Re: [PATCH 12/13] sysfs: Propagate renames to the vfs on demand

[Eric W. Biederman]

"Serge E. Hallyn" <serue@xxxxxxxxxx> writes:

> Quoting Eric W. Biederman (ebiederm@xxxxxxxxxxxx):
>> From: Eric W. Biederman <ebiederm@xxxxxxxxxxxx>
>> 
>> 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@xxxxxxxxxx>
>> Signed-off-by: Eric W. Biederman <ebiederm@xxxxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/