On Tue, 19 Oct 2021, Mel Gorman wrote:
...
> + switch(reason) {
> + case VMSCAN_THROTTLE_NOPROGRESS:
> + case VMSCAN_THROTTLE_WRITEBACK:
> + timeout = HZ/10;
> +
> + if (atomic_inc_return(&pgdat->nr_writeback_throttled) == 1) {
> + WRITE_ONCE(pgdat->nr_reclaim_start,
> + node_page_state(pgdat, NR_THROTTLED_WRITTEN));
You have introduced a behaviour change that wasn't flagged in the commit
message.
Previously nr_writeback_throttled was only incremented for
VMSCAN_THROTTLE_WRITEBACK, now it is incremented for
VMSCAN_THROTTLE_NOPROGRESS as well.
Some justification would be good.
> + }
> +
> + break;
> + case VMSCAN_THROTTLE_ISOLATED:
> + timeout = HZ/50;
> + break;
> + default:
> + WARN_ON_ONCE(1);
> + timeout = HZ;
> + break;
> }
>
> prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE);
> ret = schedule_timeout(timeout);
> finish_wait(wqh, &wait);
>
> - if (acct_writeback)
> + if (reason == VMSCAN_THROTTLE_ISOLATED)
(defect) I think you want "!=" there.
While the numbers a still magic, they are now well documented and all in
one place - a definite improvement!
Thanks,
NeilBrown
