Thread (25 messages) 25 messages, 7 authors, 2022-01-03

Re: [PATCH v2 00/33] Separate struct slab from struct page

From: Robin Murphy <hidden>
Date: 2021-12-21 17:25:17
Also in: linux-iommu, linux-mm, linux-patches

On 2021-12-20 23:58, Vlastimil Babka wrote:
On 12/16/21 16:00, Hyeonggon Yoo wrote:
quoted
On Tue, Dec 14, 2021 at 01:57:22PM +0100, Vlastimil Babka wrote:
quoted
On 12/1/21 19:14, Vlastimil Babka wrote:
quoted
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!
quoted
Let me know me if you know better way to test a patch.
Testing on your machine is just fine.
quoted
# mm/slub: Define struct slab fields for CONFIG_SLUB_CPU_PARTIAL only when enabled

Reviewed-by: Hyeonggon Yoo <redacted>
Tested-by: Hyeonggon Yoo <redacted>

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.
quoted
# 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.
quoted
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.
FYI the IOMMU changes are now queued in linux-next, so if all goes well 
you might be able to sneak that final patch in too.

Robin.
quoted
Would you update some of patches?

# mm/sl*b: Differentiate struct slab fields by sl*b implementations
Reviewed-by: Hyeonggon Yoo <redacted>
Tested-by: Hyeonggon Yoo <redacted>
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 <redacted>
Tested-by: Hyeonggon Yoo <redacted>
It still works fine on SLOB.

# mm/slab: Convert kmem_getpages() and kmem_freepages() to struct slab
Reviewed-by: Hyeonggon Yoo <redacted>
Tested-by: Hyeonggon Yoo <redacted>

# mm/slub: Convert __free_slab() to use struct slab
Reviewed-by: Hyeonggon Yoo <redacted>
Tested-by: Hyeonggon Yoo <redacted>

Thanks,
Hyeonggon.
Thanks again,
Vlastimil
_______________________________________________
iommu mailing list
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help