[PATCH 0/3] Optimize wbt and update its comments and doc

[PATCH 0/3] Optimize wbt and update its comments and doc

[Tang Yizhou]

From: Tang Yizhou <yizhou.tang@shopee.com>

Some minor optimizations and updates of comments and doc for wbt.

Tang Yizhou (3):
  blk-wbt: Optimize wbt_done() for non-throttled writes
  blk-wbt: Eliminate ambiguity in the comments of struct rq_wb
  blk-wbt: doc: Update the doc of the wbt_lat_usec interface

 Documentation/ABI/stable/sysfs-block | 10 +++++-----
 block/blk-wbt.c                      | 15 ++++++++-------
 2 files changed, 13 insertions(+), 12 deletions(-)

-- 
2.25.1

[PATCH 1/3] blk-wbt: Optimize wbt_done() for non-throttled writes

[Tang Yizhou]

From: Tang Yizhou <yizhou.tang@shopee.com>

In the current implementation, the sync_cookie and last_cookie members of
struct rq_wb are used only by read requests and not by non-throttled write
requests. Based on this, we can optimize wbt_done() by removing one if
condition check for non-throttled write requests.

Signed-off-by: Tang Yizhou <yizhou.tang@shopee.com>
---
 block/blk-wbt.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/block/blk-wbt.c b/block/blk-wbt.c
