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
