v1->v2:
-add dtc_is_global() helper
-rename "bool bg" to "include_writeback"
-fold patches using domain_dirty_avail
-reflow comment to 80col
-Fix potential NULL deref of mdtc->dirty_exceeded
This series add a lot of helpers to remove repeated code between domain
and wb; dirty limit and dirty background; global domain and wb domain.
The helpers also improve readability. More details can be found on
respective patches.
Thanks.
A simple domain hierarchy is tested:
global domain (> 20G)
|
cgroup domain1(10G)
|
wb1
|
fio
Test steps:
/* make it easy to observe */
echo 300000 > /proc/sys/vm/dirty_expire_centisecs
echo 3000 > /proc/sys/vm/dirty_writeback_centisecs
/* create cgroup domain */
cd /sys/fs/cgroup
echo "+memory +io" > cgroup.subtree_control
mkdir group1
cd group1
echo 10G > memory.high
echo 10G > memory.max
echo $$ > cgroup.procs
mkfs.ext4 -F /dev/vdb
mount /dev/vdb /bdi1/
/* run fio to generate dirty pages */
fio -name test -filename=/bdi1/file -size=xxx -ioengine=libaio -bs=4K \
-iodepth=1 -rw=write -direct=0 --time_based -runtime=600 -invalidate=0
When fio size is 1G, the wb is in freerun state and dirty pages are only
written back when dirty inode is expired after 30 seconds.
When fio size is 2G, the dirty pages keep being written back and
bandwidth of fio is limited.
Kemeng Shi (8):
writeback: factor out wb_bg_dirty_limits to remove repeated code
writeback: add general function domain_dirty_avail to calculate dirty
and avail of domain
writeback: factor out domain_over_bg_thresh to remove repeated code
writeback: Factor out code of freerun to remove repeated code
writeback: factor out wb_dirty_freerun to remove more repeated freerun
code
writeback: factor out balance_domain_limits to remove repeated code
writeback: factor out wb_dirty_exceeded to remove repeated code
writeback: factor out balance_wb_limits to remove repeated code
mm/page-writeback.c | 315 ++++++++++++++++++++++++--------------------
1 file changed, 169 insertions(+), 146 deletions(-)
--
2.30.0
Factor out balance_domain_limits to remove repeated code.
Signed-off-by: Kemeng Shi <[email protected]>
Acked-by: Tejun Heo <[email protected]>
---
mm/page-writeback.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index b9c8db7089ef..97ee5b32b4ef 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -1746,6 +1746,14 @@ static void domain_dirty_freerun(struct dirty_throttle_control *dtc,
dtc->freerun = dirty <= dirty_freerun_ceiling(thresh, bg_thresh);
}
+static void balance_domain_limits(struct dirty_throttle_control *dtc,
+ bool strictlimit)
+{
+ domain_dirty_avail(dtc, true);
+ domain_dirty_limits(dtc);
+ domain_dirty_freerun(dtc, strictlimit);
+}
+
static void wb_dirty_freerun(struct dirty_throttle_control *dtc,
bool strictlimit)
{
@@ -1802,18 +1810,13 @@ static int balance_dirty_pages(struct bdi_writeback *wb,
nr_dirty = global_node_page_state(NR_FILE_DIRTY);
- domain_dirty_avail(gdtc, true);
- domain_dirty_limits(gdtc);
- domain_dirty_freerun(gdtc, strictlimit);
-
+ balance_domain_limits(gdtc, strictlimit);
if (mdtc) {
/*
* If @wb belongs to !root memcg, repeat the same
* basic calculations for the memcg domain.
*/
- domain_dirty_avail(mdtc, true);
- domain_dirty_limits(mdtc);
- domain_dirty_freerun(mdtc, strictlimit);
+ balance_domain_limits(mdtc, strictlimit);
}
/*
--
2.30.0
Add general function domain_dirty_avail to calculate dirty and avail for
either dirty limit or background writeback in either global domain or wb
domain.
Signed-off-by: Kemeng Shi <[email protected]>
---
mm/page-writeback.c | 65 ++++++++++++++++++++++++---------------------
1 file changed, 34 insertions(+), 31 deletions(-)
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index e1f73643aca1..16a94c7df260 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -837,6 +837,34 @@ static void mdtc_calc_avail(struct dirty_throttle_control *mdtc,
mdtc->avail = filepages + min(headroom, other_clean);
}
+static inline bool dtc_is_global(struct dirty_throttle_control *dtc)
+{
+ return mdtc_gdtc(dtc) == NULL;
+}
+
+/*
+ * Dirty background will ignore pages being written as we're trying to
+ * decide whether to put more under writeback.
+ */
+static void domain_dirty_avail(struct dirty_throttle_control *dtc,
+ bool include_writeback)
+{
+ if (dtc_is_global(dtc)) {
+ dtc->avail = global_dirtyable_memory();
+ dtc->dirty = global_node_page_state(NR_FILE_DIRTY);
+ if (include_writeback)
+ dtc->dirty += global_node_page_state(NR_WRITEBACK);
+ } else {
+ unsigned long filepages = 0, headroom = 0, writeback = 0;
+
+ mem_cgroup_wb_stats(dtc->wb, &filepages, &headroom, &dtc->dirty,
+ &writeback);
+ if (include_writeback)
+ dtc->dirty += writeback;
+ mdtc_calc_avail(dtc, filepages, headroom);
+ }
+}
+
/**
* __wb_calc_thresh - @wb's share of dirty threshold
* @dtc: dirty_throttle_context of interest
@@ -899,16 +927,9 @@ unsigned long cgwb_calc_thresh(struct bdi_writeback *wb)
{
struct dirty_throttle_control gdtc = { GDTC_INIT_NO_WB };
struct dirty_throttle_control mdtc = { MDTC_INIT(wb, &gdtc) };
- unsigned long filepages = 0, headroom = 0, writeback = 0;
- gdtc.avail = global_dirtyable_memory();
- gdtc.dirty = global_node_page_state(NR_FILE_DIRTY) +
- global_node_page_state(NR_WRITEBACK);
-
- mem_cgroup_wb_stats(wb, &filepages, &headroom,
- &mdtc.dirty, &writeback);
- mdtc.dirty += writeback;
- mdtc_calc_avail(&mdtc, filepages, headroom);
+ domain_dirty_avail(&gdtc, true);
+ domain_dirty_avail(&mdtc, true);
domain_dirty_limits(&mdtc);
return __wb_calc_thresh(&mdtc, mdtc.thresh);
@@ -1719,9 +1740,8 @@ static int balance_dirty_pages(struct bdi_writeback *wb,
unsigned long m_bg_thresh = 0;
nr_dirty = global_node_page_state(NR_FILE_DIRTY);
- gdtc->avail = global_dirtyable_memory();
- gdtc->dirty = nr_dirty + global_node_page_state(NR_WRITEBACK);
+ domain_dirty_avail(gdtc, true);
domain_dirty_limits(gdtc);
if (unlikely(strictlimit)) {
@@ -1737,17 +1757,11 @@ static int balance_dirty_pages(struct bdi_writeback *wb,
}
if (mdtc) {
- unsigned long filepages, headroom, writeback;
-
/*
* If @wb belongs to !root memcg, repeat the same
* basic calculations for the memcg domain.
*/
- mem_cgroup_wb_stats(wb, &filepages, &headroom,
- &mdtc->dirty, &writeback);
- mdtc->dirty += writeback;
- mdtc_calc_avail(mdtc, filepages, headroom);
-
+ domain_dirty_avail(mdtc, true);
domain_dirty_limits(mdtc);
if (unlikely(strictlimit)) {
@@ -2119,14 +2133,8 @@ bool wb_over_bg_thresh(struct bdi_writeback *wb)
struct dirty_throttle_control * const mdtc = mdtc_valid(&mdtc_stor) ?
&mdtc_stor : NULL;
- /*
- * Similar to balance_dirty_pages() but ignores pages being written
- * as we're trying to decide whether to put more under writeback.
- */
- gdtc->avail = global_dirtyable_memory();
- gdtc->dirty = global_node_page_state(NR_FILE_DIRTY);
+ domain_dirty_avail(gdtc, false);
domain_dirty_limits(gdtc);
-
if (gdtc->dirty > gdtc->bg_thresh)
return true;
@@ -2135,13 +2143,8 @@ bool wb_over_bg_thresh(struct bdi_writeback *wb)
return true;
if (mdtc) {
- unsigned long filepages, headroom, writeback;
-
- mem_cgroup_wb_stats(wb, &filepages, &headroom, &mdtc->dirty,
- &writeback);
- mdtc_calc_avail(mdtc, filepages, headroom);
+ domain_dirty_avail(mdtc, false);
domain_dirty_limits(mdtc); /* ditto, ignore writeback */
-
if (mdtc->dirty > mdtc->bg_thresh)
return true;
--
2.30.0
Factor out wb_dirty_exceeded to remove repeated code
Signed-off-by: Kemeng Shi <[email protected]>
---
mm/page-writeback.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 97ee5b32b4ef..0f1f3e179be2 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -140,6 +140,7 @@ struct dirty_throttle_control {
unsigned long pos_ratio;
bool freerun;
+ bool dirty_exceeded;
};
/*
@@ -1775,6 +1776,13 @@ static void wb_dirty_freerun(struct dirty_throttle_control *dtc,
dirty_freerun_ceiling(dtc->wb_thresh, dtc->wb_bg_thresh);
}
+static inline void wb_dirty_exceeded(struct dirty_throttle_control *dtc,
+ bool strictlimit)
+{
+ dtc->dirty_exceeded = (dtc->wb_dirty > dtc->wb_thresh) &&
+ ((dtc->dirty > dtc->thresh) || strictlimit);
+}
+
/*
* balance_dirty_pages() must be called by processes which are generating dirty
* data. It looks at the number of dirty pages in the machine and will force
@@ -1797,7 +1805,6 @@ static int balance_dirty_pages(struct bdi_writeback *wb,
long max_pause;
long min_pause;
int nr_dirtied_pause;
- bool dirty_exceeded = false;
unsigned long task_ratelimit;
unsigned long dirty_ratelimit;
struct backing_dev_info *bdi = wb->bdi;
@@ -1866,9 +1873,7 @@ static int balance_dirty_pages(struct bdi_writeback *wb,
if (gdtc->freerun)
goto free_running;
- dirty_exceeded = (gdtc->wb_dirty > gdtc->wb_thresh) &&
- ((gdtc->dirty > gdtc->thresh) || strictlimit);
-
+ wb_dirty_exceeded(gdtc, strictlimit);
wb_position_ratio(gdtc);
sdtc = gdtc;
@@ -1883,17 +1888,14 @@ static int balance_dirty_pages(struct bdi_writeback *wb,
if (mdtc->freerun)
goto free_running;
- dirty_exceeded |= (mdtc->wb_dirty > mdtc->wb_thresh) &&
- ((mdtc->dirty > mdtc->thresh) || strictlimit);
-
+ wb_dirty_exceeded(mdtc, strictlimit);
wb_position_ratio(mdtc);
if (mdtc->pos_ratio < gdtc->pos_ratio)
sdtc = mdtc;
}
- if (dirty_exceeded != wb->dirty_exceeded)
- wb->dirty_exceeded = dirty_exceeded;
-
+ wb->dirty_exceeded = gdtc->dirty_exceeded ||
+ (mdtc && mdtc->dirty_exceeded);
if (time_is_before_jiffies(READ_ONCE(wb->bw_time_stamp) +
BANDWIDTH_INTERVAL))
__wb_update_bandwidth(gdtc, mdtc, true);
--
2.30.0
Factor out wb_dirty_freerun to remove more repeated freerun code.
Signed-off-by: Kemeng Shi <[email protected]>
---
mm/page-writeback.c | 55 +++++++++++++++++++++++----------------------
1 file changed, 28 insertions(+), 27 deletions(-)
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index b7478eacd3ff..b9c8db7089ef 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -1746,6 +1746,27 @@ static void domain_dirty_freerun(struct dirty_throttle_control *dtc,
dtc->freerun = dirty <= dirty_freerun_ceiling(thresh, bg_thresh);
}
+static void wb_dirty_freerun(struct dirty_throttle_control *dtc,
+ bool strictlimit)
+{
+ dtc->freerun = false;
+
+ /* was already handled in domain_dirty_freerun */
+ if (strictlimit)
+ return;
+
+ wb_dirty_limits(dtc);
+ /*
+ * LOCAL_THROTTLE tasks must not be throttled when below the per-wb
+ * freerun ceiling.
+ */
+ if (!(current->flags & PF_LOCAL_THROTTLE))
+ return;
+
+ dtc->freerun = dtc->wb_dirty <
+ dirty_freerun_ceiling(dtc->wb_thresh, dtc->wb_bg_thresh);
+}
+
/*
* balance_dirty_pages() must be called by processes which are generating dirty
* data. It looks at the number of dirty pages in the machine and will force
@@ -1838,19 +1859,9 @@ static int balance_dirty_pages(struct bdi_writeback *wb,
* Calculate global domain's pos_ratio and select the
* global dtc by default.
*/
- if (!strictlimit) {
- wb_dirty_limits(gdtc);
-
- if ((current->flags & PF_LOCAL_THROTTLE) &&
- gdtc->wb_dirty <
- dirty_freerun_ceiling(gdtc->wb_thresh,
- gdtc->wb_bg_thresh))
- /*
- * LOCAL_THROTTLE tasks must not be throttled
- * when below the per-wb freerun ceiling.
- */
- goto free_running;
- }
+ wb_dirty_freerun(gdtc, strictlimit);
+ if (gdtc->freerun)
+ goto free_running;
dirty_exceeded = (gdtc->wb_dirty > gdtc->wb_thresh) &&
((gdtc->dirty > gdtc->thresh) || strictlimit);
@@ -1865,20 +1876,10 @@ static int balance_dirty_pages(struct bdi_writeback *wb,
* both global and memcg domains. Choose the one
* w/ lower pos_ratio.
*/
- if (!strictlimit) {
- wb_dirty_limits(mdtc);
-
- if ((current->flags & PF_LOCAL_THROTTLE) &&
- mdtc->wb_dirty <
- dirty_freerun_ceiling(mdtc->wb_thresh,
- mdtc->wb_bg_thresh))
- /*
- * LOCAL_THROTTLE tasks must not be
- * throttled when below the per-wb
- * freerun ceiling.
- */
- goto free_running;
- }
+ wb_dirty_freerun(mdtc, strictlimit);
+ if (mdtc->freerun)
+ goto free_running;
+
dirty_exceeded |= (mdtc->wb_dirty > mdtc->wb_thresh) &&
((mdtc->dirty > mdtc->thresh) || strictlimit);
--
2.30.0
Factor out domain_over_bg_thresh from wb_over_bg_thresh to remove
repeated code.
Signed-off-by: Kemeng Shi <[email protected]>
Acked-by: Tejun Heo <[email protected]>
---
mm/page-writeback.c | 41 +++++++++++++++++++----------------------
1 file changed, 19 insertions(+), 22 deletions(-)
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 16a94c7df260..e7c6ad48f738 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2116,6 +2116,20 @@ static void wb_bg_dirty_limits(struct dirty_throttle_control *dtc)
dtc->wb_dirty = wb_stat(wb, WB_RECLAIMABLE);
}
+static bool domain_over_bg_thresh(struct dirty_throttle_control *dtc)
+{
+ domain_dirty_avail(dtc, false);
+ domain_dirty_limits(dtc);
+ if (dtc->dirty > dtc->bg_thresh)
+ return true;
+
+ wb_bg_dirty_limits(dtc);
+ if (dtc->wb_dirty > dtc->wb_bg_thresh)
+ return true;
+
+ return false;
+}
+
/**
* wb_over_bg_thresh - does @wb need to be written back?
* @wb: bdi_writeback of interest
@@ -2127,31 +2141,14 @@ static void wb_bg_dirty_limits(struct dirty_throttle_control *dtc)
*/
bool wb_over_bg_thresh(struct bdi_writeback *wb)
{
- struct dirty_throttle_control gdtc_stor = { GDTC_INIT(wb) };
- struct dirty_throttle_control mdtc_stor = { MDTC_INIT(wb, &gdtc_stor) };
- struct dirty_throttle_control * const gdtc = &gdtc_stor;
- struct dirty_throttle_control * const mdtc = mdtc_valid(&mdtc_stor) ?
- &mdtc_stor : NULL;
-
- domain_dirty_avail(gdtc, false);
- domain_dirty_limits(gdtc);
- if (gdtc->dirty > gdtc->bg_thresh)
- return true;
+ struct dirty_throttle_control gdtc = { GDTC_INIT(wb) };
+ struct dirty_throttle_control mdtc = { MDTC_INIT(wb, &gdtc) };
- wb_bg_dirty_limits(gdtc);
- if (gdtc->wb_dirty > gdtc->wb_bg_thresh)
+ if (domain_over_bg_thresh(&gdtc))
return true;
- if (mdtc) {
- domain_dirty_avail(mdtc, false);
- domain_dirty_limits(mdtc); /* ditto, ignore writeback */
- if (mdtc->dirty > mdtc->bg_thresh)
- return true;
-
- wb_bg_dirty_limits(mdtc);
- if (mdtc->wb_dirty > mdtc->wb_bg_thresh)
- return true;
- }
+ if (mdtc_valid(&mdtc))
+ return domain_over_bg_thresh(&mdtc);
return false;
}
--
2.30.0
Factor out balance_wb_limits to remove repeated code
Signed-off-by: Kemeng Shi <[email protected]>
---
mm/page-writeback.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 0f1f3e179be2..d1d385373c5b 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -1783,6 +1783,17 @@ static inline void wb_dirty_exceeded(struct dirty_throttle_control *dtc,
((dtc->dirty > dtc->thresh) || strictlimit);
}
+static void balance_wb_limits(struct dirty_throttle_control *dtc,
+ bool strictlimit)
+{
+ wb_dirty_freerun(dtc, strictlimit);
+ if (dtc->freerun)
+ return;
+
+ wb_dirty_exceeded(dtc, strictlimit);
+ wb_position_ratio(dtc);
+}
+
/*
* balance_dirty_pages() must be called by processes which are generating dirty
* data. It looks at the number of dirty pages in the machine and will force
@@ -1869,12 +1880,9 @@ static int balance_dirty_pages(struct bdi_writeback *wb,
* Calculate global domain's pos_ratio and select the
* global dtc by default.
*/
- wb_dirty_freerun(gdtc, strictlimit);
+ balance_wb_limits(gdtc, strictlimit);
if (gdtc->freerun)
goto free_running;
-
- wb_dirty_exceeded(gdtc, strictlimit);
- wb_position_ratio(gdtc);
sdtc = gdtc;
if (mdtc) {
@@ -1884,12 +1892,9 @@ static int balance_dirty_pages(struct bdi_writeback *wb,
* both global and memcg domains. Choose the one
* w/ lower pos_ratio.
*/
- wb_dirty_freerun(mdtc, strictlimit);
+ balance_wb_limits(mdtc, strictlimit);
if (mdtc->freerun)
goto free_running;
-
- wb_dirty_exceeded(mdtc, strictlimit);
- wb_position_ratio(mdtc);
if (mdtc->pos_ratio < gdtc->pos_ratio)
sdtc = mdtc;
}
--
2.30.0
Sorry about the long delay.
On Tue, May 14, 2024 at 08:52:48PM +0800, Kemeng Shi wrote:
> Add general function domain_dirty_avail to calculate dirty and avail for
> either dirty limit or background writeback in either global domain or wb
> domain.
>
> Signed-off-by: Kemeng Shi <[email protected]>
Acked-by: Tejun Heo <[email protected]>
Thanks.
--
tejun
On Tue, May 14, 2024 at 08:52:51PM +0800, Kemeng Shi wrote:
> Factor out wb_dirty_freerun to remove more repeated freerun code.
>
> Signed-off-by: Kemeng Shi <[email protected]>
Acked-by: Tejun Heo <[email protected]>
Thanks.
--
tejun
On Tue, May 14, 2024 at 08:52:53PM +0800, Kemeng Shi wrote:
> Factor out wb_dirty_exceeded to remove repeated code
>
> Signed-off-by: Kemeng Shi <[email protected]>
Acked-by: Tejun Heo <[email protected]>
Thanks.
--
tejun
Hello,
On Tue, May 14, 2024 at 08:52:54PM +0800, Kemeng Shi wrote:
> +static void balance_wb_limits(struct dirty_throttle_control *dtc,
> + bool strictlimit)
> +{
> + wb_dirty_freerun(dtc, strictlimit);
> + if (dtc->freerun)
> + return;
> +
> + wb_dirty_exceeded(dtc, strictlimit);
> + wb_position_ratio(dtc);
> +}
..
> @@ -1869,12 +1880,9 @@ static int balance_dirty_pages(struct bdi_writeback *wb,
> * Calculate global domain's pos_ratio and select the
> * global dtc by default.
> */
> - wb_dirty_freerun(gdtc, strictlimit);
> + balance_wb_limits(gdtc, strictlimit);
> if (gdtc->freerun)
> goto free_running;
> -
> - wb_dirty_exceeded(gdtc, strictlimit);
> - wb_position_ratio(gdtc);
> sdtc = gdtc;
Isn't this a bit nasty? The helper skips updating states because it knows
the caller is not going to use them? I'm not sure the slight code reduction
justifies the added subtlety.
Thanks.
--
tejun
Hello,
Sorry about the long delay. The first seven patches look fine to me and
improve code readability quite a bit. Andrew, would you mind applying the
first seven?
Thanks.
--
tejun
On Thu, 30 May 2024 08:35:05 -1000 Tejun Heo <[email protected]> wrote:
> Hello,
>
> Sorry about the long delay. The first seven patches look fine to me and
> improve code readability quite a bit. Andrew, would you mind applying the
> first seven?
>
Thanks. All 8 are in the mm-unstable branch of mm.git. I've added a
note to the eighth, to wait and see how that unfolds.
Hello,
on 5/31/2024 2:33 AM, Tejun Heo wrote:
> Hello,
>
> On Tue, May 14, 2024 at 08:52:54PM +0800, Kemeng Shi wrote:
>> +static void balance_wb_limits(struct dirty_throttle_control *dtc,
>> + bool strictlimit)
>> +{
>> + wb_dirty_freerun(dtc, strictlimit);
>> + if (dtc->freerun)
>> + return;
>> +
>> + wb_dirty_exceeded(dtc, strictlimit);
>> + wb_position_ratio(dtc);
>> +}
> ...
>> @@ -1869,12 +1880,9 @@ static int balance_dirty_pages(struct bdi_writeback *wb,
>> * Calculate global domain's pos_ratio and select the
>> * global dtc by default.
>> */
>> - wb_dirty_freerun(gdtc, strictlimit);
>> + balance_wb_limits(gdtc, strictlimit);
>> if (gdtc->freerun)
>> goto free_running;
>> -
>> - wb_dirty_exceeded(gdtc, strictlimit);
>> - wb_position_ratio(gdtc);
>> sdtc = gdtc;
>
> Isn't this a bit nasty? The helper skips updating states because it knows
> the caller is not going to use them? I'm not sure the slight code reduction
> justifies the added subtlety.
It's a general rule that wb should not be limited if the wb is in freerun state.
So I think it's intuitive to obey the rule in both balance_wb_limits and it's
caller in which case balance_wb_limits and it's caller should stop to do anything
when freerun state of wb is first seen.
But no insistant on this...
Thanks.
>
> Thanks.
>
Hello,
On Mon, Jun 03, 2024 at 02:39:18PM +0800, Kemeng Shi wrote:
> > Isn't this a bit nasty? The helper skips updating states because it knows
> > the caller is not going to use them? I'm not sure the slight code reduction
> > justifies the added subtlety.
>
> It's a general rule that wb should not be limited if the wb is in freerun state.
> So I think it's intuitive to obey the rule in both balance_wb_limits and it's
> caller in which case balance_wb_limits and it's caller should stop to do anything
> when freerun state of wb is first seen.
> But no insistant on this...
Hmm... can you at least add comments pointing out that if freerun, the
limits fields are invalid?
Thanks.
--
tejun
on 6/4/2024 2:09 AM, Tejun Heo wrote:
> Hello,
>
> On Mon, Jun 03, 2024 at 02:39:18PM +0800, Kemeng Shi wrote:
>>> Isn't this a bit nasty? The helper skips updating states because it knows
>>> the caller is not going to use them? I'm not sure the slight code reduction
>>> justifies the added subtlety.
>>
>> It's a general rule that wb should not be limited if the wb is in freerun state.
>> So I think it's intuitive to obey the rule in both balance_wb_limits and it's
>> caller in which case balance_wb_limits and it's caller should stop to do anything
>> when freerun state of wb is first seen.
>> But no insistant on this...
>
> Hmm... can you at least add comments pointing out that if freerun, the
> limits fields are invalid?
Sure, will add it in next version. Thanks
>
> Thanks.
>