[RFC,v2,0/7] mm: zswap: move writeback LRU from zpool to zswap

  This series aims to improve the zswap reclaim mechanism by reorganizing
the LRU management. In the current implementation, the LRU is maintained
within each zpool driver, resulting in duplicated code across the three
drivers. The proposed change consists in moving the LRU management from
the individual implementations up to the zswap layer.

The primary objective of this refactoring effort is to simplify the
codebase. By unifying the reclaim loop and consolidating LRU handling
within zswap, we can eliminate redundant code and improve
maintainability. Additionally, this change enables the reclamation of
stored pages in their actual LRU order. Presently, the zpool drivers
link backing pages in an LRU, causing compressed pages with different
LRU positions to be written back simultaneously.

The series consists of several patches. The first patch implements the
LRU and the reclaim loop in zswap, but it is not used yet because all
three driver implementations are marked as zpool_evictable.
The following three commits modify each zpool driver to be not
zpool_evictable, allowing the use of the reclaim loop in zswap.
As the drivers removed their shrink functions, the zpool interface is
then trimmed by removing zpool_evictable, zpool_ops, and zpool_shrink.
Finally, the code in zswap is further cleaned up by simplifying the
writeback function and removing the now unnecessary zswap_header.

Based on mm-stable + commit 399ab221f3ff
("mm: zswap: shrink until can accept") currently in mm-unstable.

V2:
- fixed lru list init/del/del_init (Johannes)
- renamed pool.lock to lru_lock and added lock ordering comment (Yosry)
- trimmed zsmalloc even more (Johannes | Nhat)
- moved ref drop out of writeback function  (Johannes)

Domenico Cerasuolo (7):
  mm: zswap: add pool shrinking mechanism
  mm: zswap: remove page reclaim logic from zbud
  mm: zswap: remove page reclaim logic from z3fold
  mm: zswap: remove page reclaim logic from zsmalloc
  mm: zswap: remove shrink from zpool interface
  mm: zswap: simplify writeback function
  mm: zswap: remove zswap_header

 include/linux/zpool.h |  19 +-
 mm/z3fold.c           | 249 +-------------------------
 mm/zbud.c             | 167 +-----------------
 mm/zpool.c            |  48 +----
 mm/zsmalloc.c         | 396 ++----------------------------------------
 mm/zswap.c            | 186 +++++++++++---------
 6 files changed, 130 insertions(+), 935 deletions(-)
  

On Tue, Jun 6, 2023 at 7:56 AM Domenico Cerasuolo
<cerasuolodomenico@gmail.com> wrote:
>
> This series aims to improve the zswap reclaim mechanism by reorganizing
> the LRU management. In the current implementation, the LRU is maintained
> within each zpool driver, resulting in duplicated code across the three
> drivers. The proposed change consists in moving the LRU management from
> the individual implementations up to the zswap layer.
>
> The primary objective of this refactoring effort is to simplify the
> codebase. By unifying the reclaim loop and consolidating LRU handling
> within zswap, we can eliminate redundant code and improve
> maintainability. Additionally, this change enables the reclamation of
> stored pages in their actual LRU order. Presently, the zpool drivers
> link backing pages in an LRU, causing compressed pages with different
> LRU positions to be written back simultaneously.
>
> The series consists of several patches. The first patch implements the
> LRU and the reclaim loop in zswap, but it is not used yet because all
> three driver implementations are marked as zpool_evictable.
> The following three commits modify each zpool driver to be not
> zpool_evictable, allowing the use of the reclaim loop in zswap.
> As the drivers removed their shrink functions, the zpool interface is
> then trimmed by removing zpool_evictable, zpool_ops, and zpool_shrink.
> Finally, the code in zswap is further cleaned up by simplifying the
> writeback function and removing the now unnecessary zswap_header.
>
> Based on mm-stable + commit 399ab221f3ff
> ("mm: zswap: shrink until can accept") currently in mm-unstable.

I tested this + commit fe1d1f7d0fb5 ("mm: zswap: support exclusive
loads") currently in mm-unstable, using zsmalloc and
CONFIG_ZSWAP_EXCLUSIVE_LOADS=y. I only ran basic zswap tests with
manual writeback induction and made sure everything is sane. I
obviously hope you did more involved testing :)

The only problem I came across is the conflict with fe1d1f7d0fb5, and
I suggested the fix in patch 1. With the fix, everything seems
correct.

So I guess, FWIW for all the patches except 2 & 3 (for zbud and z3fold):
Tested-by: Yosry Ahmed <yosryahmed@google.com>

