* [PATCHv2] zram: do not waste zram_table_entry flags bits
@ 2022-09-12 15:27 Sergey Senozhatsky
2022-09-12 15:39 ` Brian Geffon
2022-09-15 16:36 ` Minchan Kim
0 siblings, 2 replies; 6+ messages in thread
From: Sergey Senozhatsky @ 2022-09-12 15:27 UTC (permalink / raw)
To: Minchan Kim, Andrew Morton
Cc: Nitin Gupta, Brian Geffon, linux-kernel, linux-mm,
Sergey Senozhatsky
zram_table_entry::flags stores object size in the lower bits and
zram pageflags in the upper bits. However, for some reason, we
use 24 lower bits, while maximum zram object size is PAGE_SIZE,
which requires PAGE_SHIFT bits (up to 16 on arm64). This wastes
24 - PAGE_SHIFT bits that we can use for additional zram pageflags
instead.
Also add a BUILD_BUG_ON() to alert us should we run out of bits
in zram_table_entry::flags.
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
drivers/block/zram/zram_drv.c | 2 ++
drivers/block/zram/zram_drv.h | 15 +++++++--------
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index f3948abce2f7..07913bcdb5c2 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -2449,6 +2449,8 @@ static int __init zram_init(void)
{
int ret;
+ BUILD_BUG_ON(__NR_ZRAM_PAGEFLAGS > BITS_PER_LONG);
+
ret = cpuhp_setup_state_multi(CPUHP_ZCOMP_PREPARE, "block/zram:prepare",
zcomp_cpu_up_prepare, zcomp_cpu_dead);
if (ret < 0)
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index b4eecef2a11f..2b50f0521bd3 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -30,16 +30,15 @@
/*
- * The lower ZRAM_FLAG_SHIFT bits of table.flags is for
- * object size (excluding header), the higher bits is for
- * zram_pageflags.
+ * ZRAM is mainly used for memory efficiency so we want to keep memory
+ * footprint small and thus squeeze size and zram pageflags into a flags
+ * member. The lower ZRAM_FLAG_SHIFT bits is for object size (excluding
+ * header), which cannot be larger than PAGE_SIZE (requiring PAGE_SHIFT
+ * bits), the higher bits are for zram_pageflags.
*
- * zram is mainly used for memory efficiency so we want to keep memory
- * footprint small so we can squeeze size and flags into a field.
- * The lower ZRAM_FLAG_SHIFT bits is for object size (excluding header),
- * the higher bits is for zram_pageflags.
+ * We use BUILD_BUG_ON() to make sure that zram pageflags don't overflow.
*/
-#define ZRAM_FLAG_SHIFT 24
+#define ZRAM_FLAG_SHIFT (PAGE_SHIFT + 1)
/* Flags for zram pages (table[page_no].flags) */
enum zram_pageflags {
--
2.37.2.789.g6183377224-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCHv2] zram: do not waste zram_table_entry flags bits
2022-09-12 15:27 [PATCHv2] zram: do not waste zram_table_entry flags bits Sergey Senozhatsky
@ 2022-09-12 15:39 ` Brian Geffon
2022-09-13 1:54 ` Sergey Senozhatsky
2022-09-15 16:36 ` Minchan Kim
1 sibling, 1 reply; 6+ messages in thread
From: Brian Geffon @ 2022-09-12 15:39 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Andrew Morton, Nitin Gupta, LKML, linux-mm
On Mon, Sep 12, 2022 at 11:29 AM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> zram_table_entry::flags stores object size in the lower bits and
> zram pageflags in the upper bits. However, for some reason, we
> use 24 lower bits, while maximum zram object size is PAGE_SIZE,
> which requires PAGE_SHIFT bits (up to 16 on arm64). This wastes
> 24 - PAGE_SHIFT bits that we can use for additional zram pageflags
> instead.
>
> Also add a BUILD_BUG_ON() to alert us should we run out of bits
> in zram_table_entry::flags.
>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> ---
> drivers/block/zram/zram_drv.c | 2 ++
> drivers/block/zram/zram_drv.h | 15 +++++++--------
> 2 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index f3948abce2f7..07913bcdb5c2 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -2449,6 +2449,8 @@ static int __init zram_init(void)
> {
> int ret;
>
> + BUILD_BUG_ON(__NR_ZRAM_PAGEFLAGS > BITS_PER_LONG);
Should this be >= BITS_PER_LONG?
> +
> ret = cpuhp_setup_state_multi(CPUHP_ZCOMP_PREPARE, "block/zram:prepare",
> zcomp_cpu_up_prepare, zcomp_cpu_dead);
> if (ret < 0)
> diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> index b4eecef2a11f..2b50f0521bd3 100644
> --- a/drivers/block/zram/zram_drv.h
> +++ b/drivers/block/zram/zram_drv.h
> @@ -30,16 +30,15 @@
>
>
> /*
> - * The lower ZRAM_FLAG_SHIFT bits of table.flags is for
> - * object size (excluding header), the higher bits is for
> - * zram_pageflags.
> + * ZRAM is mainly used for memory efficiency so we want to keep memory
> + * footprint small and thus squeeze size and zram pageflags into a flags
> + * member. The lower ZRAM_FLAG_SHIFT bits is for object size (excluding
> + * header), which cannot be larger than PAGE_SIZE (requiring PAGE_SHIFT
> + * bits), the higher bits are for zram_pageflags.
> *
> - * zram is mainly used for memory efficiency so we want to keep memory
> - * footprint small so we can squeeze size and flags into a field.
> - * The lower ZRAM_FLAG_SHIFT bits is for object size (excluding header),
> - * the higher bits is for zram_pageflags.
> + * We use BUILD_BUG_ON() to make sure that zram pageflags don't overflow.
> */
> -#define ZRAM_FLAG_SHIFT 24
> +#define ZRAM_FLAG_SHIFT (PAGE_SHIFT + 1)
>
> /* Flags for zram pages (table[page_no].flags) */
> enum zram_pageflags {
> --
> 2.37.2.789.g6183377224-goog
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCHv2] zram: do not waste zram_table_entry flags bits
2022-09-12 15:39 ` Brian Geffon
@ 2022-09-13 1:54 ` Sergey Senozhatsky
2022-09-13 2:07 ` Brian Geffon
0 siblings, 1 reply; 6+ messages in thread
From: Sergey Senozhatsky @ 2022-09-13 1:54 UTC (permalink / raw)
To: Brian Geffon
Cc: Sergey Senozhatsky, Minchan Kim, Andrew Morton, Nitin Gupta, LKML,
linux-mm
On (22/09/12 11:39), Brian Geffon wrote:
> > zram_table_entry::flags stores object size in the lower bits and
> > zram pageflags in the upper bits. However, for some reason, we
> > use 24 lower bits, while maximum zram object size is PAGE_SIZE,
> > which requires PAGE_SHIFT bits (up to 16 on arm64). This wastes
> > 24 - PAGE_SHIFT bits that we can use for additional zram pageflags
> > instead.
> >
> > Also add a BUILD_BUG_ON() to alert us should we run out of bits
> > in zram_table_entry::flags.
> >
> > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> > ---
> > drivers/block/zram/zram_drv.c | 2 ++
> > drivers/block/zram/zram_drv.h | 15 +++++++--------
> > 2 files changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > index f3948abce2f7..07913bcdb5c2 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -2449,6 +2449,8 @@ static int __init zram_init(void)
> > {
> > int ret;
> >
> > + BUILD_BUG_ON(__NR_ZRAM_PAGEFLAGS > BITS_PER_LONG);
>
> Should this be >= BITS_PER_LONG?
__NR_ZRAM_PAGEFLAGS == BITS_PER_LONG == 64 (e.g. on 64 bit host)
means that the last valid zram pageflag (and __NR_ZRAM_PAGEFLAGS
is not a valid pageflag) is __NR_ZRAM_PAGEFLAGS - 1, which is 63
and which is a valid BIT() offset for u64.
So __NR_ZRAM_PAGEFLAGS == BITS_PER_LONG should be a valid case.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCHv2] zram: do not waste zram_table_entry flags bits
2022-09-13 1:54 ` Sergey Senozhatsky
@ 2022-09-13 2:07 ` Brian Geffon
2022-09-13 2:10 ` Sergey Senozhatsky
0 siblings, 1 reply; 6+ messages in thread
From: Brian Geffon @ 2022-09-13 2:07 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Minchan Kim, Andrew Morton, Nitin Gupta, LKML, linux-mm
On Mon, Sep 12, 2022 at 9:54 PM Sergey Senozhatsky
<senozhatsky@chromium.org> wrote:
>
> On (22/09/12 11:39), Brian Geffon wrote:
> > > zram_table_entry::flags stores object size in the lower bits and
> > > zram pageflags in the upper bits. However, for some reason, we
> > > use 24 lower bits, while maximum zram object size is PAGE_SIZE,
> > > which requires PAGE_SHIFT bits (up to 16 on arm64). This wastes
> > > 24 - PAGE_SHIFT bits that we can use for additional zram pageflags
> > > instead.
> > >
> > > Also add a BUILD_BUG_ON() to alert us should we run out of bits
> > > in zram_table_entry::flags.
> > >
> > > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> > > ---
> > > drivers/block/zram/zram_drv.c | 2 ++
> > > drivers/block/zram/zram_drv.h | 15 +++++++--------
> > > 2 files changed, 9 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > > index f3948abce2f7..07913bcdb5c2 100644
> > > --- a/drivers/block/zram/zram_drv.c
> > > +++ b/drivers/block/zram/zram_drv.c
> > > @@ -2449,6 +2449,8 @@ static int __init zram_init(void)
> > > {
> > > int ret;
> > >
> > > + BUILD_BUG_ON(__NR_ZRAM_PAGEFLAGS > BITS_PER_LONG);
> >
> > Should this be >= BITS_PER_LONG?
>
> __NR_ZRAM_PAGEFLAGS == BITS_PER_LONG == 64 (e.g. on 64 bit host)
> means that the last valid zram pageflag (and __NR_ZRAM_PAGEFLAGS
> is not a valid pageflag) is __NR_ZRAM_PAGEFLAGS - 1, which is 63
> and which is a valid BIT() offset for u64.
>
> So __NR_ZRAM_PAGEFLAGS == BITS_PER_LONG should be a valid case.
I apologize, you are correct.
Reviewed-by: Brian Geffon <bgeffon@google.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCHv2] zram: do not waste zram_table_entry flags bits
2022-09-13 2:07 ` Brian Geffon
@ 2022-09-13 2:10 ` Sergey Senozhatsky
0 siblings, 0 replies; 6+ messages in thread
From: Sergey Senozhatsky @ 2022-09-13 2:10 UTC (permalink / raw)
To: Brian Geffon
Cc: Sergey Senozhatsky, Minchan Kim, Andrew Morton, Nitin Gupta, LKML,
linux-mm
On (22/09/12 22:07), Brian Geffon wrote:
> > On (22/09/12 11:39), Brian Geffon wrote:
> > > > zram_table_entry::flags stores object size in the lower bits and
> > > > zram pageflags in the upper bits. However, for some reason, we
> > > > use 24 lower bits, while maximum zram object size is PAGE_SIZE,
> > > > which requires PAGE_SHIFT bits (up to 16 on arm64). This wastes
> > > > 24 - PAGE_SHIFT bits that we can use for additional zram pageflags
> > > > instead.
> > > >
> > > > Also add a BUILD_BUG_ON() to alert us should we run out of bits
> > > > in zram_table_entry::flags.
> > > >
> > > > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> > > > ---
> > > > drivers/block/zram/zram_drv.c | 2 ++
> > > > drivers/block/zram/zram_drv.h | 15 +++++++--------
> > > > 2 files changed, 9 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > > > index f3948abce2f7..07913bcdb5c2 100644
> > > > --- a/drivers/block/zram/zram_drv.c
> > > > +++ b/drivers/block/zram/zram_drv.c
> > > > @@ -2449,6 +2449,8 @@ static int __init zram_init(void)
> > > > {
> > > > int ret;
> > > >
> > > > + BUILD_BUG_ON(__NR_ZRAM_PAGEFLAGS > BITS_PER_LONG);
> > >
> > > Should this be >= BITS_PER_LONG?
> >
> > __NR_ZRAM_PAGEFLAGS == BITS_PER_LONG == 64 (e.g. on 64 bit host)
> > means that the last valid zram pageflag (and __NR_ZRAM_PAGEFLAGS
> > is not a valid pageflag) is __NR_ZRAM_PAGEFLAGS - 1, which is 63
> > and which is a valid BIT() offset for u64.
> >
> > So __NR_ZRAM_PAGEFLAGS == BITS_PER_LONG should be a valid case.
>
> I apologize, you are correct.
No worries! It's always good to double check.
> Reviewed-by: Brian Geffon <bgeffon@google.com>
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv2] zram: do not waste zram_table_entry flags bits
2022-09-12 15:27 [PATCHv2] zram: do not waste zram_table_entry flags bits Sergey Senozhatsky
2022-09-12 15:39 ` Brian Geffon
@ 2022-09-15 16:36 ` Minchan Kim
1 sibling, 0 replies; 6+ messages in thread
From: Minchan Kim @ 2022-09-15 16:36 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Andrew Morton, Nitin Gupta, Brian Geffon, linux-kernel, linux-mm
On Tue, Sep 13, 2022 at 12:27:44AM +0900, Sergey Senozhatsky wrote:
> zram_table_entry::flags stores object size in the lower bits and
> zram pageflags in the upper bits. However, for some reason, we
> use 24 lower bits, while maximum zram object size is PAGE_SIZE,
> which requires PAGE_SHIFT bits (up to 16 on arm64). This wastes
> 24 - PAGE_SHIFT bits that we can use for additional zram pageflags
> instead.
>
> Also add a BUILD_BUG_ON() to alert us should we run out of bits
> in zram_table_entry::flags.
>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Acked-by: Minchan Kim <minchan@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-15 16:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-12 15:27 [PATCHv2] zram: do not waste zram_table_entry flags bits Sergey Senozhatsky
2022-09-12 15:39 ` Brian Geffon
2022-09-13 1:54 ` Sergey Senozhatsky
2022-09-13 2:07 ` Brian Geffon
2022-09-13 2:10 ` Sergey Senozhatsky
2022-09-15 16:36 ` Minchan Kim
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 NNTP newsgroup(s).