From e1ab96f8cc713c673df4ab806bd90918e5ab6c98 Mon Sep 17 00:00:00 2001 From: "Matthew Wilcox (Oracle)" Date: Thu, 6 Aug 2020 23:19:32 -0700 Subject: mm/debug: handle page->mapping better in dump_page Patch series "Improvements for dump_page()", v2. Here's a sample dump of a pagecache tail page with all of the patches applied: page:000000006d1c49ca refcount:6 mapcount:0 mapping:00000000136b8d90 index:0x109 pfn:0x6c645 head:000000008bd38076 order:2 compound_mapcount:0 compound_pincount:0 aops:xfs_address_space_operations ino:800042 dentry name:"fd" flags: 0x4000000000012014(uptodate|lru|private|head) raw: 4000000000000000 ffffd46ac1b19101 ffffffff00000202 dead000000000004 raw: 0000000000000001 0000000000000000 00000000ffffffff 0000000000000000 head: 4000000000012014 ffffd46ac1b1bbc8 ffffd46ac1b1bc08 ffff91976f659560 head: 0000000000000108 ffff919773220680 00000006ffffffff 0000000000000000 page dumped because: testing This patch (of 6): If we can't call page_mapping() to get the page mapping, handle the anon/ksm/movable bits correctly. [akpm@linux-foundation.org: augmented code comment from John] Link: http://lkml.kernel.org/r/15cff11a-6762-8a6a-3f0e-dd227280cd6f@nvidia.com Signed-off-by: Matthew Wilcox (Oracle) Signed-off-by: Andrew Morton Reviewed-by: John Hubbard Acked-by: Mike Rapoport Acked-by: Vlastimil Babka Cc: William Kucharski Cc: "Kirill A. Shutemov" Link: http://lkml.kernel.org/r/20200709202117.7216-1-willy@infradead.org Link: http://lkml.kernel.org/r/20200709202117.7216-2-willy@infradead.org Signed-off-by: Linus Torvalds --- mm/debug.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) (limited to 'mm/debug.c') diff --git a/mm/debug.c b/mm/debug.c index 4f376514744d..567b80b0b9e4 100644 --- a/mm/debug.c +++ b/mm/debug.c @@ -69,8 +69,19 @@ void __dump_page(struct page *page, const char *reason) } if (page < head || (page >= head + MAX_ORDER_NR_PAGES)) { - /* Corrupt page, cannot call page_mapping */ - mapping = page->mapping; + /* + * Corrupt page, so we cannot call page_mapping. Instead, do a + * safe subset of the steps that page_mapping() does. Caution: + * this will be misleading for tail pages, PageSwapCache pages, + * and potentially other situations. (See the page_mapping() + * implementation for what's missing here.) + */ + unsigned long tmp = (unsigned long)page->mapping; + + if (tmp & PAGE_MAPPING_ANON) + mapping = NULL; + else + mapping = (void *)(tmp & ~PAGE_MAPPING_FLAGS); head = page; compound = false; } else { -- cgit v1.2.3 From 452b557c9500d27903a20c450ca8d2919c3742be Mon Sep 17 00:00:00 2001 From: "Matthew Wilcox (Oracle)" Date: Thu, 6 Aug 2020 23:19:35 -0700 Subject: mm/debug: dump compound page information on a second line Simplify both the implementation and the output by splitting all the compound page information onto a second line. Reported-by: John Hubbard Signed-off-by: Matthew Wilcox (Oracle) Signed-off-by: Andrew Morton Tested-by: John Hubbard Reviewed-by: John Hubbard Acked-by: Mike Rapoport Acked-by: Vlastimil Babka Cc: "Kirill A. Shutemov" Cc: William Kucharski Link: http://lkml.kernel.org/r/20200709202117.7216-3-willy@infradead.org Signed-off-by: Linus Torvalds --- mm/debug.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) (limited to 'mm/debug.c') diff --git a/mm/debug.c b/mm/debug.c index 567b80b0b9e4..b0ebf83eec89 100644 --- a/mm/debug.c +++ b/mm/debug.c @@ -95,27 +95,21 @@ void __dump_page(struct page *page, const char *reason) */ mapcount = PageSlab(head) ? 0 : page_mapcount(page); - if (compound) + pr_warn("page:%px refcount:%d mapcount:%d mapping:%p index:%#lx\n", + page, page_ref_count(head), mapcount, mapping, + page_to_pgoff(page)); + if (compound) { if (hpage_pincount_available(page)) { - pr_warn("page:%px refcount:%d mapcount:%d mapping:%p " - "index:%#lx head:%px order:%u " - "compound_mapcount:%d compound_pincount:%d\n", - page, page_ref_count(head), mapcount, - mapping, page_to_pgoff(page), head, - compound_order(head), compound_mapcount(page), - compound_pincount(page)); + pr_warn("head:%px order:%u compound_mapcount:%d compound_pincount:%d\n", + head, compound_order(head), + compound_mapcount(head), + compound_pincount(head)); } else { - pr_warn("page:%px refcount:%d mapcount:%d mapping:%p " - "index:%#lx head:%px order:%u " - "compound_mapcount:%d\n", - page, page_ref_count(head), mapcount, - mapping, page_to_pgoff(page), head, - compound_order(head), compound_mapcount(page)); + pr_warn("head:%px order:%u compound_mapcount:%d\n", + head, compound_order(head), + compound_mapcount(head)); } - else - pr_warn("page:%px refcount:%d mapcount:%d mapping:%p index:%#lx\n", - page, page_ref_count(page), mapcount, - mapping, page_to_pgoff(page)); + } if (PageKsm(page)) type = "ksm "; else if (PageAnon(page)) -- cgit v1.2.3 From 0b93d59e90d4403dd7c6bccaf27bd5bc91396866 Mon Sep 17 00:00:00 2001 From: "Matthew Wilcox (Oracle)" Date: Thu, 6 Aug 2020 23:19:39 -0700 Subject: mm/debug: print head flags in dump_page Tail page flags contain very little useful information. Print the head page's flags instead. While the flags will contain "head" for tail pages, this should not be too confusing as the previous line starts with the word "head:" and so the flags should be interpreted as belonging to the head page. Signed-off-by: Matthew Wilcox (Oracle) Signed-off-by: Andrew Morton Reviewed-by: John Hubbard Acked-by: Mike Rapoport Acked-by: Vlastimil Babka Cc: "Kirill A. Shutemov" Cc: William Kucharski Link: http://lkml.kernel.org/r/20200709202117.7216-4-willy@infradead.org Signed-off-by: Linus Torvalds --- mm/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'mm/debug.c') diff --git a/mm/debug.c b/mm/debug.c index b0ebf83eec89..27837c18027c 100644 --- a/mm/debug.c +++ b/mm/debug.c @@ -168,7 +168,7 @@ void __dump_page(struct page *page, const char *reason) out_mapping: BUILD_BUG_ON(ARRAY_SIZE(pageflag_names) != __NR_PAGEFLAGS + 1); - pr_warn("%sflags: %#lx(%pGp)%s\n", type, page->flags, &page->flags, + pr_warn("%sflags: %#lx(%pGp)%s\n", type, head->flags, &head->flags, page_cma ? " CMA" : ""); hex_only: -- cgit v1.2.3 From 9ad3826575abd1c096cf678a87dd860395c46d78 Mon Sep 17 00:00:00 2001 From: "Matthew Wilcox (Oracle)" Date: Thu, 6 Aug 2020 23:19:42 -0700 Subject: mm/debug: switch dump_page to get_kernel_nofault This is simpler to use than copy_from_kernel_nofault(). Also make some of the related error messages less verbose. Signed-off-by: Matthew Wilcox (Oracle) Signed-off-by: Andrew Morton Acked-by: Mike Rapoport Acked-by: Vlastimil Babka Cc: John Hubbard Cc: "Kirill A. Shutemov" Cc: William Kucharski Link: http://lkml.kernel.org/r/20200709202117.7216-5-willy@infradead.org Signed-off-by: Linus Torvalds --- mm/debug.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) (limited to 'mm/debug.c') diff --git a/mm/debug.c b/mm/debug.c index 27837c18027c..4c62ee5f0ae6 100644 --- a/mm/debug.c +++ b/mm/debug.c @@ -115,54 +115,50 @@ void __dump_page(struct page *page, const char *reason) else if (PageAnon(page)) type = "anon "; else if (mapping) { - const struct inode *host; + struct inode *host; const struct address_space_operations *a_ops; - const struct hlist_node *dentry_first; - const struct dentry *dentry_ptr; + struct hlist_node *dentry_first; + struct dentry *dentry_ptr; struct dentry dentry; /* * mapping can be invalid pointer and we don't want to crash * accessing it, so probe everything depending on it carefully */ - if (copy_from_kernel_nofault(&host, &mapping->host, - sizeof(struct inode *)) || - copy_from_kernel_nofault(&a_ops, &mapping->a_ops, - sizeof(struct address_space_operations *))) { - pr_warn("failed to read mapping->host or a_ops, mapping not a valid kernel address?\n"); + if (get_kernel_nofault(host, &mapping->host) || + get_kernel_nofault(a_ops, &mapping->a_ops)) { + pr_warn("failed to read mapping contents, not a valid kernel address?\n"); goto out_mapping; } if (!host) { - pr_warn("mapping->a_ops:%ps\n", a_ops); + pr_warn("aops:%ps\n", a_ops); goto out_mapping; } - if (copy_from_kernel_nofault(&dentry_first, - &host->i_dentry.first, sizeof(struct hlist_node *))) { - pr_warn("mapping->a_ops:%ps with invalid mapping->host inode address %px\n", - a_ops, host); + if (get_kernel_nofault(dentry_first, &host->i_dentry.first)) { + pr_warn("aops:%ps with invalid host inode %px\n", + a_ops, host); goto out_mapping; } if (!dentry_first) { - pr_warn("mapping->a_ops:%ps\n", a_ops); + pr_warn("aops:%ps\n", a_ops); goto out_mapping; } dentry_ptr = container_of(dentry_first, struct dentry, d_u.d_alias); - if (copy_from_kernel_nofault(&dentry, dentry_ptr, - sizeof(struct dentry))) { - pr_warn("mapping->aops:%ps with invalid mapping->host->i_dentry.first %px\n", - a_ops, dentry_ptr); + if (get_kernel_nofault(dentry, dentry_ptr)) { + pr_warn("aops:%ps with invalid dentry %px\n", a_ops, + dentry_ptr); } else { /* * if dentry is corrupted, the %pd handler may still * crash, but it's unlikely that we reach here with a * corrupted struct page */ - pr_warn("mapping->aops:%ps dentry name:\"%pd\"\n", - a_ops, &dentry); + pr_warn("aops:%ps dentry name:\"%pd\"\n", a_ops, + &dentry); } } out_mapping: -- cgit v1.2.3 From 9bdaf2cc5eb570a875c6871ef7bee86a8c5be2d7 Mon Sep 17 00:00:00 2001 From: "Matthew Wilcox (Oracle)" Date: Thu, 6 Aug 2020 23:19:45 -0700 Subject: mm/debug: print the inode number in dump_page The inode number helps correlate this page with debug messages elsewhere in the kernel. Signed-off-by: Matthew Wilcox (Oracle) Signed-off-by: Andrew Morton Reviewed-by: John Hubbard Acked-by: Mike Rapoport Cc: "Kirill A. Shutemov" Cc: Vlastimil Babka Cc: William Kucharski Link: http://lkml.kernel.org/r/20200709202117.7216-6-willy@infradead.org Signed-off-by: Linus Torvalds --- mm/debug.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'mm/debug.c') diff --git a/mm/debug.c b/mm/debug.c index 4c62ee5f0ae6..c4dbe4def83d 100644 --- a/mm/debug.c +++ b/mm/debug.c @@ -143,7 +143,7 @@ void __dump_page(struct page *page, const char *reason) } if (!dentry_first) { - pr_warn("aops:%ps\n", a_ops); + pr_warn("aops:%ps ino:%lx\n", a_ops, host->i_ino); goto out_mapping; } @@ -157,8 +157,8 @@ void __dump_page(struct page *page, const char *reason) * crash, but it's unlikely that we reach here with a * corrupted struct page */ - pr_warn("aops:%ps dentry name:\"%pd\"\n", a_ops, - &dentry); + pr_warn("aops:%ps ino:%lx dentry name:\"%pd\"\n", + a_ops, host->i_ino, &dentry); } } out_mapping: -- cgit v1.2.3 From 54a75157d97976f35e63e37e31e39bda622cbf39 Mon Sep 17 00:00:00 2001 From: "Matthew Wilcox (Oracle)" Date: Thu, 6 Aug 2020 23:19:48 -0700 Subject: mm/debug: print hashed address of struct page The actual address of the struct page isn't particularly helpful, while the hashed address helps match with other messages elsewhere. Add the PFN that the page refers to in order to help diagnose problems where the page is improperly aligned for the purpose. Signed-off-by: Matthew Wilcox (Oracle) Signed-off-by: Andrew Morton Reviewed-by: John Hubbard Acked-by: Mike Rapoport Cc: "Kirill A. Shutemov" Cc: Vlastimil Babka Cc: William Kucharski Link: http://lkml.kernel.org/r/20200709202117.7216-7-willy@infradead.org Signed-off-by: Linus Torvalds --- mm/debug.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'mm/debug.c') diff --git a/mm/debug.c b/mm/debug.c index c4dbe4def83d..8f569db9a514 100644 --- a/mm/debug.c +++ b/mm/debug.c @@ -95,17 +95,17 @@ void __dump_page(struct page *page, const char *reason) */ mapcount = PageSlab(head) ? 0 : page_mapcount(page); - pr_warn("page:%px refcount:%d mapcount:%d mapping:%p index:%#lx\n", + pr_warn("page:%p refcount:%d mapcount:%d mapping:%p index:%#lx pfn:%#lx\n", page, page_ref_count(head), mapcount, mapping, - page_to_pgoff(page)); + page_to_pgoff(page), page_to_pfn(page)); if (compound) { if (hpage_pincount_available(page)) { - pr_warn("head:%px order:%u compound_mapcount:%d compound_pincount:%d\n", + pr_warn("head:%p order:%u compound_mapcount:%d compound_pincount:%d\n", head, compound_order(head), compound_mapcount(head), compound_pincount(head)); } else { - pr_warn("head:%px order:%u compound_mapcount:%d\n", + pr_warn("head:%p order:%u compound_mapcount:%d\n", head, compound_order(head), compound_mapcount(head)); } -- cgit v1.2.3 From 6dc5ea16c86f753951f53085aa04df49cf17cb50 Mon Sep 17 00:00:00 2001 From: John Hubbard Date: Thu, 6 Aug 2020 23:19:51 -0700 Subject: mm, dump_page: do not crash with bad compound_mapcount() If a compound page is being split while dump_page() is being run on that page, we can end up calling compound_mapcount() on a page that is no longer compound. This leads to a crash (already seen at least once in the field), due to the VM_BUG_ON_PAGE() assertion inside compound_mapcount(). (The above is from Matthew Wilcox's analysis of Qian Cai's bug report.) A similar problem is possible, via compound_pincount() instead of compound_mapcount(). In order to avoid this kind of crash, make dump_page() slightly more robust, by providing a pair of simpler routines that don't contain assertions: head_mapcount() and head_pincount(). For debug tools, we don't want to go *too* far in this direction, but this is a simple small fix, and the crash has already been seen, so it's a good trade-off. Reported-by: Qian Cai Suggested-by: Matthew Wilcox Signed-off-by: John Hubbard Signed-off-by: Andrew Morton Acked-by: Vlastimil Babka Cc: Kirill A. Shutemov Cc: Mike Rapoport Cc: William Kucharski Link: http://lkml.kernel.org/r/20200804214807.169256-1-jhubbard@nvidia.com Signed-off-by: Linus Torvalds --- include/linux/mm.h | 14 ++++++++++++-- mm/debug.c | 6 +++--- 2 files changed, 15 insertions(+), 5 deletions(-) (limited to 'mm/debug.c') diff --git a/include/linux/mm.h b/include/linux/mm.h index dc7b87310c10..303a47a9769d 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -779,6 +779,11 @@ static inline void *kvcalloc(size_t n, size_t size, gfp_t flags) extern void kvfree(const void *addr); extern void kvfree_sensitive(const void *addr, size_t len); +static inline int head_mapcount(struct page *head) +{ + return atomic_read(compound_mapcount_ptr(head)) + 1; +} + /* * Mapcount of compound page as a whole, does not include mapped sub-pages. * @@ -788,7 +793,7 @@ static inline int compound_mapcount(struct page *page) { VM_BUG_ON_PAGE(!PageCompound(page), page); page = compound_head(page); - return atomic_read(compound_mapcount_ptr(page)) + 1; + return head_mapcount(page); } /* @@ -901,11 +906,16 @@ static inline bool hpage_pincount_available(struct page *page) return PageCompound(page) && compound_order(page) > 1; } +static inline int head_pincount(struct page *head) +{ + return atomic_read(compound_pincount_ptr(head)); +} + static inline int compound_pincount(struct page *page) { VM_BUG_ON_PAGE(!hpage_pincount_available(page), page); page = compound_head(page); - return atomic_read(compound_pincount_ptr(page)); + return head_pincount(page); } static inline void set_compound_order(struct page *page, unsigned int order) diff --git a/mm/debug.c b/mm/debug.c index 8f569db9a514..ca8d1cacdecc 100644 --- a/mm/debug.c +++ b/mm/debug.c @@ -102,12 +102,12 @@ void __dump_page(struct page *page, const char *reason) if (hpage_pincount_available(page)) { pr_warn("head:%p order:%u compound_mapcount:%d compound_pincount:%d\n", head, compound_order(head), - compound_mapcount(head), - compound_pincount(head)); + head_mapcount(head), + head_pincount(head)); } else { pr_warn("head:%p order:%u compound_mapcount:%d\n", head, compound_order(head), - compound_mapcount(head)); + head_mapcount(head)); } } if (PageKsm(page)) -- cgit v1.2.3