2021-03-16 09:30:06

by Changheun Lee

[permalink] [raw]
Subject: [RESEND PATCH v5 2/2] bio: add limit_bio_size sysfs

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 2638d3446b79..cd371a821855 100644
--- a/Documentation/block/queue-sysfs.rst
+++ b/Documentation/block/queue-sysfs.rst
@@ -273,4 +273,11 @@ devices are described in the ZBC (Zoned Block Commands) and ZAC
do not support zone commands, they will be treated as regular block devices
and zoned will report "none".

+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 b513f1683af0..840d97f427e6 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -288,6 +288,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)
@@ -615,6 +616,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,
@@ -648,6 +650,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.28.0


2021-04-07 14:50:08

by Greg KH

[permalink] [raw]
Subject: Re: [RESEND PATCH v5 2/2] bio: add limit_bio_size sysfs

On Wed, Apr 07, 2021 at 10:21:17AM +0900, Changheun Lee wrote:
> > On 3/16/21 12:44 AM, Changheun Lee wrote:
> > > 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 2638d3446b79..cd371a821855 100644
> > > --- a/Documentation/block/queue-sysfs.rst
> > > +++ b/Documentation/block/queue-sysfs.rst
> > > @@ -273,4 +273,11 @@ devices are described in the ZBC (Zoned Block Commands) and ZAC
> > > do not support zone commands, they will be treated as regular block devices
> > > and zoned will report "none".
> > >
> > > +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 b513f1683af0..840d97f427e6 100644
> > > --- a/block/blk-sysfs.c
> > > +++ b/block/blk-sysfs.c
> > > @@ -288,6 +288,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)
> > > @@ -615,6 +616,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,
> > > @@ -648,6 +650,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,
> >
> > Has it been considered to introduce a function to set the BIO size limit
> > instead of introducing a new sysfs attribute? See also
> > blk_queue_max_hw_sectors().
>
> A function to set has been not considered yet.
> But sysfs attribute should be supported I think. Because it can be
> different depending on each system environment including policy.

But what tool is now responsible for setting this new value? Where will
it get that information from? And why can't the kernel just
automatically set this correctly in the first place without any need for
userspace interaction?

thanks,

greg k-h

2021-04-07 15:26:27

by Bart Van Assche

[permalink] [raw]
Subject: Re: [RESEND PATCH v5 2/2] bio: add limit_bio_size sysfs

On 3/16/21 12:44 AM, Changheun Lee wrote:
> 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 2638d3446b79..cd371a821855 100644
> --- a/Documentation/block/queue-sysfs.rst
> +++ b/Documentation/block/queue-sysfs.rst
> @@ -273,4 +273,11 @@ devices are described in the ZBC (Zoned Block Commands) and ZAC
> do not support zone commands, they will be treated as regular block devices
> and zoned will report "none".
>
> +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 b513f1683af0..840d97f427e6 100644
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -288,6 +288,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)
> @@ -615,6 +616,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,
> @@ -648,6 +650,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,

Has it been considered to introduce a function to set the BIO size limit
instead of introducing a new sysfs attribute? See also
blk_queue_max_hw_sectors().

Thanks,

Bart.

2021-04-07 16:11:32

by Changheun Lee

[permalink] [raw]
Subject: Re: [RESEND PATCH v5 2/2] bio: add limit_bio_size sysfs

> On 3/16/21 12:44 AM, Changheun Lee wrote:
> > 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 2638d3446b79..cd371a821855 100644
> > --- a/Documentation/block/queue-sysfs.rst
> > +++ b/Documentation/block/queue-sysfs.rst
> > @@ -273,4 +273,11 @@ devices are described in the ZBC (Zoned Block Commands) and ZAC
> > do not support zone commands, they will be treated as regular block devices
> > and zoned will report "none".
> >
> > +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 b513f1683af0..840d97f427e6 100644
> > --- a/block/blk-sysfs.c
> > +++ b/block/blk-sysfs.c
> > @@ -288,6 +288,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)
> > @@ -615,6 +616,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,
> > @@ -648,6 +650,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,
>
> Has it been considered to introduce a function to set the BIO size limit
> instead of introducing a new sysfs attribute? See also
> blk_queue_max_hw_sectors().

A function to set has been not considered yet.
But sysfs attribute should be supported I think. Because it can be
different depending on each system environment including policy.

>
> Thanks,
>
> Bart.

Thanks,

Changheun Lee