>
> V2:
> - fixed lru list init/del/del_init (Johannes)
> - renamed pool.lock to lru_lock and added lock ordering comment (Yosry)
> - trimmed zsmalloc even more (Johannes | Nhat)
> - moved ref drop out of writeback function  (Johannes)
>
> Domenico Cerasuolo (7):
>   mm: zswap: add pool shrinking mechanism
>   mm: zswap: remove page reclaim logic from zbud
>   mm: zswap: remove page reclaim logic from z3fold
>   mm: zswap: remove page reclaim logic from zsmalloc
>   mm: zswap: remove shrink from zpool interface
>   mm: zswap: simplify writeback function
>   mm: zswap: remove zswap_header
>
>  include/linux/zpool.h |  19 +-
>  mm/z3fold.c           | 249 +-------------------------
>  mm/zbud.c             | 167 +-----------------
>  mm/zpool.c            |  48 +----
>  mm/zsmalloc.c         | 396 ++----------------------------------------
>  mm/zswap.c            | 186 +++++++++++---------
>  6 files changed, 130 insertions(+), 935 deletions(-)
>
> --
> 2.34.1
>
  

On Wed, Jun 7, 2023 at 11:16 AM Yosry Ahmed <yosryahmed@google.com> wrote:
>
> On Tue, Jun 6, 2023 at 7:56 AM Domenico Cerasuolo
> <cerasuolodomenico@gmail.com> wrote:
> >
> > This series aims to improve the zswap reclaim mechanism by reorganizing
> > the LRU management. In the current implementation, the LRU is maintained
> > within each zpool driver, resulting in duplicated code across the three
> > drivers. The proposed change consists in moving the LRU management from
> > the individual implementations up to the zswap layer.
> >
> > The primary objective of this refactoring effort is to simplify the
> > codebase. By unifying the reclaim loop and consolidating LRU handling
> > within zswap, we can eliminate redundant code and improve
> > maintainability. Additionally, this change enables the reclamation of
> > stored pages in their actual LRU order. Presently, the zpool drivers
> > link backing pages in an LRU, causing compressed pages with different
> > LRU positions to be written back simultaneously.
> >
> > The series consists of several patches. The first patch implements the
> > LRU and the reclaim loop in zswap, but it is not used yet because all
> > three driver implementations are marked as zpool_evictable.
> > The following three commits modify each zpool driver to be not
> > zpool_evictable, allowing the use of the reclaim loop in zswap.
> > As the drivers removed their shrink functions, the zpool interface is
> > then trimmed by removing zpool_evictable, zpool_ops, and zpool_shrink.
> > Finally, the code in zswap is further cleaned up by simplifying the
> > writeback function and removing the now unnecessary zswap_header.
> >
> > Based on mm-stable + commit 399ab221f3ff
> > ("mm: zswap: shrink until can accept") currently in mm-unstable.
>
> I tested this + commit fe1d1f7d0fb5 ("mm: zswap: support exclusive
> loads") currently in mm-unstable, using zsmalloc and
> CONFIG_ZSWAP_EXCLUSIVE_LOADS=y. I only ran basic zswap tests with
> manual writeback induction and made sure everything is sane. I
> obviously hope you did more involved testing :)
>
> The only problem I came across is the conflict with fe1d1f7d0fb5, and
> I suggested the fix in patch 1. With the fix, everything seems
> correct.
>
> So I guess, FWIW for all the patches except 2 & 3 (for zbud and z3fold):
> Tested-by: Yosry Ahmed <yosryahmed@google.com>

Thanks a lot for the effort! I'll rebase and test it again before submitting the
new version.

>
> >
> > V2:
> > - fixed lru list init/del/del_init (Johannes)
> > - renamed pool.lock to lru_lock and added lock ordering comment (Yosry)
> > - trimmed zsmalloc even more (Johannes | Nhat)
> > - moved ref drop out of writeback function  (Johannes)
> >
> > Domenico Cerasuolo (7):
> >   mm: zswap: add pool shrinking mechanism
> >   mm: zswap: remove page reclaim logic from zbud
> >   mm: zswap: remove page reclaim logic from z3fold
> >   mm: zswap: remove page reclaim logic from zsmalloc
> >   mm: zswap: remove shrink from zpool interface
> >   mm: zswap: simplify writeback function
> >   mm: zswap: remove zswap_header
> >
> >  include/linux/zpool.h |  19 +-
> >  mm/z3fold.c           | 249 +-------------------------
> >  mm/zbud.c             | 167 +-----------------
> >  mm/zpool.c            |  48 +----
> >  mm/zsmalloc.c         | 396 ++----------------------------------------
> >  mm/zswap.c            | 186 +++++++++++---------
> >  6 files changed, 130 insertions(+), 935 deletions(-)
> >
> > --
> > 2.34.1
> >
  

