mbox series

[00/15] add zpdesc memory descriptor for zswap.zpool

Message ID 20240621054658.1220796-1-alexs@kernel.org (mailing list archive)
Headers show
Series add zpdesc memory descriptor for zswap.zpool | expand

Message

alexs@kernel.org June 21, 2024, 5:46 a.m. UTC
From: Alex Shi <alexs@kernel.org>

According to Metthew's plan, the page descriptor will be replace by a 8
bytes mem_desc on destination purpose.
https://lore.kernel.org/lkml/YvV1KTyzZ+Jrtj9x@casper.infradead.org/

Here is a implement on z3fold to replace page descriptor by zpdesc,
which is still overlay on struct page now. but it's a step move forward
above destination.

To name the struct zpdesc instead of z3fold_desc, since there are 3 zpool
usages under zswap, zbud, z3fold, zsmalloc. It looks like we may extend the
zpdesc to zbud and zsmalloc, combined their usage into one.

For zpdesc(page), z3fold just uses the 5th member zppage_flag, which
match with page.private. Potentially uses the first member flags for
headless PG_locked, list_head lru and page.mapping|PAGE_MAPPING_MOVABLE
for page migration.

This patachset could save 26Kbyetes z3fold.o size, basely saving come
from the page to folio conversion.

Thanks a lot!
Alex

Alex Shi (15):
  mm/z3fold: add zpdesc struct and helper and use them in
    z3fold_page_isolate
  mm/z3fold: use zpdesc in z3fold_page_migrate
  mm/z3fold: use zpdesc in z3fold_page_putback
  mm/z3fold: use zpdesc in get/put_z3fold_header funcs
  mm/z3fold: use zpdesc in init_z3fold_page
  mm/z3fold: use zpdesc in free_z3fold_page
  mm/z3fold: convert page to zpdesc in __release_z3fold_page
  mm/z3fold: use zpdesc free_pages_work
  mm/z3fold: use zpdesc in z3fold_compact_page and do_compact_page
  mm/z3fold: use zpdesc in __z3fold_alloc
  mm/z3fold: use zpdesc in z3fold_alloc
  mm/z3fold: use zpdesc in free_z3fold_page and z3fold_free
  mm/z3fold: use zpdesc in z3fold_map/z3fold_unmap
  mm/z3fold: introduce __zpdesc_set_movable
  mm/z3fold: introduce __zpdesc_clear_movable

 mm/z3fold.c | 190 +++++++++++++++++++++++++++-------------------------
 mm/zpdesc.h |  87 ++++++++++++++++++++++++
 2 files changed, 184 insertions(+), 93 deletions(-)
 create mode 100644 mm/zpdesc.h

Comments

Alex Shi June 21, 2024, 6:43 a.m. UTC | #1
On 6/21/24 1:46 PM, alexs@kernel.org wrote:
> From: Alex Shi <alexs@kernel.org>
> 
> According to Metthew's plan, the page descriptor will be replace by a 8
> bytes mem_desc on destination purpose.
> https://lore.kernel.org/lkml/YvV1KTyzZ+Jrtj9x@casper.infradead.org/
> 
> Here is a implement on z3fold to replace page descriptor by zpdesc,
> which is still overlay on struct page now. but it's a step move forward
> above destination.
> 
> To name the struct zpdesc instead of z3fold_desc, since there are 3 zpool
> usages under zswap, zbud, z3fold, zsmalloc. It looks like we may extend the
> zpdesc to zbud and zsmalloc, combined their usage into one.
BTW, 
Thanks for David and Willy's suggestion!
Since all zpool candidates are single page users, then we could use zpdesc to
descript them all in future.

Thanks

