Message ID | 20210412025831.31498-2-nanich.lee@samsung.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | limit bio max size | expand |
Hi Changheun, Thank you for the patch! Yet something to improve: [auto build test ERROR on mkp-scsi/for-next] [also build test ERROR on scsi/for-next linus/master v5.12-rc7] [cannot apply to block/for-next next-20210409] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Changheun-Lee/bio-limit-bio-max-size/20210412-115922 base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next config: i386-tinyconfig (attached as .config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce (this is a W=1 build): # https://github.com/0day-ci/linux/commit/5d00aa42f58575968c4a7a4b374addaf4f9a5624 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Changheun-Lee/bio-limit-bio-max-size/20210412-115922 git checkout 5d00aa42f58575968c4a7a4b374addaf4f9a5624 # save the attached .config to linux build tree make W=1 ARCH=i386 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): In file included from include/linux/blkdev.h:19, from include/linux/blk-cgroup.h:23, from include/linux/writeback.h:14, from include/linux/memcontrol.h:22, from include/linux/swap.h:9, from include/linux/suspend.h:5, from arch/x86/kernel/asm-offsets.c:13: include/linux/bio.h: In function 'bio_full': >> include/linux/bio.h:122:29: error: implicit declaration of function 'bio_max_size'; did you mean 'bio_max_segs'? [-Werror=implicit-function-declaration] 122 | if (bio->bi_iter.bi_size > bio_max_size(bio) - len) | ^~~~~~~~~~~~ | bio_max_segs cc1: some warnings being treated as errors -- In file included from include/linux/blkdev.h:19, from include/linux/blk-cgroup.h:23, from include/linux/writeback.h:14, from include/linux/memcontrol.h:22, from include/linux/swap.h:9, from include/linux/suspend.h:5, from arch/x86/kernel/asm-offsets.c:13: include/linux/bio.h: In function 'bio_full': >> include/linux/bio.h:122:29: error: implicit declaration of function 'bio_max_size'; did you mean 'bio_max_segs'? [-Werror=implicit-function-declaration] 122 | if (bio->bi_iter.bi_size > bio_max_size(bio) - len) | ^~~~~~~~~~~~ | bio_max_segs cc1: some warnings being treated as errors make[2]: *** [scripts/Makefile.build:116: arch/x86/kernel/asm-offsets.s] Error 1 make[2]: Target '__build' not remade because of errors. make[1]: *** [Makefile:1233: prepare0] Error 2 make[1]: Target 'prepare' not remade because of errors. make: *** [Makefile:215: __sub-make] Error 2 make: Target 'prepare' not remade because of errors. vim +122 include/linux/bio.h 108 109 /** 110 * bio_full - check if the bio is full 111 * @bio: bio to check 112 * @len: length of one segment to be added 113 * 114 * Return true if @bio is full and one segment with @len bytes can't be 115 * added to the bio, otherwise return false 116 */ 117 static inline bool bio_full(struct bio *bio, unsigned len) 118 { 119 if (bio->bi_vcnt >= bio->bi_max_vecs) 120 return true; 121 > 122 if (bio->bi_iter.bi_size > bio_max_size(bio) - len) 123 return true; 124 125 return false; 126 } 127 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Changheun, Thank you for the patch! Yet something to improve: [auto build test ERROR on mkp-scsi/for-next] [also build test ERROR on scsi/for-next linus/master v5.12-rc7] [cannot apply to block/for-next next-20210409] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Changheun-Lee/bio-limit-bio-max-size/20210412-115922 base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next config: nios2-defconfig (attached as .config) compiler: nios2-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/5d00aa42f58575968c4a7a4b374addaf4f9a5624 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Changheun-Lee/bio-limit-bio-max-size/20210412-115922 git checkout 5d00aa42f58575968c4a7a4b374addaf4f9a5624 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nios2 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All error/warnings (new ones prefixed by >>): In file included from include/linux/blkdev.h:19, from include/linux/blk-cgroup.h:23, from include/linux/writeback.h:14, from include/linux/memcontrol.h:22, from include/linux/swap.h:9, from block/bio.c:6: include/linux/bio.h: In function 'bio_full': include/linux/bio.h:122:29: error: implicit declaration of function 'bio_max_size'; did you mean 'bio_max_segs'? [-Werror=implicit-function-declaration] 122 | if (bio->bi_iter.bi_size > bio_max_size(bio) - len) | ^~~~~~~~~~~~ | bio_max_segs block/bio.c: At top level: >> block/bio.c:258:14: warning: no previous prototype for 'bio_max_size' [-Wmissing-prototypes] 258 | unsigned int bio_max_size(struct bio *bio) | ^~~~~~~~~~~~ >> block/bio.c:258:14: error: conflicting types for 'bio_max_size' In file included from include/linux/blkdev.h:19, from include/linux/blk-cgroup.h:23, from include/linux/writeback.h:14, from include/linux/memcontrol.h:22, from include/linux/swap.h:9, from block/bio.c:6: include/linux/bio.h:122:29: note: previous implicit declaration of 'bio_max_size' was here 122 | if (bio->bi_iter.bi_size > bio_max_size(bio) - len) | ^~~~~~~~~~~~ block/bio.c: In function 'bio_max_size': >> block/bio.c:260:31: error: 'struct bio' has no member named 'bi_disk' 260 | struct request_queue *q = bio->bi_disk->queue; | ^~ cc1: some warnings being treated as errors vim +/bio_max_size +258 block/bio.c 257 > 258 unsigned int bio_max_size(struct bio *bio) 259 { > 260 struct request_queue *q = bio->bi_disk->queue; 261 262 if (blk_queue_limit_bio_size(q)) 263 return blk_queue_get_max_sectors(q, bio_op(bio)) 264 << SECTOR_SHIFT; 265 266 return UINT_MAX; 267 } 268 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/block/bio.c b/block/bio.c index 50e579088aca..e4d6169106b6 100644 --- a/block/bio.c +++ b/block/bio.c @@ -255,6 +255,17 @@ void bio_init(struct bio *bio, struct bio_vec *table, } EXPORT_SYMBOL(bio_init); +unsigned int bio_max_size(struct bio *bio) +{ + struct request_queue *q = bio->bi_disk->queue; + + if (blk_queue_limit_bio_size(q)) + return blk_queue_get_max_sectors(q, bio_op(bio)) + << SECTOR_SHIFT; + + return UINT_MAX; +} + /** * bio_reset - reinitialize a bio * @bio: bio to reset @@ -866,7 +877,7 @@ bool __bio_try_merge_page(struct bio *bio, struct page *page, struct bio_vec *bv = &bio->bi_io_vec[bio->bi_vcnt - 1]; if (page_is_mergeable(bv, page, len, off, same_page)) { - if (bio->bi_iter.bi_size > UINT_MAX - len) { + if (bio->bi_iter.bi_size > bio_max_size(bio) - len) { *same_page = false; return false; } diff --git a/block/blk-settings.c b/block/blk-settings.c index b4aa2f37fab6..1d94b97cea4f 100644 --- a/block/blk-settings.c +++ b/block/blk-settings.c @@ -928,6 +928,23 @@ void blk_queue_set_zoned(struct gendisk *disk, enum blk_zoned_model model) } EXPORT_SYMBOL_GPL(blk_queue_set_zoned); +/** + * blk_queue_set_limit_bio_size - set limit bio size flag + * @q: the request queue for the device + * @limit: limit bio size on(true), or off + * + * bio max size will be limited to queue max sectors size, + * if limit is true. + */ +void blk_queue_set_limit_bio_size(struct request_queue *q, bool limit) +{ + if (limit) + blk_queue_flag_set(QUEUE_FLAG_LIMIT_BIO_SIZE, q); + else + blk_queue_flag_clear(QUEUE_FLAG_LIMIT_BIO_SIZE, q); +} +EXPORT_SYMBOL_GPL(blk_queue_set_limit_bio_size); + static int __init blk_settings_init(void) { blk_max_low_pfn = max_low_pfn - 1; diff --git a/include/linux/bio.h b/include/linux/bio.h index d0246c92a6e8..830c784967c0 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -119,7 +119,7 @@ static inline bool bio_full(struct bio *bio, unsigned len) if (bio->bi_vcnt >= bio->bi_max_vecs) return true; - if (bio->bi_iter.bi_size > UINT_MAX - len) + if (bio->bi_iter.bi_size > bio_max_size(bio) - len) return true; return false; diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 158aefae1030..c69a6ed7a189 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -617,6 +617,7 @@ struct request_queue { #define QUEUE_FLAG_RQ_ALLOC_TIME 27 /* record rq->alloc_time_ns */ #define QUEUE_FLAG_HCTX_ACTIVE 28 /* at least one blk-mq hctx is active */ #define QUEUE_FLAG_NOWAIT 29 /* device supports NOWAIT */ +#define QUEUE_FLAG_LIMIT_BIO_SIZE 30 /* limit bio size */ #define QUEUE_FLAG_MQ_DEFAULT ((1 << QUEUE_FLAG_IO_STAT) | \ (1 << QUEUE_FLAG_SAME_COMP) | \ @@ -663,6 +664,8 @@ bool blk_queue_flag_test_and_set(unsigned int flag, struct request_queue *q); #define blk_queue_fua(q) test_bit(QUEUE_FLAG_FUA, &(q)->queue_flags) #define blk_queue_registered(q) test_bit(QUEUE_FLAG_REGISTERED, &(q)->queue_flags) #define blk_queue_nowait(q) test_bit(QUEUE_FLAG_NOWAIT, &(q)->queue_flags) +#define blk_queue_limit_bio_size(q) \ + test_bit(QUEUE_FLAG_LIMIT_BIO_SIZE, &(q)->queue_flags) extern void blk_set_pm_only(struct request_queue *q); extern void blk_clear_pm_only(struct request_queue *q); @@ -1183,6 +1186,7 @@ extern void blk_queue_required_elevator_features(struct request_queue *q, unsigned int features); extern bool blk_queue_can_use_dma_map_merging(struct request_queue *q, struct device *dev); +extern void blk_queue_set_limit_bio_size(struct request_queue *q, bool limit); /* * Number of physical segments as sent to the device.
bio size can grow up to 4GB when muli-page bvec is enabled. but sometimes it would lead to inefficient behaviors. in case of large chunk direct I/O, - 32MB chunk read in user space - all pages for 32MB would be merged to a bio structure if the pages physical addresses are contiguous. it makes some delay to submit until merge complete. bio max size should be limited to a proper size. When 32MB chunk read with direct I/O option is coming from userspace, kernel behavior is below now in do_direct_IO() loop. it's timeline. | bio merge for 32MB. total 8,192 pages are merged. | total elapsed time is over 2ms. |------------------ ... ----------------------->| | 8,192 pages merged a bio. | at this time, first bio submit is done. | 1 bio is split to 32 read request and issue. |---------------> |---------------> |---------------> ...... |---------------> |--------------->| total 19ms elapsed to complete 32MB read done from device. | If bio max size is limited with 1MB, behavior is changed below. | bio merge for 1MB. 256 pages are merged for each bio. | total 32 bio will be made. | total elapsed time is over 2ms. it's same. | but, first bio submit timing is fast. about 100us. |--->|--->|--->|---> ... -->|--->|--->|--->|--->| | 256 pages merged a bio. | at this time, first bio submit is done. | and 1 read request is issued for 1 bio. |---------------> |---------------> |---------------> ...... |---------------> |--------------->| total 17ms elapsed to complete 32MB read done from device. | As a result, read request issue timing is faster if bio max size is limited. Current kernel behavior with multipage bvec, super large bio can be created. And it lead to delay first I/O request issue. Signed-off-by: Changheun Lee <nanich.lee@samsung.com> --- block/bio.c | 13 ++++++++++++- block/blk-settings.c | 17 +++++++++++++++++ include/linux/bio.h | 2 +- include/linux/blkdev.h | 4 ++++ 4 files changed, 34 insertions(+), 2 deletions(-)