From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Moyer Subject: Re: [PATCH v2 alt 2] fs/direct-io.c: don't try to allocate more than BIO_MAX_PAGES in a bio Date: Mon, 17 Jan 2011 13:28:52 -0500 Message-ID: References: <1294977946.21790.20.camel@obelisk.thedillows.org> <1295023584.24487.16.camel@lap75545.ornl.gov> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "linux-kernel\@vger.kernel.org" , "linux-fsdevel\@vger.kernel.org" To: David Dillow Return-path: In-Reply-To: <1295023584.24487.16.camel@lap75545.ornl.gov> (David Dillow's message of "Fri, 14 Jan 2011 11:46:24 -0500") Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org David Dillow writes: > When using devices that support max_segments > BIO_MAX_PAGES (256), > direct IO tries to allocate a bio with more pages than allowed, which > leads to an oops in dio_bio_alloc(). Clamp the request to the supported > maximum, and change dio_bio_alloc() to reflect that bio_alloc() will > always return a bio when called with __GFP_WAIT and a valid number of > vectors. I like this version better than the first, just clean up the comment to conform to CodingStyle and you can add my Reviewed-by: Jeff Moyer Be sure to CC akpm on the next posting, as DIO changes usually go in through his tree. Cheers, Jeff > Signed-off-by: David Dillow > -- > Second alternate, my preferred that reflects that bio_alloc() cannot > fail when properly called. I can take or leave the comment. > > > diff --git a/fs/direct-io.c b/fs/direct-io.c > index 85882f6..96a01d8 100644 > --- a/fs/direct-io.c > +++ b/fs/direct-io.c > @@ -325,13 +325,17 @@ void dio_end_io(struct bio *bio, int error) > } > EXPORT_SYMBOL_GPL(dio_end_io); > > -static int > +static void > dio_bio_alloc(struct dio *dio, struct block_device *bdev, > sector_t first_sector, int nr_vecs) > { > struct bio *bio; > > + /* bio_alloc() is guaranteed to return a bio when called with > + * __GFP_WAIT and we request a valid number of vectors. > + */ > bio = bio_alloc(GFP_KERNEL, nr_vecs); > + BUG_ON(!bio); > > bio->bi_bdev = bdev; > bio->bi_sector = first_sector; > @@ -342,7 +346,6 @@ dio_bio_alloc(struct dio *dio, struct block_device *bdev, > > dio->bio = bio; > dio->logical_offset_in_bio = dio->cur_page_fs_offset; > - return 0; > } > > /* > @@ -583,8 +586,9 @@ static int dio_new_bio(struct dio *dio, sector_t start_sector) > goto out; > sector = start_sector << (dio->blkbits - 9); > nr_pages = min(dio->pages_in_io, bio_get_nr_vecs(dio->map_bh.b_bdev)); > + nr_pages = min(nr_pages, BIO_MAX_PAGES); > BUG_ON(nr_pages <= 0); > - ret = dio_bio_alloc(dio, dio->map_bh.b_bdev, sector, nr_pages); > + dio_bio_alloc(dio, dio->map_bh.b_bdev, sector, nr_pages); > dio->boundary = 0; > out: > return ret;