linux-kernel - Re: [RFC PATCH v2 00/21] mm/zsmalloc: Split zsdesc from struct page

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]

Date:   Fri, 21 Jul 2023 06:52:05 +0900
From:   Hyeonggon Yoo <42.hyeyoo@...il.com>
To:     Yosry Ahmed <yosryahmed@...gle.com>
Cc:     Sergey Senozhatsky <senozhatsky@...omium.org>,
        Minchan Kim <minchan@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, Matthew Wilcox <willy@...radead.org>,
        Mike Rapoport <rppt@...nel.org>
Subject: Re: [RFC PATCH v2 00/21] mm/zsmalloc: Split zsdesc from struct page

On Fri, Jul 21, 2023 at 6:39 AM Yosry Ahmed <yosryahmed@...gle.com> wrote:
>
> <snip>
> >
> > > > > It seems to me though the sizeof(zsdesc) is actually 56 bytes (on
> > > > > 64-bit), so sizeof(zsdesc) + sizeof(memdesc) would be equal to the
> > > > > current size of struct page. If that's true, then there is no loss,
> > > >
> > > > Yeah, zsdesc would be 56 bytes on 64 bit CPUs as memcg_data field is
> > > > not used in zsmalloc.
> > > > More fields in the current struct page might not be needed in the
> > > > future, although it's hard to say at the moment.
> > > > but it's not a loss.
> > >
> > > Is page->memcg_data something that we can drop? Aren't there code
> > > paths that will check page->memcg_data even for kernel pages (e.g.
> > > __folio_put() -> __folio_put_small() -> mem_cgroup_uncharge() ) ?
> >
> > zsmalloc pages are not accounted for via __GFP_ACCOUNT,
>
> Yeah, but the code in the free path above will check page->memcg_data
> nonetheless to check if it is charged.

Right.

> I think to drop memcg_data we need to enlighten the code that some pages
> do not even have memcg_data at all

I agree with you. It should be one of the milestones for all of this to work.
It won't be complicated for the code to be aware of it, because there will be
a freeing (and uncharging if need) routine per type of descriptors.