Re: [PATCH v2 00/33] Separate struct slab from struct page
From: Hyeonggon Yoo <hidden>
Date: 2021-12-29 11:23:06
Also in:
cgroups, linux-iommu, linux-patches
On Wed, Dec 22, 2021 at 05:56:50PM +0100, Vlastimil Babka wrote:
On 12/14/21 13:57, 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-v2r2Pushed 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, I've pushed another update branch slab-struct_slab-v4r1, and also to -next. I've shortened git commit log lines to make checkpatch happier, so no range-diff as it would be too long. I believe it would be useless spam to post the whole series now, shortly before xmas, so I will do it at rc8 time, to hopefully collect remaining reviews. But if anyone wants a mailed version, I can do that.
Hello Matthew and Vlastimil.
it's part 3 of review.
# mm: Convert struct page to struct slab in functions used by other subsystems
Reviewed-by: Hyeonggon Yoo <redacted>
# mm/slub: Convert most struct page to struct slab by spatch
Reviewed-by: Hyeonggon Yoo <redacted>
Tested-by: Hyeonggon Yoo <redacted>
with a question below.
-static int check_slab(struct kmem_cache *s, struct page *page)
+static int check_slab(struct kmem_cache *s, struct slab *slab)
{
int maxobj;
- if (!PageSlab(page)) {
- slab_err(s, page, "Not a valid slab page");
+ if (!folio_test_slab(slab_folio(slab))) {
+ slab_err(s, slab, "Not a valid slab page");
return 0;
}
Can't we guarantee that struct slab * always points to a slab?
for struct page * it can be !PageSlab(page) because struct page *
can be other than slab. but struct slab * can only be slab
unlike struct page. code will be simpler if we guarantee that
struct slab * always points to a slab (or NULL).
# mm/slub: Convert pfmemalloc_match() to take a struct slab
It's confusing to me because the original pfmemalloc_match() is removed
and pfmemalloc_match_unsafe() was renamed to pfmemalloc_match() and
converted to use slab_test_pfmemalloc() helper.
But I agree with the resulting code. so:
Reviewed-by: Hyeonggon Yoo <redacted>
# mm/slub: Convert alloc_slab_page() to return a struct slab
Reviewed-by: Hyeonggon Yoo <redacted>
Tested-by: Hyeonggon Yoo <redacted>
# mm/slub: Convert print_page_info() to print_slab_info()
Reviewed-by: Hyeonggon Yoo <redacted>
I hope to review rest of patches in a week.
Thanks,
Hyeonggon
Changes in v4: - rebase to 5.16-rc6 to avoid a conflict with mainline - collect acks/reviews/tested-by from Johannes, Roman, Hyeonggon Yoo - thanks! - in patch "mm/slub: Convert detached_freelist to use a struct slab" renamed free_nonslab_page() to free_large_kmalloc() and use folio there, as suggested by Roman - in "mm/memcg: Convert slab objcgs from struct page to struct slab" change one caller of slab_objcgs_check() to slab_objcgs() as suggested by Johannes, realize the other caller should be also changed, and remove slab_objcgs_check() completely.