> 
> For zpdesc(page), z3fold just uses the 5th member zppage_flag, which
> match with page.private. Potentially uses the first member flags for
> headless PG_locked, list_head lru and page.mapping|PAGE_MAPPING_MOVABLE
> for page migration.> 
> This patachset could save 26Kbyetes z3fold.o size, basely saving come
> from the page to folio conversion.
> 
> Thanks a lot!
> Alex
> 
> Alex Shi (15):
>   mm/z3fold: add zpdesc struct and helper and use them in
>     z3fold_page_isolate
>   mm/z3fold: use zpdesc in z3fold_page_migrate
>   mm/z3fold: use zpdesc in z3fold_page_putback
>   mm/z3fold: use zpdesc in get/put_z3fold_header funcs
>   mm/z3fold: use zpdesc in init_z3fold_page
>   mm/z3fold: use zpdesc in free_z3fold_page
>   mm/z3fold: convert page to zpdesc in __release_z3fold_page
>   mm/z3fold: use zpdesc free_pages_work
>   mm/z3fold: use zpdesc in z3fold_compact_page and do_compact_page
>   mm/z3fold: use zpdesc in __z3fold_alloc
>   mm/z3fold: use zpdesc in z3fold_alloc
>   mm/z3fold: use zpdesc in free_z3fold_page and z3fold_free
>   mm/z3fold: use zpdesc in z3fold_map/z3fold_unmap
>   mm/z3fold: introduce __zpdesc_set_movable
>   mm/z3fold: introduce __zpdesc_clear_movable
> 
>  mm/z3fold.c | 190 +++++++++++++++++++++++++++-------------------------
>  mm/zpdesc.h |  87 ++++++++++++++++++++++++
>  2 files changed, 184 insertions(+), 93 deletions(-)
>  create mode 100644 mm/zpdesc.h
>
Yosry Ahmed June 24, 2024, 9:46 p.m. UTC | #2
On Thu, Jun 20, 2024 at 10:42 PM <alexs@kernel.org> wrote:
>
> From: Alex Shi <alexs@kernel.org>
>
> According to Metthew's plan, the page descriptor will be replace by a 8
> bytes mem_desc on destination purpose.
> https://lore.kernel.org/lkml/YvV1KTyzZ+Jrtj9x@casper.infradead.org/
>
> Here is a implement on z3fold to replace page descriptor by zpdesc,
> which is still overlay on struct page now. but it's a step move forward
> above destination.
>
> To name the struct zpdesc instead of z3fold_desc, since there are 3 zpool
> usages under zswap, zbud, z3fold, zsmalloc. It looks like we may extend the
> zpdesc to zbud and zsmalloc, combined their usage into one.

Please do not focus your development efforts on z3fold. We really want
to deprecate/remove it, as well as zbud eventually. See [1].

For zsmalloc, there is already an ongoing effort to split zsdesc from
struct page [2].

[1]https://lore.kernel.org/lkml/CAJD7tkbRF6od-2x_L8-A1QL3=2Ww13sCj4S3i4bNndqF+3+_Vg@mail.gmail.com/

[2]https://lore.kernel.org/lkml/20230713042037.980211-1-42.hyeyoo@gmail.com/

