On Wed, Sep 06 2017, Mikulas Patocka wrote: > On Wed, 6 Sep 2017, NeilBrown wrote: > >> Thanks for the extra explanation. I've thought some more about this I can >> see a way forward that I am comfortable with. Please let me know what >> you think. >> I haven't tested this patch yet, but I believe that applying it first will >> clear the way for my other patch to work without reintroducing >> deadlocks. >> >> Thanks, >> NeilBrown > > What's the purpose of this patch? If the current code that offloads bios > on current->bio_list to a rescue thread works, why do you want to change > it? That is a fair question. My primary reason is that I don't like the 'rescue' functionality built in to biosets, and I want to get rid of it together with all uses for BIOSET_NEED_RESCUER. I think this functionality is unnecessary and adds complexity. It superficially seems like it should fix any deadlock issue created by the ordering that generic_make_request() requires, but as you discovered with dm-snap, it isn't sufficient by itself. So it is an incomplete solution that mostly isn't necessary, and I don't think it is good design to keep such things around. I also like uniformity where practical. The block layer provides a simple coherent infrastructure for splitting requests when needed, and if simple guidelines are following concerning how the resulting bios are submitted to generic_make_request(), then deadlocks can be avoided. Most device use this common infrastructure, the exceptions being dm and bcache (though I really haven't wrapped my mind around bcache yet so I cannot say much in concrete terms about what it does). So I would like dm to handle over-large bios by allocating from the ->bio_split pool, cloning, bio_chain and generic_make_request(). On reflection, I think that just this change might address all your deadlock issues. We might be able to get rid of the dm_offload stuff completely, though I'm not 100% sure yet. So in brief: 1/ use same approach to bio splitting everywhere - an approach that is resistant to deadlocks. 2/ discard the incomplete and unnecessary BIOSET_NEED_RESCUER functionality. Thanks, NeilBrown
Attachment:
signature.asc
Description: PGP signature
-- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
