| Message ID | 20240530134547.970075-1-longman@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | blk-throttle: Fix incorrect display of io.max | expand |
On 5/30/24 09:45, Waiman Long wrote: > Commit bf20ab538c81 ("blk-throttle: remove CONFIG_BLK_DEV_THROTTLING_LOW") > attempts to revert the code change introduced by commit cd5ab1b0fcb4 > ("blk-throttle: add .low interface"). However, it leaves behind the > bps_conf[] and iops_conf[] fields in the throtl_grp structure which > aren't set anywhere in the new blk-throttle.c code but are still being > used by tg_prfill_limit() to display the limits in io.max. Now io.max > always displays the following values if a block queue is used: > > <m>:<n> rbps=0 wbps=0 riops=0 wiops=0 > > Fix this problem by removing bps_conf[] and iops_conf[] and use bps[] > and iops[] instead to complete the revert. > > Fixes: bf20ab538c81 ("blk-throttle: remove CONFIG_BLK_DEV_THROTTLING_LOW") > Reported-by: Justin Forbes <jforbes@redhat.com> > Closes: https://github.com/containers/podman/issues/22701#issuecomment-2120627789 > Signed-off-by: Waiman Long <longman@redhat.com> > --- > block/blk-throttle.c | 24 ++++++++++++------------ > block/blk-throttle.h | 8 ++------ > 2 files changed, 14 insertions(+), 18 deletions(-) > > diff --git a/block/blk-throttle.c b/block/blk-throttle.c > index d907040859f9..da619654f418 100644 > --- a/block/blk-throttle.c > +++ b/block/blk-throttle.c > @@ -1347,32 +1347,32 @@ static u64 tg_prfill_limit(struct seq_file *sf, struct blkg_policy_data *pd, > bps_dft = U64_MAX; > iops_dft = UINT_MAX; > > - if (tg->bps_conf[READ] == bps_dft && > - tg->bps_conf[WRITE] == bps_dft && > - tg->iops_conf[READ] == iops_dft && > - tg->iops_conf[WRITE] == iops_dft) > + if (tg->bps[READ] == bps_dft && > + tg->bps[WRITE] == bps_dft && > + tg->iops[READ] == iops_dft && > + tg->iops[WRITE] == iops_dft) > return 0; > > seq_printf(sf, "%s", dname); > - if (tg->bps_conf[READ] == U64_MAX) > + if (tg->bps[READ] == U64_MAX) > seq_printf(sf, " rbps=max"); > else > - seq_printf(sf, " rbps=%llu", tg->bps_conf[READ]); > + seq_printf(sf, " rbps=%llu", tg->bps[READ]); > > - if (tg->bps_conf[WRITE] == U64_MAX) > + if (tg->bps[WRITE] == U64_MAX) > seq_printf(sf, " wbps=max"); > else > - seq_printf(sf, " wbps=%llu", tg->bps_conf[WRITE]); > + seq_printf(sf, " wbps=%llu", tg->bps[WRITE]); > > - if (tg->iops_conf[READ] == UINT_MAX) > + if (tg->iops[READ] == UINT_MAX) > seq_printf(sf, " riops=max"); > else > - seq_printf(sf, " riops=%u", tg->iops_conf[READ]); > + seq_printf(sf, " riops=%u", tg->iops[READ]); > > - if (tg->iops_conf[WRITE] == UINT_MAX) > + if (tg->iops[WRITE] == UINT_MAX) > seq_printf(sf, " wiops=max"); > else > - seq_printf(sf, " wiops=%u", tg->iops_conf[WRITE]); > + seq_printf(sf, " wiops=%u", tg->iops[WRITE]); > > seq_printf(sf, "\n"); > return 0; > diff --git a/block/blk-throttle.h b/block/blk-throttle.h > index 32503fd83a84..8c365541a275 100644 > --- a/block/blk-throttle.h > +++ b/block/blk-throttle.h > @@ -95,15 +95,11 @@ struct throtl_grp { > bool has_rules_bps[2]; > bool has_rules_iops[2]; > > - /* internally used bytes per second rate limits */ > + /* bytes per second rate limits */ > uint64_t bps[2]; > - /* user configured bps limits */ > - uint64_t bps_conf[2]; > > - /* internally used IOPS limits */ > + /* IOPS limits */ > unsigned int iops[2]; > - /* user configured IOPS limits */ > - unsigned int iops_conf[2]; > > /* Number of bytes dispatched in current slice */ > uint64_t bytes_disp[2]; Add Yu Kuai <yukuai3@huawei.com> to cc.
On Thu, May 30, 2024 at 09:45:47AM -0400, Waiman Long wrote: > Commit bf20ab538c81 ("blk-throttle: remove CONFIG_BLK_DEV_THROTTLING_LOW") > attempts to revert the code change introduced by commit cd5ab1b0fcb4 > ("blk-throttle: add .low interface"). However, it leaves behind the > bps_conf[] and iops_conf[] fields in the throtl_grp structure which > aren't set anywhere in the new blk-throttle.c code but are still being > used by tg_prfill_limit() to display the limits in io.max. Now io.max > always displays the following values if a block queue is used: > > <m>:<n> rbps=0 wbps=0 riops=0 wiops=0 > > Fix this problem by removing bps_conf[] and iops_conf[] and use bps[] > and iops[] instead to complete the revert. > > Fixes: bf20ab538c81 ("blk-throttle: remove CONFIG_BLK_DEV_THROTTLING_LOW") > Reported-by: Justin Forbes <jforbes@redhat.com> > Closes: https://github.com/containers/podman/issues/22701#issuecomment-2120627789 > Signed-off-by: Waiman Long <longman@redhat.com> Acked-by: Tejun Heo <tj@kernel.org> Thanks.
在 2024/05/30 21:49, Waiman Long 写道: > > On 5/30/24 09:45, Waiman Long wrote: >> Commit bf20ab538c81 ("blk-throttle: remove >> CONFIG_BLK_DEV_THROTTLING_LOW") >> attempts to revert the code change introduced by commit cd5ab1b0fcb4 >> ("blk-throttle: add .low interface"). However, it leaves behind the >> bps_conf[] and iops_conf[] fields in the throtl_grp structure which >> aren't set anywhere in the new blk-throttle.c code but are still being >> used by tg_prfill_limit() to display the limits in io.max. Now io.max >> always displays the following values if a block queue is used: >> >> <m>:<n> rbps=0 wbps=0 riops=0 wiops=0 >> >> Fix this problem by removing bps_conf[] and iops_conf[] and use bps[] >> and iops[] instead to complete the revert. >> >> Fixes: bf20ab538c81 ("blk-throttle: remove >> CONFIG_BLK_DEV_THROTTLING_LOW") >> Reported-by: Justin Forbes <jforbes@redhat.com> >> Closes: >> https://github.com/containers/podman/issues/22701#issuecomment-2120627789 >> Signed-off-by: Waiman Long <longman@redhat.com> LGTM Reviewed-by: Yu Kuai <yukuai3@huawei.com> >> --- >> block/blk-throttle.c | 24 ++++++++++++------------ >> block/blk-throttle.h | 8 ++------ >> 2 files changed, 14 insertions(+), 18 deletions(-) >> >> diff --git a/block/blk-throttle.c b/block/blk-throttle.c >> index d907040859f9..da619654f418 100644 >> --- a/block/blk-throttle.c >> +++ b/block/blk-throttle.c >> @@ -1347,32 +1347,32 @@ static u64 tg_prfill_limit(struct seq_file >> *sf, struct blkg_policy_data *pd, >> bps_dft = U64_MAX; >> iops_dft = UINT_MAX; >> - if (tg->bps_conf[READ] == bps_dft && >> - tg->bps_conf[WRITE] == bps_dft && >> - tg->iops_conf[READ] == iops_dft && >> - tg->iops_conf[WRITE] == iops_dft) >> + if (tg->bps[READ] == bps_dft && >> + tg->bps[WRITE] == bps_dft && >> + tg->iops[READ] == iops_dft && >> + tg->iops[WRITE] == iops_dft) >> return 0; >> seq_printf(sf, "%s", dname); >> - if (tg->bps_conf[READ] == U64_MAX) >> + if (tg->bps[READ] == U64_MAX) >> seq_printf(sf, " rbps=max"); >> else >> - seq_printf(sf, " rbps=%llu", tg->bps_conf[READ]); >> + seq_printf(sf, " rbps=%llu", tg->bps[READ]); >> - if (tg->bps_conf[WRITE] == U64_MAX) >> + if (tg->bps[WRITE] == U64_MAX) >> seq_printf(sf, " wbps=max"); >> else >> - seq_printf(sf, " wbps=%llu", tg->bps_conf[WRITE]); >> + seq_printf(sf, " wbps=%llu", tg->bps[WRITE]); >> - if (tg->iops_conf[READ] == UINT_MAX) >> + if (tg->iops[READ] == UINT_MAX) >> seq_printf(sf, " riops=max"); >> else >> - seq_printf(sf, " riops=%u", tg->iops_conf[READ]); >> + seq_printf(sf, " riops=%u", tg->iops[READ]); >> - if (tg->iops_conf[WRITE] == UINT_MAX) >> + if (tg->iops[WRITE] == UINT_MAX) >> seq_printf(sf, " wiops=max"); >> else >> - seq_printf(sf, " wiops=%u", tg->iops_conf[WRITE]); >> + seq_printf(sf, " wiops=%u", tg->iops[WRITE]); >> seq_printf(sf, "\n"); >> return 0; >> diff --git a/block/blk-throttle.h b/block/blk-throttle.h >> index 32503fd83a84..8c365541a275 100644 >> --- a/block/blk-throttle.h >> +++ b/block/blk-throttle.h >> @@ -95,15 +95,11 @@ struct throtl_grp { >> bool has_rules_bps[2]; >> bool has_rules_iops[2]; >> - /* internally used bytes per second rate limits */ >> + /* bytes per second rate limits */ >> uint64_t bps[2]; >> - /* user configured bps limits */ >> - uint64_t bps_conf[2]; >> - /* internally used IOPS limits */ >> + /* IOPS limits */ >> unsigned int iops[2]; >> - /* user configured IOPS limits */ >> - unsigned int iops_conf[2]; >> /* Number of bytes dispatched in current slice */ >> uint64_t bytes_disp[2]; > > Add Yu Kuai <yukuai3@huawei.com> to cc. > > > . >
On Thu, 30 May 2024 09:45:47 -0400, Waiman Long wrote: > Commit bf20ab538c81 ("blk-throttle: remove CONFIG_BLK_DEV_THROTTLING_LOW") > attempts to revert the code change introduced by commit cd5ab1b0fcb4 > ("blk-throttle: add .low interface"). However, it leaves behind the > bps_conf[] and iops_conf[] fields in the throtl_grp structure which > aren't set anywhere in the new blk-throttle.c code but are still being > used by tg_prfill_limit() to display the limits in io.max. Now io.max > always displays the following values if a block queue is used: > > [...] Applied, thanks! [1/1] blk-throttle: Fix incorrect display of io.max commit: 0a751df4566c86e5a24f2a03290dad3d0f215692 Best regards,
diff --git a/block/blk-throttle.c b/block/blk-throttle.c index d907040859f9..da619654f418 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -1347,32 +1347,32 @@ static u64 tg_prfill_limit(struct seq_file *sf, struct blkg_policy_data *pd, bps_dft = U64_MAX; iops_dft = UINT_MAX; - if (tg->bps_conf[READ] == bps_dft && - tg->bps_conf[WRITE] == bps_dft && - tg->iops_conf[READ] == iops_dft && - tg->iops_conf[WRITE] == iops_dft) + if (tg->bps[READ] == bps_dft && + tg->bps[WRITE] == bps_dft && + tg->iops[READ] == iops_dft && + tg->iops[WRITE] == iops_dft) return 0; seq_printf(sf, "%s", dname); - if (tg->bps_conf[READ] == U64_MAX) + if (tg->bps[READ] == U64_MAX) seq_printf(sf, " rbps=max"); else - seq_printf(sf, " rbps=%llu", tg->bps_conf[READ]); + seq_printf(sf, " rbps=%llu", tg->bps[READ]); - if (tg->bps_conf[WRITE] == U64_MAX) + if (tg->bps[WRITE] == U64_MAX) seq_printf(sf, " wbps=max"); else - seq_printf(sf, " wbps=%llu", tg->bps_conf[WRITE]); + seq_printf(sf, " wbps=%llu", tg->bps[WRITE]); - if (tg->iops_conf[READ] == UINT_MAX) + if (tg->iops[READ] == UINT_MAX) seq_printf(sf, " riops=max"); else - seq_printf(sf, " riops=%u", tg->iops_conf[READ]); + seq_printf(sf, " riops=%u", tg->iops[READ]); - if (tg->iops_conf[WRITE] == UINT_MAX) + if (tg->iops[WRITE] == UINT_MAX) seq_printf(sf, " wiops=max"); else - seq_printf(sf, " wiops=%u", tg->iops_conf[WRITE]); + seq_printf(sf, " wiops=%u", tg->iops[WRITE]); seq_printf(sf, "\n"); return 0; diff --git a/block/blk-throttle.h b/block/blk-throttle.h index 32503fd83a84..8c365541a275 100644 --- a/block/blk-throttle.h +++ b/block/blk-throttle.h @@ -95,15 +95,11 @@ struct throtl_grp { bool has_rules_bps[2]; bool has_rules_iops[2]; - /* internally used bytes per second rate limits */ + /* bytes per second rate limits */ uint64_t bps[2]; - /* user configured bps limits */ - uint64_t bps_conf[2]; - /* internally used IOPS limits */ + /* IOPS limits */ unsigned int iops[2]; - /* user configured IOPS limits */ - unsigned int iops_conf[2]; /* Number of bytes dispatched in current slice */ uint64_t bytes_disp[2];
Commit bf20ab538c81 ("blk-throttle: remove CONFIG_BLK_DEV_THROTTLING_LOW") attempts to revert the code change introduced by commit cd5ab1b0fcb4 ("blk-throttle: add .low interface"). However, it leaves behind the bps_conf[] and iops_conf[] fields in the throtl_grp structure which aren't set anywhere in the new blk-throttle.c code but are still being used by tg_prfill_limit() to display the limits in io.max. Now io.max always displays the following values if a block queue is used: <m>:<n> rbps=0 wbps=0 riops=0 wiops=0 Fix this problem by removing bps_conf[] and iops_conf[] and use bps[] and iops[] instead to complete the revert. Fixes: bf20ab538c81 ("blk-throttle: remove CONFIG_BLK_DEV_THROTTLING_LOW") Reported-by: Justin Forbes <jforbes@redhat.com> Closes: https://github.com/containers/podman/issues/22701#issuecomment-2120627789 Signed-off-by: Waiman Long <longman@redhat.com> --- block/blk-throttle.c | 24 ++++++++++++------------ block/blk-throttle.h | 8 ++------ 2 files changed, 14 insertions(+), 18 deletions(-)