>
> For zpdesc(page), z3fold just uses the 5th member zppage_flag, which
> match with page.private. Potentially uses the first member flags for
> headless PG_locked, list_head lru and page.mapping|PAGE_MAPPING_MOVABLE
> for page migration.
>
> This patachset could save 26Kbyetes z3fold.o size, basely saving come
> from the page to folio conversion.
>
> Thanks a lot!
> Alex
>
> Alex Shi (15):
>   mm/z3fold: add zpdesc struct and helper and use them in
>     z3fold_page_isolate
>   mm/z3fold: use zpdesc in z3fold_page_migrate
>   mm/z3fold: use zpdesc in z3fold_page_putback
>   mm/z3fold: use zpdesc in get/put_z3fold_header funcs
>   mm/z3fold: use zpdesc in init_z3fold_page
>   mm/z3fold: use zpdesc in free_z3fold_page
>   mm/z3fold: convert page to zpdesc in __release_z3fold_page
>   mm/z3fold: use zpdesc free_pages_work
>   mm/z3fold: use zpdesc in z3fold_compact_page and do_compact_page
>   mm/z3fold: use zpdesc in __z3fold_alloc
>   mm/z3fold: use zpdesc in z3fold_alloc
>   mm/z3fold: use zpdesc in free_z3fold_page and z3fold_free
>   mm/z3fold: use zpdesc in z3fold_map/z3fold_unmap
>   mm/z3fold: introduce __zpdesc_set_movable
>   mm/z3fold: introduce __zpdesc_clear_movable
>
>  mm/z3fold.c | 190 +++++++++++++++++++++++++++-------------------------
>  mm/zpdesc.h |  87 ++++++++++++++++++++++++
>  2 files changed, 184 insertions(+), 93 deletions(-)
>  create mode 100644 mm/zpdesc.h
>
> --
> 2.43.0
>
>
Alex Shi June 25, 2024, 8:11 a.m. UTC | #3
On 6/25/24 5:46 AM, Yosry Ahmed wrote:
> On Thu, Jun 20, 2024 at 10:42 PM <alexs@kernel.org> wrote:
>>
>> From: Alex Shi <alexs@kernel.org>
>>
>> According to Metthew's plan, the page descriptor will be replace by a 8
>> bytes mem_desc on destination purpose.
>> https://lore.kernel.org/lkml/YvV1KTyzZ+Jrtj9x@casper.infradead.org/
>>
>> Here is a implement on z3fold to replace page descriptor by zpdesc,
>> which is still overlay on struct page now. but it's a step move forward
>> above destination.
>>
>> To name the struct zpdesc instead of z3fold_desc, since there are 3 zpool
>> usages under zswap, zbud, z3fold, zsmalloc. It looks like we may extend the
>> zpdesc to zbud and zsmalloc, combined their usage into one.
> 
> Please do not focus your development efforts on z3fold. We really want
> to deprecate/remove it, as well as zbud eventually. See [1].
> 
> For zsmalloc, there is already an ongoing effort to split zsdesc from
> struct page [2].
> 
> [1]https://lore.kernel.org/lkml/CAJD7tkbRF6od-2x_L8-A1QL3=2Ww13sCj4S3i4bNndqF+3+_Vg@mail.gmail.com/

Hi Yosry,

Thanks a lot for the info and comments! It's my stupid w/o checking the email list before work on it.
Anyway don't know if z3fold would be removed, jut left this tested patchset here if someone need it.

> 
> [2]https://lore.kernel.org/lkml/20230713042037.980211-1-42.hyeyoo@gmail.com/

David had pointed out this to me few weeks ago too. This patchset hasn't updated nearly a year. If Yoo don't object I'd like to pick up from his left and update it to latest zsmalloc.c.

Thanks
Alex
> 
>>
>> For zpdesc(page), z3fold just uses the 5th member zppage_flag, which
>> match with page.private. Potentially uses the first member flags for
>> headless PG_locked, list_head lru and page.mapping|PAGE_MAPPING_MOVABLE
>> for page migration.
>>
>> This patachset could save 26Kbyetes z3fold.o size, basely saving come
>> from the page to folio conversion.
>>
>> Thanks a lot!
>> Alex
>>
>> Alex Shi (15):
>>   mm/z3fold: add zpdesc struct and helper and use them in
>>     z3fold_page_isolate
>>   mm/z3fold: use zpdesc in z3fold_page_migrate
>>   mm/z3fold: use zpdesc in z3fold_page_putback
>>   mm/z3fold: use zpdesc in get/put_z3fold_header funcs
>>   mm/z3fold: use zpdesc in init_z3fold_page
>>   mm/z3fold: use zpdesc in free_z3fold_page
>>   mm/z3fold: convert page to zpdesc in __release_z3fold_page
>>   mm/z3fold: use zpdesc free_pages_work
>>   mm/z3fold: use zpdesc in z3fold_compact_page and do_compact_page
>>   mm/z3fold: use zpdesc in __z3fold_alloc
>>   mm/z3fold: use zpdesc in z3fold_alloc
>>   mm/z3fold: use zpdesc in free_z3fold_page and z3fold_free
>>   mm/z3fold: use zpdesc in z3fold_map/z3fold_unmap
>>   mm/z3fold: introduce __zpdesc_set_movable
>>   mm/z3fold: introduce __zpdesc_clear_movable
>>
>>  mm/z3fold.c | 190 +++++++++++++++++++++++++++-------------------------
>>  mm/zpdesc.h |  87 ++++++++++++++++++++++++
>>  2 files changed, 184 insertions(+), 93 deletions(-)
>>  create mode 100644 mm/zpdesc.h
>>
>> --
>> 2.43.0
>>
>>
Hyeonggon Yoo June 25, 2024, 9:28 a.m. UTC | #4
On Tue, Jun 25, 2024 at 5:11 PM Alex Shi <seakeel@gmail.com> wrote:

