Commit 91cb02b7 authored by Andrew Morton's avatar Andrew Morton Committed by Linus Torvalds

[PATCH] swapcache bugfixes

Fixes a few lock ranking bugs (and deadlocks) related to
swap_list_lock(), swap_device_lock(), mapping->page_lock and
mapping->private_lock.

- Cannot call block_flushpage->try_to_free_buffers() inside
  mapping->page_lock.  Because __set_page_dirty_buffers() takes
  ->page_lock inside ->private-lock.

- Cannot call swap_free->swap_list_lock/swap_device_lock inside
  mapping->page_lock because exclusive_swap_page() takes ->page_lock
  inside swap_info_get().


The patch also removes all the block_flushpage() calls from the swap
code in favour of a direct call to try_to_free_buffers().

The theory is that the page is locked, there is no I/O underway, nobody
else has access to the buffers so they MUST be freeable.  A bunch of
BUG() checks have been added, and unless someone manages to trigger
one, the "block_flushpage() inside spinlock" problem is fixed.
parent 3aeb30b0
...@@ -53,6 +53,8 @@ ...@@ -53,6 +53,8 @@
* pagemap_lru_lock * pagemap_lru_lock
* ->i_shared_lock (vmtruncate) * ->i_shared_lock (vmtruncate)
* ->private_lock (__free_pte->__set_page_dirty_buffers) * ->private_lock (__free_pte->__set_page_dirty_buffers)
* ->swap_list_lock
* ->swap_device_lock (exclusive_swap_page, others)
* ->mapping->page_lock * ->mapping->page_lock
* ->inode_lock (__mark_inode_dirty) * ->inode_lock (__mark_inode_dirty)
* ->sb_lock (fs/fs-writeback.c) * ->sb_lock (fs/fs-writeback.c)
......
...@@ -14,7 +14,7 @@ ...@@ -14,7 +14,7 @@
#include <linux/init.h> #include <linux/init.h>
#include <linux/pagemap.h> #include <linux/pagemap.h>
#include <linux/smp_lock.h> #include <linux/smp_lock.h>
#include <linux/buffer_head.h> /* for block_sync_page()/block_flushpage() */ #include <linux/buffer_head.h> /* block_sync_page()/try_to_free_buffers() */
#include <asm/pgtable.h> #include <asm/pgtable.h>
...@@ -150,11 +150,15 @@ void delete_from_swap_cache(struct page *page) ...@@ -150,11 +150,15 @@ void delete_from_swap_cache(struct page *page)
{ {
swp_entry_t entry; swp_entry_t entry;
if (!PageLocked(page)) /*
* I/O should have completed and nobody can have a ref against the
* page's buffers
*/
BUG_ON(!PageLocked(page));
BUG_ON(PageWriteback(page));
if (page_has_buffers(page) && !try_to_free_buffers(page))
BUG(); BUG();
block_flushpage(page, 0);
entry.val = page->index; entry.val = page->index;
write_lock(&swapper_space.page_lock); write_lock(&swapper_space.page_lock);
...@@ -219,7 +223,15 @@ int move_from_swap_cache(struct page *page, unsigned long index, ...@@ -219,7 +223,15 @@ int move_from_swap_cache(struct page *page, unsigned long index,
void **pslot; void **pslot;
int err; int err;
if (!PageLocked(page)) /*
* Drop the buffers now, before taking the page_lock. Because
* mapping->private_lock nests outside mapping->page_lock.
* This "must" succeed. The page is locked and all I/O has completed
* and nobody else has a ref against its buffers.
*/
BUG_ON(!PageLocked(page));
BUG_ON(PageWriteback(page));
if (page_has_buffers(page) && !try_to_free_buffers(page))
BUG(); BUG();
write_lock(&swapper_space.page_lock); write_lock(&swapper_space.page_lock);
...@@ -229,10 +241,8 @@ int move_from_swap_cache(struct page *page, unsigned long index, ...@@ -229,10 +241,8 @@ int move_from_swap_cache(struct page *page, unsigned long index,
if (!err) { if (!err) {
swp_entry_t entry; swp_entry_t entry;
block_flushpage(page, 0);
entry.val = page->index; entry.val = page->index;
__delete_from_swap_cache(page); __delete_from_swap_cache(page);
swap_free(entry);
*pslot = page; *pslot = page;
page->flags &= ~(1 << PG_uptodate | 1 << PG_error | page->flags &= ~(1 << PG_uptodate | 1 << PG_error |
...@@ -248,11 +258,16 @@ int move_from_swap_cache(struct page *page, unsigned long index, ...@@ -248,11 +258,16 @@ int move_from_swap_cache(struct page *page, unsigned long index,
/* fix that up */ /* fix that up */
list_del(&page->list); list_del(&page->list);
list_add(&page->list, &mapping->dirty_pages); list_add(&page->list, &mapping->dirty_pages);
write_unlock(&mapping->page_lock);
write_unlock(&swapper_space.page_lock);
/* Do this outside ->page_lock */
swap_free(entry);
return 0;
} }
write_unlock(&mapping->page_lock); write_unlock(&mapping->page_lock);
write_unlock(&swapper_space.page_lock); write_unlock(&swapper_space.page_lock);
return err; return err;
} }
......
...@@ -16,7 +16,7 @@ ...@@ -16,7 +16,7 @@
#include <linux/namei.h> #include <linux/namei.h>
#include <linux/shm.h> #include <linux/shm.h>
#include <linux/blkdev.h> #include <linux/blkdev.h>
#include <linux/buffer_head.h> /* for block_flushpage() */ #include <linux/buffer_head.h> /* for try_to_free_buffers() */
#include <asm/pgtable.h> #include <asm/pgtable.h>
#include <linux/swapops.h> #include <linux/swapops.h>
...@@ -326,7 +326,9 @@ int remove_exclusive_swap_page(struct page *page) ...@@ -326,7 +326,9 @@ int remove_exclusive_swap_page(struct page *page)
swap_info_put(p); swap_info_put(p);
if (retval) { if (retval) {
block_flushpage(page, 0); BUG_ON(PageWriteback(page));
if (page_has_buffers(page) && !try_to_free_buffers(page))
BUG();
swap_free(entry); swap_free(entry);
page_cache_release(page); page_cache_release(page);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment