Linux-mm Archive mirror
 help / color / mirror / Atom feed
* [PATCH mm-unstable v1] mm/mglru: make memcg_lru->lock irq safe
@ 2023-06-19 19:38 Yu Zhao
  2023-06-20 20:19 ` Yosry Ahmed
  0 siblings, 1 reply; 2+ messages in thread
From: Yu Zhao @ 2023-06-19 19:38 UTC (permalink / raw
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, Yu Zhao, syzbot+87c490fd2be656269b6a

lru_gen_rotate_memcg() can happen in softirq if
memory.soft_limit_in_bytes is set. This requires memcg_lru->lock to be
irq safe.

This problem only affects memcg v1.

Reported-by: syzbot+87c490fd2be656269b6a@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=87c490fd2be656269b6a
Fixes: e4dde56cd208 ("mm: multi-gen LRU: per-node lru_gen_folio lists")
Signed-off-by: Yu Zhao <yuzhao@google.com>
---
 mm/vmscan.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 45d17c7cc555..27f90896f789 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4759,10 +4759,11 @@ static void lru_gen_rotate_memcg(struct lruvec *lruvec, int op)
 {
 	int seg;
 	int old, new;
+	unsigned long flags;
 	int bin = get_random_u32_below(MEMCG_NR_BINS);
 	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
 
-	spin_lock(&pgdat->memcg_lru.lock);
+	spin_lock_irqsave(&pgdat->memcg_lru.lock, flags);
 
 	VM_WARN_ON_ONCE(hlist_nulls_unhashed(&lruvec->lrugen.list));
 
@@ -4797,7 +4798,7 @@ static void lru_gen_rotate_memcg(struct lruvec *lruvec, int op)
 	if (!pgdat->memcg_lru.nr_memcgs[old] && old == get_memcg_gen(pgdat->memcg_lru.seq))
 		WRITE_ONCE(pgdat->memcg_lru.seq, pgdat->memcg_lru.seq + 1);
 
-	spin_unlock(&pgdat->memcg_lru.lock);
+	spin_unlock_irqrestore(&pgdat->memcg_lru.lock, flags);
 }
 
 void lru_gen_online_memcg(struct mem_cgroup *memcg)
@@ -4810,7 +4811,7 @@ void lru_gen_online_memcg(struct mem_cgroup *memcg)
 		struct pglist_data *pgdat = NODE_DATA(nid);
 		struct lruvec *lruvec = get_lruvec(memcg, nid);
 
-		spin_lock(&pgdat->memcg_lru.lock);
+		spin_lock_irq(&pgdat->memcg_lru.lock);
 
 		VM_WARN_ON_ONCE(!hlist_nulls_unhashed(&lruvec->lrugen.list));
 
@@ -4821,7 +4822,7 @@ void lru_gen_online_memcg(struct mem_cgroup *memcg)
 
 		lruvec->lrugen.gen = gen;
 
-		spin_unlock(&pgdat->memcg_lru.lock);
+		spin_unlock_irq(&pgdat->memcg_lru.lock);
 	}
 }
 
@@ -4845,7 +4846,7 @@ void lru_gen_release_memcg(struct mem_cgroup *memcg)
 		struct pglist_data *pgdat = NODE_DATA(nid);
 		struct lruvec *lruvec = get_lruvec(memcg, nid);
 
-		spin_lock(&pgdat->memcg_lru.lock);
+		spin_lock_irq(&pgdat->memcg_lru.lock);
 
 		VM_WARN_ON_ONCE(hlist_nulls_unhashed(&lruvec->lrugen.list));
 
@@ -4857,7 +4858,7 @@ void lru_gen_release_memcg(struct mem_cgroup *memcg)
 		if (!pgdat->memcg_lru.nr_memcgs[gen] && gen == get_memcg_gen(pgdat->memcg_lru.seq))
 			WRITE_ONCE(pgdat->memcg_lru.seq, pgdat->memcg_lru.seq + 1);
 
-		spin_unlock(&pgdat->memcg_lru.lock);
+		spin_unlock_irq(&pgdat->memcg_lru.lock);
 	}
 }
 
-- 
2.41.0.185.g7c58973941-goog



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH mm-unstable v1] mm/mglru: make memcg_lru->lock irq safe
  2023-06-19 19:38 [PATCH mm-unstable v1] mm/mglru: make memcg_lru->lock irq safe Yu Zhao
@ 2023-06-20 20:19 ` Yosry Ahmed
  0 siblings, 0 replies; 2+ messages in thread
