On 12/15/21 02:03, Roman Gushchin 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: > > Hi Vlastimil! > > I've started to review this patchset (btw, a really nice work, I like > the resulting code way more). Because I'm looking at v3 and I don't have > the whole v2 in my mailbox, here is what I've now: Thanks a lot, Roman! > * mm: add virt_to_folio() and folio_address() > Reviewed-by: Roman Gushchin <guro@xxxxxx> > > * mm/slab: Dissolve slab_map_pages() in its caller > Reviewed-by: Roman Gushchin <guro@xxxxxx> > > * mm/slub: Make object_err() static > Reviewed-by: Roman Gushchin <guro@xxxxxx> > > * mm: Split slab into its own type > 1) Shouldn't SLAB_MATCH() macro use struct folio instead of struct page for the > comparison? Folio doesn't have define most of the fields, and matching some to page and others to folio seems like unnecessary complication. Maybe as part of the final struct page cleanup when the slab fields are gone from struct page, the rest could all be in folio - I'll check once we get there. > 2) page_slab() is used only in kasan and only in one place, so maybe it's better > to not introduce it as a generic helper? Yeah that's the case after the series, but as part of the incremental steps, page_slab() gets used in many places. I'll consider removing it on top though. > Other than that > Reviewed-by: Roman Gushchin <guro@xxxxxx> > > * mm: Add account_slab() and unaccount_slab() > 1) maybe change the title to convert/replace instead of add? Done. > 2) maybe move later changes to memcg_alloc_page_obj_cgroups() to this patch? Maybe possible, but that would distort the series more than I'd like to at this rc6 time. > Reviewed-by: Roman Gushchin <guro@xxxxxx> > > * mm: Convert virt_to_cache() to use struct slab > Reviewed-by: Roman Gushchin <guro@xxxxxx> > > * mm: Convert __ksize() to struct slab > It looks like certain parts of __ksize() can be merged between slab, slub and slob? > Reviewed-by: Roman Gushchin <guro@xxxxxx> > > * mm: Use struct slab in kmem_obj_info() > Reviewed-by: Roman Gushchin <guro@xxxxxx> > > > I'll try to finish reviewing the patchset until the end of the week. > > Thanks! > > Roman