Re: [dm-devel] [RFC] block: fix blk_queue_split() resource exhaustion

From: Lars Ellenberg
Date: Fri Jul 08 2016 - 09:00:50 EST


On Fri, Jul 08, 2016 at 07:39:36PM +1000, NeilBrown wrote:
> >> To make the patch "perfect", and maybe even more elegant we could treat
> >> ->remainder and ->recursion more alike.
> >> i.e.:
> >> - generic make request has a private "stack" of requests.
> >> - before calling ->make_request_fn(), both ->remainder and ->recursion
> >> are initialised
> >> - after ->make_request_fn(), ->remainder are spliced in to top of
> >> 'stack', then ->recursion is spliced onto that.
> >> - If stack is not empty, the top request is popped and we loop to top.
> >>
> >> This reliably follows in-order execution, and handles siblings correctly
> >> (in submitted order) if/when a request splits off multiple siblings.
> >
> > The only splitting that creates siblings on the current level
> > is blk_queue_split(), which splits the current bio into
> > "front piece" and "remainder", already processed in this order.
>
> Yes.
> I imagine that a driver *could* split a bio into three parts with a
> single allocation, but I cannot actually see any point in doing it. So
> I was over-complicating things.
>
> >
> > Anything else creating "siblings" is not creating siblings for the
> > current layer, but for the next deeper layer, which are queue on
> > "recursion" and also processed in the order they have been generated.
> >
> >> I think that as long a requests are submitted in the order they are
> >> created at each level there is no reason to expect performance
> >> regressions.
> >> All we are doing is changing the ordering between requests generated at
> >> different levels, and I think we are restoring a more natural order.
> >
> > I believe both patches combined are doing exactly this already.
> > I could rename .remainder to .todo or .incoming, though.
>
> :-) neither "remainder" or "recursion" seem like brilliant names to me,
> but I don't have anything better to suggest. Naming is hard!
> As long as a comment explains the name clearly I could cope with X and Y.

...

> I think we just might be in violent agreement.

I thought so, too :-)

Should I merge both patches,
rename to ".queue" and ".tmp",
and submit for inclusion?

Lars Ellenberg