reiserfs locking fix
PATCH 5/15 reiserfs locking fix
reiserfs is using b_inode_buffers and fsync_buffers_list() for attaching dependent buffers to its journal. For writeout prior to commit.
This worked OK when a global lock was used everywhere, but the locking is currently incorrect - try_to_free_buffers() is taking a different lock when detaching buffers from their "foreign" inode. So list_head corruption could occur on SMP.
The patch implements a reiserfs_releasepage() which holds the journal-wide buffer lock while it runs try_to_free_buffers(), so all those list_heads are protected. The lock is held across the try_to_free_buffers() call as well, so nobody will attach one of this page's buffers to a list while try_to_free_buffers() is running.
=====================================
fs/reiserfs/inode.c renamed from fs/reiserfs/inode.c~reiserfs_releasepage +38 -0
@@ -2096,9 +2096,47 @@ static int reiserfs_commit_write(structreturn ret ; } +/* +* Returns 1 if the page's buffers were dropped. The page is locked. +* +* Takes j_dirty_buffers_lock to protect the b_assoc_buffers list_heads +* in the buffers at page_buffers(page). +* +* FIXME: Chris says the buffer list is not used with `mount -o notail', +* so in that case the fs can avoid the extra locking. Create a second +* address_space_operations with a NULL ->releasepage and install that +* into new address_spaces. +*/ +static int reiserfs_releasepage (struct page *page,int unused_gfp_flags) +{ +struct inode *inode = page->mapping->host ; +struct reiserfs_journal *j =SB_JOURNAL (inode->i_sb) ; +struct buffer_head *head ; +struct buffer_head *bh ; +int ret =1 ; + +spin_lock (&j->j_dirty_buffers_lock) ; + head =page_buffers (page) ; + bh = head ; +do { +if (!buffer_dirty (bh) && !buffer_locked (bh)) { +list_del_init (&bh->b_assoc_buffers) ; + }else { + ret =0 ; +break ; + } + bh = bh->b_this_page ; + }while (bh != head) ; +if (ret) + ret =try_to_free_buffers (page) ; +spin_unlock (&j->j_dirty_buffers_lock) ; +return ret ; +} +struct address_space_operations reiserfs_address_space_operations = { writepage: reiserfs_writepage, readpage: reiserfs_readpage, + releasepage: reiserfs_releasepage, sync_page: block_sync_page, prepare_write: reiserfs_prepare_write, commit_write: reiserfs_commit_write,
fs/reiserfs/file.c renamed from fs/reiserfs/file.c~reiserfs_releasepage +6 -0
@@ -72,6 +72,12 @@ static void reiserfs_vfs_truncate_file(s }/* Sync a reiserfs file. */ + +/* +* FIXME: sync_mapping_buffers() never has anything to sync. Can +* be removed... +*/ + static int reiserfs_sync_file( struct file * p_s_filp, struct dentry * p_s_dentry,
-
Quoted 10 lines · Andrew Morton
reiserfs is using
b_inode_buffersandfsync_buffers_list()for attaching dependent buffers to its journal. For writeout prior to commit.This worked OK when a global lock was used everywhere, but the locking is currently incorrect -
try_to_free_buffers()is taking a different lock when detaching buffers from their "foreign" inode. Solist_headcorruption could occur on SMP.
Thanks Andrew, this is working for me here.
-chris