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

[Jan Kara <jack@suse.cz>]

On Wed 03-10-12 16:58:50, Fernando Luis Vazquez Cao wrote:
> On 2012/09/26 18:09, Jan Kara wrote:
> >>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).
> >   I see. Well, from a brief check I don't think we should ever get to a
> >superblock without MS_BORN set. All functions iterating over superblocks
> >return only superblocks with MS_BORN set. In particular freeze_bdev() even
> >has an active reference itself and ioctl_fsfreeze() has a file open on the
> >sb which guarantees an active reference as well...
> 
> As you say when we get there via the superblock level API
> it is guaranteed that we have at least one active reference
> and that MS_BORN is set. However, freeze_bdev() iterates
> over superblocks using get_active_super() which can return
> a superblock without the MS_BORN flag set; during sys_mount
> mount_fs() sets the MS_BORN flag *after* sget() inserts the
> superblock in all the lists.
> 
> If the analysis above is correct we do need the MS_BORN
> check.
  Checking again I agree with you. But that seems more like an issue with
get_active_super() which shouldn't return superblock without MS_BORN.
Whatever... that can be left for a separate patch.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR