On Mon, Jun 04 2018, Kent Overstreet wrote:

> On Tue, Jun 05, 2018 at 07:16:51AM +1000, NeilBrown wrote:
>> I really should get back to BIOSET_NEED_RESCUER and see if I can discard
>> it completely.
>> Kent - the only remaining user is bcache, and the main reason I haven't
>> removed it is that I cannot follow the code (or at least, I couldn't
>> last time I tried).  Are you able to see if it is still needed?
>
> Oh man, rescueurs make my head hurt. I remember you changed something so that
> they weren't needed most of the time, but I can't remember what you did - can
> you remind me what that was and whatever else you can remember offhand?

(and closures make my head hurt :-)

There were two closely related changes.
One is make sure that generic_make_request processed bios strictly
in a depth-first order.  The other is to ensure that all (stacking)
block device drivers never block in their make_request_fn waiting
for something that have themselves submitted.

Together these mean that a thread in generic_make_request will never
block waiting for a bio that might be queued in current->bio_list.
The bio chosen by generic_make_request will be a leaf and won't depend
on anything else in the list, and the handler for that request won't
block after submitting a lower-level request.

The  second property is the important one for drivers and the one we
need to ensure that bcache follows.
A common bad pattern is something like:

 while (too_big(bio)) {
    split = bio_spilt(bio,...))
    process(split)
 }
 process(bio)

That needs to be more like
  if (too_big(bio)) {
     split = bio_split(bio,...);
     generic_make_request(bio);
     bio = split;
  }
  process(bio)

so that generic_make_request() gets to do that while-loop
and can control the order in which bios are handled.

I cannot convince myself that bcache doesn't have something similar to
that while loop (structured with closures).

Thanks,
NeilBrown

Attachment: signature.asc
Description: PGP signature

Reply via email to