[v2,2/2] btrfs: qgroup: Remove the unnecesaary spin lock for qgroup_rescan_running|queued
diff mbox series

Message ID 20200207053821.25643-3-wqu@suse.com
State New
Headers show
Series
  • btrfs: qgroup: Fix deadlock where btrfs_qgroup_wait_for_completion() waits for never-queued work
Related show

Commit Message

Qu Wenruo Feb. 7, 2020, 5:38 a.m. UTC
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(-)

Comments

David Sterba Feb. 24, 2020, 4:45 p.m. UTC | #1
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.
Qu Wenruo Feb. 24, 2020, 11:44 p.m. UTC | #2
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
David Sterba Feb. 25, 2020, 4:49 p.m. UTC | #3
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.

Patch
diff mbox series

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)