On Wed, Aug 08, 2012 at 03:58:39PM -0700, Tejun Heo wrote: > On Mon, Aug 06, 2012 at 03:08:37PM -0700, Kent Overstreet wrote: > > + * > > + * BIG FAT WARNING: > > + * > > + * If you're calling this from under generic_make_request() (i.e. > > + * current->bio_list != NULL), you should mask out __GFP_WAIT and punt to > > + * workqueue if the allocation fails. Otherwise, your code will probably > > + * deadlock. > > If the condition is detectable, WARN_ON_ONCE() please. I know I said I liked this idea, but I changed my mind. Sticking a WARN_ON_ONCE() there is saying that passing __GFP_WAIT from under generic_make_request() is always wrong - it might as well be a BUG_ON() except warn is better for the user. If that's true, then an assertion is completely wrong because we can just do the right thing instead - mask out __GFP_WAIT if current->bio_list != NULL and document that it can fail in that situation. Which is what my original code did. The alternative is accepting that there are situations where it is technically possible to pass __GFP_WAIT from under generic_make_request() without deadlocking and allow it, but my position is still that that is far too subtle to expect that it'll be gotten right (especially considering the ways that the code is wrong today wrt deadlocks). But honestly this is turning into bikeshedding. The current bio splitting and merge_bvec_fn stuff is crap, and there are worse potential deadlocks/bugs in the existing code than what we're arguing over here. -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
