On Fri, 9 Sep 2016 15:38:38 -0700 Santosh Shilimkar <santosh.shilim...@oracle.com> wrote:
> 'commit 62c230bc1790 ("mm: add support for a filesystem to activate swap > files and use direct_IO for writing swap pages")' replaced swap_aops > dirty hook from __set_page_dirty_no_writeback() to swap_set_page_dirty(). > As such for normal cases without these special SWP flags > code path falls back to __set_page_dirty_no_writeback() > so behaviour is expected to be same as before. > > But swap_set_page_dirty() makes use of helper page_swap_info() to > get sis(swap_info_struct) to check for the flags like SWP_FILE, > SWP_BLKDEV etc as desired for those features. This helper has > BUG_ON(!PageSwapCache(page)) which is racy and safe only for > set_page_dirty_lock() path. For set_page_dirty() path which is > often needed for cases to be called from irq context, kswapd() > can togele the flag behind the back while the call is > getting executed when system is low on memory and heavy > swapping is ongoing. > > This ends up with undesired kernel panic. Patch just moves > the check outside the helper to its users appropriately > to fix kernel panic for the described path. Couple > of users of helpers already take care of SwapCache > condition so I skipped them. > > Cc: Mel Gorman <mgor...@suse.de> > Cc: Joe Perches <j...@perches.com> > Cc: Andrew Morton <a...@linux-foundation.org> > Cc: Peter Zijlstra <a.p.zijls...@chello.nl> > Cc: Rik van Riel <r...@redhat.com> > Cc: David S. Miller <da...@davemloft.net> > Cc: Jens Axboe <ax...@fb.com> > Cc: Michal Hocko <mho...@suse.com> > Cc: Hugh Dickins <hu...@google.com> > Cc: Al Viro <v...@zeniv.linux.org.uk> I'll add Cc: <sta...@vger.kernel.org> [4.7.x] > --- a/mm/page_io.c > +++ b/mm/page_io.c > @@ -264,6 +264,7 @@ int __swap_writepage(struct page *page, struct > writeback_control *wbc, > int ret; > struct swap_info_struct *sis = page_swap_info(page); > > + BUG_ON(!PageSwapCache(page)); > if (sis->flags & SWP_FILE) { > struct kiocb kiocb; > struct file *swap_file = sis->swap_file; > @@ -337,6 +338,7 @@ int swap_readpage(struct page *page) > int ret = 0; > struct swap_info_struct *sis = page_swap_info(page); > > + BUG_ON(!PageSwapCache(page)); > VM_BUG_ON_PAGE(!PageLocked(page), page); > VM_BUG_ON_PAGE(PageUptodate(page), page); > if (frontswap_load(page) == 0) { > @@ -386,6 +388,7 @@ int swap_set_page_dirty(struct page *page) > > if (sis->flags & SWP_FILE) { > struct address_space *mapping = sis->swap_file->f_mapping; > + BUG_ON(!PageSwapCache(page)); > return mapping->a_ops->set_page_dirty(page); > } else { > return __set_page_dirty_no_writeback(page); I guess this is OK for 4.8 but for later kernels, let's quieten it down a bit? From: Andrew Morton <a...@linux-foundation.org> Subject: mm/page_io.c: replace some BUG_ON()s with VM_BUG_ON_PAGE() So they are CONFIG_DEBUG_VM-only and more informative. Cc: Al Viro <v...@zeniv.linux.org.uk> Cc: David S. Miller <da...@davemloft.net> Cc: Hugh Dickins <hu...@google.com> Cc: Jens Axboe <ax...@fb.com> Cc: Joe Perches <j...@perches.com> Cc: Mel Gorman <mgor...@suse.de> Cc: Michal Hocko <mho...@suse.com> Cc: Peter Zijlstra <a.p.zijls...@chello.nl> Cc: Rik van Riel <r...@redhat.com> Cc: Santosh Shilimkar <santosh.shilim...@oracle.com> Signed-off-by: Andrew Morton <a...@linux-foundation.org> --- mm/page_io.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff -puN mm/page_io.c~mm-fix-the-page_swap_info-bug_on-check-fix mm/page_io.c --- a/mm/page_io.c~mm-fix-the-page_swap_info-bug_on-check-fix +++ a/mm/page_io.c @@ -264,7 +264,7 @@ int __swap_writepage(struct page *page, int ret; struct swap_info_struct *sis = page_swap_info(page); - BUG_ON(!PageSwapCache(page)); + VM_BUG_ON_PAGE(!PageSwapCache(page), page); if (sis->flags & SWP_FILE) { struct kiocb kiocb; struct file *swap_file = sis->swap_file; @@ -338,7 +338,7 @@ int swap_readpage(struct page *page) int ret = 0; struct swap_info_struct *sis = page_swap_info(page); - BUG_ON(!PageSwapCache(page)); + VM_BUG_ON_PAGE(!PageSwapCache(page), page); VM_BUG_ON_PAGE(!PageLocked(page), page); VM_BUG_ON_PAGE(PageUptodate(page), page); if (frontswap_load(page) == 0) { @@ -388,7 +388,8 @@ int swap_set_page_dirty(struct page *pag if (sis->flags & SWP_FILE) { struct address_space *mapping = sis->swap_file->f_mapping; - BUG_ON(!PageSwapCache(page)); + + VM_BUG_ON_PAGE(!PageSwapCache(page), page); return mapping->a_ops->set_page_dirty(page); } else { return __set_page_dirty_no_writeback(page); diff -puN mm/swapfile.c~mm-fix-the-page_swap_info-bug_on-check-fix mm/swapfile.c _