Re: [PATCH 7/9] fsfreeze: freeze_super and thaw_bdev don't play well together

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2012/09/26 01:39, Jan Kara wrote:
@@ -1365,29 +1363,27 @@ static void sb_wait_write(struct super_b
   * freezing. Then we transition to SB_FREEZE_COMPLETE state. This state is
   * mostly auxiliary for filesystems to verify they do not modify frozen fs.
   *
- * sb->s_writers.frozen is protected by sb->s_umount.
+ * sb->s_writers.frozen and sb->s_freeze_count are protected by sb->s_umount.
   */
  int freeze_super(struct super_block *sb)
  {
-	int ret;
+	int ret = 0;
  	atomic_inc(&sb->s_active);
  	down_write(&sb->s_umount);
-	if (sb->s_writers.frozen != SB_UNFROZEN) {
-		deactivate_locked_super(sb);
-		return -EBUSY;
-	}
  	if (!(sb->s_flags & MS_BORN)) {
-		up_write(&sb->s_umount);
-		return 0;	/* sic - it's "nothing to do" */
+		atomic_dec(&sb->s_active);
+		goto out_active;	/* sic - it's "nothing to do" */
   Why not 'goto out_deactivate' here instead of manually decrementing
s_active?
I was afraid that calling deactivate_locked_super()
when the MS_BORN flag is set could release the
last active reference to the superblock and
end up freeing it.
   Well, the caller has to have the sb pinned somehow so that it can safely
pass it into freeze_super(). So deactivate_locked_super() cannot really
end up freeing the superblock...

I should have explained my fears better. If no one else
is holding an active reference we will end up executing:

fs->kill_sb(s);
...
put_filesystem(fs);
put_super(s);

in deactivate_locked_super(). If s_count is greater than 0
the superblock will not be freed, as you say, however we
still do "fs->kill_sb(s)" and "put_filesystem(fs)" and I am not
sure whether this is safe when MS_BORN flag is not set in
the superblock. I will check how MS_BORN is being used
once more and try to figure it out (if you are familiar with
MS_BORN I would appreciate it your advise).


By the way, I probably should explain in the
patch that that piece of code fixes a reference
leak bug in the current code.
   Yep, that would be nice.

Will do.

Thanks,
Fernando
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux