On Thu, Jul 20, 2023 at 2:52 PM Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx> wrote: > > 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. Right. For this patch series, do we need to maintain memcg_data in zsdec to avoid any subtle problems?