On Wed, Jun 7, 2023 at 2:24 AM Domenico Cerasuolo
<cerasuolodomenico@gmail.com> wrote:
>
> On Wed, Jun 7, 2023 at 11:16 AM Yosry Ahmed <yosryahmed@google.com> wrote:
> >
> > On Tue, Jun 6, 2023 at 7:56 AM Domenico Cerasuolo
> > <cerasuolodomenico@gmail.com> wrote:
> > >
> > > This series aims to improve the zswap reclaim mechanism by reorganizing
> > > the LRU management. In the current implementation, the LRU is maintained
> > > within each zpool driver, resulting in duplicated code across the three
> > > drivers. The proposed change consists in moving the LRU management from
> > > the individual implementations up to the zswap layer.
> > >
> > > The primary objective of this refactoring effort is to simplify the
> > > codebase. By unifying the reclaim loop and consolidating LRU handling
> > > within zswap, we can eliminate redundant code and improve
> > > maintainability. Additionally, this change enables the reclamation of
> > > stored pages in their actual LRU order. Presently, the zpool drivers
> > > link backing pages in an LRU, causing compressed pages with different
> > > LRU positions to be written back simultaneously.
> > >
> > > The series consists of several patches. The first patch implements the
> > > LRU and the reclaim loop in zswap, but it is not used yet because all
> > > three driver implementations are marked as zpool_evictable.
> > > The following three commits modify each zpool driver to be not
> > > zpool_evictable, allowing the use of the reclaim loop in zswap.
> > > As the drivers removed their shrink functions, the zpool interface is
> > > then trimmed by removing zpool_evictable, zpool_ops, and zpool_shrink.
> > > Finally, the code in zswap is further cleaned up by simplifying the
> > > writeback function and removing the now unnecessary zswap_header.
> > >
> > > Based on mm-stable + commit 399ab221f3ff
> > > ("mm: zswap: shrink until can accept") currently in mm-unstable.
> >
> > I tested this + commit fe1d1f7d0fb5 ("mm: zswap: support exclusive
> > loads") currently in mm-unstable, using zsmalloc and
> > CONFIG_ZSWAP_EXCLUSIVE_LOADS=y. I only ran basic zswap tests with
> > manual writeback induction and made sure everything is sane. I
> > obviously hope you did more involved testing :)
> >
> > The only problem I came across is the conflict with fe1d1f7d0fb5, and
> > I suggested the fix in patch 1. With the fix, everything seems
> > correct.
> >
> > So I guess, FWIW for all the patches except 2 & 3 (for zbud and z3fold):
> > Tested-by: Yosry Ahmed <yosryahmed@google.com>
>
> Thanks a lot for the effort! I'll rebase and test it again before submitting the
> new version.

Perhaps give v2 a little bit more time to give other folks a chance to
take a look as well, save yourself (and probably Andrew) the trouble
of sending a new version for every single review :)

>
> >
> > >
> > > V2:
> > > - fixed lru list init/del/del_init (Johannes)
> > > - renamed pool.lock to lru_lock and added lock ordering comment (Yosry)
> > > - trimmed zsmalloc even more (Johannes | Nhat)
> > > - moved ref drop out of writeback function  (Johannes)
> > >
> > > Domenico Cerasuolo (7):
> > >   mm: zswap: add pool shrinking mechanism
> > >   mm: zswap: remove page reclaim logic from zbud
> > >   mm: zswap: remove page reclaim logic from z3fold
> > >   mm: zswap: remove page reclaim logic from zsmalloc
> > >   mm: zswap: remove shrink from zpool interface
> > >   mm: zswap: simplify writeback function
> > >   mm: zswap: remove zswap_header
> > >
> > >  include/linux/zpool.h |  19 +-
> > >  mm/z3fold.c           | 249 +-------------------------
> > >  mm/zbud.c             | 167 +-----------------
> > >  mm/zpool.c            |  48 +----
> > >  mm/zsmalloc.c         | 396 ++----------------------------------------
> > >  mm/zswap.c            | 186 +++++++++++---------
> > >  6 files changed, 130 insertions(+), 935 deletions(-)
> > >
> > > --
> > > 2.34.1
> > >