From: Yosry Ahmed @ 2023-06-20 20:19 UTC (permalink / raw
  To: Yu Zhao; +Cc: Andrew Morton, linux-mm, linux-kernel,
	syzbot+87c490fd2be656269b6a

On Mon, Jun 19, 2023 at 12:38 PM Yu Zhao <yuzhao@google.com> wrote:
>
> lru_gen_rotate_memcg() can happen in softirq if
> memory.soft_limit_in_bytes is set. This requires memcg_lru->lock to be
> irq safe.
>
> This problem only affects memcg v1.
>
> Reported-by: syzbot+87c490fd2be656269b6a@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=87c490fd2be656269b6a
> Fixes: e4dde56cd208 ("mm: multi-gen LRU: per-node lru_gen_folio lists")
> Signed-off-by: Yu Zhao <yuzhao@google.com>
> ---
>  mm/vmscan.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 45d17c7cc555..27f90896f789 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4759,10 +4759,11 @@ static void lru_gen_rotate_memcg(struct lruvec *lruvec, int op)
>  {
>         int seg;
>         int old, new;
> +       unsigned long flags;
>         int bin = get_random_u32_below(MEMCG_NR_BINS);
>         struct pglist_data *pgdat = lruvec_pgdat(lruvec);
>
> -       spin_lock(&pgdat->memcg_lru.lock);
> +       spin_lock_irqsave(&pgdat->memcg_lru.lock, flags);

Nit: I think it might be useful to add a comment here that this is
needed due to the call path from memcg_check_events() ->
mem_cgroup_update_tree() -- so that if that call path changes we can
come back and remove the irq-disablement.

FWIW:
Reviewed-by: Yosry Ahmed <yosryahmed@google.com>

>
>         VM_WARN_ON_ONCE(hlist_nulls_unhashed(&lruvec->lrugen.list));
>
> @@ -4797,7 +4798,7 @@ static void lru_gen_rotate_memcg(struct lruvec *lruvec, int op)
>         if (!pgdat->memcg_lru.nr_memcgs[old] && old == get_memcg_gen(pgdat->memcg_lru.seq))
>                 WRITE_ONCE(pgdat->memcg_lru.seq, pgdat->memcg_lru.seq + 1);
>
> -       spin_unlock(&pgdat->memcg_lru.lock);
> +       spin_unlock_irqrestore(&pgdat->memcg_lru.lock, flags);
>  }
>
>  void lru_gen_online_memcg(struct mem_cgroup *memcg)
> @@ -4810,7 +4811,7 @@ void lru_gen_online_memcg(struct mem_cgroup *memcg)
>                 struct pglist_data *pgdat = NODE_DATA(nid);
>                 struct lruvec *lruvec = get_lruvec(memcg, nid);
>
> -               spin_lock(&pgdat->memcg_lru.lock);
> +               spin_lock_irq(&pgdat->memcg_lru.lock);
>
>                 VM_WARN_ON_ONCE(!hlist_nulls_unhashed(&lruvec->lrugen.list));
>
> @@ -4821,7 +4822,7 @@ void lru_gen_online_memcg(struct mem_cgroup *memcg)
>
>                 lruvec->lrugen.gen = gen;
>
> -               spin_unlock(&pgdat->memcg_lru.lock);
> +               spin_unlock_irq(&pgdat->memcg_lru.lock);
>         }
>  }
>
> @@ -4845,7 +4846,7 @@ void lru_gen_release_memcg(struct mem_cgroup *memcg)
>                 struct pglist_data *pgdat = NODE_DATA(nid);
>                 struct lruvec *lruvec = get_lruvec(memcg, nid);
>
> -               spin_lock(&pgdat->memcg_lru.lock);
> +               spin_lock_irq(&pgdat->memcg_lru.lock);
>
>                 VM_WARN_ON_ONCE(hlist_nulls_unhashed(&lruvec->lrugen.list));
>
> @@ -4857,7 +4858,7 @@ void lru_gen_release_memcg(struct mem_cgroup *memcg)
>                 if (!pgdat->memcg_lru.nr_memcgs[gen] && gen == get_memcg_gen(pgdat->memcg_lru.seq))
>                         WRITE_ONCE(pgdat->memcg_lru.seq, pgdat->memcg_lru.seq + 1);
>
> -               spin_unlock(&pgdat->memcg_lru.lock);
> +               spin_unlock_irq(&pgdat->memcg_lru.lock);
>         }
>  }
>
> --
> 2.41.0.185.g7c58973941-goog
>
>


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-06-20 20:20 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-19 19:38 [PATCH mm-unstable v1] mm/mglru: make memcg_lru->lock irq safe Yu Zhao
2023-06-20 20:19 ` Yosry Ahmed

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).