On Fri, Jul 21, 2023 at 6:39 AM Yosry Ahmed <yosryahmed@xxxxxxxxxx> 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.
