There is no logical reason to refuse storing same-filled pages more
efficiently and opt for compression. Remove the userspace knob.
Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
---
mm/zswap.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/mm/zswap.c b/mm/zswap.c
index 498a6c5839bef..0fc27ae950c74 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -123,14 +123,6 @@ static unsigned int zswap_accept_thr_percent = 90; /* of max pool size */
module_param_named(accept_threshold_percent, zswap_accept_thr_percent,
uint, 0644);
-/*
- * Enable/disable handling same-value filled pages (enabled by default).
- * If disabled every page is considered non-same-value filled.
- */
-static bool zswap_same_filled_pages_enabled = true;
-module_param_named(same_filled_pages_enabled, zswap_same_filled_pages_enabled,
- bool, 0644);
-
/* Enable/disable handling non-same-value filled pages (enabled by default) */
static bool zswap_non_same_filled_pages_enabled = true;
module_param_named(non_same_filled_pages_enabled, zswap_non_same_filled_pages_enabled,
@@ -1392,9 +1384,6 @@ static bool zswap_is_folio_same_filled(struct folio *folio, unsigned long *value
unsigned int pos, last_pos = PAGE_SIZE / sizeof(*page) - 1;
bool ret;
- if (!zswap_same_filled_pages_enabled)
- return false;
-
page = kmap_local_folio(folio, 0);
val = page[0];
--
2.44.0.396.g6e790dbe36-googOn Mon, Mar 25, 2024 at 11:50:13PM +0000, Yosry Ahmed wrote: > There is no logical reason to refuse storing same-filled pages more > efficiently and opt for compression. Remove the userspace knob. > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> Acked-by: Johannes Weiner <hannes@cmpxchg.org> I also think the non_same_filled_pages_enabled option should go away. Both of these tunables are pretty bizarre.
On Thu, Mar 28, 2024 at 12:11 PM Johannes Weiner <hannes@cmpxchg.org> wrote: > > On Mon, Mar 25, 2024 at 11:50:13PM +0000, Yosry Ahmed wrote: > > There is no logical reason to refuse storing same-filled pages more > > efficiently and opt for compression. Remove the userspace knob. > > > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> > > Acked-by: Johannes Weiner <hannes@cmpxchg.org> > > I also think the non_same_filled_pages_enabled option should go > away. Both of these tunables are pretty bizarre. Happy to remove both in the next version :) Thanks!
On Thu, Mar 28, 2024 at 1:06 PM Yosry Ahmed <yosryahmed@google.com> wrote: > > On Thu, Mar 28, 2024 at 12:11 PM Johannes Weiner <hannes@cmpxchg.org> wrote: > > > > On Mon, Mar 25, 2024 at 11:50:13PM +0000, Yosry Ahmed wrote: > > > There is no logical reason to refuse storing same-filled pages more > > > efficiently and opt for compression. Remove the userspace knob. > > > > > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> > > > > Acked-by: Johannes Weiner <hannes@cmpxchg.org> > > > > I also think the non_same_filled_pages_enabled option should go > > away. Both of these tunables are pretty bizarre. > > Happy to remove both in the next version :) I thought non_same_filled_pages_enabled was introduced with the initial support for same-filled pages, but it was introduced separately (and much more recently): https://lore.kernel.org/all/7dbafa963e8bab43608189abbe2067f4b9287831.1641247624.git.maciej.szmigiero@oracle.com/ I am CCing Maciej to hear more about the use case for this. > > Thanks!
On 29.03.2024 03:14, Yosry Ahmed wrote: > On Thu, Mar 28, 2024 at 1:06 PM Yosry Ahmed <yosryahmed@google.com> wrote: >> >> On Thu, Mar 28, 2024 at 12:11 PM Johannes Weiner <hannes@cmpxchg.org> wrote: >>> >>> On Mon, Mar 25, 2024 at 11:50:13PM +0000, Yosry Ahmed wrote: >>>> There is no logical reason to refuse storing same-filled pages more >>>> efficiently and opt for compression. Remove the userspace knob. >>>> >>>> Signed-off-by: Yosry Ahmed <yosryahmed@google.com> >>> >>> Acked-by: Johannes Weiner <hannes@cmpxchg.org> >>> >>> I also think the non_same_filled_pages_enabled option should go >>> away. Both of these tunables are pretty bizarre. >> >> Happy to remove both in the next version :) > > I thought non_same_filled_pages_enabled was introduced with the > initial support for same-filled pages, but it was introduced > separately (and much more recently): > https://lore.kernel.org/all/7dbafa963e8bab43608189abbe2067f4b9287831.1641247624.git.maciej.szmigiero@oracle.com/ > > I am CCing Maciej to hear more about the use case for this. Thanks for CCing me. I introduced "non_same_filled_pages_enabled" a few years ago to enable using zswap in a lightweight mode where it is only used for its ability to store same-filled pages effectively. As far as I remember, there were some interactions between full zswap and the cgroup memory controller - like, it made it easier for an aggressive workload to exceed its cgroup memory.high limits. On the other hand, "same_filled_pages_enabled" sounds like some kind of a debugging option since I don't see any real reason to disable it either. Thanks, Maciej
On Fri, Mar 29, 2024 at 03:02:10PM +0100, Maciej S. Szmigiero wrote:
> On 29.03.2024 03:14, Yosry Ahmed wrote:
> > On Thu, Mar 28, 2024 at 1:06 PM Yosry Ahmed <yosryahmed@google.com> wrote:
> >>
> >> On Thu, Mar 28, 2024 at 12:11 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
> >>>
> >>> On Mon, Mar 25, 2024 at 11:50:13PM +0000, Yosry Ahmed wrote:
> >>>> There is no logical reason to refuse storing same-filled pages more
> >>>> efficiently and opt for compression. Remove the userspace knob.
> >>>>
> >>>> Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> >>>
> >>> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> >>>
> >>> I also think the non_same_filled_pages_enabled option should go
> >>> away. Both of these tunables are pretty bizarre.
> >>
> >> Happy to remove both in the next version :)
> >
> > I thought non_same_filled_pages_enabled was introduced with the
> > initial support for same-filled pages, but it was introduced
> > separately (and much more recently):
> > https://lore.kernel.org/all/7dbafa963e8bab43608189abbe2067f4b9287831.1641247624.git.maciej.szmigiero@oracle.com/
> >
> > I am CCing Maciej to hear more about the use case for this.
>
> Thanks for CCing me.
>
> I introduced "non_same_filled_pages_enabled" a few years ago to
> enable using zswap in a lightweight mode where it is only used for
> its ability to store same-filled pages effectively.
But all the pages it rejects go to disk swap instead, which is much
slower than compression...
> As far as I remember, there were some interactions between full
> zswap and the cgroup memory controller - like, it made it easier
> for an aggressive workload to exceed its cgroup memory.high limits.
Ok, that makes sense! A container fairness measure, rather than a
performance optimization.
Fair enough, but that's moot then with cgroup accounting of the
backing memory, f4840ccfca25 ("zswap: memcg accounting").
Thanks for prodiving context.
On Fri, Mar 29, 2024 at 10:45 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> On Fri, Mar 29, 2024 at 03:02:10PM +0100, Maciej S. Szmigiero wrote:
> > On 29.03.2024 03:14, Yosry Ahmed wrote:
> > > On Thu, Mar 28, 2024 at 1:06 PM Yosry Ahmed <yosryahmed@google.com> wrote:
> > >>
> > >> On Thu, Mar 28, 2024 at 12:11 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
> > >>>
> > >>> On Mon, Mar 25, 2024 at 11:50:13PM +0000, Yosry Ahmed wrote:
> > >>>> There is no logical reason to refuse storing same-filled pages more
> > >>>> efficiently and opt for compression. Remove the userspace knob.
> > >>>>
> > >>>> Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> > >>>
> > >>> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> > >>>
> > >>> I also think the non_same_filled_pages_enabled option should go
> > >>> away. Both of these tunables are pretty bizarre.
> > >>
> > >> Happy to remove both in the next version :)
> > >
> > > I thought non_same_filled_pages_enabled was introduced with the
> > > initial support for same-filled pages, but it was introduced
> > > separately (and much more recently):
> > > https://lore.kernel.org/all/7dbafa963e8bab43608189abbe2067f4b9287831.1641247624.git.maciej.szmigiero@oracle.com/
> > >
> > > I am CCing Maciej to hear more about the use case for this.
> >
> > Thanks for CCing me.
> >
> > I introduced "non_same_filled_pages_enabled" a few years ago to
> > enable using zswap in a lightweight mode where it is only used for
> > its ability to store same-filled pages effectively.
>
> But all the pages it rejects go to disk swap instead, which is much
> slower than compression...
>
> > As far as I remember, there were some interactions between full
> > zswap and the cgroup memory controller - like, it made it easier
> > for an aggressive workload to exceed its cgroup memory.high limits.
>
> Ok, that makes sense! A container fairness measure, rather than a
> performance optimization.
>
> Fair enough, but that's moot then with cgroup accounting of the
> backing memory, f4840ccfca25 ("zswap: memcg accounting").
Right, this should no longer be needed with the zswap charging.
Maciej, is this still being used on kernels with f4840ccfca25 (5.19+)?
Any objections to removing it now?
On 29.03.2024 19:22, Yosry Ahmed wrote:
> On Fri, Mar 29, 2024 at 10:45 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
>>
>> On Fri, Mar 29, 2024 at 03:02:10PM +0100, Maciej S. Szmigiero wrote:
>>> On 29.03.2024 03:14, Yosry Ahmed wrote:
>>>> On Thu, Mar 28, 2024 at 1:06 PM Yosry Ahmed <yosryahmed@google.com> wrote:
>>>>>
>>>>> On Thu, Mar 28, 2024 at 12:11 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
>>>>>>
>>>>>> On Mon, Mar 25, 2024 at 11:50:13PM +0000, Yosry Ahmed wrote:
>>>>>>> There is no logical reason to refuse storing same-filled pages more
>>>>>>> efficiently and opt for compression. Remove the userspace knob.
>>>>>>>
>>>>>>> Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
>>>>>>
>>>>>> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
>>>>>>
>>>>>> I also think the non_same_filled_pages_enabled option should go
>>>>>> away. Both of these tunables are pretty bizarre.
>>>>>
>>>>> Happy to remove both in the next version :)
>>>>
>>>> I thought non_same_filled_pages_enabled was introduced with the
>>>> initial support for same-filled pages, but it was introduced
>>>> separately (and much more recently):
>>>> https://lore.kernel.org/all/7dbafa963e8bab43608189abbe2067f4b9287831.1641247624.git.maciej.szmigiero@oracle.com/
>>>>
>>>> I am CCing Maciej to hear more about the use case for this.
>>>
>>> Thanks for CCing me.
>>>
>>> I introduced "non_same_filled_pages_enabled" a few years ago to
>>> enable using zswap in a lightweight mode where it is only used for
>>> its ability to store same-filled pages effectively.
>>
>> But all the pages it rejects go to disk swap instead, which is much
>> slower than compression...
>>
>>> As far as I remember, there were some interactions between full
>>> zswap and the cgroup memory controller - like, it made it easier
>>> for an aggressive workload to exceed its cgroup memory.high limits.
>>
>> Ok, that makes sense! A container fairness measure, rather than a
>> performance optimization.
>>
>> Fair enough, but that's moot then with cgroup accounting of the
>> backing memory, f4840ccfca25 ("zswap: memcg accounting").
>
> Right, this should no longer be needed with the zswap charging.
>
> Maciej, is this still being used on kernels with f4840ccfca25 (5.19+)?
> Any objections to removing it now?
I don't object to its removal as long as stable kernel trees aren't
affected.
Thanks,
Maciej
On Mon, Apr 1, 2024 at 3:38 AM Maciej S. Szmigiero
<mail@maciej.szmigiero.name> wrote:
>
> On 29.03.2024 19:22, Yosry Ahmed wrote:
> > On Fri, Mar 29, 2024 at 10:45 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
> >>
> >> On Fri, Mar 29, 2024 at 03:02:10PM +0100, Maciej S. Szmigiero wrote:
> >>> On 29.03.2024 03:14, Yosry Ahmed wrote:
> >>>> On Thu, Mar 28, 2024 at 1:06 PM Yosry Ahmed <yosryahmed@google.com> wrote:
> >>>>>
> >>>>> On Thu, Mar 28, 2024 at 12:11 PM Johannes Weiner <hannes@cmpxchg.org> wrote:
> >>>>>>
> >>>>>> On Mon, Mar 25, 2024 at 11:50:13PM +0000, Yosry Ahmed wrote:
> >>>>>>> There is no logical reason to refuse storing same-filled pages more
> >>>>>>> efficiently and opt for compression. Remove the userspace knob.
> >>>>>>>
> >>>>>>> Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
> >>>>>>
> >>>>>> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> >>>>>>
> >>>>>> I also think the non_same_filled_pages_enabled option should go
> >>>>>> away. Both of these tunables are pretty bizarre.
> >>>>>
> >>>>> Happy to remove both in the next version :)
> >>>>
> >>>> I thought non_same_filled_pages_enabled was introduced with the
> >>>> initial support for same-filled pages, but it was introduced
> >>>> separately (and much more recently):
> >>>> https://lore.kernel.org/all/7dbafa963e8bab43608189abbe2067f4b9287831.1641247624.git.maciej.szmigiero@oracle.com/
> >>>>
> >>>> I am CCing Maciej to hear more about the use case for this.
> >>>
> >>> Thanks for CCing me.
> >>>
> >>> I introduced "non_same_filled_pages_enabled" a few years ago to
> >>> enable using zswap in a lightweight mode where it is only used for
> >>> its ability to store same-filled pages effectively.
> >>
> >> But all the pages it rejects go to disk swap instead, which is much
> >> slower than compression...
> >>
> >>> As far as I remember, there were some interactions between full
> >>> zswap and the cgroup memory controller - like, it made it easier
> >>> for an aggressive workload to exceed its cgroup memory.high limits.
> >>
> >> Ok, that makes sense! A container fairness measure, rather than a
> >> performance optimization.
> >>
> >> Fair enough, but that's moot then with cgroup accounting of the
> >> backing memory, f4840ccfca25 ("zswap: memcg accounting").
> >
> > Right, this should no longer be needed with the zswap charging.
> >
> > Maciej, is this still being used on kernels with f4840ccfca25 (5.19+)?
> > Any objections to removing it now?
>
> I don't object to its removal as long as stable kernel trees aren't
> affected.
Yeah this isn't something that would be backported to stable kernels.
Thanks for confirming.
On 2024/3/26 07:50, Yosry Ahmed wrote: > There is no logical reason to refuse storing same-filled pages more > efficiently and opt for compression. Remove the userspace knob. > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> LGTM, should we also remove zswap_non_same_filled_pages_enabled? Not sure if it has real usage... Reviewed-by: Chengming Zhou <chengming.zhou@linux.dev> > --- > mm/zswap.c | 11 ----------- > 1 file changed, 11 deletions(-) > > diff --git a/mm/zswap.c b/mm/zswap.c > index 498a6c5839bef..0fc27ae950c74 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -123,14 +123,6 @@ static unsigned int zswap_accept_thr_percent = 90; /* of max pool size */ > module_param_named(accept_threshold_percent, zswap_accept_thr_percent, > uint, 0644); > > -/* > - * Enable/disable handling same-value filled pages (enabled by default). > - * If disabled every page is considered non-same-value filled. > - */ > -static bool zswap_same_filled_pages_enabled = true; > -module_param_named(same_filled_pages_enabled, zswap_same_filled_pages_enabled, > - bool, 0644); > - > /* Enable/disable handling non-same-value filled pages (enabled by default) */ > static bool zswap_non_same_filled_pages_enabled = true; > module_param_named(non_same_filled_pages_enabled, zswap_non_same_filled_pages_enabled, > @@ -1392,9 +1384,6 @@ static bool zswap_is_folio_same_filled(struct folio *folio, unsigned long *value > unsigned int pos, last_pos = PAGE_SIZE / sizeof(*page) - 1; > bool ret; > > - if (!zswap_same_filled_pages_enabled) > - return false; > - > page = kmap_local_folio(folio, 0); > val = page[0]; >
On Tue, Mar 26, 2024 at 7:44 PM Chengming Zhou <chengming.zhou@linux.dev> wrote: > > On 2024/3/26 07:50, Yosry Ahmed wrote: > > There is no logical reason to refuse storing same-filled pages more > > efficiently and opt for compression. Remove the userspace knob. > > > > Signed-off-by: Yosry Ahmed <yosryahmed@google.com> > > LGTM, should we also remove zswap_non_same_filled_pages_enabled? > Not sure if it has real usage... I am not aware of usages, but in theory you can use it if you exclusively use zswap to optimize swapping zero-filled pages. Not sure if anyone actually does that though. We can remove it if everyone else agrees. > > Reviewed-by: Chengming Zhou <chengming.zhou@linux.dev> Thanks!
On Mon, Mar 25, 2024 at 4:50 PM Yosry Ahmed <yosryahmed@google.com> wrote: > > There is no logical reason to refuse storing same-filled pages more > efficiently and opt for compression. Remove the userspace knob. > Agree. Absolutely no idea why we have this knob in the first place - if you have cycles to compress, you probably have some extra cycles to check same-filled page state? And that is the only cost I can think of - it wins on probably all other aspects: memory usage, "decompression", no need to write back these pages etc. Any actual zswap user who has data or counter-use case should chime in, but FWIW, my vote is: Reviewed-by: Nhat Pham <nphamcs@gmail.com>
© 2016 - 2026 Red Hat, Inc.