Re: [PATCH 3/5] New blk_make_request(), takes bio, returns a request

[Boaz Harrosh <bharrosh@panasas.com>]

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.)

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.
+ * 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)