>
>
> On 6/25/24 5:46 AM, Yosry Ahmed wrote:
> > On Thu, Jun 20, 2024 at 10:42 PM <alexs@kernel.org> wrote:
> >>
> >> From: Alex Shi <alexs@kernel.org>
> >>
> >> According to Metthew's plan, the page descriptor will be replace by a 8
> >> bytes mem_desc on destination purpose.
> >> https://lore.kernel.org/lkml/YvV1KTyzZ+Jrtj9x@casper.infradead.org/
> >>
> >> Here is a implement on z3fold to replace page descriptor by zpdesc,
> >> which is still overlay on struct page now. but it's a step move forward
> >> above destination.
> >>
> >> To name the struct zpdesc instead of z3fold_desc, since there are 3
> zpool
> >> usages under zswap, zbud, z3fold, zsmalloc. It looks like we may extend
> the
> >> zpdesc to zbud and zsmalloc, combined their usage into one.
> >
> > For zsmalloc, there is already an ongoing effort to split zsdesc from
> > struct page [2].
> >
> > [2]
> https://lore.kernel.org/lkml/20230713042037.980211-1-42.hyeyoo@gmail.com/
>
> David had pointed out this to me few weeks ago too. This patchset hasn't
> updated nearly a year. If Yoo don't object I'd like to pick up from his
> left and update it to latest zsmalloc.c.
>

Hi Alex and Yosry,

Thank you for mentioning this! I still believe the work is worth pursuing,
but recently I haven't had the capacity to push it further.

I'm on board with you taking it over and updating it with the latest source
code. If you have any questions while bringing it up to date, don't
hesitate please reach out to me.
Yosry Ahmed June 25, 2024, 10:30 a.m. UTC | #5
On Tue, Jun 25, 2024 at 1:11 AM Alex Shi <seakeel@gmail.com> wrote:
>
>
>
> On 6/25/24 5:46 AM, Yosry Ahmed wrote:
> > On Thu, Jun 20, 2024 at 10:42 PM <alexs@kernel.org> wrote:
> >>
> >> From: Alex Shi <alexs@kernel.org>
> >>
> >> According to Metthew's plan, the page descriptor will be replace by a 8
> >> bytes mem_desc on destination purpose.
> >> https://lore.kernel.org/lkml/YvV1KTyzZ+Jrtj9x@casper.infradead.org/
> >>
> >> Here is a implement on z3fold to replace page descriptor by zpdesc,
> >> which is still overlay on struct page now. but it's a step move forward
> >> above destination.
> >>
> >> To name the struct zpdesc instead of z3fold_desc, since there are 3 zpool
> >> usages under zswap, zbud, z3fold, zsmalloc. It looks like we may extend the
> >> zpdesc to zbud and zsmalloc, combined their usage into one.
> >
> > Please do not focus your development efforts on z3fold. We really want
> > to deprecate/remove it, as well as zbud eventually. See [1].
> >
> > For zsmalloc, there is already an ongoing effort to split zsdesc from
> > struct page [2].
> >
> > [1]https://lore.kernel.org/lkml/CAJD7tkbRF6od-2x_L8-A1QL3=2Ww13sCj4S3i4bNndqF+3+_Vg@mail.gmail.com/
>
> Hi Yosry,
>
> Thanks a lot for the info and comments! It's my stupid w/o checking the email list before work on it.
> Anyway don't know if z3fold would be removed, jut left this tested patchset here if someone need it.

