| Message ID | 20230413104034.1086717-3-yosryahmed@google.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | Ignore non-LRU-based reclaim in memcg reclaim | expand |
On 13.04.23 12:40, Yosry Ahmed wrote: > Move set_task_reclaim_state() near flush_reclaim_state() so that all > helpers manipulating reclaim_state are in close proximity. > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> > --- Hm, it's rather a simple helper to set the reclaim_state for a task, not to modify it. No strong opinion, but I'd just leave it as it.
On Thu, Apr 13, 2023 at 4:19 AM David Hildenbrand <david@redhat.com> wrote: > > On 13.04.23 12:40, Yosry Ahmed wrote: > > Move set_task_reclaim_state() near flush_reclaim_state() so that all > > helpers manipulating reclaim_state are in close proximity. > > > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> > > --- > > Hm, it's rather a simple helper to set the reclaim_state for a task, not > to modify it. > > No strong opinion, but I'd just leave it as it. It's just personal taste to have helpers acting on the same data structure next to one another. I don't feel strongly about it either, I left it as a separate patch so that we can simply drop it. Peter also thought the same, so maybe I should just drop it. > > -- > Thanks, > > David / dhildenb >
On Thu 13-04-23 10:40:33, Yosry Ahmed wrote: > Move set_task_reclaim_state() near flush_reclaim_state() so that all > helpers manipulating reclaim_state are in close proximity. > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> Acked-by: Michal Hocko <mhocko@suse.com>
diff --git a/mm/vmscan.c b/mm/vmscan.c index be657832be48..cb7d5a17c2b2 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -188,18 +188,6 @@ struct scan_control { */ int vm_swappiness = 60; -static void set_task_reclaim_state(struct task_struct *task, - struct reclaim_state *rs) -{ - /* Check for an overwrite */ - WARN_ON_ONCE(rs && task->reclaim_state); - - /* Check for the nulling of an already-nulled member */ - WARN_ON_ONCE(!rs && !task->reclaim_state); - - task->reclaim_state = rs; -} - LIST_HEAD(shrinker_list); DECLARE_RWSEM(shrinker_rwsem); @@ -511,6 +499,18 @@ static bool writeback_throttling_sane(struct scan_control *sc) } #endif +static void set_task_reclaim_state(struct task_struct *task, + struct reclaim_state *rs) +{ + /* Check for an overwrite */ + WARN_ON_ONCE(rs && task->reclaim_state); + + /* Check for the nulling of an already-nulled member */ + WARN_ON_ONCE(!rs && !task->reclaim_state); + + task->reclaim_state = rs; +} + /* * flush_reclaim_state(): add pages reclaimed outside of LRU-based reclaim to * scan_control->nr_reclaimed.
Move set_task_reclaim_state() near flush_reclaim_state() so that all helpers manipulating reclaim_state are in close proximity. Signed-off-by: Yosry Ahmed <yosryahmed@google.com> --- mm/vmscan.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)