On Tue, May 19 2009, Boaz Harrosh wrote: > On 05/19/2009 01:13 PM, Jens Axboe wrote: > > On Tue, May 19 2009, Boaz Harrosh wrote: > <snip> > >> Thanks Jens, for your comment. > >> > >> I have three sources of bio allocations. > >> 1. bio_map_kern which uses bio_kmalloc (recently fixed by Tejun) > >> 2. by osdblk which does a clone and will not ever wait. > >> (I've fixed the code to split up the IO on allocation failure into > >> smaller requests (will repost soon)) > >> 3. Future code in exofs and pNFS-Client that will only ever use bio_kmalloc. > > > > All of those are fine! > > > >> Should we add something to the Documentation, and/or above doc_book comment > >> to warn off users? > > > > Yes I think so. I'm generally weary of adding interfaces that are easy > > to misuse. This one has that potential, but it also has merits. So I'll > > merge your series, if you could send a patch updating the > > comment/docbook, then that would be great. > > > > As my English sucks, please read proof below addition. I will repost later > today. > > Should I just post this one patch, or all the 5? > (alternatively I have these on a public git tree reabased on > block/for-next branch.) Just this one is fine, I already merged the series. > Thanks Boaz > --- > diff --git a/block/blk-core.c b/block/blk-core.c > index 89261d2..4dc4e32 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -910,6 +910,15 @@ EXPORT_SYMBOL(blk_get_request); > * need bouncing, by calling the appropriate masked or flagged allocator, > * suitable for the target device. Otherwise the call to blk_queue_bounce will > * BUG. > + * > + * WARNING: When allocating/cloning a bio-chain, careful consideration should be > + * given to how you allocate bios. In particular, you cannot use __GFP_WAIT for > + * anything but the first bio in the chain. Otherwise you risk deadlocking, > + * waiting for a bio to be returned to the pool, which will never return, since > + * it was not submitted yet. Perhaps something like: Otherwise you risk waiting for IO completion of a bio that hasn't been submitted yet, thus resulting in a deadlock. > + * Alternatively bios should be allocated using bio_kmalloc only. > + * If possible a long IO should be split into smaller parts when allocation > + * fails. Partial allocation should not be an error, or you risk a live-lock. > */ > struct request *blk_make_request(struct request_queue *q, struct bio *bio, > gfp_t gfp_mask) Alternatively bios should be allocated using bio_kmalloc() instead of bio_alloc(), as that avoids the mempool deadlock. If possible a big IO should be ... -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
