Eric,

Thanks a lot for the updated patchset.

I haven't look at the details yet, but I've already integrated it to 
your network namespaces patchset and ported the whole thing to 
2.6.22-rc4-mm2 today.
It is far easier to do when you don't have to think about the sysfs stuff :)

I only performed some basic testing on it so far as I'm home today, but 
it already seems to work better than the patchset I ported to 
2.6.21-mm2. It solves the issues I still had with sysfs.

Thanks again.

Regards,
Benjamin


Eric W. Biederman wrote:
> While shadow directories appear to be a good idea, the current scheme
> of controlling their creation and destruction outside of sysfs appears
> to be a locking and maintenance nightmare in the face of sysfs
> directories dynamically coming and going.  Which can now occur for
> directories containing network devices when CONFIG_SYSFS_DEPRECATED is
> not set.
> 
> This patch removes everything from the initial shadow directory
> support that allowed the shadow directory creation to be controlled at
> a higher level.  So except for a few bits of sysfs_rename_dir
> everything from commit b592fcfe7f06c15ec11774b5be7ce0de3aa86e73 is now
> gone.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
> ---
> These patches are against 2.6.22-rc4-mm2  Hopefully that is new enough
> to catch all of the in flight sysfs patches.
> 
>  fs/sysfs/dir.c          |  172 ++++++++++-------------------------------------
>  fs/sysfs/group.c        |    1 -
>  fs/sysfs/inode.c        |   10 ---
>  fs/sysfs/mount.c        |    2 +-
>  fs/sysfs/sysfs.h        |    6 --
>  include/linux/kobject.h |    4 -
>  include/linux/sysfs.h   |   24 +------
>  lib/kobject.c           |   44 ++----------
>  8 files changed, 48 insertions(+), 215 deletions(-)
> 
> diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
> index b4074ad..b1da4fc 100644
> --- a/fs/sysfs/dir.c
> +++ b/fs/sysfs/dir.c
> @@ -415,10 +415,9 @@ int sysfs_create_subdir(struct kobject * k, const char * n, struct dentry ** d)
>  /**
>   *	sysfs_create_dir - create a directory for an object.
>   *	@kobj:		object we're creating directory for. 
> - *	@shadow_parent:	parent parent object.
>   */
>  
> -int sysfs_create_dir(struct kobject * kobj, struct dentry *shadow_parent)
> +int sysfs_create_dir(struct kobject * kobj)
>  {
>  	struct dentry * dentry = NULL;
>  	struct dentry * parent;
> @@ -426,9 +425,7 @@ int sysfs_create_dir(struct kobject * kobj, struct dentry *shadow_parent)
>  
>  	BUG_ON(!kobj);
>  
> -	if (shadow_parent)
> -		parent = shadow_parent;
> -	else if (kobj->parent)
> +	if (kobj->parent)
>  		parent = kobj->parent->dentry;
>  	else if (sysfs_mount && sysfs_mount->mnt_sb)
>  		parent = sysfs_mount->mnt_sb->s_root;
> @@ -516,12 +513,26 @@ void sysfs_remove_subdir(struct dentry * d)
>  }
>  
>  
> -static void __sysfs_remove_dir(struct dentry *dentry)
> +/**
> + *	sysfs_remove_dir - remove an object's directory.
> + *	@kobj:	object.
> + *
> + *	The only thing special about this is that we remove any files in
> + *	the directory before we remove the directory, and we've inlined
> + *	what used to be sysfs_rmdir() below, instead of calling separately.
> + */
> +
> +void sysfs_remove_dir(struct kobject * kobj)
>  {
> +	struct dentry *dentry = kobj->dentry;
>  	struct sysfs_dirent *removed = NULL;
>  	struct sysfs_dirent *parent_sd;
>  	struct sysfs_dirent **pos;
>  
> +	spin_lock(&kobj_sysfs_assoc_lock);
> +	kobj->dentry = NULL;
> +	spin_unlock(&kobj_sysfs_assoc_lock);
> +
>  	if (!dentry)
>  		return;
>  
> @@ -555,55 +566,35 @@ static void __sysfs_remove_dir(struct dentry *dentry)
>  	remove_dir(dentry);
>  }
>  
> -/**
> - *	sysfs_remove_dir - remove an object's directory.
> - *	@kobj:	object.
> - *
> - *	The only thing special about this is that we remove any files in
> - *	the directory before we remove the directory, and we've inlined
> - *	what used to be sysfs_rmdir() below, instead of calling separately.
> - */
> -
> -void sysfs_remove_dir(struct kobject * kobj)
> -{
> -	struct dentry *d = kobj->dentry;
> -
> -	spin_lock(&kobj_sysfs_assoc_lock);
> -	kobj->dentry = NULL;
> -	spin_unlock(&kobj_sysfs_assoc_lock);
> -
> -	__sysfs_remove_dir(d);
> -}
> -
> -int sysfs_rename_dir(struct kobject * kobj, struct dentry *new_parent,
> -		     const char *new_name)
> +int sysfs_rename_dir(struct kobject * kobj, const char *new_name)
>  {
> -	struct sysfs_dirent *sd = kobj->dentry->d_fsdata;
> -	struct sysfs_dirent *parent_sd = new_parent->d_fsdata;
> -	struct dentry *new_dentry;
> +	struct sysfs_dirent *sd;
> +	struct sysfs_dirent *parent_sd;
> +	struct inode *inode;
> +	struct dentry *new_dentry, *parent;
>  	char *dup_name;
>  	int error;
>  
> -	if (!new_parent)
> -		return -EFAULT;
> +	if (!kobj->parent)
> +		return -EINVAL;
> +
> +	parent = dget(kobj->dentry->d_parent);
> +	inode = parent->d_inode;
> +
> +	sd = kobj->dentry->d_fsdata;
>  
>  	down_write(&sysfs_rename_sem);
> -	mutex_lock(&new_parent->d_inode->i_mutex);
> +	mutex_lock(&inode->i_mutex);
>  
> -	new_dentry = lookup_one_len(new_name, new_parent, strlen(new_name));
> +	parent_sd = parent->d_fsdata;
> +	new_dentry = lookup_one_len(new_name, parent_sd->s_dentry, strlen(new_name));
>  	if (IS_ERR(new_dentry)) {
>  		error = PTR_ERR(new_dentry);
>  		goto out_unlock;
>  	}
>  
> -	/* By allowing two different directories with the same
> -	 * d_parent we allow this routine to move between different
> -	 * shadows of the same directory
> -	 */
>  	error = -EINVAL;
> -	if (kobj->dentry->d_parent->d_inode != new_parent->d_inode ||
> -	    new_dentry->d_parent->d_inode != new_parent->d_inode ||
> -	    new_dentry == kobj->dentry)
> +	if (new_dentry == kobj->dentry)
>  		goto out_dput;
>  
>  	error = -EEXIST;
> @@ -643,8 +634,11 @@ int sysfs_rename_dir(struct kobject * kobj, struct dentry *new_parent,
>   out_dput:
>  	dput(new_dentry);
>   out_unlock:
> -	mutex_unlock(&new_parent->d_inode->i_mutex);
> +	mutex_unlock(&inode->i_mutex);
>  	up_write(&sysfs_rename_sem);
> +
> +	dput(parent);
> +
>  	return error;
>  }
>  
> @@ -837,98 +831,6 @@ static loff_t sysfs_dir_lseek(struct file * file, loff_t offset, int origin)
>  	return offset;
>  }
>  
> -
> -/**
> - *	sysfs_make_shadowed_dir - Setup so a directory can be shadowed
> - *	@kobj:	object we're creating shadow of.
> - */
> -
> -int sysfs_make_shadowed_dir(struct kobject *kobj,
> -	void * (*follow_link)(struct dentry *, struct nameidata *))
> -{
> -	struct inode *inode;
> -	struct inode_operations *i_op;
> -
> -	inode = kobj->dentry->d_inode;
> -	if (inode->i_op != &sysfs_dir_inode_operations)
> -		return -EINVAL;
> -
> -	i_op = kmalloc(sizeof(*i_op), GFP_KERNEL);
> -	if (!i_op)
> -		return -ENOMEM;
> -
> -	memcpy(i_op, &sysfs_dir_inode_operations, sizeof(*i_op));
> -	i_op->follow_link = follow_link;
> -
> -	/* Locking of inode->i_op?
> -	 * Since setting i_op is a single word write and they
> -	 * are atomic we should be ok here.
> -	 */
> -	inode->i_op = i_op;
> -	return 0;
> -}
> -
> -/**
> - *	sysfs_create_shadow_dir - create a shadow directory for an object.
> - *	@kobj:	object we're creating directory for.
> - *
> - *	sysfs_make_shadowed_dir must already have been called on this
> - *	directory.
> - */
> -
> -struct dentry *sysfs_create_shadow_dir(struct kobject *kobj)
> -{
> -	struct dentry *dir = kobj->dentry;
> -	struct inode *inode = dir->d_inode;
> -	struct dentry *parent = dir->d_parent;
> -	struct sysfs_dirent *parent_sd = parent->d_fsdata;
> -	struct dentry *shadow;
> -	struct sysfs_dirent *sd;
> -
> -	shadow = ERR_PTR(-EINVAL);
> -	if (!sysfs_is_shadowed_inode(inode))
> -		goto out;
> -
> -	shadow = d_alloc(parent, &dir->d_name);
> -	if (!shadow)
> -		goto nomem;
> -
> -	sd = sysfs_new_dirent("_SHADOW_", inode->i_mode, SYSFS_DIR);
> -	if (!sd)
> -		goto nomem;
> -	sd->s_elem.dir.kobj = kobj;
> -	/* point to parent_sd but don't attach to it */
> -	sd->s_parent = sysfs_get(parent_sd);
> -	sysfs_attach_dirent(sd, NULL, shadow);
> -
> -	d_instantiate(shadow, igrab(inode));
> -	inc_nlink(inode);
> -	inc_nlink(parent->d_inode);
> -
> -	dget(shadow);		/* Extra count - pin the dentry in core */
> -
> -out:
> -	return shadow;
> -nomem:
> -	dput(shadow);
> -	shadow = ERR_PTR(-ENOMEM);
> -	goto out;
> -}
> -
> -/**
> - *	sysfs_remove_shadow_dir - remove an object's directory.
> - *	@shadow: dentry of shadow directory
> - *
> - *	The only thing special about this is that we remove any files in
> - *	the directory before we remove the directory, and we've inlined
> - *	what used to be sysfs_rmdir() below, instead of calling separately.
> - */
> -
> -void sysfs_remove_shadow_dir(struct dentry *shadow)
> -{
> -	__sysfs_remove_dir(shadow);
> -}
> -
>  const struct file_operations sysfs_dir_operations = {
>  	.open		= sysfs_dir_open,
>  	.release	= sysfs_dir_close,
> diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
> index 52eed2a..82e0f55 100644
> --- a/fs/sysfs/group.c
> +++ b/fs/sysfs/group.c
> @@ -13,7 +13