2024-01-30 00:09:13

by Yosry Ahmed

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] mm/zswap: don't return LRU_SKIP if we have dropped lru lock

On Sun, Jan 28, 2024 at 01:28:49PM +0000, Chengming Zhou wrote:
> LRU_SKIP can only be returned if we don't ever dropped lru lock, or
> we need to return LRU_RETRY to restart from the head of lru list.
>
> Otherwise, the iteration might continue from a cursor position that
> was freed while the locks were dropped.

Does this warrant a stable backport?

>
> Actually we may need to introduce another LRU_STOP to really terminate
> the ongoing shrinking scan process, when we encounter a warm page
> already in the swap cache. The current list_lru implementation
> doesn't have this function to early break from __list_lru_walk_one.
>
> Fixes: b5ba474f3f51 ("zswap: shrink zswap pool based on memory pressure")
> Acked-by: Johannes Weiner <[email protected]>
> Reviewed-by: Nhat Pham <[email protected]>
> Signed-off-by: Chengming Zhou <[email protected]>

Acked-by: Yosry Ahmed <[email protected]>


2024-01-30 00:13:16

by Nhat Pham

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] mm/zswap: don't return LRU_SKIP if we have dropped lru lock

On Mon, Jan 29, 2024 at 4:09 PM Yosry Ahmed <[email protected]> wrote:
>
> On Sun, Jan 28, 2024 at 01:28:49PM +0000, Chengming Zhou wrote:
> > LRU_SKIP can only be returned if we don't ever dropped lru lock, or
> > we need to return LRU_RETRY to restart from the head of lru list.
> >
> > Otherwise, the iteration might continue from a cursor position that
> > was freed while the locks were dropped.
>
> Does this warrant a stable backport?

IUC, the zswap shrinker was merged in 6.8, and we're still in the RC's
for 6.8, right? If this patch goes into 6.8 then no need?
Otherwise, yeah it should go to 6.8 stable IMHO.

>
> >
> > Actually we may need to introduce another LRU_STOP to really terminate
> > the ongoing shrinking scan process, when we encounter a warm page
> > already in the swap cache. The current list_lru implementation
> > doesn't have this function to early break from __list_lru_walk_one.
> >
> > Fixes: b5ba474f3f51 ("zswap: shrink zswap pool based on memory pressure")
> > Acked-by: Johannes Weiner <[email protected]>
> > Reviewed-by: Nhat Pham <[email protected]>
> > Signed-off-by: Chengming Zhou <[email protected]>
>
> Acked-by: Yosry Ahmed <[email protected]>

2024-01-30 00:58:15

by Yosry Ahmed

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] mm/zswap: don't return LRU_SKIP if we have dropped lru lock

On Mon, Jan 29, 2024 at 04:12:54PM -0800, Nhat Pham wrote:
> On Mon, Jan 29, 2024 at 4:09 PM Yosry Ahmed <[email protected]> wrote:
> >
> > On Sun, Jan 28, 2024 at 01:28:49PM +0000, Chengming Zhou wrote:
> > > LRU_SKIP can only be returned if we don't ever dropped lru lock, or
> > > we need to return LRU_RETRY to restart from the head of lru list.
> > >
> > > Otherwise, the iteration might continue from a cursor position that
> > > was freed while the locks were dropped.
> >
> > Does this warrant a stable backport?
>
> IUC, the zswap shrinker was merged in 6.8, and we're still in the RC's
> for 6.8, right? If this patch goes into 6.8 then no need?
> Otherwise, yeah it should go to 6.8 stable IMHO.

For some reason I thought the shrinker went into v6.7, my bad. Then I
guess it should only go into v6.8.