| Message ID | 20200207053821.25643-3-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: qgroup: Fix deadlock where btrfs_qgroup_wait_for_completion() waits for never-queued work | expand |
On Fri, Feb 07, 2020 at 01:38:21PM +0800, Qu Wenruo wrote: > Those two members are all protected by > btrfs_fs_info::qgroup_rescan_lock, thus no need for the extra spinlock. Two members refers to btrfs_fs_info::qgroup_rescan_lock and what else? Byt the subject it's something 'queued' but I can't find what it's referring to.
On 2020/2/25 上午12:45, David Sterba wrote: > On Fri, Feb 07, 2020 at 01:38:21PM +0800, Qu Wenruo wrote: >> Those two members are all protected by >> btrfs_fs_info::qgroup_rescan_lock, thus no need for the extra spinlock. > > Two members refers to btrfs_fs_info::qgroup_rescan_lock and what else? > Byt the subject it's something 'queued' but I can't find what it's > referring to. > My bad, with the latest version, there is only qgroup_rescan_running, no qgroup_rescan_queued. Just one member now. Do I need to resend the patch with commit message updated? Thanks, Qu
On Tue, Feb 25, 2020 at 07:44:12AM +0800, Qu Wenruo wrote: > > > On 2020/2/25 上午12:45, David Sterba wrote: > > On Fri, Feb 07, 2020 at 01:38:21PM +0800, Qu Wenruo wrote: > >> Those two members are all protected by > >> btrfs_fs_info::qgroup_rescan_lock, thus no need for the extra spinlock. > > > > Two members refers to btrfs_fs_info::qgroup_rescan_lock and what else? > > Byt the subject it's something 'queued' but I can't find what it's > > referring to. > > > My bad, with the latest version, there is only qgroup_rescan_running, no > qgroup_rescan_queued. > > Just one member now. > > Do I need to resend the patch with commit message updated? Not needed, I only wanted to clarify if I'm not missing something. I'll update changelog and push to misc-next.
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c index 812f51f67903..e07d6a6b2049 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -3247,7 +3247,6 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid, } mutex_lock(&fs_info->qgroup_rescan_lock); - spin_lock(&fs_info->qgroup_lock); if (init_flags) { if (fs_info->qgroup_flags & BTRFS_QGROUP_STATUS_FLAG_RESCAN) { @@ -3262,7 +3261,6 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid, } if (ret) { - spin_unlock(&fs_info->qgroup_lock); mutex_unlock(&fs_info->qgroup_rescan_lock); return ret; } @@ -3273,8 +3271,6 @@ qgroup_rescan_init(struct btrfs_fs_info *fs_info, u64 progress_objectid, sizeof(fs_info->qgroup_rescan_progress)); fs_info->qgroup_rescan_progress.objectid = progress_objectid; init_completion(&fs_info->qgroup_rescan_completion); - - spin_unlock(&fs_info->qgroup_lock); mutex_unlock(&fs_info->qgroup_rescan_lock); btrfs_init_work(&fs_info->qgroup_rescan_work, @@ -3351,9 +3347,7 @@ int btrfs_qgroup_wait_for_completion(struct btrfs_fs_info *fs_info, int ret = 0; mutex_lock(&fs_info->qgroup_rescan_lock); - spin_lock(&fs_info->qgroup_lock); running = fs_info->qgroup_rescan_running; - spin_unlock(&fs_info->qgroup_lock); mutex_unlock(&fs_info->qgroup_rescan_lock); if (!running)
Those two members are all protected by btrfs_fs_info::qgroup_rescan_lock, thus no need for the extra spinlock. Suggested-by: Josef Bacik <josef@toxicpanda.com> Signed-off-by: Qu Wenruo <wqu@suse.com> --- Changelog: v2: - New patch split in v2 --- fs/btrfs/qgroup.c | 6 ------ 1 file changed, 6 deletions(-)