I found a inefficient behavior from multipage bvec. Large chunk DIO
scenario is that. This patch series could be a solution to improve it.
Changheun Lee (3):
bio: limit bio max size
ufs: set QUEUE_FLAG_LIMIT_BIO_SIZE
bio: add limit_bio_size sysfs
Documentation/ABI/testing/sysfs-block | 10 ++++++++++
Documentation/block/queue-sysfs.rst | 7 +++++++
block/bio.c | 13 ++++++++++++-
block/blk-settings.c | 17 +++++++++++++++++
block/blk-sysfs.c | 3 +++
drivers/scsi/scsi_lib.c | 2 ++
drivers/scsi/ufs/ufshcd.c | 1 +
include/linux/bio.h | 2 +-
include/linux/blkdev.h | 4 ++++
include/scsi/scsi_host.h | 2 ++
10 files changed, 59 insertions(+), 2 deletions(-)
--
2.29.0
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 <[email protected]>
---
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(-)
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.
--
2.29.0
Add limit_bio_size block sysfs node to limit bio size.
Queue flag QUEUE_FLAG_LIMIT_BIO_SIZE will be set if limit_bio_size is set.
And bio max size will be limited by queue max sectors via
QUEUE_FLAG_LIMIT_BIO_SIZE set.
Signed-off-by: Changheun Lee <[email protected]>
---
Documentation/ABI/testing/sysfs-block | 10 ++++++++++
Documentation/block/queue-sysfs.rst | 7 +++++++
block/blk-sysfs.c | 3 +++
3 files changed, 20 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block
index e34cdeeeb9d4..86a7b15410cf 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -316,3 +316,13 @@ Description:
does not complete in this time then the block driver timeout
handler is invoked. That timeout handler can decide to retry
the request, to fail it or to start a device recovery strategy.
+
+What: /sys/block/<disk>/queue/limit_bio_size
+Date: Feb, 2021
+Contact: Changheun Lee <[email protected]>
+Description:
+ (RW) Toggle for set/clear QUEUE_FLAG_LIMIT_BIO_SIZE queue flag.
+ Queue flag QUEUE_FLAG_LIMIT_BIO_SIZE will be set if limit_bio_size
+ is set. And bio max size will be limited by queue max sectors.
+ QUEUE_FLAG_LIMIT_BIO_SIZE will be cleared if limit_bio_size is
+ cleard. And limit of bio max size will be cleard.
diff --git a/Documentation/block/queue-sysfs.rst b/Documentation/block/queue-sysfs.rst
index 4dc7f0d499a8..220d183a4c11 100644
--- a/Documentation/block/queue-sysfs.rst
+++ b/Documentation/block/queue-sysfs.rst
@@ -286,4 +286,11 @@ sequential zones of zoned block devices (devices with a zoned attributed
that reports "host-managed" or "host-aware"). This value is always 0 for
regular block devices.
+limit_bio_size (RW)
+-------------------
+This indicates QUEUE_FLAG_LIMIT_BIO_SIZE queue flag value. And
+QUEUE_FLAG_LIMIT_BIO_SIZE can be changed via set(1)/clear(0) this node.
+bio max size will be limited by queue max sectors via set this node. And
+limit of bio max size will be cleard via clear this node.
+
Jens Axboe <[email protected]>, February 2009
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 0f4f0c8a7825..4625d5319daf 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -294,6 +294,7 @@ QUEUE_SYSFS_BIT_FNS(nonrot, NONROT, 1);
QUEUE_SYSFS_BIT_FNS(random, ADD_RANDOM, 0);
QUEUE_SYSFS_BIT_FNS(iostats, IO_STAT, 0);
QUEUE_SYSFS_BIT_FNS(stable_writes, STABLE_WRITES, 0);
+QUEUE_SYSFS_BIT_FNS(limit_bio_size, LIMIT_BIO_SIZE, 0);
#undef QUEUE_SYSFS_BIT_FNS
static ssize_t queue_zoned_show(struct request_queue *q, char *page)
@@ -625,6 +626,7 @@ QUEUE_RW_ENTRY(queue_nonrot, "rotational");
QUEUE_RW_ENTRY(queue_iostats, "iostats");
QUEUE_RW_ENTRY(queue_random, "add_random");
QUEUE_RW_ENTRY(queue_stable_writes, "stable_writes");
+QUEUE_RW_ENTRY(queue_limit_bio_size, "limit_bio_size");
static struct attribute *queue_attrs[] = {
&queue_requests_entry.attr,
@@ -659,6 +661,7 @@ static struct attribute *queue_attrs[] = {
&queue_rq_affinity_entry.attr,
&queue_iostats_entry.attr,
&queue_stable_writes_entry.attr,
+ &queue_limit_bio_size_entry.attr,
&queue_random_entry.attr,
&queue_poll_entry.attr,
&queue_wc_entry.attr,
--
2.29.0
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 <[email protected]>
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/[email protected]
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 <[email protected]>
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/[email protected]