index a50d4cd55f41..30886d44f6cd 100644
--- a/block/blk-wbt.c
+++ b/block/blk-wbt.c
@@ -248,13 +248,14 @@ static void wbt_done(struct rq_qos *rqos, struct request *rq)
 	struct rq_wb *rwb = RQWB(rqos);
 
 	if (!wbt_is_tracked(rq)) {
-		if (rwb->sync_cookie == rq) {
-			rwb->sync_issue = 0;
-			rwb->sync_cookie = NULL;
-		}
+		if (wbt_is_read(rq)) {
+			if (rwb->sync_cookie == rq) {
+				rwb->sync_issue = 0;
+				rwb->sync_cookie = NULL;
+			}
 
-		if (wbt_is_read(rq))
 			wb_timestamp(rwb, &rwb->last_comp);
+		}
 	} else {
 		WARN_ON_ONCE(rq == rwb->sync_cookie);
 		__wbt_done(rqos, wbt_flags(rq));
-- 
2.25.1

Re: [PATCH 1/3] blk-wbt: Optimize wbt_done() for non-throttled writes

[Jan Kara]

On Thu 24-07-25 16:29:59, Tang Yizhou wrote:
> From: Tang Yizhou <yizhou.tang@shopee.com>
> 
> In the current implementation, the sync_cookie and last_cookie members of
> struct rq_wb are used only by read requests and not by non-throttled write
> requests. Based on this, we can optimize wbt_done() by removing one if
> condition check for non-throttled write requests.
> 
> Signed-off-by: Tang Yizhou <yizhou.tang@shopee.com>

Nah, I'm undecided if this is actually worth it. But it looks correct and
doesn't make the code harder to follow so I guess feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  block/blk-wbt.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/block/blk-wbt.c b/block/blk-wbt.c
> index a50d4cd55f41..30886d44f6cd 100644
> --- a/block/blk-wbt.c
> +++ b/block/blk-wbt.c
> @@ -248,13 +248,14 @@ static void wbt_done(struct rq_qos *rqos, struct request *rq)
>  	struct rq_wb *rwb = RQWB(rqos);
>  
>  	if (!wbt_is_tracked(rq)) {
> -		if (rwb->sync_cookie == rq) {
> -			rwb->sync_issue = 0;
> -			rwb->sync_cookie = NULL;
> -		}
> +		if (wbt_is_read(rq)) {
> +			if (rwb->sync_cookie == rq) {
> +				rwb->sync_issue = 0;
> +				rwb->sync_cookie = NULL;
> +			}
>  
> -		if (wbt_is_read(rq))
>  			wb_timestamp(rwb, &rwb->last_comp);
> +		}
>  	} else {
>  		WARN_ON_ONCE(rq == rwb->sync_cookie);
>  		__wbt_done(rqos, wbt_flags(rq));
> -- 
> 2.25.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 1/3] blk-wbt: Optimize wbt_done() for non-throttled writes

[Yu Kuai]

Hi,

在 2025/7/24 16:29, Tang Yizhou 写道:
> From: Tang Yizhou <yizhou.tang@shopee.com>
>
> In the current implementation, the sync_cookie and last_cookie members of
> struct rq_wb are used only by read requests and not by non-throttled write
> requests. Based on this, we can optimize wbt_done() by removing one if
> condition check for non-throttled write requests.
>
> Signed-off-by: Tang Yizhou <yizhou.tang@shopee.com>
> ---
>   block/blk-wbt.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/block/blk-wbt.c b/block/blk-wbt.c
> index a50d4cd55f41..30886d44f6cd 100644
> --- a/block/blk-wbt.c
> +++ b/block/blk-wbt.c
> @@ -248,13 +248,14 @@ static void wbt_done(struct rq_qos *rqos, struct request *rq)
>   	struct rq_wb *rwb = RQWB(rqos);
>   
>   	if (!wbt_is_tracked(rq)) {
> -		if (rwb->sync_cookie == rq) {
> -			rwb->sync_issue = 0;
> -			rwb->sync_cookie = NULL;
> -		}
> +		if (wbt_is_read(rq)) {
> +			if (rwb->sync_cookie == rq) {
> +				rwb->sync_issue = 0;
> +				rwb->sync_cookie = NULL;
> +			}
>   
> -		if (wbt_is_read(rq))
>   			wb_timestamp(rwb, &rwb->last_comp);
> +		}
>   	} else {
>   		WARN_ON_ONCE(rq == rwb->sync_cookie);
>   		__wbt_done(rqos, wbt_flags(rq));
At lease this is correct logically.

Reviewed-by: Yu Kuai <yukuai3@huawei.com>

[PATCH 2/3] blk-wbt: Eliminate ambiguity in the comments of struct rq_wb

[Tang Yizhou]

From: Tang Yizhou <yizhou.tang@shopee.com>

In the current implementation, the last_issue and last_comp members of
struct rq_wb are used only by read requests and not by non-throttled write
requests. Therefore, eliminate the ambiguity here.

Signed-off-by: Tang Yizhou <yizhou.tang@shopee.com>
---
 block/blk-wbt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/blk-wbt.c b/block/blk-wbt.c
index 30886d44f6cd..eb8037bae0bd 100644
--- a/block/blk-wbt.c
+++ b/block/blk-wbt.c
@@ -85,8 +85,8 @@ struct rq_wb {
 	u64 sync_issue;
 	void *sync_cookie;
 
-	unsigned long last_issue;		/* last non-throttled issue */
-	unsigned long last_comp;		/* last non-throttled comp */
+	unsigned long last_issue;	/* issue time of last read rq */
+	unsigned long last_comp;	/* completion time of last read rq */
 	unsigned long min_lat_nsec;
 	struct rq_qos rqos;
 	struct rq_wait rq_wait[WBT_NUM_RWQ];
-- 
2.25.1

Re: [PATCH 2/3] blk-wbt: Eliminate ambiguity in the comments of struct rq_wb

[Jan Kara]

On Thu 24-07-25 16:30:00, Tang Yizhou wrote:
> From: Tang Yizhou <yizhou.tang@shopee.com>
> 
> In the current implementation, the last_issue and last_comp members of
> struct rq_wb are used only by read requests and not by non-throttled write
> requests. Therefore, eliminate the ambiguity here.
> 
> Signed-off-by: Tang Yizhou <yizhou.tang@shopee.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  block/blk-wbt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/block/blk-wbt.c b/block/blk-wbt.c
> index 30886d44f6cd..eb8037bae0bd 100644
> --- a/block/blk-wbt.c
> +++ b/block/blk-wbt.c
> @@ -85,8 +85,8 @@ struct rq_wb {
>  	u64 sync_issue;
>  	void *sync_cookie;
>  
> -	unsigned long last_issue;		/* last non-throttled issue */
> -	unsigned long last_comp;		/* last non-throttled comp */
> +	unsigned long last_issue;	/* issue time of last read rq */
> +	unsigned long last_comp;	/* completion time of last read rq */
>  	unsigned long min_lat_nsec;
>  	struct rq_qos rqos;
>  	struct rq_wait rq_wait[WBT_NUM_RWQ];
> -- 
> 2.25.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 2/3] blk-wbt: Eliminate ambiguity in the comments of struct rq_wb

[Yu Kuai]

Hi,

在 2025/7/24 16:30, Tang Yizhou 写道:
> From: Tang Yizhou <yizhou.tang@shopee.com>
>
> In the current implementation, the last_issue and last_comp members of
> struct rq_wb are used only by read requests and not by non-throttled write
> requests. Therefore, eliminate the ambiguity here.
>
> Signed-off-by: Tang Yizhou <yizhou.tang@shopee.com>
> ---
>   block/blk-wbt.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/block/blk-wbt.c b/block/blk-wbt.c
> index 30886d44f6cd..eb8037bae0bd 100644
> --- a/block/blk-wbt.c
> +++ b/block/blk-wbt.c
> @@ -85,8 +85,8 @@ struct rq_wb {
>   	u64 sync_issue;
>   	void *sync_cookie;
>   
> -	unsigned long last_issue;		/* last non-throttled issue */
> -	unsigned long last_comp;		/* last non-throttled comp */
> +	unsigned long last_issue;	/* issue time of last read rq */
> +	unsigned long last_comp;	/* completion time of last read rq */
>   	unsigned long min_lat_nsec;
>   	struct rq_qos rqos;
>   	struct rq_wait rq_wait[WBT_NUM_RWQ];
Not sure if this is worth it, I'm more concerned about backport trouble.

Thanks,
Kuai

[PATCH 3/3] blk-wbt: doc: Update the doc of the wbt_lat_usec interface

[Tang Yizhou]

From: Tang Yizhou <yizhou.tang@shopee.com>

The symbol wb_window_usec cannot be found. Update the doc to reflect the
latest implementation, in other words, the cur_win_nsec member of struct
rq_wb.

Signed-off-by: Tang Yizhou <yizhou.tang@shopee.com>
---
 Documentation/ABI/stable/sysfs-block | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/stable/sysfs-block b/Documentation/ABI/stable/sysfs-block
index 4ba771b56b3b..7bb4dce73eca 100644
--- a/Documentation/ABI/stable/sysfs-block
+++ b/Documentation/ABI/stable/sysfs-block
@@ -731,11 +731,11 @@ Contact:	linux-block@vger.kernel.org
 Description:
 		[RW] If the device is registered for writeback throttling, then
 		this file shows the target minimum read latency. If this latency
-		is exceeded in a given window of time (see wb_window_usec), then
-		the writeback throttling will start scaling back writes. Writing
-		a value of '0' to this file disables the feature. Writing a
-		value of '-1' to this file resets the value to the default
-		setting.
+		is exceeded in a given window of time (see the cur_win_nsec
+		member of struct rq_wb), then the writeback throttling will
+		start scaling back writes. Writing a value of '0' to this file
+		disables the feature. Writing a value of '-1' to this file
+		resets the value to the default setting.
 
 
 What:		/sys/block/<disk>/queue/write_cache
-- 
2.25.1

Re: [PATCH 3/3] blk-wbt: doc: Update the doc of the wbt_lat_usec interface

[Jan Kara]

On Thu 24-07-25 16:30:01, Tang Yizhou wrote:
> From: Tang Yizhou <yizhou.tang@shopee.com>
> 
> The symbol wb_window_usec cannot be found. Update the doc to reflect the
> latest implementation, in other words, the cur_win_nsec member of struct
> rq_wb.
> 
> Signed-off-by: Tang Yizhou <yizhou.tang@shopee.com>

I think the name should be actually 'curr_win_nsec' because that's the name
of this value shown in debugfs.

								Honza

> ---
>  Documentation/ABI/stable/sysfs-block | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/ABI/stable/sysfs-block b/Documentation/ABI/stable/sysfs-block
> index 4ba771b56b3b..7bb4dce73eca 100644
> --- a/Documentation/ABI/stable/sysfs-block
> +++ b/Documentation/ABI/stable/sysfs-block
> @@ -731,11 +731,11 @@ Contact:	linux-block@vger.kernel.org
>  Description:
>  		[RW] If the device is registered for writeback throttling, then
>  		this file shows the target minimum read latency. If this latency
> -		is exceeded in a given window of time (see wb_window_usec), then
> -		the writeback throttling will start scaling back writes. Writing
> -		a value of '0' to this file disables the feature. Writing a
> -		value of '-1' to this file resets the value to the default
> -		setting.
> +		is exceeded in a given window of time (see the cur_win_nsec
> +		member of struct rq_wb), then the writeback throttling will
> +		start scaling back writes. Writing a value of '0' to this file
> +		disables the feature. Writing a value of '-1' to this file
> +		resets the value to the default setting.
>  
>  
>  What:		/sys/block/<disk>/queue/write_cache
> -- 
> 2.25.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 3/3] blk-wbt: doc: Update the doc of the wbt_lat_usec interface

[Yu Kuai]

Hi,

在 2025/7/25 23:49, Jan Kara 写道:
> On Thu 24-07-25 16:30:01, Tang Yizhou wrote:
>> From: Tang Yizhou <yizhou.tang@shopee.com>
>>
>> The symbol wb_window_usec cannot be found. Update the doc to reflect the
>> latest implementation, in other words, the cur_win_nsec member of struct
>> rq_wb.
>>
>> Signed-off-by: Tang Yizhou <yizhou.tang@shopee.com>
> I think the name should be actually 'curr_win_nsec' because that's the name
> of this value shown in debugfs.
Agreed, or you can mention both the field in rq_wb and debugfs name.

Thanks,
Kuai
>
> 								Honza
>
>> ---
>>   Documentation/ABI/stable/sysfs-block | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/ABI/stable/sysfs-block b/Documentation/ABI/stable/sysfs-block
>> index 4ba771b56b3b..7bb4dce73eca 100644
>> --- a/Documentation/ABI/stable/sysfs-block
>> +++ b/Documentation/ABI/stable/sysfs-block
>> @@ -731,11 +731,11 @@ Contact:	linux-block@vger.kernel.org
>>   Description:
>>   		[RW] If the device is registered for writeback throttling, then
>>   		this file shows the target minimum read latency. If this latency
>> -		is exceeded in a given window of time (see wb_window_usec), then
>> -		the writeback throttling will start scaling back writes. Writing
>> -		a value of '0' to this file disables the feature. Writing a
>> -		value of '-1' to this file resets the value to the default
>> -		setting.
>> +		is exceeded in a given window of time (see the cur_win_nsec
>> +		member of struct rq_wb), then the writeback throttling will
>> +		start scaling back writes. Writing a value of '0' to this file
>> +		disables the feature. Writing a value of '-1' to this file
>> +		resets the value to the default setting.
>>   
>>   
>>   What:		/sys/block/<disk>/queue/write_cache
>> -- 
>> 2.25.1
>>

Re: [PATCH 3/3] blk-wbt: doc: Update the doc of the wbt_lat_usec interface

[Yizhou Tang]

On Sat, Jul 26, 2025 at 2:16 AM Yu Kuai <yukuai@kernel.org> wrote:
>
> Hi,
>
> 在 2025/7/25 23:49, Jan Kara 写道:
> > On Thu 24-07-25 16:30:01, Tang Yizhou wrote:
> >> From: Tang Yizhou <yizhou.tang@shopee.com>
> >>
> >> The symbol wb_window_usec cannot be found. Update the doc to reflect the
> >> latest implementation, in other words, the cur_win_nsec member of struct
> >> rq_wb.
> >>
> >> Signed-off-by: Tang Yizhou <yizhou.tang@shopee.com>
> > I think the name should be actually 'curr_win_nsec' because that's the name
> > of this value shown in debugfs.
> Agreed, or you can mention both the field in rq_wb and debugfs name.
>

I will change it to cur_win_nsec in the next patch.

Thanks,
Yi

> Thanks,
> Kuai
> >
> >                                                               Honza
> >
> >> ---
> >>   Documentation/ABI/stable/sysfs-block | 10 +++++-----
> >>   1 file changed, 5 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/Documentation/ABI/stable/sysfs-block b/Documentation/ABI/stable/sysfs-block
> >> index 4ba771b56b3b..7bb4dce73eca 100644
> >> --- a/Documentation/ABI/stable/sysfs-block
> >> +++ b/Documentation/ABI/stable/sysfs-block
> >> @@ -731,11 +731,11 @@ Contact:       linux-block@vger.kernel.org
> >>   Description:
> >>              [RW] If the device is registered for writeback throttling, then
> >>              this file shows the target minimum read latency. If this latency
> >> -            is exceeded in a given window of time (see wb_window_usec), then
> >> -            the writeback throttling will start scaling back writes. Writing
> >> -            a value of '0' to this file disables the feature. Writing a
> >> -            value of '-1' to this file resets the value to the default
> >> -            setting.
> >> +            is exceeded in a given window of time (see the cur_win_nsec
> >> +            member of struct rq_wb), then the writeback throttling will
> >> +            start scaling back writes. Writing a value of '0' to this file
> >> +            disables the feature. Writing a value of '-1' to this file
> >> +            resets the value to the default setting.
> >>
> >>
> >>   What:              /sys/block/<disk>/queue/write_cache
> >> --
> >> 2.25.1
> >>
>

Re: [PATCH 3/3] blk-wbt: doc: Update the doc of the wbt_lat_usec interface

[Yizhou Tang]

On Fri, Jul 25, 2025 at 11:49 PM Jan Kara <jack@suse.cz> wrote:
>
> On Thu 24-07-25 16:30:01, Tang Yizhou wrote:
> > From: Tang Yizhou <yizhou.tang@shopee.com>
> >
> > The symbol wb_window_usec cannot be found. Update the doc to reflect the
> > latest implementation, in other words, the cur_win_nsec member of struct
> > rq_wb.
> >
> > Signed-off-by: Tang Yizhou <yizhou.tang@shopee.com>
>
> I think the name should be actually 'curr_win_nsec' because that's the name
> of this value shown in debugfs.
>
>                                                                 Honza

Agreed, I will make the change in the next patch.

Thanks,
Yi

>
> > ---
> >  Documentation/ABI/stable/sysfs-block | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/ABI/stable/sysfs-block b/Documentation/ABI/stable/sysfs-block
> > index 4ba771b56b3b..7bb4dce73eca 100644
> > --- a/Documentation/ABI/stable/sysfs-block
> > +++ b/Documentation/ABI/stable/sysfs-block
> > @@ -731,11 +731,11 @@ Contact:        linux-block@vger.kernel.org
> >  Description:
> >               [RW] If the device is registered for writeback throttling, then
> >               this file shows the target minimum read latency. If this latency
> > -             is exceeded in a given window of time (see wb_window_usec), then
> > -             the writeback throttling will start scaling back writes. Writing
> > -             a value of '0' to this file disables the feature. Writing a
> > -             value of '-1' to this file resets the value to the default
> > -             setting.
> > +             is exceeded in a given window of time (see the cur_win_nsec
> > +             member of struct rq_wb), then the writeback throttling will
> > +             start scaling back writes. Writing a value of '0' to this file
> > +             disables the feature. Writing a value of '-1' to this file
> > +             resets the value to the default setting.
> >
> >
> >  What:                /sys/block/<disk>/queue/write_cache
> > --
> > 2.25.1
> >
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR