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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2021-12-20 23:58, Vlastimil Babka wrote:
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.

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.


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
_______________________________________________
iommu mailing list
iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/iommu



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [Monitors]

  Powered by Linux