On Tue, Apr 4, 2023 at 3:31 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > > On Tue, 4 Apr 2023 15:28:16 -0700 Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > > > On Tue, 4 Apr 2023 15:00:57 -0700 Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote: > > > > > ... > > > > > > > > > > > > > Without refactoring the code that adds reclaim_state->reclaimed to > > > > > scan_control->nr_reclaimed into a helper (flush_reclaim_state()), the > > > > > change would need to be done in two places instead of one, and I > > > > > wouldn't know where to put the huge comment. > > > > > > > > Well, all depends on how desirable it it that we backport. If "not > > > > desirable" then leave things as-is. If at least "possibly desirable" > > > > then a simple patch with the two changes and no elaborate comment will > > > > suit. > > > > > > > > > > I would rather leave the current series as-is with an elaborate > > > comment. I can send a separate single patch as a backport to stable if > > > this is something that we usually do (though I am not sure how to > > > format such patch). > > > > -stable maintainers prefer to take something which has already been > > accepted by Linus. > > > > The series could be as simple as > > > > simple-two-liner.patch > > revert-simple-two-liner.patch > > this-series-as-is.patch > > > > simple-two-liner.patch goes into 6.3-rcX and -stable. The other > > patches into 6.4-rc1. > > But the key question remains: how desirable is a backport? > > Looking at the changelogs I'm not seeing a clear statement of the > impact upon real-world users' real-world workloads. (This is a hint). > So I am unable to judge. > > Please share your thoughts on this. I think it's nice to have but not really important. It occasionally causes writes to memory.reclaim to report false positives and *might* cause unnecessary retrying when charging memory, but probably too rare to be a practical problem. Personally, I intend to backport to our kernel at Google because it's a simple enough fix and we have occasionally seen test flakiness without it. I have a reworked version of the series that only has 2 patches: - simple-two-liner-patch (actually 5 lines) - one patch including all refactoring squashed (introducing flush_reclaim_state() with the huge comment, introducing mm_account_reclaimed_pages(), and moving set_task_reclaim_state() around). Let me know if you want me to send it as v5, or leave the current v4 if you think backporting is not generally important. >
