From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f66.google.com ([74.125.83.66]:47096 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751408AbeAZXqi (ORCPT ); Fri, 26 Jan 2018 18:46:38 -0500 Received: by mail-pg0-f66.google.com with SMTP id s9so1204537pgq.13 for ; Fri, 26 Jan 2018 15:46:37 -0800 (PST) Date: Fri, 26 Jan 2018 15:46:35 -0800 From: Omar Sandoval To: Al Viro Cc: linux-fsdevel@vger.kernel.org, kernel-team@fb.com, "Eric W . Biederman" , Tejun Heo Subject: Re: [PATCH] fs: only sync() superblocks reachable from the current namespace Message-ID: <20180126234635.GE19033@vader.DHCP.thefacebook.com> References: <05434cda5cc3b461b5d70467b094904ad23fdc11.1517007510.git.osandov@fb.com> <20180126231350.GC13338@ZenIV.linux.org.uk> <20180126231729.GD13338@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180126231729.GD13338@ZenIV.linux.org.uk> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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?