It's partially our fault for leaving z3fold knowing that it is headed
toward deprecation/removal. FWIW, I tried to remove it or mark it as
deprecated, but there was some resistance :/
Alex Shi June 25, 2024, 1:39 p.m. UTC | #6
On 6/25/24 5:28 PM, Hyeonggon Yoo wrote:
> 
> 
> On Tue, Jun 25, 2024 at 5:11 PM Alex Shi <seakeel@gmail.com <mailto:seakeel@gmail.com>> wrote:
> 
> 
> 
>     On 6/25/24 5:46 AM, Yosry Ahmed wrote:
>     > On Thu, Jun 20, 2024 at 10:42 PM <alexs@kernel.org <mailto:alexs@kernel.org>> wrote:
>     >>
>     >> From: Alex Shi <alexs@kernel.org <mailto:alexs@kernel.org>>
>     >>
>     >> According to Metthew's plan, the page descriptor will be replace by a 8
>     >> bytes mem_desc on destination purpose.
>     >> https://lore.kernel.org/lkml/YvV1KTyzZ+Jrtj9x@casper.infradead.org/ <https://lore.kernel.org/lkml/YvV1KTyzZ+Jrtj9x@casper.infradead.org/>
>     >>
>     >> Here is a implement on z3fold to replace page descriptor by zpdesc,
>     >> which is still overlay on struct page now. but it's a step move forward
>     >> above destination.
>     >>
>     >> To name the struct zpdesc instead of z3fold_desc, since there are 3 zpool
>     >> usages under zswap, zbud, z3fold, zsmalloc. It looks like we may extend the
>     >> zpdesc to zbud and zsmalloc, combined their usage into one.
>     >
>     > For zsmalloc, there is already an ongoing effort to split zsdesc from
>     > struct page [2].
>     >
>     > [2]https://lore.kernel.org/lkml/20230713042037.980211-1-42.hyeyoo@gmail.com/ <https://lore.kernel.org/lkml/20230713042037.980211-1-42.hyeyoo@gmail.com/>
> 
>     David had pointed out this to me few weeks ago too. This patchset hasn't updated nearly a year. If Yoo don't object I'd like to pick up from his left and update it to latest zsmalloc.c.
> 
> 
> Hi Alex and Yosry,
> 
> Thank you for mentioning this! I still believe the work is worth pursuing, but recently I haven't had the capacity to push it further.
> 
> I'm on board with you taking it over and updating it with the latest source code. If you have any questions while bringing it up to date, don't hesitate please reach out to me.


Hi Yoo,

Thanks a lot for generous offer! I will update the patchset and send you for review ASAP. :)

Cheers!
Alex
Alex Shi June 25, 2024, 1:44 p.m. UTC | #7
On 6/25/24 6:30 PM, Yosry Ahmed wrote:
> On Tue, Jun 25, 2024 at 1:11 AM Alex Shi <seakeel@gmail.com> wrote:
>>
>>
>>
>> On 6/25/24 5:46 AM, Yosry Ahmed wrote:
>>> On Thu, Jun 20, 2024 at 10:42 PM <alexs@kernel.org> wrote:
>>>>
>>>> From: Alex Shi <alexs@kernel.org>
>>>>
>>>> According to Metthew's plan, the page descriptor will be replace by a 8
>>>> bytes mem_desc on destination purpose.
>>>> https://lore.kernel.org/lkml/YvV1KTyzZ+Jrtj9x@casper.infradead.org/
>>>>
>>>> Here is a implement on z3fold to replace page descriptor by zpdesc,
>>>> which is still overlay on struct page now. but it's a step move forward
>>>> above destination.
>>>>
>>>> To name the struct zpdesc instead of z3fold_desc, since there are 3 zpool
>>>> usages under zswap, zbud, z3fold, zsmalloc. It looks like we may extend the
>>>> zpdesc to zbud and zsmalloc, combined their usage into one.
>>>
>>> Please do not focus your development efforts on z3fold. We really want
>>> to deprecate/remove it, as well as zbud eventually. See [1].
>>>
>>> For zsmalloc, there is already an ongoing effort to split zsdesc from
>>> struct page [2].
>>>
>>> [1]https://lore.kernel.org/lkml/CAJD7tkbRF6od-2x_L8-A1QL3=2Ww13sCj4S3i4bNndqF+3+_Vg@mail.gmail.com/
>>
>> Hi Yosry,
>>
>> Thanks a lot for the info and comments! It's my stupid w/o checking the email list before work on it.
>> Anyway don't know if z3fold would be removed, jut left this tested patchset here if someone need it.
> 
> It's partially our fault for leaving z3fold knowing that it is headed
> toward deprecation/removal. FWIW, I tried to remove it or mark it as
> deprecated, but there was some resistance :/

