On 12/16/21 16:00, Hyeonggon Yoo wrote: > On Tue, Dec 14, 2021 at 01:57:22PM +0100, Vlastimil Babka wrote: >> On 12/1/21 19:14, Vlastimil Babka wrote: >> > Folks from non-slab subsystems are Cc'd only to patches affecting them, and >> > this cover letter. >> > >> > Series also available in git, based on 5.16-rc3: >> > https://git.kernel.org/pub/scm/linux/kernel/git/vbabka/linux.git/log/?h=slab-struct_slab-v2r2 >> >> Pushed a new branch slab-struct-slab-v3r3 with accumulated fixes and small tweaks >> and a new patch from Hyeonggon Yoo on top. To avoid too much spam, here's a range diff: > > Reviewing the whole patch series takes longer than I thought. > I'll try to review and test rest of patches when I have time. > > I added Tested-by if kernel builds okay and kselftests > does not break the kernel on my machine. > (with CONFIG_SLAB/SLUB/SLOB depending on the patch), Thanks! > Let me know me if you know better way to test a patch. Testing on your machine is just fine. > # mm/slub: Define struct slab fields for CONFIG_SLUB_CPU_PARTIAL only when enabled > > Reviewed-by: Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx> > Tested-by: Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx> > > Comment: > Works on both SLUB_CPU_PARTIAL and !SLUB_CPU_PARTIAL. > btw, do we need slabs_cpu_partial attribute when we don't use > cpu partials? (!SLUB_CPU_PARTIAL) The sysfs attribute? Yeah we should be consistent to userspace expecting to read it (even with zeroes), regardless of config. > # mm/slub: Simplify struct slab slabs field definition > Comment: > > This is how struct page looks on the top of v3r3 branch: > struct page { > [...] > struct { /* slab, slob and slub */ > union { > struct list_head slab_list; > struct { /* Partial pages */ > struct page *next; > #ifdef CONFIG_64BIT > int pages; /* Nr of pages left */ > #else > short int pages; > #endif > }; > }; > [...] > > It's not consistent with struct slab. Hm right. But as we don't actually use the struct page version anymore, and it's not one of the fields checked by SLAB_MATCH(), we can ignore this. > I think this is because "mm: Remove slab from struct page" was dropped. That was just postponed until iommu changes are in. Matthew mentioned those might be merged too, so that final cleanup will happen too and take care of the discrepancy above, so no need for extra churn to address it speficially. > Would you update some of patches? > > # mm/sl*b: Differentiate struct slab fields by sl*b implementations > Reviewed-by: Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx> > Tested-by: Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx> > Works SL[AUO]B on my machine and makes code much better. > > # mm/slob: Convert SLOB to use struct slab and struct folio > Reviewed-by: Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx> > Tested-by: Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx> > It still works fine on SLOB. > > # mm/slab: Convert kmem_getpages() and kmem_freepages() to struct slab > Reviewed-by: Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx> > Tested-by: Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx> > > # mm/slub: Convert __free_slab() to use struct slab > Reviewed-by: Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx> > Tested-by: Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx> > > Thanks, > Hyeonggon. Thanks again, Vlastimil
