| Message ID | 20240522074629.2420423-3-Sukrit.Bhatnagar@sony.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Improve dump_page() output for slab pages | expand |
On Wed, May 22, 2024 at 04:46:29PM +0900, Sukrit Bhatnagar wrote: > The function dump_page() prints "anon" even for slab pages. > This is not correct, especially now that struct slab is separated from > struct page, and that the slab pages cannot be mapped to userspace. > > [ 7.071985] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x102768 > [ 7.072602] head: order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0 > [ 7.073085] anon flags: 0x8000000000000840(slab|head|zone=2) > [ 7.073777] raw: 8000000000000840 ffff8881000419c0 0000000000000000 dead000000000001 > > This debugging output may be misleading, and it is not easy to understand > unless we read the source code. > > If the folio tests true for slab, do not print information that does not > apply to it. Instead, print the slab flags stored in the kmem_cache field. > > [ 7.248722] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff888103e6aa87> > [ 7.249135] head: order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0 > [ 7.249429] slab flags: 0x8000000000000840(slab|head|zone=2) > [ 7.249664] cache flags: 0x10310(HWCACHE_ALIGN|PANIC|TYPESAFE_BY_RCU|CMPXCHG_DOUBLE) > [ 7.249999] raw: 8000000000000000 ffffea00040f9a01 ffffea00040f9bc8 dead000000000400 You haven't tested this against the current codebase ... > @@ -98,6 +101,8 @@ static void __dump_folio(struct folio *folio, struct page *page, > is_migrate_cma_folio(folio, pfn) ? " CMA" : ""); > if (page_has_type(&folio->page)) > pr_warn("page_type: %pGt\n", &folio->page.page_type); > + else if (folio_test_slab(folio)) > + pr_warn("cache flags: %pGs\n", &((struct slab *)&folio->page)->slab_cache->flags); > ... because page_has_type() is now true for slab; there is no more PG_slab. I think you also want: folio_slab(folio)->slab_cache->flags Anyway, we have print_slab_info() which is currently static in slub.c. Maybe that needs to become non-static and dump_page() should call that for slabs?
Hi Matthew, On 2024-05-22 21:32, Matthew Wilcox wrote: > On Wed, May 22, 2024 at 04:46:29PM +0900, Sukrit Bhatnagar wrote: >> If the folio tests true for slab, do not print information that does not >> apply to it. Instead, print the slab flags stored in the kmem_cache field. >> >> [ 7.248722] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff888103e6aa87> >> [ 7.249135] head: order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0 >> [ 7.249429] slab flags: 0x8000000000000840(slab|head|zone=2) >> [ 7.249664] cache flags: 0x10310(HWCACHE_ALIGN|PANIC|TYPESAFE_BY_RCU|CMPXCHG_DOUBLE) >> [ 7.249999] raw: 8000000000000000 ffffea00040f9a01 ffffea00040f9bc8 dead000000000400 > > You haven't tested this against the current codebase ... > >> @@ -98,6 +101,8 @@ static void __dump_folio(struct folio *folio, struct page *page, >> is_migrate_cma_folio(folio, pfn) ? " CMA" : ""); >> if (page_has_type(&folio->page)) >> pr_warn("page_type: %pGt\n", &folio->page.page_type); >> + else if (folio_test_slab(folio)) >> + pr_warn("cache flags: %pGs\n", &((struct slab *)&folio->page)->slab_cache->flags); >> > > ... because page_has_type() is now true for slab; there is no more > PG_slab. I think you also want: > > folio_slab(folio)->slab_cache->flags I didn't notice your other patch about removing PG_slab; it pretty much solves this issue and much more. (I had created these patches a few weeks ago.) > Anyway, we have print_slab_info() which is currently static in slub.c. > Maybe that needs to become non-static and dump_page() should call that > for slabs? Thank you for the suggestions. print_slab_info() has a slightly different output string format, which does not match with the dump_page() output style. Adding it as-it-is looks a bit weird to me. Other than that, I think it may be useful to print it (which would happen only when SLAB_DEBUG is enabled). Also: print_slab_info() is printing the folio's flags. Maybe that needs a change? -- Sukrit
diff --git a/mm/debug.c b/mm/debug.c index 2ef516f310e8..b6892dd279cb 100644 --- a/mm/debug.c +++ b/mm/debug.c @@ -16,6 +16,7 @@ #include <linux/ctype.h> #include "internal.h" +#include "slab.h" #include <trace/events/migrate.h> /* @@ -80,7 +81,9 @@ static void __dump_folio(struct folio *folio, struct page *page, if (folio->memcg_data) pr_warn("memcg:%lx\n", folio->memcg_data); #endif - if (folio_test_ksm(folio)) + if (folio_test_slab(folio)) + type = "slab "; + else if (folio_test_ksm(folio)) type = "ksm "; else if (folio_test_anon(folio)) type = "anon "; @@ -98,6 +101,8 @@ static void __dump_folio(struct folio *folio, struct page *page, is_migrate_cma_folio(folio, pfn) ? " CMA" : ""); if (page_has_type(&folio->page)) pr_warn("page_type: %pGt\n", &folio->page.page_type); + else if (folio_test_slab(folio)) + pr_warn("cache flags: %pGs\n", &((struct slab *)&folio->page)->slab_cache->flags); print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32, sizeof(unsigned long), page,
The function dump_page() prints "anon" even for slab pages. This is not correct, especially now that struct slab is separated from struct page, and that the slab pages cannot be mapped to userspace. [ 7.071985] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x102768 [ 7.072602] head: order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0 [ 7.073085] anon flags: 0x8000000000000840(slab|head|zone=2) [ 7.073777] raw: 8000000000000840 ffff8881000419c0 0000000000000000 dead000000000001 This debugging output may be misleading, and it is not easy to understand unless we read the source code. If the folio tests true for slab, do not print information that does not apply to it. Instead, print the slab flags stored in the kmem_cache field. [ 7.248722] page: refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff888103e6aa87> [ 7.249135] head: order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0 [ 7.249429] slab flags: 0x8000000000000840(slab|head|zone=2) [ 7.249664] cache flags: 0x10310(HWCACHE_ALIGN|PANIC|TYPESAFE_BY_RCU|CMPXCHG_DOUBLE) [ 7.249999] raw: 8000000000000000 ffffea00040f9a01 ffffea00040f9bc8 dead000000000400 Signed-off-by: Sukrit Bhatnagar <Sukrit.Bhatnagar@sony.com> --- mm/debug.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)