Re: [PATCH v2 9/8] don't fool lockdep in freeze_super() and thaw_super() paths
From: Jan Kara <jack@suse.cz>
Date: 2015-08-13 11:01:55
Also in:
lkml
On Wed 12-08-15 15:11:38, Oleg Nesterov wrote:
On 08/11, Oleg Nesterov wrote:quoted
The only essential change is that I dropped the lockdep improvements as we discussed. This means that 8/8 was changed a bit, and I decided to add the new documentation patch, see 3/8.Update: The recent [PATCH 0/2] xfs: kill lockdep false positives from readdir changes from Dave fixed the problems ILOCK false-positives. So we can add the additional patch which (modulo comments) just turns v2 back into v1. Dave, Jan, you seem to agree with these patches. How should we route this all?
Regarding the routing, ideally Al Viro should take these as a VFS maintainer.
------------------------------------------------------------------------------- Subject: [PATCH v2 9/8] don't fool lockdep in freeze_super() and thaw_super() paths sb_wait_write()->percpu_rwsem_release() fools lockdep to avoid the false-positives. Now that xfs was fixed by Dave we can remove it and change freeze_super() and thaw_super() to run with s_writers.rw_sem locks held; we add two trivial helpers for that, sb_freeze_release() and sb_freeze_acquire(). While at it, kill the outdated part of the comment above sb_wait_write. Signed-off-by: Oleg Nesterov <oleg@redhat.com>
The patch looks good. Just one nit:
+ for (level = SB_FREEZE_LEVELS; --level >= 0; ) + percpu_rwsem_release(sb->s_writers.rw_sem + level, 0, _THIS_IP_);
It is more common (and to me more readable) to have the loop written as: for (level = SB_FREEZE_LEVELS - 1; level >= 0; level--) I agree what you do is shorter but IMHO it's just an unnecessary obfuscation :) Otherwise feel free to add: Reviewed-by: Jan Kara <jack@suse.com> Honza
quoted hunk ↗ jump to hunk
+} + +/* + * Tell lockdep we are holding these locks before we call ->unfreeze_fs(sb). + */ +static void sb_freeze_acquire(struct super_block *sb) { int level; for (level = 0; level < SB_FREEZE_LEVELS; ++level) percpu_rwsem_acquire(sb->s_writers.rw_sem + level, 0, _THIS_IP_); +} + +static void sb_freeze_unlock(struct super_block *sb) +{ + int level; for (level = SB_FREEZE_LEVELS; --level >= 0; ) percpu_up_write(sb->s_writers.rw_sem + level);@@ -1329,6 +1336,7 @@ int freeze_super(struct super_block *sb) * sees write activity when frozen is set to SB_FREEZE_COMPLETE. */ sb->s_writers.frozen = SB_FREEZE_COMPLETE; + sb_freeze_release(sb); up_write(&sb->s_umount); return 0; }@@ -1355,11 +1363,14 @@ int thaw_super(struct super_block *sb) goto out; } + sb_freeze_acquire(sb); + if (sb->s_op->unfreeze_fs) { error = sb->s_op->unfreeze_fs(sb); if (error) { printk(KERN_ERR "VFS:Filesystem thaw failed\n"); + sb_freeze_release(sb); up_write(&sb->s_umount); return error; }-- 1.5.5.1
-- Jan Kara [off-list ref] SUSE Labs, CR