To: "Serge E. Hallyn" <serue@us.ibm.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>, Kay Sievers <kay.sievers@vrfy.org>,
       Greg KH <greg@kroah.com>, linux-kernel@vger.kernel.org,
       Tejun Heo <tj@kernel.org>, Cornelia Huck <cornelia.huck@de.ibm.com>,
       linux-fsdevel@vger.kernel.org, Eric Dumazet <eric.dumazet@gmail.com>,
       Benjamin LaHaise <bcrl@lhnet.ca>,
       "Eric W. Biederman" <ebiederm@aristanetworks.com>
References: <m13a4vbz8r.fsf@fess.ebiederm.org>
	<1257249429-12384-12-git-send-email-ebiederm@xmission.com>
	<20091104214938.GA21033@us.ibm.com>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Wed, 04 Nov 2009 13:59:36 -0800
In-Reply-To: <20091104214938.GA21033@us.ibm.com> (Serge E. Hallyn's message of "Wed\, 4 Nov 2009 15\:49\:38 -0600")
Message-ID: <m13a4ux86v.fsf@fess.ebiederm.org>
User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.2 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Subject: Re: [PATCH 12/13] sysfs: Propagate renames to the vfs on demand
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2452
Lines: 65

"Serge E. Hallyn" <serue@us.ibm.com> writes:

> Quoting Eric W. Biederman (ebiederm@xmission.com):
>> From: Eric W. Biederman <ebiederm@xmission.com>
>> 
>> By teaching sysfs_revalidate to hide a dentry for
>> a sysfs_dirent if the sysfs_dirent has been renamed,
>> and by teaching sysfs_lookup to return the original
>> dentry if the sysfs dirent has been renamed.  I can
>> show the results of renames correctly without having to
>> update the dcache during the directory rename.
>> 
>> This massively simplifies the rename logic allowing a lot
>> of weird sysfs special cases to be removed along with
>> a lot of now unnecesary helper code.
>> 
>> Acked-by: Tejun Heo <tj@kernel.org>
>> Signed-off-by: Eric W. Biederman <ebiederm@aristanetworks.com>
>
> Patch looks *great*, except:
>
> ...
>
>> @@ -315,6 +274,14 @@ static int sysfs_dentry_revalidate(struct dentry *dentry, struct nameidata *nd)
>>  	if (sd->s_flags & SYSFS_FLAG_REMOVED)
>>  		goto out_bad;
>> 
>> +	/* The sysfs dirent has been moved? */
>> +	if (dentry->d_parent->d_fsdata != sd->s_parent)
>> +		goto out_bad;
>> +
>> +	/* The sysfs dirent has been renamed */
>> +	if (strcmp(dentry->d_name.name, sd->s_name) != 0)
>> +		goto out_bad;
>> +
>>  	mutex_unlock(&sysfs_mutex);
>>  out_valid:
>>  	return 1;
>> @@ -322,6 +289,12 @@ out_bad:
>>  	/* Remove the dentry from the dcache hashes.
>>  	 * If this is a deleted dentry we use d_drop instead of d_delete
>>  	 * so sysfs doesn't need to cope with negative dentries.
>> +	 *
>> +	 * If this is a dentry that has simply been renamed we
>> +	 * use d_drop to remove it from the dcache lookup on its
>> +	 * old parent.  If this dentry persists later when a lookup
>> +	 * is performed at its new name the dentry will be readded
>> +	 * to the dcache hashes.
>>  	 */
>>  	is_dir = (sysfs_type(sd) == SYSFS_DIR);
>>  	mutex_unlock(&sysfs_mutex);
>
> After this, if (is_dir) and (have_submounts(dentry)) then you'll
> still goto out_valid and return 1.  Is that what you want?

It isn't what I want but it is what the VFS requires.  If let the vfs
continue on it's delusional state we will leak the vfs mount and
everything mounted on top of it, with no way to remove the mounts.

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/