diff mbox series

[v6,2/3] mm: vmscan: move set_task_reclaim_state() near flush_reclaim_state()

Message ID 20230413104034.1086717-3-yosryahmed@google.com
State New
Headers
Series Ignore non-LRU-based reclaim in memcg reclaim |

Commit Message

Yosry Ahmed April 13, 2023, 10:40 a.m. UTC 
  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(-)

Comments

David Hildenbrand April 13, 2023, 11:19 a.m. UTC | #1 
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.
Yosry Ahmed April 13, 2023, 11:26 a.m. UTC | #2 
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
>
Michal Hocko April 14, 2023, 8:16 a.m. UTC | #3 
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 mbox series

Patch

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.