05 Mar, 2010

1 commit

  • Currently various places in the VFS call vfs_dq_init directly. This means
    we tie the quota code into the VFS. Get rid of that and make the
    filesystem responsible for the initialization. For most metadata operations
    this is a straight forward move into the methods, but for truncate and
    open it's a bit more complicated.

    For truncate we currently only call vfs_dq_init for the sys_truncate case
    because open already takes care of it for ftruncate and open(O_TRUNC) - the
    new code causes an additional vfs_dq_init for those which is harmless.

    For open the initialization is moved from do_filp_open into the open method,
    which means it happens slightly earlier now, and only for regular files.
    The latter is fine because we don't need to initialize it for operations
    on special files, and we already do it as part of the namespace operations
    for directories.

    Add a dquot_file_open helper that filesystems that support generic quotas
    can use to fill in ->open.

    Signed-off-by: Christoph Hellwig
    Signed-off-by: Jan Kara

    Christoph Hellwig
     

09 Jan, 2010

1 commit

  • …t/frederic/random-tracing

    * 'reiserfs/kill-bkl' of git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing:
    reiserfs: Relax reiserfs_xattr_set_handle() while acquiring xattr locks
    reiserfs: Fix unreachable statement
    reiserfs: Don't call reiserfs_get_acl() with the reiserfs lock
    reiserfs: Relax lock on xattr removing
    reiserfs: Relax the lock before truncating pages
    reiserfs: Fix recursive lock on lchown
    reiserfs: Fix mistake in down_write() conversion

    Linus Torvalds
     

07 Jan, 2010

1 commit


05 Jan, 2010

1 commit

  • When we remove an xattr, we call lookup_and_delete_xattr()
    that takes some private xattr inodes mutexes. But we hold
    the reiserfs lock at this time, which leads to dependency
    inversions.

    We can safely call lookup_and_delete_xattr() without the
    reiserfs lock, where xattr inodes lookups only need the
    xattr inodes mutexes.

    Signed-off-by: Frederic Weisbecker
    Cc: Christian Kujau
    Cc: Alexander Beregalov
    Cc: Chris Mason
    Cc: Ingo Molnar

    Frederic Weisbecker
     

03 Jan, 2010

2 commits

  • Fix a mistake in commit 0719d3434747889b314a1e8add776418c4148bcf
    (reiserfs: Fix reiserfs lock i_xattr_sem dependency inversion)
    that has converted a down_write() into a down_read() accidentally.

    Signed-off-by: Frederic Weisbecker
    Cc: Christian Kujau
    Cc: Alexander Beregalov
    Cc: Chris Mason
    Cc: Ingo Molnar

    Frederic Weisbecker
     
  • …t/frederic/random-tracing

    * 'reiserfs/kill-bkl' of git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing:
    reiserfs: Safely acquire i_mutex from xattr_rmdir
    reiserfs: Safely acquire i_mutex from reiserfs_for_each_xattr
    reiserfs: Fix journal mutex <-> inode mutex lock inversion
    reiserfs: Fix unwanted recursive reiserfs lock in reiserfs_unlink()
    reiserfs: Relax lock before open xattr dir in reiserfs_xattr_set_handle()
    reiserfs: Relax reiserfs lock while freeing the journal
    reiserfs: Fix reiserfs lock <-> i_mutex dependency inversion on xattr
    reiserfs: Warn on lock relax if taken recursively
    reiserfs: Fix reiserfs lock <-> i_xattr_sem dependency inversion
    reiserfs: Fix remaining in-reclaim-fs <-> reclaim-fs-on locking inversion
    reiserfs: Fix reiserfs lock <-> inode mutex dependency inversion
    reiserfs: Fix reiserfs lock and journal lock inversion dependency
    reiserfs: Fix possible recursive lock

    Linus Torvalds
     

02 Jan, 2010

