Re: [PATCH] fs: only sync() superblocks reachable from the current namespace

[Omar Sandoval <osandov@osandov.com>]

On Fri, Jan 26, 2018 at 11:17:29PM +0000, Al Viro wrote:
> On Fri, Jan 26, 2018 at 11:13:51PM +0000, Al Viro wrote:
> 
> > Erm...  And just what is protecting the list here?

Addressed in v2/v3, sorry about that.

> > >  static void fdatawrite_one_bdev(struct block_device *bdev, void *arg)
> > > @@ -107,12 +138,18 @@ static void fdatawait_one_bdev(struct block_device *bdev, void *arg)
> > >   */
> > >  SYSCALL_DEFINE0(sync)
> > >  {
> > > -	int nowait = 0, wait = 1;
> > > +	struct sb_sync arg = {
> > > +		.mnt_ns = current->nsproxy->mnt_ns,
> > > +	};
> > > +
> > > +	if (arg.mnt_ns == init_task.nsproxy->mnt_ns)
> > > +		arg.mnt_ns = NULL;
> > >  
> > >  	wakeup_flusher_threads(WB_REASON_SYNC);
> > > -	iterate_supers(sync_inodes_one_sb, NULL);
> > > -	iterate_supers(sync_fs_one_sb, &nowait);
> > > -	iterate_supers(sync_fs_one_sb, &wait);
> > > +	iterate_supers(sync_inodes_one_sb, &arg);
> > > +	iterate_supers(sync_fs_one_sb, &arg);
> > > +	arg.wait = 1;
> > > +	iterate_supers(sync_fs_one_sb, &arg);
> > 
> > So now sync() includes O(total vfsmounts on the system) walking the lists, no
> > matter what

This bit I thought about, and the only alternative I could come up with
was walking mnt_ns->list which makes it O(vfsmounts in the namespace)
which is at least capped at sysctl_mount_max. The downside of that is
that we have to track duplicates to handle super blocks mounted in more
than one place. And, of course, it doesn't address your other points.

> > *and* in a situation when a lazy-unmounted filesystem is held active
> > by an opened file sync(2) won't touch that filesystem.  Unless done in the
> > magical namespace init(8) happens to run in.

Hm, so we could always sync the filesystem if s_mounts is empty to cover
this.

> BTW, if your process happens to have inherited an opened file from parent,
> then unshares the namespace and unmounts the filesystem that file came
> from, sync(2) won't affect the writes on that descriptor.

But for this, I don't have any good ideas. Either we track these things
in the hot path or do something O(number of open fds). Do you have any
suggestions?