Re: [PATCH v3] block: make sure big bio is splitted into at most 256 bvecs
flat view
From: Christoph Hellwig <hch@infradead.org>
Date: 2016-08-15 18:23:35
Also in:
linux-block, lkml
On Mon, Aug 15, 2016 at 11:11:22PM +0800, Ming Lei wrote:
After arbitrary bio size is supported, the incoming bio may be very big. We have to split the bio into small bios so that each holds at most BIO_MAX_PAGES bvecs for safety reason, such as bio_clone().
I still think working around a rough driver submitting too large I/O is a bad thing until we've done a full audit of all consuming bios through ->make_request, and we've enabled it for the common path as well.
quoted hunk ↗ jump to hunk
bool do_split = true; struct bio *new = NULL; const unsigned max_sectors = get_max_io_size(q, bio); + unsigned bvecs = 0; + + *no_merge = true; bio_for_each_segment(bv, bio, iter) { /* + * With arbitrary bio size, the incoming bio may be very + * big. We have to split the bio into small bios so that + * each holds at most BIO_MAX_PAGES bvecs because + * bio_clone() can fail to allocate big bvecs. + * + * It should have been better to apply the limit per + * request queue in which bio_clone() is involved, + * instead of globally. The biggest blocker is + * bio_clone() in bio bounce. + * + * If bio is splitted by this reason, we should allow + * to continue bios merging. + * + * TODO: deal with bio bounce's bio_clone() gracefully + * and convert the global limit into per-queue limit. + */ + if (bvecs++ >= BIO_MAX_PAGES) { + *no_merge = false; + goto split; + }
That being said this simple if check here is simple enough that it's probably fine. But I see no need to uglify the whole code path with that no_merge flag. Please drop if for now, and if we start caring for this path in common code we should just move the REQ_NOMERGE setting into the actual blk_bio_*_split helpers.