6 commits

  • Relax the reiserfs lock before taking the inode mutex from
    xattr_rmdir() to avoid the usual reiserfs lock inode mutex
    bad dependency.

    Signed-off-by: Frederic Weisbecker
    Tested-by: Christian Kujau
    Cc: Alexander Beregalov
    Cc: Chris Mason
    Cc: Ingo Molnar

    Frederic Weisbecker
     
  • Relax the reiserfs lock before taking the inode mutex from
    reiserfs_for_each_xattr() to avoid the usual bad dependencies:

    =======================================================
    [ INFO: possible circular locking dependency detected ]
    2.6.32-atom #179
    -------------------------------------------------------
    rm/3242 is trying to acquire lock:
    (&sb->s_type->i_mutex_key#4/3){+.+.+.}, at: [] reiserfs_for_each_xattr+0x23f/0x290

    but task is already holding lock:
    (&REISERFS_SB(s)->lock){+.+.+.}, at: [] reiserfs_write_lock+0x29/0x40

    which lock already depends on the new lock.

    the existing dependency chain (in reverse order) is:

    -> #1 (&REISERFS_SB(s)->lock){+.+.+.}:
    [] __lock_acquire+0x11ff/0x19e0
    [] lock_acquire+0x68/0x90
    [] mutex_lock_nested+0x5b/0x340
    [] reiserfs_write_lock_once+0x29/0x50
    [] reiserfs_lookup+0x62/0x140
    [] __lookup_hash+0xef/0x110
    [] lookup_one_len+0x8d/0xc0
    [] open_xa_dir+0xea/0x1b0
    [] reiserfs_for_each_xattr+0x70/0x290
    [] reiserfs_delete_xattrs+0x1a/0x60
    [] reiserfs_delete_inode+0x9f/0x150
    [] generic_delete_inode+0xa2/0x170
    [] generic_drop_inode+0x4f/0x70
    [] iput+0x47/0x50
    [] do_unlinkat+0xd5/0x160
    [] sys_unlinkat+0x23/0x40
    [] sysenter_do_call+0x12/0x32

    -> #0 (&sb->s_type->i_mutex_key#4/3){+.+.+.}:
    [] __lock_acquire+0x18f6/0x19e0
    [] lock_acquire+0x68/0x90
    [] mutex_lock_nested+0x5b/0x340
    [] reiserfs_for_each_xattr+0x23f/0x290
    [] reiserfs_delete_xattrs+0x1a/0x60
    [] reiserfs_delete_inode+0x9f/0x150
    [] generic_delete_inode+0xa2/0x170
    [] generic_drop_inode+0x4f/0x70
    [] iput+0x47/0x50
    [] do_unlinkat+0xd5/0x160
    [] sys_unlinkat+0x23/0x40
    [] sysenter_do_call+0x12/0x32

    other info that might help us debug this:

    1 lock held by rm/3242:
    #0: (&REISERFS_SB(s)->lock){+.+.+.}, at: [] reiserfs_write_lock+0x29/0x40

    stack backtrace:
    Pid: 3242, comm: rm Not tainted 2.6.32-atom #179
    Call Trace:
    [] ? printk+0x18/0x1a
    [] print_circular_bug+0xca/0xd0
    [] __lock_acquire+0x18f6/0x19e0
    [] ? mark_held_locks+0x62/0x80
    [] ? trace_hardirqs_on+0xb/0x10
    [] ? mutex_unlock+0x8/0x10
    [] lock_acquire+0x68/0x90
    [] ? reiserfs_for_each_xattr+0x23f/0x290
    [] ? reiserfs_for_each_xattr+0x23f/0x290
    [] mutex_lock_nested+0x5b/0x340
    [] ? reiserfs_for_each_xattr+0x23f/0x290
    [] reiserfs_for_each_xattr+0x23f/0x290
    [] ? delete_one_xattr+0x0/0x100
    [] reiserfs_delete_xattrs+0x1a/0x60
    [] ? reiserfs_write_lock_once+0x29/0x50
    [] reiserfs_delete_inode+0x9f/0x150
    [] ? _atomic_dec_and_lock+0x4f/0x70
    [] ? reiserfs_delete_inode+0x0/0x150
    [] generic_delete_inode+0xa2/0x170
    [] generic_drop_inode+0x4f/0x70
    [] iput+0x47/0x50
    [] do_unlinkat+0xd5/0x160
    [] ? mutex_unlock+0x8/0x10
    [] ? vfs_readdir+0x7d/0xb0
    [] ? filldir64+0x0/0xf0
    [] ? sysenter_exit+0xf/0x16
    [] ? trace_hardirqs_on_caller+0x124/0x170
    [] sys_unlinkat+0x23/0x40
    [] sysenter_do_call+0x12/0x32

    Signed-off-by: Frederic Weisbecker
    Tested-by: Christian Kujau
    Cc: Alexander Beregalov
    Cc: Chris Mason
    Cc: Ingo Molnar

    Frederic Weisbecker
     
  • We need to relax the reiserfs lock before locking the inode mutex
    from xattr_unlink(), otherwise we'll face the usual bad dependencies:

    =======================================================
    [ INFO: possible circular locking dependency detected ]
    2.6.32-atom #178
    -------------------------------------------------------
    rm/3202 is trying to acquire lock:
    (&journal->j_mutex){+.+...}, at: [] do_journal_begin_r+0x94/0x360

    but task is already holding lock:
    (&sb->s_type->i_mutex_key#4/2){+.+...}, at: [] xattr_unlink+0x57/0xb0

    which lock already depends on the new lock.

    the existing dependency chain (in reverse order) is:

    -> #2 (&sb->s_type->i_mutex_key#4/2){+.+...}:
    [] __lock_acquire+0x11ff/0x19e0
    [] lock_acquire+0x68/0x90
    [] mutex_lock_nested+0x5b/0x340
    [] xattr_unlink+0x57/0xb0
    [] delete_one_xattr+0x29/0x100
    [] reiserfs_for_each_xattr+0x10b/0x290
    [] reiserfs_delete_xattrs+0x1a/0x60
    [] reiserfs_delete_inode+0x9f/0x150
    [] generic_delete_inode+0xa2/0x170
    [] generic_drop_inode+0x4f/0x70
    [] iput+0x47/0x50
    [] do_unlinkat+0xd5/0x160
    [] sys_unlinkat+0x23/0x40
    [] sysenter_do_call+0x12/0x32

    -> #1 (&REISERFS_SB(s)->lock){+.+.+.}:
    [] __lock_acquire+0x11ff/0x19e0
    [] lock_acquire+0x68/0x90
    [] mutex_lock_nested+0x5b/0x340
    [] reiserfs_write_lock+0x29/0x40
    [] do_journal_begin_r+0x9c/0x360
    [] journal_begin+0x80/0x130
    [] reiserfs_remount+0x223/0x4e0
    [] do_remount_sb+0xa6/0x140
    [] do_mount+0x560/0x750
    [] sys_mount+0x84/0xb0
    [] sysenter_do_call+0x12/0x32

    -> #0 (&journal->j_mutex){+.+...}:
    [] __lock_acquire+0x18f6/0x19e0
    [] lock_acquire+0x68/0x90
    [] mutex_lock_nested+0x5b/0x340
    [] do_journal_begin_r+0x94/0x360
    [] journal_begin+0x80/0x130
    [] reiserfs_unlink+0x83/0x2e0
    [] xattr_unlink+0x64/0xb0
    [] delete_one_xattr+0x29/0x100
    [] reiserfs_for_each_xattr+0x10b/0x290
    [] reiserfs_delete_xattrs+0x1a/0x60
    [] reiserfs_delete_inode+0x9f/0x150
    [] generic_delete_inode+0xa2/0x170
    [] generic_drop_inode+0x4f/0x70
    [] iput+0x47/0x50
    [] do_unlinkat+0xd5/0x160
    [] sys_unlinkat+0x23/0x40
    [] sysenter_do_call+0x12/0x32

    other info that might help us debug this:

    2 locks held by rm/3202:
    #0: (&sb->s_type->i_mutex_key#4/3){+.+.+.}, at: [] reiserfs_for_each_xattr+0x9b/0x290
    #1: (&sb->s_type->i_mutex_key#4/2){+.+...}, at: [] xattr_unlink+0x57/0xb0

    stack backtrace:
    Pid: 3202, comm: rm Not tainted 2.6.32-atom #178
    Call Trace:
    [] ? printk+0x18/0x1a
    [] print_circular_bug+0xca/0xd0
    [] __lock_acquire+0x18f6/0x19e0
    [] ? xattr_unlink+0x57/0xb0
    [] lock_acquire+0x68/0x90
    [] ? do_journal_begin_r+0x94/0x360
    [] ? do_journal_begin_r+0x94/0x360
    [] mutex_lock_nested+0x5b/0x340
    [] ? do_journal_begin_r+0x94/0x360
    [] do_journal_begin_r+0x94/0x360
    [] ? run_timer_softirq+0x1a6/0x220
    [] ? __do_softirq+0x50/0x140
    [] journal_begin+0x80/0x130
    [] ? __do_softirq+0xf2/0x140
    [] ? hrtimer_interrupt+0xdf/0x220
    [] reiserfs_unlink+0x83/0x2e0
    [] ? mark_held_locks+0x62/0x80
    [] ? trace_hardirqs_on_thunk+0xc/0x10
    [] ? restore_all_notrace+0x0/0x18
    [] ? xattr_unlink+0x57/0xb0
    [] xattr_unlink+0x64/0xb0
    [] delete_one_xattr+0x29/0x100
    [] reiserfs_for_each_xattr+0x10b/0x290
    [] ? delete_one_xattr+0x0/0x100
    [] ? mutex_lock_nested+0x299/0x340
    [] reiserfs_delete_xattrs+0x1a/0x60
    [] ? reiserfs_write_lock_once+0x29/0x50
    [] reiserfs_delete_inode+0x9f/0x150
    [] ? _atomic_dec_and_lock+0x4f/0x70
    [] ? reiserfs_delete_inode+0x0/0x150
    [] generic_delete_inode+0xa2/0x170
    [] generic_drop_inode+0x4f/0x70
    [] iput+0x47/0x50
    [] do_unlinkat+0xd5/0x160
    [] ? mutex_unlock+0x8/0x10
    [] ? vfs_readdir+0x7d/0xb0
    [] ? filldir64+0x0/0xf0
    [] ? sysenter_exit+0xf/0x16
    [] ? trace_hardirqs_on_caller+0x124/0x170
    [] sys_unlinkat+0x23/0x40
    [] sysenter_do_call+0x12/0x32

    Signed-off-by: Frederic Weisbecker
    Tested-by: Christian Kujau
    Cc: Alexander Beregalov
    Cc: Chris Mason
    Cc: Ingo Molnar

    Frederic Weisbecker
     
  • We call xattr_lookup() from reiserfs_xattr_get(). We then hold
    the reiserfs lock when we grab the i_mutex. But later, we may
    relax the reiserfs lock, creating dependency inversion between
    both locks.

    The lookups and creation jobs ar already protected by the
    inode mutex, so we can safely relax the reiserfs lock, dropping
    the unwanted reiserfs lock -> i_mutex dependency, as shown
    in the following lockdep report:

    =======================================================
    [ INFO: possible circular locking dependency detected ]
    2.6.32-atom #173
    -------------------------------------------------------
    cp/3204 is trying to acquire lock:
    (&REISERFS_SB(s)->lock){+.+.+.}, at: [] reiserfs_write_lock_once+0x29/0x50

    but task is already holding lock:
    (&sb->s_type->i_mutex_key#4/3){+.+.+.}, at: [] open_xa_dir+0xd8/0x1b0

    which lock already depends on the new lock.

    the existing dependency chain (in reverse order) is:

    -> #1 (&sb->s_type->i_mutex_key#4/3){+.+.+.}:
    [] __lock_acquire+0x11ff/0x19e0
    [] lock_acquire+0x68/0x90
    [] mutex_lock_nested+0x5b/0x340
    [] open_xa_dir+0x43/0x1b0
    [] reiserfs_for_each_xattr+0x62/0x260
    [] reiserfs_delete_xattrs+0x1a/0x60
    [] reiserfs_delete_inode+0x9f/0x150
    [] generic_delete_inode+0xa2/0x170
    [] generic_drop_inode+0x4f/0x70
    [] iput+0x47/0x50
    [] do_unlinkat+0xd5/0x160
    [] sys_unlink+0x10/0x20
    [] sysenter_do_call+0x12/0x32

    -> #0 (&REISERFS_SB(s)->lock){+.+.+.}:
    [] __lock_acquire+0x18f6/0x19e0
    [] lock_acquire+0x68/0x90
    [] mutex_lock_nested+0x5b/0x340
    [] reiserfs_write_lock_once+0x29/0x50
    [] reiserfs_lookup+0x62/0x140
    [] __lookup_hash+0xef/0x110
    [] lookup_one_len+0x8d/0xc0
    [] open_xa_dir+0xea/0x1b0
    [] xattr_lookup+0x15/0x160
    [] reiserfs_xattr_get+0x56/0x2a0
    [] reiserfs_get_acl+0xa2/0x360
    [] reiserfs_cache_default_acl+0x3a/0x160
    [] reiserfs_mkdir+0x6c/0x2c0
    [] vfs_mkdir+0xd6/0x180
    [] sys_mkdirat+0xc0/0xd0
    [] sys_mkdir+0x20/0x30
    [] sysenter_do_call+0x12/0x32

    other info that might help us debug this:

    2 locks held by cp/3204:
    #0: (&sb->s_type->i_mutex_key#4/1){+.+.+.}, at: [] lookup_create+0x26/0xa0
    #1: (&sb->s_type->i_mutex_key#4/3){+.+.+.}, at: [] open_xa_dir+0xd8/0x1b0

    stack backtrace:
    Pid: 3204, comm: cp Not tainted 2.6.32-atom #173
    Call Trace:
    [] ? printk+0x18/0x1a
    [] print_circular_bug+0xca/0xd0
    [] __lock_acquire+0x18f6/0x19e0
    [] ? check_usage+0x6a/0x460
    [] lock_acquire+0x68/0x90
    [] ? reiserfs_write_lock_once+0x29/0x50
    [] ? reiserfs_write_lock_once+0x29/0x50
    [] mutex_lock_nested+0x5b/0x340
    [] ? reiserfs_write_lock_once+0x29/0x50
    [] reiserfs_write_lock_once+0x29/0x50
    [] reiserfs_lookup+0x62/0x140
    [] ? debug_check_no_locks_freed+0x8a/0x140
    [] ? trace_hardirqs_on_caller+0x124/0x170
    [] __lookup_hash+0xef/0x110
    [] lookup_one_len+0x8d/0xc0
    [] open_xa_dir+0xea/0x1b0
    [] xattr_lookup+0x15/0x160
    [] reiserfs_xattr_get+0x56/0x2a0
    [] reiserfs_get_acl+0xa2/0x360
    [] ? new_inode+0x27/0xa0
    [] reiserfs_cache_default_acl+0x3a/0x160
    [] ? _spin_unlock+0x27/0x40
    [] reiserfs_mkdir+0x6c/0x2c0
    [] ? __d_lookup+0x108/0x190
    [] ? mark_held_locks+0x62/0x80
    [] ? mutex_lock_nested+0x2bd/0x340
    [] ? generic_permission+0x1a/0xa0
    [] ? security_inode_permission+0x1e/0x20
    [] vfs_mkdir+0xd6/0x180
    [] sys_mkdirat+0xc0/0xd0
    [] ? up_read+0x16/0x30
    [] ? restore_all_notrace+0x0/0x18
    [] sys_mkdir+0x20/0x30
    [] sysenter_do_call+0x12/0x32

    Signed-off-by: Frederic Weisbecker
    Tested-by: Christian Kujau
    Cc: Alexander Beregalov
    Cc: Chris Mason
    Cc: Ingo Molnar

    Frederic Weisbecker
     
  • While deleting the xattrs of an inode, we hold the reiserfs lock
    and grab the inode->i_mutex of the targeted inode and the root
    private xattr directory.

    Later on, we may relax the reiserfs lock for various reasons, this
    creates inverted dependencies.

    We can remove the reiserfs lock -> i_mutex dependency by relaxing
    the former before calling open_xa_dir(). This is fine because the
    lookup and creation of xattr private directories done in
    open_xa_dir() are covered by the targeted inode mutexes. And deeper
    operations in the tree are still done under the write lock.

    This fixes the following lockdep report:

    =======================================================
    [ INFO: possible circular locking dependency detected ]
    2.6.32-atom #173
    -------------------------------------------------------
    cp/3204 is trying to acquire lock:
    (&REISERFS_SB(s)->lock){+.+.+.}, at: [] reiserfs_write_lock_once+0x29/0x50

    but task is already holding lock:
    (&sb->s_type->i_mutex_key#4/3){+.+.+.}, at: [] open_xa_dir+0xd8/0x1b0

    which lock already depends on the new lock.

    the existing dependency chain (in reverse order) is:

    -> #1 (&sb->s_type->i_mutex_key#4/3){+.+.+.}:
    [] __lock_acquire+0x11ff/0x19e0
    [] lock_acquire+0x68/0x90
    [] mutex_lock_nested+0x5b/0x340
    [] open_xa_dir+0x43/0x1b0
    [] reiserfs_for_each_xattr+0x62/0x260
    [] reiserfs_delete_xattrs+0x1a/0x60
    [] reiserfs_delete_inode+0x9f/0x150
    [] generic_delete_inode+0xa2/0x170
    [] generic_drop_inode+0x4f/0x70
    [] iput+0x47/0x50
    [] do_unlinkat+0xd5/0x160
    [] sys_unlink+0x10/0x20
    [] sysenter_do_call+0x12/0x32

    -> #0 (&REISERFS_SB(s)->lock){+.+.+.}:
    [] __lock_acquire+0x18f6/0x19e0
    [] lock_acquire+0x68/0x90
    [] mutex_lock_nested+0x5b/0x340
    [] reiserfs_write_lock_once+0x29/0x50
    [] reiserfs_lookup+0x62/0x140
    [] __lookup_hash+0xef/0x110
    [] lookup_one_len+0x8d/0xc0
    [] open_xa_dir+0xea/0x1b0
    [] xattr_lookup+0x15/0x160
    [] reiserfs_xattr_get+0x56/0x2a0
    [] reiserfs_get_acl+0xa2/0x360
    [] reiserfs_cache_default_acl+0x3a/0x160
    [] reiserfs_mkdir+0x6c/0x2c0
    [] vfs_mkdir+0xd6/0x180
    [] sys_mkdirat+0xc0/0xd0
    [] sys_mkdir+0x20/0x30
    [] sysenter_do_call+0x12/0x32

    other info that might help us debug this:

    2 locks held by cp/3204:
    #0: (&sb->s_type->i_mutex_key#4/1){+.+.+.}, at: [] lookup_create+0x26/0xa0
    #1: (&sb->s_type->i_mutex_key#4/3){+.+.+.}, at: [] open_xa_dir+0xd8/0x1b0

    stack backtrace:
    Pid: 3204, comm: cp Not tainted 2.6.32-atom #173
    Call Trace:
    [] ? printk+0x18/0x1a
    [] print_circular_bug+0xca/0xd0
    [] __lock_acquire+0x18f6/0x19e0
    [] ? check_usage+0x6a/0x460
    [] lock_acquire+0x68/0x90
    [] ? reiserfs_write_lock_once+0x29/0x50
    [] ? reiserfs_write_lock_once+0x29/0x50
    [] mutex_lock_nested+0x5b/0x340
    [] ? reiserfs_write_lock_once+0x29/0x50
    [] reiserfs_write_lock_once+0x29/0x50
    [] reiserfs_lookup+0x62/0x140
    [] ? debug_check_no_locks_freed+0x8a/0x140
    [] ? trace_hardirqs_on_caller+0x124/0x170
    [] __lookup_hash+0xef/0x110
    [] lookup_one_len+0x8d/0xc0
    [] open_xa_dir+0xea/0x1b0
    [] xattr_lookup+0x15/0x160
    [] reiserfs_xattr_get+0x56/0x2a0
    [] reiserfs_get_acl+0xa2/0x360
    [] ? new_inode+0x27/0xa0
    [] reiserfs_cache_default_acl+0x3a/0x160
    [] ? _spin_unlock+0x27/0x40
    [] reiserfs_mkdir+0x6c/0x2c0
    [] ? __d_lookup+0x108/0x190
    [] ? mark_held_locks+0x62/0x80
    [] ? mutex_lock_nested+0x2bd/0x340
    [] ? generic_permission+0x1a/0xa0
    [] ? security_inode_permission+0x1e/0x20
    [] vfs_mkdir+0xd6/0x180
    [] sys_mkdirat+0xc0/0xd0
    [] ? up_read+0x16/0x30
    [] ? restore_all_notrace+0x0/0x18
    [] sys_mkdir+0x20/0x30
    [] sysenter_do_call+0x12/0x32

    v2: Don't drop reiserfs_mutex_lock_nested_safe() as we'll still
    need it later

    Signed-off-by: Frederic Weisbecker
    Tested-by: Christian Kujau
    Cc: Alexander Beregalov
    Cc: Chris Mason
    Cc: Ingo Molnar

    Frederic Weisbecker
     
  • i_xattr_sem depends on the reiserfs lock. But after we grab
    i_xattr_sem, we may relax/relock the reiserfs lock while waiting
    on a freezed filesystem, creating a dependency inversion between
    the two locks.

    In order to avoid the i_xattr_sem -> reiserfs lock dependency, let's
    create a reiserfs_down_read_safe() that acts like
    reiserfs_mutex_lock_safe(): relax the reiserfs lock while grabbing
    another lock to avoid undesired dependencies induced by the
    heivyweight reiserfs lock.

    This fixes the following warning:

    [ 990.005931] =======================================================
    [ 990.012373] [ INFO: possible circular locking dependency detected ]
    [ 990.013233] 2.6.33-rc1 #1
    [ 990.013233] -------------------------------------------------------
    [ 990.013233] dbench/1891 is trying to acquire lock:
    [ 990.013233] (&REISERFS_SB(s)->lock){+.+.+.}, at: [] reiserfs_write_lock+0x35/0x50
    [ 990.013233]
    [ 990.013233] but task is already holding lock:
    [ 990.013233] (&REISERFS_I(inode)->i_xattr_sem){+.+.+.}, at: [] reiserfs_xattr_set_handle+0x8a/0x470
    [ 990.013233]
    [ 990.013233] which lock already depends on the new lock.
    [ 990.013233]
    [ 990.013233]
    [ 990.013233] the existing dependency chain (in reverse order) is:
    [ 990.013233]
    [ 990.013233] -> #1 (&REISERFS_I(inode)->i_xattr_sem){+.+.+.}:
    [ 990.013233] [] __lock_acquire+0xf9c/0x1560
    [ 990.013233] [] lock_acquire+0x8f/0xb0
    [ 990.013233] [] down_write+0x44/0x80
    [ 990.013233] [] reiserfs_xattr_set_handle+0x8a/0x470
    [ 990.013233] [] reiserfs_xattr_set+0xb0/0x150
    [ 990.013233] [] user_set+0x8a/0x90
    [ 990.013233] [] reiserfs_setxattr+0xaa/0xb0
    [ 990.013233] [] __vfs_setxattr_noperm+0x36/0xa0
    [ 990.013233] [] vfs_setxattr+0xbc/0xc0
    [ 990.013233] [] setxattr+0xc0/0x150
    [ 990.013233] [] sys_fsetxattr+0x8d/0xa0
    [ 990.013233] [] system_call_fastpath+0x16/0x1b
    [ 990.013233]
    [ 990.013233] -> #0 (&REISERFS_SB(s)->lock){+.+.+.}:
    [ 990.013233] [] __lock_acquire+0x12d0/0x1560
    [ 990.013233] [] lock_acquire+0x8f/0xb0
    [ 990.013233] [] __mutex_lock_common+0x47/0x3b0
    [ 990.013233] [] mutex_lock_nested+0x3e/0x50
    [ 990.013233] [] reiserfs_write_lock+0x35/0x50
    [ 990.013233] [] reiserfs_prepare_write+0x45/0x180
    [ 990.013233] [] reiserfs_xattr_set_handle+0x2a6/0x470
    [ 990.013233] [] reiserfs_xattr_set+0xb0/0x150
    [ 990.013233] [] user_set+0x8a/0x90
    [ 990.013233] [] reiserfs_setxattr+0xaa/0xb0
    [ 990.013233] [] __vfs_setxattr_noperm+0x36/0xa0
    [ 990.013233] [] vfs_setxattr+0xbc/0xc0
    [ 990.013233] [] setxattr+0xc0/0x150
    [ 990.013233] [] sys_fsetxattr+0x8d/0xa0
    [ 990.013233] [] system_call_fastpath+0x16/0x1b
    [ 990.013233]
    [ 990.013233] other info that might help us debug this:
    [ 990.013233]
    [ 990.013233] 2 locks held by dbench/1891:
    [ 990.013233] #0: (&sb->s_type->i_mutex_key#12){+.+.+.}, at: [] vfs_setxattr+0x78/0xc0
    [ 990.013233] #1: (&REISERFS_I(inode)->i_xattr_sem){+.+.+.}, at: [] reiserfs_xattr_set_handle+0x8a/0x470
    [ 990.013233]
    [ 990.013233] stack backtrace:
    [ 990.013233] Pid: 1891, comm: dbench Not tainted 2.6.33-rc1 #1
    [ 990.013233] Call Trace:
    [ 990.013233] [] print_circular_bug+0xe9/0xf0
    [ 990.013233] [] __lock_acquire+0x12d0/0x1560
    [ 990.013233] [] ? reiserfs_xattr_set_handle+0x8a/0x470
    [ 990.013233] [] lock_acquire+0x8f/0xb0
    [ 990.013233] [] ? reiserfs_write_lock+0x35/0x50
    [ 990.013233] [] ? reiserfs_xattr_set_handle+0x8a/0x470
    [ 990.013233] [] __mutex_lock_common+0x47/0x3b0
    [ 990.013233] [] ? reiserfs_write_lock+0x35/0x50
    [ 990.013233] [] ? reiserfs_write_lock+0x35/0x50
    [ 990.013233] [] ? mark_held_locks+0x72/0xa0
    [ 990.013233] [] ? __mutex_unlock_slowpath+0xbd/0x140
    [ 990.013233] [] ? trace_hardirqs_on_caller+0x14d/0x1a0
    [ 990.013233] [] mutex_lock_nested+0x3e/0x50
    [ 990.013233] [] reiserfs_write_lock+0x35/0x50
    [ 990.013233] [] reiserfs_prepare_write+0x45/0x180
    [ 990.013233] [] reiserfs_xattr_set_handle+0x2a6/0x470
    [ 990.013233] [] reiserfs_xattr_set+0xb0/0x150
    [ 990.013233] [] ? __mutex_lock_common+0x284/0x3b0
    [ 990.013233] [] user_set+0x8a/0x90
    [ 990.013233] [] reiserfs_setxattr+0xaa/0xb0
    [ 990.013233] [] __vfs_setxattr_noperm+0x36/0xa0
    [ 990.013233] [] vfs_setxattr+0xbc/0xc0
    [ 990.013233] [] setxattr+0xc0/0x150
    [ 990.013233] [] ? sched_clock_cpu+0xb8/0x100
    [ 990.013233] [] ? trace_hardirqs_off+0xd/0x10
    [ 990.013233] [] ? cpu_clock+0x43/0x50
    [ 990.013233] [] ? fget+0xb0/0x110
    [ 990.013233] [] ? fget+0x0/0x110
    [ 990.013233] [] ? sysret_check+0x27/0x62
    [ 990.013233] [] sys_fsetxattr+0x8d/0xa0
    [ 990.013233] [] system_call_fastpath+0x16/0x1b

    Reported-and-tested-by: Christian Kujau
    Signed-off-by: Frederic Weisbecker
    Cc: Alexander Beregalov
    Cc: Chris Mason
    Cc: Ingo Molnar

    Frederic Weisbecker
     

17 Dec, 2009

2 commits

  • The reiserfs lock -> inode mutex dependency gets inverted when we
    relax the lock while walking to the tree.

    To fix this, use a specialized version of reiserfs_mutex_lock_safe
    that takes care of mutex subclasses. Then we can grab the inode
    mutex with I_MUTEX_XATTR subclass without any reiserfs lock
    dependency.

    This fixes the following report:

    [ INFO: possible circular locking dependency detected ]
    2.6.32-06793-gf405425-dirty #2
    -------------------------------------------------------
    mv/18566 is trying to acquire lock:
    (&REISERFS_SB(s)->lock){+.+.+.}, at: [] reiserfs_write_lock+0x28=
    /0x40

    but task is already holding lock:
    (&sb->s_type->i_mutex_key#5/3){+.+.+.}, at: []
    reiserfs_for_each_xattr+0x10c/0x380

    which lock already depends on the new lock.

    the existing dependency chain (in reverse order) is:

    -> #1 (&sb->s_type->i_mutex_key#5/3){+.+.+.}:
    [] validate_chain+0xa23/0xf70
    [] __lock_acquire+0x4e5/0xa70
    [] lock_acquire+0x7a/0xa0
    [] mutex_lock_nested+0x5f/0x2b0
    [] reiserfs_for_each_xattr+0x84/0x380
    [] reiserfs_delete_xattrs+0x15/0x50
    [] reiserfs_delete_inode+0x8f/0x140
    [] generic_delete_inode+0x9c/0x150
    [] generic_drop_inode+0x3d/0x60
    [] iput+0x47/0x50
    [] do_unlinkat+0xdb/0x160
    [] sys_unlink+0x10/0x20
    [] sysenter_do_call+0x12/0x36

    -> #0 (&REISERFS_SB(s)->lock){+.+.+.}:
    [] validate_chain+0xf68/0xf70
    [] __lock_acquire+0x4e5/0xa70
    [] lock_acquire+0x7a/0xa0
    [] mutex_lock_nested+0x5f/0x2b0
    [] reiserfs_write_lock+0x28/0x40
    [] search_by_key+0x1f7b/0x21b0
    [] search_by_entry_key+0x1f/0x3b0
    [] reiserfs_find_entry+0x77/0x400
    [] reiserfs_lookup+0x85/0x130
    [] __lookup_hash+0xb4/0x110
    [] lookup_one_len+0xb3/0x100
    [] reiserfs_for_each_xattr+0x120/0x380
    [] reiserfs_delete_xattrs+0x15/0x50
    [] reiserfs_delete_inode+0x8f/0x140
    [] generic_delete_inode+0x9c/0x150
    [] generic_drop_inode+0x3d/0x60
    [] iput+0x47/0x50
    [] dentry_iput+0x6f/0xf0
    [] d_kill+0x24/0x50
    [] dput+0x5b/0x120
    [] sys_renameat+0x1b9/0x230
    [] sys_rename+0x28/0x30
    [] sysenter_do_call+0x12/0x36

    other info that might help us debug this:

    2 locks held by mv/18566:
    #0: (&sb->s_type->i_mutex_key#5/1){+.+.+.}, at: []
    lock_rename+0xcc/0xd0
    #1: (&sb->s_type->i_mutex_key#5/3){+.+.+.}, at: []
    reiserfs_for_each_xattr+0x10c/0x380

    stack backtrace:
    Pid: 18566, comm: mv Tainted: G C 2.6.32-06793-gf405425-dirty #2
    Call Trace:
    [] ? printk+0x18/0x1e
    [] print_circular_bug+0xc0/0xd0
    [] validate_chain+0xf68/0xf70
    [] ? trace_hardirqs_off+0xb/0x10
    [] __lock_acquire+0x4e5/0xa70
    [] lock_acquire+0x7a/0xa0
    [] ? reiserfs_write_lock+0x28/0x40
    [] mutex_lock_nested+0x5f/0x2b0
    [] ? reiserfs_write_lock+0x28/0x40
    [] ? reiserfs_write_lock+0x28/0x40
    [] ? schedule+0x27a/0x440
    [] reiserfs_write_lock+0x28/0x40
    [] search_by_key+0x1f7b/0x21b0
    [] ? __lock_acquire+0x506/0xa70
    [] ? lock_release_non_nested+0x1e7/0x340
    [] ? reiserfs_write_lock+0x28/0x40
    [] ? trace_hardirqs_on_caller+0x124/0x170
    [] ? trace_hardirqs_on+0xb/0x10
    [] ? T.316+0x15/0x1a0
    [] ? sched_clock_cpu+0x9d/0x100
    [] search_by_entry_key+0x1f/0x3b0
    [] ? __mutex_unlock_slowpath+0x9a/0x120
    [] ? trace_hardirqs_on_caller+0x124/0x170
    [] reiserfs_find_entry+0x77/0x400
    [] reiserfs_lookup+0x85/0x130
    [] ? sched_clock_cpu+0x9d/0x100
    [] __lookup_hash+0xb4/0x110
    [] lookup_one_len+0xb3/0x100
    [] reiserfs_for_each_xattr+0x120/0x380
    [] ? delete_one_xattr+0x0/0x1c0
    [] ? math_error+0x22/0x150
    [] ? reiserfs_write_lock+0x28/0x40
    [] reiserfs_delete_xattrs+0x15/0x50
    [] ? reiserfs_write_lock+0x28/0x40
    [] reiserfs_delete_inode+0x8f/0x140
    [] ? generic_delete_inode+0x5f/0x150
    [] ? reiserfs_delete_inode+0x0/0x140
    [] generic_delete_inode+0x9c/0x150
    [] generic_drop_inode+0x3d/0x60
    [] iput+0x47/0x50
    [] dentry_iput+0x6f/0xf0
    [] d_kill+0x24/0x50
    [] dput+0x5b/0x120
    [] sys_renameat+0x1b9/0x230
    [] ? sched_clock_cpu+0x9d/0x100
    [] ? trace_hardirqs_off+0xb/0x10
    [] ? cpu_clock+0x4e/0x60
    [] ? do_page_fault+0x155/0x370
    [] ? up_read+0x16/0x30
    [] ? do_page_fault+0x155/0x370
    [] sys_rename+0x28/0x30
    [] sysenter_do_call+0x12/0x36

    Reported-by: Alexander Beregalov
    Signed-off-by: Frederic Weisbecker
    Cc: Chris Mason
    Cc: Ingo Molnar
    Cc: Thomas Gleixner

    Frederic Weisbecker
     
  • Add a flags argument to struct xattr_handler and pass it to all xattr
    handler methods. This allows using the same methods for multiple
    handlers, e.g. for the ACL methods which perform exactly the same action
    for the access and default ACLs, just using a different underlying
    attribute. With a little more groundwork it'll also allow sharing the
    methods for the regular user/trusted/secure handlers in extN, ocfs2 and
    jffs2 like it's already done for xfs in this patch.

    Also change the inode argument to the handlers to a dentry to allow
    using the handlers mechnism for filesystems that require it later,
    e.g. cifs.

    [with GFS2 bits updated by Steven Whitehouse ]

    Signed-off-by: Christoph Hellwig
    Reviewed-by: James Morris
    Acked-by: Joel Becker
    Signed-off-by: Al Viro

    Christoph Hellwig
     

14 Sep, 2009

2 commits

  • reiserfs_xattr_init is called with the reiserfs write lock held, but
    if the ".reiserfs_priv" entry is not created, we take the superblock
    root directory inode mutex until .reiserfs_priv is created.

    This creates a lock dependency inversion against other sites such as
    reiserfs_file_release() which takes an inode mutex and the reiserfs
    lock after.

    Signed-off-by: Frederic Weisbecker
    Cc: Jeff Mahoney
    Cc: Chris Mason
    Cc: Ingo Molnar
    Cc: Alexander Beregalov
    Cc: Laurent Riffard

    Frederic Weisbecker
     
  • While searching a pathname, an inode mutex can be acquired
    in do_lookup() which calls reiserfs_lookup() which in turn
    acquires the write lock.

    On the other side reiserfs_fill_super() can acquire the write_lock
    and then call reiserfs_lookup_privroot() which can acquire an
    inode mutex (the root of the mount point).

    So we theoretically risk an AB - BA lock inversion that could lead
    to a deadlock.

    As for other lock dependencies found since the bkl to mutex
    conversion, the fix is to use reiserfs_mutex_lock_safe() which
    drops the lock dependency to the write lock.

    [ Impact: fix a possible deadlock with reiserfs ]

    Cc: Jeff Mahoney
    Cc: Chris Mason
    Cc: Ingo Molnar
    Cc: Alexander Beregalov
    Signed-off-by: Frederic Weisbecker

    Frederic Weisbecker
     

13 Jul, 2009

1 commit

  • * Remove smp_lock.h from files which don't need it (including some headers!)
    * Add smp_lock.h to files which do need it
    * Make smp_lock.h include conditional in hardirq.h
    It's needed only for one kernel_locked() usage which is under CONFIG_PREEMPT

    This will make hardirq.h inclusion cheaper for every PREEMPT=n config
    (which includes allmodconfig/allyesconfig, BTW)

    Signed-off-by: Alexey Dobriyan
    Signed-off-by: Linus Torvalds

    Alexey Dobriyan
     

12 Jun, 2009

1 commit


18 May, 2009

3 commits


09 May, 2009

4 commits

  • With Al Viro's patch to move privroot lookup to fs mount, there's no need
    to have special code to hide the privroot in reiserfs_lookup.

    I've also cleaned up the privroot hiding in reiserfs_readdir_dentry and
    removed the last user of reiserfs_xattrs().

    Signed-off-by: Jeff Mahoney
    Signed-off-by: Al Viro

    Jeff Mahoney
     
  • The xattr_root caching was broken from my previous patch set. It wouldn't
    cause corruption, but could cause decreased performance due to allocating
    a larger chunk of the journal (~ 27 blocks) than it would actually use.

    This patch loads the xattr root dentry at xattr initialization and creates
    it on-demand. Since we're using the cached dentry, there's no point
    in keeping lookup_or_create_dir around, so that's removed.

    Signed-off-by: Jeff Mahoney
    Signed-off-by: Al Viro

    Jeff Mahoney
     
  • ... even if it's a negative dentry. That way we can set ->d_op on
    root before anyone could race with us. Simplify d_compare(), while
    we are at it.

    Signed-off-by: Al Viro

    Al Viro
     
  • 2.6.30-rc3 introduced some sanity checks in the VFS code to avoid NFS
    bugs by ensuring that lookup_one_len is always called under i_mutex.

    This patch expands the i_mutex locking to enclose lookup_one_len. This was
    always required, but not not enforced in the reiserfs code since it
    does locking around the xattr interactions with the xattr_sem.

    This is obvious enough, and it survived an overnight 50 thread ACL test.

    Signed-off-by: Jeff Mahoney
    Signed-off-by: Al Viro

    Jeff Mahoney
     

31 Mar, 2009

15 commits

  • This patch ifdefs xattr_create when xattrs aren't enabled.

    Signed-off-by: Jeff Mahoney
    Signed-off-by: Linus Torvalds

    Jeff Mahoney
     
  • * reiserfs-updates: (35 commits)
    reiserfs: rename [cn]_* variables
    reiserfs: rename p_._ variables
    reiserfs: rename p_s_tb to tb
    reiserfs: rename p_s_inode to inode
    reiserfs: rename p_s_bh to bh
    reiserfs: rename p_s_sb to sb
    reiserfs: strip trailing whitespace
    reiserfs: cleanup path functions
    reiserfs: factor out buffer_info initialization
    reiserfs: add atomic addition of selinux attributes during inode creation
    reiserfs: use generic readdir for operations across all xattrs
    reiserfs: journaled xattrs
    reiserfs: use generic xattr handlers
    reiserfs: remove i_has_xattr_dir
    reiserfs: make per-inode xattr locking more fine grained
    reiserfs: eliminate per-super xattr lock
    reiserfs: simplify xattr internal file lookups/opens
    reiserfs: Clean up xattrs when REISERFS_FS_XATTR is unset
    reiserfs: remove IS_PRIVATE helpers
    reiserfs: remove link detection code
    ...

    Fixed up conflicts manually due to:
    - quota name cleanups vs variable naming changes:
    fs/reiserfs/inode.c
    fs/reiserfs/namei.c
    fs/reiserfs/stree.c
    fs/reiserfs/xattr.c
    - exported include header cleanups
    include/linux/reiserfs_fs.h

    Linus Torvalds
     
  • The current reiserfs xattr implementation open codes reiserfs_readdir
    and frees the path before calling the filldir function. Typically, the
    filldir function is something that modifies the file system, such as a
    chown or an inode deletion that also require reading of an inode
    associated with each direntry. Since the file system is modified, the
    path retained becomes invalid for the next run. In addition, it runs
    backwards in attempt to minimize activity.

    This is clearly suboptimal from a code cleanliness perspective as well
    as performance-wise.

    This patch implements a generic reiserfs_for_each_xattr that uses the
    generic readdir and a specific filldir routine that simply populates an
    array of dentries and then performs a specific operation on them. When
    all files have been operated on, it then calls the operation on the
    directory itself.

    The result is a noticable code reduction and better performance.

    Signed-off-by: Jeff Mahoney
    Signed-off-by: Linus Torvalds

    Jeff Mahoney
     
  • Deadlocks are possible in the xattr code between the journal lock and the
    xattr sems.

    This patch implements journalling for xattr operations. The benefit is
    twofold:
    * It gets rid of the deadlock possibility by always ensuring that xattr
    write operations are initiated inside a transaction.
    * It corrects the problem where xattr backing files aren't considered any
    differently than normal files, despite the fact they are metadata.

    I discussed the added journal load with Chris Mason, and we decided that
    since xattrs (versus other journal activity) is fairly rare, the introduction
    of larger transactions to support journaled xattrs wouldn't be too big a deal.

    Signed-off-by: Jeff Mahoney
    Signed-off-by: Linus Torvalds

    Jeff Mahoney
     
  • Christoph Hellwig had asked me quite some time ago to port the reiserfs
    xattrs to the generic xattr interface.

    This patch replaces the reiserfs-specific xattr handling code with the
    generic struct xattr_handler.

    However, since reiserfs doesn't split the prefix and name when accessing
    xattrs, it can't leverage generic_{set,get,list,remove}xattr without
    needlessly reconstructing the name on the back end.

    Update 7/26/07: Added missing dput() to deletion path.
    Update 8/30/07: Added missing mark_inode_dirty when i_mode is used to
    represent an ACL and no previous ACL existed.

    Signed-off-by: Jeff Mahoney
    Signed-off-by: Linus Torvalds

    Jeff Mahoney
     
  • With the changes to xattr root locking, the i_has_xattr_dir flag
    is no longer needed. This patch removes it.

    Signed-off-by: Jeff Mahoney
    Signed-off-by: Linus Torvalds

    Jeff Mahoney
     
  • The per-inode locking can be made more fine-grained to surround just the
    interaction with the filesystem itself. This really only applies to
    protecting reads during a write, since concurrent writes are barred with
    inode->i_mutex at the vfs level.

    Signed-off-by: Jeff Mahoney
    Signed-off-by: Linus Torvalds

    Jeff Mahoney
     
  • With the switch to using inode->i_mutex locking during lookups/creation
    in the xattr root, the per-super xattr lock is no longer needed.

    This patch removes it.

    Signed-off-by: Jeff Mahoney
    Signed-off-by: Linus Torvalds

    Jeff Mahoney
     
  • The xattr file open/lookup code is needlessly complex. We can use
    vfs-level operations to perform the same work, and also simplify the
    locking constraints. The locking advantages will be exploited in future
    patches.

    Signed-off-by: Jeff Mahoney
    Signed-off-by: Linus Torvalds

    Jeff Mahoney
     
  • The current reiserfs xattr implementation will not clean up old xattr
    files if files are deleted when REISERFS_FS_XATTR is unset. This
    results in inaccessible lost files, wasting space.

    This patch compiles in basic xattr knowledge, such as how to delete them
    and change ownership for quota tracking. If the file system has never
    used xattrs, then the operation is quite fast: it returns immediately
    when it sees there is no .reiserfs_priv directory.

    Signed-off-by: Jeff Mahoney
    Signed-off-by: Linus Torvalds

    Jeff Mahoney
     
  • There are a number of helper functions for marking a reiserfs inode
    private that were leftover from reiserfs did its own thing wrt to
    private inodes. S_PRIVATE has been in the kernel for some time, so this
    patch removes the helpers and uses IS_PRIVATE instead.

    Signed-off-by: Jeff Mahoney
    Signed-off-by: Linus Torvalds

    Jeff Mahoney
     
  • Early in the reiserfs xattr development, there was a plan to use
    hardlinks to save disk space for identical xattrs. That code never
    materialized and isn't going to, so this patch removes the detection
    code.

    Signed-off-by: Jeff Mahoney
    Signed-off-by: Linus Torvalds

    Jeff Mahoney
     
  • This patch changes reiserfs_get_page to take an offset rather than an
    index since no callers calculate the index differently.

    Signed-off-by: Jeff Mahoney
    Signed-off-by: Linus Torvalds

    Jeff Mahoney
     
  • This patch removes the xinode and mapping variables from
    reiserfs_xattr_{get,set}.

    Signed-off-by: Jeff Mahoney
    Signed-off-by: Linus Torvalds

    Jeff Mahoney
     
  • This patch makes many paths that are currently using warnings to handle
    the error.

    Signed-off-by: Jeff Mahoney
    Signed-off-by: Linus Torvalds

    Jeff Mahoney