Yes, It happens. Community is too huge. Always someone want sth.
Nhat Pham June 25, 2024, 5:22 p.m. UTC | #8
On Tue, Jun 25, 2024 at 3:32 AM Yosry Ahmed <yosryahmed@google.com> wrote:
>
> On Tue, Jun 25, 2024 at 1:11 AM Alex Shi <seakeel@gmail.com> wrote:
> >
> > Thanks a lot for the info and comments! It's my stupid w/o checking the email list before work on it.
> > Anyway don't know if z3fold would be removed, jut left this tested patchset here if someone need it.
>
> It's partially our fault for leaving z3fold knowing that it is headed
> toward deprecation/removal. FWIW, I tried to remove it or mark it as
> deprecated, but there was some resistance :/
>
Our apologies, Alex. Thank you for your contribution regardless!

Regarding zbud and z3fold, this is the second time this conversation
has come up within a week or so. Chengming was trying to revert the
multiple zpool changes. zsmalloc (after we re-introduce the class
locks) does not seem to regress (at least based on benchmarking), but
z3fold and zbud suffer. I think we are starting to pay the price of
maintaining z3fold and zbud:

1. Future improvement to related subsystems now hurts z3fold.
Developers/maintainers have to spend extra mental capacity to consider
this, and users could potentially see worse performance if they select
z3fold/zbud unknowingly.

2. Contributors are confused on where they should spend their effort
on improving.

Can we at least have a roadmap for deprecating these 2? Something
along the line of:

1. Deprecate either zbud or z3fold. We do not really need both of them.

2. Make zsmalloc independent of MMU, somehow (IIRC, compaction was one
reason right? maybe making zsmalloc compaction dependent on MMU
availability?).

3. Deprecate the remaining one - zsmalloc can be used in all deployments now.

4. (Optional) Maybe adding another allocator for the edge cases that
zsmalloc does not handle well? I'd need to see numbers to be convinced
that this is the case. I think I saw somewhere that Vitaly was working
on zblock - look forward to seeing this :)
Yosry Ahmed June 25, 2024, 9:02 p.m. UTC | #9
On Tue, Jun 25, 2024 at 10:22 AM Nhat Pham <nphamcs@gmail.com> wrote:
>
> On Tue, Jun 25, 2024 at 3:32 AM Yosry Ahmed <yosryahmed@google.com> wrote:
> >
> > On Tue, Jun 25, 2024 at 1:11 AM Alex Shi <seakeel@gmail.com> wrote:
> > >
> > > Thanks a lot for the info and comments! It's my stupid w/o checking the email list before work on it.
> > > Anyway don't know if z3fold would be removed, jut left this tested patchset here if someone need it.
> >
> > It's partially our fault for leaving z3fold knowing that it is headed
> > toward deprecation/removal. FWIW, I tried to remove it or mark it as
> > deprecated, but there was some resistance :/
> >
> Our apologies, Alex. Thank you for your contribution regardless!
>
> Regarding zbud and z3fold, this is the second time this conversation
> has come up within a week or so. Chengming was trying to revert the
> multiple zpool changes. zsmalloc (after we re-introduce the class
> locks) does not seem to regress (at least based on benchmarking), but
> z3fold and zbud suffer. I think we are starting to pay the price of
> maintaining z3fold and zbud:
>
> 1. Future improvement to related subsystems now hurts z3fold.
> Developers/maintainers have to spend extra mental capacity to consider
> this, and users could potentially see worse performance if they select
> z3fold/zbud unknowingly.
>
> 2. Contributors are confused on where they should spend their effort
> on improving.
>
> Can we at least have a roadmap for deprecating these 2? Something
> along the line of:

100% agreed. I think we can start with z3fold given that it doesn't
offer significant advantages over zbud in my testing, and zbud is
probably more popular since it was the default zpool for a while.

Then we should be able to remove zbud as well after we take care of
the MMU dependency in zsmalloc. After that, if no new allocators show
up in a while, we can drop the zpool abstraction entirely.

Just my 2c.