linux-kernel - Re: [PATCH] zram: do not waste zram_table

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CADyq12yWFMyTRCQmmGoLg7epvXRWu-XqKMS4N2vEomvvuWNpBA@mail.gmail.com>
Date:   Mon, 12 Sep 2022 10:20:49 -0400
From:   Brian Geffon <bgeffon@...gle.com>
To:     Sergey Senozhatsky <senozhatsky@...omium.org>
Cc:     Minchan Kim <minchan@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Nitin Gupta <ngupta@...are.org>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-mm <linux-mm@...ck.org>
Subject: Re: [PATCH] zram: do not waste zram_table_entry flags bits

On Mon, Sep 12, 2022 at 1:38 AM Sergey Senozhatsky
<senozhatsky@...omium.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.
>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@...omium.org>
> ---
>  drivers/block/zram/zram_drv.h | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> index b4eecef2a11f..138be8326866 100644
> --- a/drivers/block/zram/zram_drv.h
> +++ b/drivers/block/zram/zram_drv.h
> @@ -30,16 +30,13 @@
>
>
>  /*
> - * 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 so we can squeeze size and flags into a field.
> + * ZRAM is mainly used for memory efficiency so we want to keep memory
> + * footprint small and thus squeeze size and flags into a flags member.
>   * The lower ZRAM_FLAG_SHIFT bits is for object size (excluding header),
> - * the higher bits is for zram_pageflags.
> + * which cannot be larger than PAGE_SIZE (requiring PAGE_SHIFT bits),
> + * the higher bits are for zram_pageflags.
>   */
> -#define ZRAM_FLAG_SHIFT 24
> +#define ZRAM_FLAG_SHIFT (PAGE_SHIFT + 1)

Why not just hard code 16 with an explanation that it cannot be
increased further using the analysis you did in the other thread? It's
going to be tricky to reason about how many free flag bits actually
remain with PAGE_SHIFT across all architectures, especially given we
have no architecture specific flags.

>
>  /* Flags for zram pages (table[page_no].flags) */
>  enum zram_pageflags {
> --
> 2.37.2.789.g6183377224-goog
>