From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [PATCH 01/33] block: add a lower-level bio_add_page interface To: Christoph Hellwig Cc: linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-block@vger.kernel.org, linux-mm@kvack.org References: <20180509074830.16196-1-hch@lst.de> <20180509074830.16196-2-hch@lst.de> <37c16316-aa3a-e3df-79d0-9fca37a5996f@codeaurora.org> <20180516180503.GA6627@lst.de> From: Ritesh Harjani Message-ID: Date: Thu, 17 May 2018 09:48:30 +0530 MIME-Version: 1.0 In-Reply-To: <20180516180503.GA6627@lst.de> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: On 5/16/2018 11:35 PM, Christoph Hellwig wrote: > On Wed, May 16, 2018 at 10:36:14AM +0530, Ritesh Harjani wrote: >> 1. if bio_full is true that means no space in bio->bio_io_vec[] no? >> Than how come we are still proceeding ahead with only warning? >> While originally in bio_add_page we used to return after checking >> bio_full. Callers can still call __bio_add_page directly right. > > I you don't know if the bio is full or not don't use __bio_add_page, > keep using bio_add_page. The WARN_ON is just a debug tool to catch > cases where the developer did use it incorrectly. > >> 2. IF above is correct why don't we set the bio->bi_max_vecs to the size >> of the slab instead of keeeping it to nr_iovecs which user requested? >> (in bio_alloc_bioset) > > Because we limit the user to the number that the user requested. Not > that this patch changes anything about that. > >> 3. Could you please help understand why for cloned bio we still allow >> __bio_add_page to work? why not WARN and return like in original code? > > It doesn't work, and I have now added the WARN_ON to deal with any > incorrect usage. > >>> - if (bio->bi_vcnt >= bio->bi_max_vecs) >>> - return 0; >> Originally here we were supposed to return and not proceed further. >> Should __bio_add_page not have similar checks to safeguard crossing >> the bio_io_vec[] boundary? > > No, __bio_add_page is the "I know what I am doing" interface. > Thanks for explaining. I guess I missed reading the comment made on top of function __bio_add_page. "The caller must ensure that @bio has space for another bvec" This discussion helped me understand a bit about bios & bio_vec. Thanks!! Ritesh -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.