Hi,
I tripped over a crash in reiserfs which happened due to PageSwapCache
page being passed to reiserfs_set_page_dirty(). Now it's not that hard to
make reiserfs_set_page_dirty() check that case but I really wonder: Does it
make sense to call mapping->a_ops->set_page_dirty() for a PageSwapCache
page? The page is going to be written via direct IO so from the POV of the
filesystem there's no need for any dirtiness tracking. Also there are
several ->set_page_dirty() implementations which will spectacularly crash
because they do things like page->mapping->host, or call
__set_page_dirty_buffers() which expects buffer heads in page->private.
Or what is the reason for calling filesystem's set_page_dirty() function?
Honza
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
>From 02b238f142b0564fb1cfba133c2a4e0b211330cf Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@xxxxxxx>
Date: Mon, 17 Sep 2012 18:32:19 +0200
Subject: [PATCH] mm: Remove swap_set_page_dirty()
It doesn't make much sense to call filesystem's ->set_page_dirty() method for
PageSwapCache page. It will be written through direct IO so filesystem doesn't
care about its dirtiness and several filesystems actually don't count with such
pages getting into their ->set_page_dirty() functions.
Signed-off-by: Jan Kara <jack@xxxxxxx>
---
mm/page_io.c | 12 ------------
mm/swap_state.c | 2 +-
2 files changed, 1 insertions(+), 13 deletions(-)
diff --git a/mm/page_io.c b/mm/page_io.c
index 78eee32..8520a4f 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -278,15 +278,3 @@ int swap_readpage(struct page *page)
out:
return ret;
}
-
-int swap_set_page_dirty(struct page *page)
-{
- struct swap_info_struct *sis = page_swap_info(page);
-
- if (sis->flags & SWP_FILE) {
- struct address_space *mapping = sis->swap_file->f_mapping;
- return mapping->a_ops->set_page_dirty(page);
- } else {
- return __set_page_dirty_no_writeback(page);
- }
-}
diff --git a/mm/swap_state.c b/mm/swap_state.c
index 0cb36fb..01852cd 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -27,7 +27,7 @@
*/
static const struct address_space_operations swap_aops = {
.writepage = swap_writepage,
- .set_page_dirty = swap_set_page_dirty,
+ .set_page_dirty = set_page_dirty_no_writeback,
.migratepage = migrate_page,
};
--
1.7.1