lkml.org 
[lkml]   [2019]   [Jan]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.20 56/57] Btrfs: fix deadlock when enabling quotas due to concurrent snapshot creation
    Date
    4.20-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Filipe Manana <fdmanana@suse.com>

    commit 9a6f209e36500efac51528132a3e3083586eda5f upstream.

    If the quota enable and snapshot creation ioctls are called concurrently
    we can get into a deadlock where the task enabling quotas will deadlock
    on the fs_info->qgroup_ioctl_lock mutex because it attempts to lock it
    twice, or the task creating a snapshot tries to commit the transaction
    while the task enabling quota waits for the former task to commit the
    transaction while holding the mutex. The following time diagrams show how
    both cases happen.

    First scenario:

    CPU 0 CPU 1

    btrfs_ioctl()
    btrfs_ioctl_quota_ctl()
    btrfs_quota_enable()
    mutex_lock(fs_info->qgroup_ioctl_lock)
    btrfs_start_transaction()

    btrfs_ioctl()
    btrfs_ioctl_snap_create_v2
    create_snapshot()
    --> adds snapshot to the
    list pending_snapshots
    of the current
    transaction

    btrfs_commit_transaction()
    create_pending_snapshots()
    create_pending_snapshot()
    qgroup_account_snapshot()
    btrfs_qgroup_inherit()
    mutex_lock(fs_info->qgroup_ioctl_lock)
    --> deadlock, mutex already locked
    by this task at
    btrfs_quota_enable()

    Second scenario:

    CPU 0 CPU 1

    btrfs_ioctl()
    btrfs_ioctl_quota_ctl()
    btrfs_quota_enable()
    mutex_lock(fs_info->qgroup_ioctl_lock)
    btrfs_start_transaction()

    btrfs_ioctl()
    btrfs_ioctl_snap_create_v2
    create_snapshot()
    --> adds snapshot to the
    list pending_snapshots
    of the current
    transaction

    btrfs_commit_transaction()
    --> waits for task at
    CPU 0 to release
    its transaction
    handle

    btrfs_commit_transaction()
    --> sees another task started
    the transaction commit first
    --> releases its transaction
    handle
    --> waits for the transaction
    commit to be completed by
    the task at CPU 1

    create_pending_snapshot()
    qgroup_account_snapshot()
    btrfs_qgroup_inherit()
    mutex_lock(fs_info->qgroup_ioctl_lock)
    --> deadlock, task at CPU 0
    has the mutex locked but
    it is waiting for us to
    finish the transaction
    commit

    So fix this by setting the quota enabled flag in fs_info after committing
    the transaction at btrfs_quota_enable(). This ends up serializing quota
    enable and snapshot creation as if the snapshot creation happened just
    before the quota enable request. The quota rescan task, scheduled after
    committing the transaction in btrfs_quote_enable(), will do the accounting.

    Fixes: 6426c7ad697d ("btrfs: qgroup: Fix qgroup accounting when creating snapshot")
    Signed-off-by: Filipe Manana <fdmanana@suse.com>
    Signed-off-by: David Sterba <dsterba@suse.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    fs/btrfs/qgroup.c | 14 ++++++++++----
    1 file changed, 10 insertions(+), 4 deletions(-)

    --- a/fs/btrfs/qgroup.c
    +++ b/fs/btrfs/qgroup.c
    @@ -1013,16 +1013,22 @@ out_add_root:
    btrfs_abort_transaction(trans, ret);
    goto out_free_path;
    }
    - spin_lock(&fs_info->qgroup_lock);
    - fs_info->quota_root = quota_root;
    - set_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
    - spin_unlock(&fs_info->qgroup_lock);

    ret = btrfs_commit_transaction(trans);
    trans = NULL;
    if (ret)
    goto out_free_path;

    + /*
    + * Set quota enabled flag after committing the transaction, to avoid
    + * deadlocks on fs_info->qgroup_ioctl_lock with concurrent snapshot
    + * creation.
    + */
    + spin_lock(&fs_info->qgroup_lock);
    + fs_info->quota_root = quota_root;
    + set_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
    + spin_unlock(&fs_info->qgroup_lock);
    +
    ret = qgroup_rescan_init(fs_info, 0, 1);
    if (!ret) {
    qgroup_rescan_zero_tracking(fs_info);

    \
     
     \ /
      Last update: 2019-01-15 17:46    [W:5.021 / U:0.196 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site