* [PATCH v2 0/2] mm: fix arithmetic for bdi min_ratio and @ 2023-12-19 2:42 Jingbo Xu 2023-12-19 2:42 ` [PATCH v2 1/2] mm: fix arithmetic for bdi min_ratio Jingbo Xu 2023-12-19 2:42 ` [PATCH v2 2/2] mm: fix arithmetic for max_prop_frac when setting max_ratio Jingbo Xu 0 siblings, 2 replies; 7+ messages in thread From: Jingbo Xu @ 2023-12-19 2:42 UTC (permalink / raw) To: shr, akpm; +Cc: linux-mm, linux-kernel, joseph.qi, linux-fsdevel, linux-block changes since v1: - split the previous v1 patch into two separate patches with corresponding "Fixes" tag (Andrew Morton) - patch 2: remove "UL" suffix for the "100" constant v1: https://lore.kernel.org/all/20231218031640.77983-1-jefflexu@linux.alibaba.com/ Jingbo Xu (2): mm: fix arithmetic for bdi min_ratio mm: fix arithmetic for max_prop_frac when setting max_ratio mm/page-writeback.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.19.1.6.gb485710b ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] mm: fix arithmetic for bdi min_ratio 2023-12-19 2:42 [PATCH v2 0/2] mm: fix arithmetic for bdi min_ratio and Jingbo Xu @ 2023-12-19 2:42 ` Jingbo Xu 2023-12-19 2:42 ` [PATCH v2 2/2] mm: fix arithmetic for max_prop_frac when setting max_ratio Jingbo Xu 1 sibling, 0 replies; 7+ messages in thread From: Jingbo Xu @ 2023-12-19 2:42 UTC (permalink / raw) To: shr, akpm; +Cc: linux-mm, linux-kernel, joseph.qi, linux-fsdevel, linux-block Since now bdi->min_ratio is part per million, fix the wrong arithmetic. Otherwise it will fail with -EINVAL when setting a reasonable min_ratio, as it tries to set min_ratio to (min_ratio * BDI_RATIO_SCALE) in percentage unit, which exceeds 100% anyway. # cat /sys/class/bdi/253\:0/min_ratio 0 # cat /sys/class/bdi/253\:0/max_ratio 100 # echo 1 > /sys/class/bdi/253\:0/min_ratio -bash: echo: write error: Invalid argument Fixes: 8021fb3232f2 ("mm: split off __bdi_set_min_ratio() function") Reported-by: Joseph Qi <joseph.qi@linux.alibaba.com> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com> --- mm/page-writeback.c | 1 - 1 file changed, 1 deletion(-) diff --git a/mm/page-writeback.c b/mm/page-writeback.c index ee2fd6a6af40..2140382dd768 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -692,7 +692,6 @@ static int __bdi_set_min_ratio(struct backing_dev_info *bdi, unsigned int min_ra if (min_ratio > 100 * BDI_RATIO_SCALE) return -EINVAL; - min_ratio *= BDI_RATIO_SCALE; spin_lock_bh(&bdi_lock); if (min_ratio > bdi->max_ratio) { -- 2.19.1.6.gb485710b ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] mm: fix arithmetic for max_prop_frac when setting max_ratio 2023-12-19 2:42 [PATCH v2 0/2] mm: fix arithmetic for bdi min_ratio and Jingbo Xu 2023-12-19 2:42 ` [PATCH v2 1/2] mm: fix arithmetic for bdi min_ratio Jingbo Xu @ 2023-12-19 2:42 ` Jingbo Xu 2023-12-19 4:06 ` Matthew Wilcox 1 sibling, 1 reply; 7+ messages in thread From: Jingbo Xu @ 2023-12-19 2:42 UTC (permalink / raw) To: shr, akpm; +Cc: linux-mm, linux-kernel, joseph.qi, linux-fsdevel, linux-block Since now bdi->max_ratio is part per million, fix the wrong arithmetic for max_prop_frac when setting max_ratio. Otherwise the miscalculated max_prop_frac will affect the incrementing of writeout completion count when max_ratio is not 100%. Fixes: efc3e6ad53ea ("mm: split off __bdi_set_max_ratio() function") Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com> --- mm/page-writeback.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mm/page-writeback.c b/mm/page-writeback.c index 2140382dd768..dda59b368c01 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -728,7 +728,8 @@ static int __bdi_set_max_ratio(struct backing_dev_info *bdi, unsigned int max_ra ret = -EINVAL; } else { bdi->max_ratio = max_ratio; - bdi->max_prop_frac = (FPROP_FRAC_BASE * max_ratio) / 100; + bdi->max_prop_frac = div64_u64(FPROP_FRAC_BASE * max_ratio, + 100 * BDI_RATIO_SCALE); } spin_unlock_bh(&bdi_lock); -- 2.19.1.6.gb485710b ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] mm: fix arithmetic for max_prop_frac when setting max_ratio 2023-12-19 2:42 ` [PATCH v2 2/2] mm: fix arithmetic for max_prop_frac when setting max_ratio Jingbo Xu @ 2023-12-19 4:06 ` Matthew Wilcox 2023-12-19 5:58 ` Jingbo Xu 0 siblings, 1 reply; 7+ messages in thread From: Matthew Wilcox @ 2023-12-19 4:06 UTC (permalink / raw) To: Jingbo Xu Cc: shr, akpm, linux-mm, linux-kernel, joseph.qi, linux-fsdevel, linux-block On Tue, Dec 19, 2023 at 10:42:46AM +0800, Jingbo Xu wrote: > } else { > bdi->max_ratio = max_ratio; > - bdi->max_prop_frac = (FPROP_FRAC_BASE * max_ratio) / 100; > + bdi->max_prop_frac = div64_u64(FPROP_FRAC_BASE * max_ratio, > + 100 * BDI_RATIO_SCALE); > } Why use div64_u64 here? FPROP_FRAC_BASE is an unsigned long. max_ratio is an unsigned int, so the numerator is an unsigned long. BDI_RATIO_SCALE is 10,000, so the numerator is an unsigned int. There's no 64-bit arithmetic needed here. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] mm: fix arithmetic for max_prop_frac when setting max_ratio 2023-12-19 4:06 ` Matthew Wilcox @ 2023-12-19 5:58 ` Jingbo Xu 2023-12-19 13:01 ` Matthew Wilcox 0 siblings, 1 reply; 7+ messages in thread From: Jingbo Xu @ 2023-12-19 5:58 UTC (permalink / raw) To: Matthew Wilcox Cc: shr, akpm, linux-mm, linux-kernel, joseph.qi, linux-fsdevel, linux-block On 12/19/23 12:06 PM, Matthew Wilcox wrote: > On Tue, Dec 19, 2023 at 10:42:46AM +0800, Jingbo Xu wrote: >> } else { >> bdi->max_ratio = max_ratio; >> - bdi->max_prop_frac = (FPROP_FRAC_BASE * max_ratio) / 100; >> + bdi->max_prop_frac = div64_u64(FPROP_FRAC_BASE * max_ratio, >> + 100 * BDI_RATIO_SCALE); >> } > > Why use div64_u64 here? > > FPROP_FRAC_BASE is an unsigned long. max_ratio is an unsigned int, so > the numerator is an unsigned long. BDI_RATIO_SCALE is 10,000, so the > numerator is an unsigned int. There's no 64-bit arithmetic needed here. Yes, div64_u64() is actually not needed here. So it seems bdi->max_prop_frac = FPROP_FRAC_BASE * max_ratio / 100 / BDI_RATIO_SCALE; is adequate? -- Thanks, Jingbo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] mm: fix arithmetic for max_prop_frac when setting max_ratio 2023-12-19 5:58 ` Jingbo Xu @ 2023-12-19 13:01 ` Matthew Wilcox 2023-12-19 14:07 ` Jingbo Xu 0 siblings, 1 reply; 7+ messages in thread From: Matthew Wilcox @ 2023-12-19 13:01 UTC (permalink / raw) To: Jingbo Xu Cc: shr, akpm, linux-mm, linux-kernel, joseph.qi, linux-fsdevel, linux-block On Tue, Dec 19, 2023 at 01:58:21PM +0800, Jingbo Xu wrote: > On 12/19/23 12:06 PM, Matthew Wilcox wrote: > > On Tue, Dec 19, 2023 at 10:42:46AM +0800, Jingbo Xu wrote: > >> } else { > >> bdi->max_ratio = max_ratio; > >> - bdi->max_prop_frac = (FPROP_FRAC_BASE * max_ratio) / 100; > >> + bdi->max_prop_frac = div64_u64(FPROP_FRAC_BASE * max_ratio, > >> + 100 * BDI_RATIO_SCALE); > >> } > > > > Why use div64_u64 here? > > > > FPROP_FRAC_BASE is an unsigned long. max_ratio is an unsigned int, so > > the numerator is an unsigned long. BDI_RATIO_SCALE is 10,000, so the > > numerator is an unsigned int. There's no 64-bit arithmetic needed here. > > Yes, div64_u64() is actually not needed here. So it seems > > bdi->max_prop_frac = FPROP_FRAC_BASE * max_ratio / 100 / BDI_RATIO_SCALE; > > is adequate? I'd rather spell that as: bdi->max_prop_frac = (FPROP_FRAC_BASE * max_ratio) / (100 * BDI_RATIO_SCALE); It's closer to how you'd write it out mathematically and so it reads more easily. At least for me. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] mm: fix arithmetic for max_prop_frac when setting max_ratio 2023-12-19 13:01 ` Matthew Wilcox @ 2023-12-19 14:07 ` Jingbo Xu 0 siblings, 0 replies; 7+ messages in thread From: Jingbo Xu @ 2023-12-19 14:07 UTC (permalink / raw) To: Matthew Wilcox Cc: shr, akpm, linux-mm, linux-kernel, joseph.qi, linux-fsdevel, linux-block On 12/19/23 9:01 PM, Matthew Wilcox wrote: > On Tue, Dec 19, 2023 at 01:58:21PM +0800, Jingbo Xu wrote: >> On 12/19/23 12:06 PM, Matthew Wilcox wrote: >>> On Tue, Dec 19, 2023 at 10:42:46AM +0800, Jingbo Xu wrote: >>>> } else { >>>> bdi->max_ratio = max_ratio; >>>> - bdi->max_prop_frac = (FPROP_FRAC_BASE * max_ratio) / 100; >>>> + bdi->max_prop_frac = div64_u64(FPROP_FRAC_BASE * max_ratio, >>>> + 100 * BDI_RATIO_SCALE); >>>> } >>> >>> Why use div64_u64 here? >>> >>> FPROP_FRAC_BASE is an unsigned long. max_ratio is an unsigned int, so >>> the numerator is an unsigned long. BDI_RATIO_SCALE is 10,000, so the >>> numerator is an unsigned int. There's no 64-bit arithmetic needed here. >> >> Yes, div64_u64() is actually not needed here. So it seems >> >> bdi->max_prop_frac = FPROP_FRAC_BASE * max_ratio / 100 / BDI_RATIO_SCALE; >> >> is adequate? > > I'd rather spell that as: > > bdi->max_prop_frac = (FPROP_FRAC_BASE * max_ratio) / > (100 * BDI_RATIO_SCALE); > > It's closer to how you'd write it out mathematically and so it reads > more easily. At least for me. Thanks, I would send v3 soon. -- Thanks, Jingbo ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-12-19 14:12 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-12-19 2:42 [PATCH v2 0/2] mm: fix arithmetic for bdi min_ratio and Jingbo Xu 2023-12-19 2:42 ` [PATCH v2 1/2] mm: fix arithmetic for bdi min_ratio Jingbo Xu 2023-12-19 2:42 ` [PATCH v2 2/2] mm: fix arithmetic for max_prop_frac when setting max_ratio Jingbo Xu 2023-12-19 4:06 ` Matthew Wilcox 2023-12-19 5:58 ` Jingbo Xu 2023-12-19 13:01 ` Matthew Wilcox 2023-12-19 14:07 ` Jingbo Xu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).