From: ebiederm@xmission.com (Eric W. Biederman)
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: Al Viro <viro@zeniv.linux.org.uk>, "Serge E. Hallyn" <serge@hallyn.com>,
        Linux-Fsdevel <linux-fsdevel@vger.kernel.org>,
        Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Andy Lutomirski <luto@amacapital.net>, Rob Landley <rob@landley.net>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Christoph Hellwig <hch@infradead.org>, Karel Zak <kzak@redhat.com>,
        "J. Bruce Fields" <bfields@fieldses.org>
References: <CAJfpegsxgnSRUW-E5HM3uT5QfGyUtn_v=i4Ppkkkutp34287AA@mail.gmail.com>
	<87a9kkax0j.fsf@xmission.com>
	<CAJfpeguv+7giYNpAuXE9Ja_9BEwB0-fZBVgRSeVqpzSXgQYZ6Q@mail.gmail.com>
	<8761v7h2pt.fsf@tw-ebiederman.twitter.com>
	<CAJfpegtT7y-1HhbEVAMKkdQugTG_w7G_epGtQHGvQLpcZB5FVA@mail.gmail.com>
	<87li281wx6.fsf_-_@xmission.com> <87ob28kqks.fsf_-_@xmission.com>
	<87txc0jbvt.fsf_-_@xmission.com>
	<20140218171708.GD4026@tucsk.piliscsaba.szeredi.hu>
Date: Mon, 24 Feb 2014 19:24:27 -0800
In-Reply-To: <20140218171708.GD4026@tucsk.piliscsaba.szeredi.hu> (Miklos
	Szeredi's message of "Tue, 18 Feb 2014 18:17:08 +0100")
Message-ID: <87mwhfdgd0.fsf@xmission.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [PATCH 05/11] vfs: Add a function to lazily unmount all mounts from any dentry.
Sender: linux-kernel-owner@vger.kernel.org

Miklos Szeredi <miklos@szeredi.hu> writes:

> On Sat, Feb 15, 2014 at 01:37:26PM -0800, Eric W. Biederman wrote:
>> 
>> v2: Always drop the lock when exiting early.
>> v3: Make detach_mounts robust about freeing several
>>     mounts on the same mountpoint at one time, and remove
>>     the unneeded mnt_list list test.
>> v4: Document the purpose of detach_mounts and why new_mountpoint is
>>     safe to call.
>> 
>> Signed-off-by: Eric W. Biederman <ebiederman@twitter.com>
>> ---
>>  fs/mount.h     |    2 ++
>>  fs/namespace.c |   39 +++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 41 insertions(+), 0 deletions(-)
>> 
>> diff --git a/fs/mount.h b/fs/mount.h
>> index 50a72d46e7a6..2b470f34e665 100644
>> --- a/fs/mount.h
>> +++ b/fs/mount.h
>> @@ -84,6 +84,8 @@ extern struct mount *__lookup_mnt_last(struct vfsmount *, struct dentry *);
>>  
>>  extern bool legitimize_mnt(struct vfsmount *, unsigned);
>>  
>> +extern void detach_mounts(struct dentry *dentry);
>> +
>>  static inline void get_mnt_ns(struct mnt_namespace *ns)
>>  {
>>  	atomic_inc(&ns->count);
>> diff --git a/fs/namespace.c b/fs/namespace.c
>> index 33db9e95bd5c..7abbf722ce18 100644
>> --- a/fs/namespace.c
>> +++ b/fs/namespace.c
>> @@ -1359,6 +1359,45 @@ static int do_umount(struct mount *mnt, int flags)
>>  	return retval;
>>  }
>>  
>> +/*
>> + * detach_mounts - lazily unmount all mounts on the specified dentry
>> + *
>> + * During unlink, rmdir, and d_drop it is possible to loose the path
>> + * to an existing mountpoint, and wind up leaking the mount.
>> + * detach_mounts allows lazily unmounting those mounts instead of
>> + * leaking them.
>> + * 
>> + * The caller may hold dentry->d_inode->i_mutex.
>> + */
>> +void detach_mounts(struct dentry *dentry)
>> +{
>> +	struct mountpoint *mp;
>> +	struct mount *mnt;
>> +
>> +	namespace_lock();
>> +	if (!d_mountpoint(dentry))
>> +		goto out_unlock;
>> +
>> +	/* 
>> +	 * The namespace lock and d_mountpoint being set guarantees
>> +	 * that new_mountpoint will just be a lookup of the existing
>> +	 * mountpoint structure.
>> +	 */
>> +	mp = new_mountpoint(dentry);
>
> Howabout a get_mountpoint(dentry) helper, that returns NULL if it turns out to
> be not a mountpoint?  And, as an added bonus, you can drop the comment above as
> well.

I hate to admit it but that is a nice change.   Especially as it allows
removing the d_mounpoint check inside of namespace_lock.  I still need a
cheak d_mounpoint check outside of namespace lock but inside it can go.

The first time I looked at doing that I missed something and the change
looked too awkward to be worth implementing :(

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/