ceph: switch back to testing for NULL folio->private in ceph_dirty_folio
Subsystems (7)
PATCH 1/1 ceph: switch back to testing for NULL folio->private in ceph_dirty_folio
Willy requested that we change this back to warning on folio->private being non-NULl. He's trying to kill off the PG_private flag, and so we'd like to catch where it's non-NULL.
Add a VM_WARN_ON_FOLIO (since it doesn't exist yet) and change over to using that instead of VM_BUG_ON_FOLIO along with testing the ->private pointer.
--- fs/ceph/addr.c | 2 +- include/linux/mmdebug.h | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-)
fs/ceph/addr.c +1 -1
@@ -122,7 +122,7 @@ static bool ceph_dirty_folio(struct address_space *mapping, struct folio *folio)* Reference snap context in folio->private. Also set * PagePrivate so that we get invalidate_folio callback. */ -VM_BUG_ON_FOLIO (folio_test_private (folio), folio); +VM_WARN_ON_FOLIO (folio->private, folio);folio_attach_private (folio, snapc);return ceph_fscache_dirty_folio (mapping, folio);
include/linux/mmdebug.h +9 -0
@@ -54,6 +54,15 @@ void dump_mm(const struct mm_struct *mm); } \unlikely (__ret_warn_once); \ }) +#define VM_WARN_ON_FOLIO (cond, folio) ({ \ + int __ret_warn = !!(cond); \ + \ + if (unlikely(__ret_warn)) { \ + dump_page(&folio->page, "VM_WARN_ON_FOLIO(" __stringify(cond)")");\ + WARN_ON(1); \ + } \ + unlikely(__ret_warn); \ +})#define VM_WARN_ON_ONCE_FOLIO (cond, folio) ({ \ static bool __section(".data.once") __warned; \ int __ret_warn_once = !!(cond); \
Signature · 1 line
2.36.1
Quoted 38 lines · Jeff Layton
Willy requested that we change this back to warning on
folio->privatebeing non-NULl. He's trying to kill off thePG_privateflag, and so we'd like to catch where it's non-NULL.Add a
VM_WARN_ON_FOLIO(since it doesn't exist yet) and change over to using that instead ofVM_BUG_ON_FOLIOalong with testing the ->private pointer.Cc Matthew Wilcox <[email protected]>Signed-off-by Jeff Layton <[email protected]>--- fs/ceph/addr.c | 2 +- include/linux/mmdebug.h | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-)fs/ceph/addr.c +1
-1@@ -122,7 +122,7 @@ static bool ceph_dirty_folio(struct address_space *mapping, struct folio *folio)* Reference snap context in folio->private. Also set * PagePrivate so that we get invalidate_folio callback. */ -VM_BUG_ON_FOLIO (folio_test_private (folio), folio); +VM_WARN_ON_FOLIO (folio->private, folio);folio_attach_private (folio, snapc);return ceph_fscache_dirty_folio (mapping, folio);include/linux/mmdebug.h +2
-0@@ -54,6 +54,15 @@ void dump_mm(const struct mm_struct *mm); } \unlikely (__ret_warn_once); \ }) +#define VM_WARN_ON_FOLIO (cond, folio) ({ \ + int __ret_warn = !!(cond); \
include/linux/mmdebug.h void dump_mm(const struct mm_struct *mm); · line 68
Quoted 10 lines
include/linux/mmdebug.h +7
-0+ \ +if (unlikely (__ret_warn)) { \ + dump_page(&folio->page, "VM_WARN_ON_FOLIO(" __stringify(cond)")");\ +WARN_ON (1 ); \ + } \ +unlikely (__ret_warn); \ +})#define VM_WARN_ON_ONCE_FOLIO (cond, folio) ({ \ static bool __section(".data.once") __warned; \ int __ret_warn_once = !!(cond); \
All tests passed except the known issues.
Merged into testing branch, thanks Jeff!
-- Xiubo
Quoted 35 lines · Jeff Layton
Willy requested that we change this back to warning on
folio->privatebeing non-NULl. He's trying to kill off thePG_privateflag, and so we'd like to catch where it's non-NULL.Add a
VM_WARN_ON_FOLIO(since it doesn't exist yet) and change over to using that instead ofVM_BUG_ON_FOLIOalong with testing the ->private pointer.Cc Matthew Wilcox <[email protected]>Signed-off-by Jeff Layton <[email protected]>--- fs/ceph/addr.c | 2 +- include/linux/mmdebug.h | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-)fs/ceph/addr.c +1
-1@@ -122,7 +122,7 @@ static bool ceph_dirty_folio(struct address_space *mapping, struct folio *folio)* Reference snap context in folio->private. Also set * PagePrivate so that we get invalidate_folio callback. */ -VM_BUG_ON_FOLIO (folio_test_private (folio), folio); +VM_WARN_ON_FOLIO (folio->private, folio);folio_attach_private (folio, snapc);return ceph_fscache_dirty_folio (mapping, folio);include/linux/mmdebug.h +0
-0@@ -54,6 +54,15 @@ void dump_mm(const struct mm_struct *mm); } \unlikely (__ret_warn_once); \
include/linux/mmdebug.h void dump_mm(const struct mm_struct *mm); · line 65
Quoted 10 lines
include/linux/mmdebug.h +9
-0}) +#define VM_WARN_ON_FOLIO (cond, folio) ({ \ + int __ret_warn = !!(cond); \ + \ + if (unlikely(__ret_warn)) { \ + dump_page(&folio->page, "VM_WARN_ON_FOLIO(" __stringify(cond)")");\ + WARN_ON(1); \ + } \ + unlikely(__ret_warn); \ +})
I have fixed the compile warning reported by kernel test robot by defining it in case the DEBUG_VM is disabled in testing branch.
-- Xiubo
Quoted 3 lines
include/linux/mmdebug.h +0
-0#define VM_WARN_ON_ONCE_FOLIO (cond, folio) ({ \ static bool __section(".data.once") __warned; \ int __ret_warn_once = !!(cond); \
Matthew Wilcox
maintainerQuoted 17 lines · Xiubo Li
Quoted 15 lines · Jeff Layton
Willy requested that we change this back to warning on
folio->privatebeing non-NULl. He's trying to kill off thePG_privateflag, and so we'd like to catch where it's non-NULL.Add a
VM_WARN_ON_FOLIO(since it doesn't exist yet) and change over to using that instead ofVM_BUG_ON_FOLIOalong with testing the ->private pointer.Cc Matthew Wilcox <[email protected]>Signed-off-by Jeff Layton <[email protected]>--- fs/ceph/addr.c | 2 +- include/linux/mmdebug.h | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-)
Quoted 12 lines
fs/ceph/addr.c static bool ceph_dirty_folio(struct address_space *mapping, struct folio *folio) · line 127
Quoted 12 lines
fs/ceph/addr.c +1
-1@@ -122,7 +122,7 @@ static bool ceph_dirty_folio(struct address_space *mapping, struct folio *folio)* Reference snap context in folio->private. Also set * PagePrivate so that we get invalidate_folio callback. */ -VM_BUG_ON_FOLIO (folio_test_private (folio), folio); +VM_WARN_ON_FOLIO (folio->private, folio);folio_attach_private (folio, snapc);return ceph_fscache_dirty_folio (mapping, folio);
I found a couple of places where page->private needs to be NULLed out. Neither of them are Ceph's fault. I decided that testing whether folio->private and PG_private are in agreement was better done in folio_unlock() than in any of the other potential places we could check for it.
mm/filemap.c +3 -0
@@ -1535,6 +1535,9 @@ void folio_unlock(struct folio *folio)BUILD_BUG_ON (PG_waiters !=7 );BUILD_BUG_ON (PG_locked >7 );VM_BUG_ON_FOLIO (!folio_test_locked (folio), folio); +VM_BUG_ON_FOLIO (!folio_test_private (folio) && + !folio_test_swapbacked (folio) && +folio_get_private (folio), folio);if (clear_bit_unlock_is_negative_byte (PG_locked,folio_flags (folio,0 )))folio_wake_bit (folio, PG_locked); }
mm/huge_memory.c +1 -0
@@ -2438,6 +2438,7 @@ static void __split_huge_page_tail(struct page *head, int tail, page_tail); page_tail->mapping = head->mapping; page_tail->index = head->index + tail; + page_tail->private =0 ;/* Page flags must be visible before we make the page non-compound. */ smp_wmb ();
mm/migrate.c +2 -0
@@ -1157,6 +1157,8 @@ static int unmap_and_move(new_page_t get_new_page, newpage =get_new_page (page, private);if (!newpage)return -ENOMEM ; +BUG_ON (compound_order (newpage) !=compound_order (page)); + newpage->private =0 ; rc =__unmap_and_move (page, newpage, force, mode);if (rc ==MIGRATEPAGE_SUCCESS )
Quoted 26 lines · Matthew Wilcox
Quoted 25 lines · Xiubo Li
Quoted 24 lines · Jeff Layton
Willy requested that we change this back to warning on
folio->privatebeing non-NULl. He's trying to kill off thePG_privateflag, and so we'd like to catch where it's non-NULL.Add a
VM_WARN_ON_FOLIO(since it doesn't exist yet) and change over to using that instead ofVM_BUG_ON_FOLIOalong with testing the ->private pointer.Cc Matthew Wilcox <[email protected]>Signed-off-by Jeff Layton <[email protected]>--- fs/ceph/addr.c | 2 +- include/linux/mmdebug.h | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-)fs/ceph/addr.c +0
-1@@ -122,7 +122,7 @@ static bool ceph_dirty_folio(struct address_space *mapping, struct folio *folio)* Reference snap context in folio->private. Also set * PagePrivate so that we get invalidate_folio callback. */ -VM_BUG_ON_FOLIO (folio_test_private (folio), folio);
Quoted 8 lines
Quoted 3 lines
fs/ceph/addr.c static bool ceph_dirty_folio(struct address_space *mapping, struct folio *folio) · line 127
Quoted 3 lines
fs/ceph/addr.c +1
-0+VM_WARN_ON_FOLIO (folio->private, folio);folio_attach_private (folio, snapc);return ceph_fscache_dirty_folio (mapping, folio);I found a couple of places where
page->privateneeds to be NULLed out. Neither of them are Ceph's fault. I decided that testing whetherfolio->privateandPG_privateare in agreement was better done infolio_unlock()than in any of the other potential places we could check for it.
Hi Willy,
Cool. I will test this patch today. Thanks!
-- Xiubo
Quoted 40 lines
mm/filemap.c +3
-0@@ -1535,6 +1535,9 @@ void folio_unlock(struct folio *folio)BUILD_BUG_ON (PG_waiters !=7 );BUILD_BUG_ON (PG_locked >7 );VM_BUG_ON_FOLIO (!folio_test_locked (folio), folio); +VM_BUG_ON_FOLIO (!folio_test_private (folio) && + !folio_test_swapbacked (folio) && +folio_get_private (folio), folio);if (clear_bit_unlock_is_negative_byte (PG_locked,folio_flags (folio,0 )))folio_wake_bit (folio, PG_locked); }mm/huge_memory.c +1
-0@@ -2438,6 +2438,7 @@ static void __split_huge_page_tail(struct page *head, int tail, page_tail); page_tail->mapping = head->mapping; page_tail->index = head->index + tail; + page_tail->private =0 ;/* Page flags must be visible before we make the page non-compound. */ smp_wmb ();mm/migrate.c +2
-0@@ -1157,6 +1157,8 @@ static int unmap_and_move(new_page_t get_new_page, newpage =get_new_page (page, private);if (!newpage)return -ENOMEM ; +BUG_ON (compound_order (newpage) !=compound_order (page)); + newpage->private =0 ; rc =__unmap_and_move (page, newpage, force, mode);if (rc ==MIGRATEPAGE_SUCCESS )