[Patch?] Teach sysfs_get_name not to use a dentry

[Patch?] Teach sysfs_get_name not to use a dentry

[Adam J. Richter]

Hi Maneesh,

	The following patch changes syfs_getname to avoid using
sysfs_dirent->s_dentry for getting the names of directories (as
this pointer may be NULL in a future version where sysfs would
be able to release the inode and dentry for each sysfs directory).
It does so by defining different sysfs_dirent.s_type values depending
on whether the diretory represents a kobject or an attribute_group.

	This patch is ground work for unpinning the struct inode
and struct dentry used by each sysfs directory.  The patch also may
facilitate eliminating the sysfs_dentry for each member of an
attribute group.  The patch has no benefits by itself, but I'm posting
it now separately in the hopes of making it easier for people
to spot problems, and, also, because if it is integrated, I think
it will make the rest of the change to unpin directories (which
I have not yet written) easier to understand.

	This patch is against 2.6.10-rc2-bk13 with both Chris Wright's
patch to allocate sysfs_dirent's from a kmem_cache and my patch
for removing sysfs_dirent.s_count already applied (in other words,
bk13 with two patches --> bk13 with three patches).

	I have verified that "find /sys" produces the same output
before and after applying this patch, and I have also been successfully
runing the tests that you previously suggested (loading and unloading
the dummy network device modules with running "ls -lR" on /sys and
cat'ing the files in /sys/class/net, all in a repeating loop).

	Please let me know what you think.  If you could get this
patch integrated and forwarded downstream, that would be great, and
would be a step toward encouraging small incremental patches if you
prefer those when feasible, but it can also wait for the patch to
unpin sysfs directories that is your preference.

                    __     ______________
Adam J. Richter        \ /
adam@yggdrasil.com      | g g d r a s i l
--- linux.prev/include/linux/sysfs.h	2004-12-02 10:14:06.000000000 +0800
+++ linux/include/linux/sysfs.h	2004-12-02 13:27:04.000000000 +0800
@@ -70,10 +70,11 @@
 
 #define SYSFS_ROOT		0x0001
 #define SYSFS_DIR		0x0002
 #define SYSFS_KOBJ_ATTR 	0x0004
 #define SYSFS_KOBJ_BIN_ATTR	0x0008
+#define SYSFS_ATTR_GROUP	0x0010
 #define SYSFS_KOBJ_LINK 	0x0020
 #define SYSFS_NOT_PINNED	(SYSFS_KOBJ_ATTR | SYSFS_KOBJ_BIN_ATTR | SYSFS_KOBJ_LINK)
 
 #ifdef CONFIG_SYSFS
 
diff -u5 linux.prev/fs/sysfs/dir.c linux/fs/sysfs/dir.c
--- linux.prev/fs/sysfs/dir.c	2004-12-02 11:55:02.000000000 +0800
+++ linux/fs/sysfs/dir.c	2004-12-02 13:14:51.000000000 +0800
@@ -89,23 +89,23 @@
 {
 	inode->i_op = &sysfs_symlink_inode_operations;
 	return 0;
 }
 
-static int create_dir(struct kobject * k, struct dentry * p,
-		      const char * n, struct dentry ** d)
+static int create_dir(void *element, struct dentry * p,
+		      const char * n, struct dentry ** d, int type)
 {
 	int error;
 	umode_t mode = S_IFDIR| S_IRWXU | S_IRUGO | S_IXUGO;
 
 	down(&p->d_inode->i_sem);
 	*d = sysfs_get_dentry(p,n);
 	if (!IS_ERR(*d)) {
 		error = sysfs_create(*d, mode, init_dir);
 		if (!error) {
-			error = sysfs_make_dirent(p->d_fsdata, *d, k, mode,
-						SYSFS_DIR);
+			error = sysfs_make_dirent(p->d_fsdata, *d, element,
+						  mode, type);
 			if (!error) {
 				p->d_inode->i_nlink++;
 				(*d)->d_op = &sysfs_dentry_ops;
 				d_rehash(*d);
 			}
@@ -118,13 +118,15 @@
 	up(&p->d_inode->i_sem);
 	return error;
 }
 
 
-int sysfs_create_subdir(struct kobject * k, const char * n, struct dentry ** d)
+int sysfs_create_subdir(struct kobject * k,
+			const struct attribute_group *grp,
+			struct dentry ** d)
 {
-	return create_dir(k,k->dentry,n,d);
+	return create_dir(grp, k->dentry, grp->name, d, SYSFS_ATTR_GROUP);
 }
 
 /**
  *	sysfs_create_dir - create a directory for an object.
  *	@parent:	parent parent object.
@@ -144,11 +146,11 @@
 	else if (sysfs_mount && sysfs_mount->mnt_sb)
 		parent = sysfs_mount->mnt_sb->s_root;
 	else
 		return -EFAULT;
 
-	error = create_dir(kobj,parent,kobject_name(kobj),&dentry);
+	error = create_dir(kobj,parent,kobject_name(kobj),&dentry, SYSFS_DIR);
 	if (!error)
 		kobj->dentry = dentry;
 	return error;
 }
 
diff -u5 linux.prev/fs/sysfs/group.c linux/fs/sysfs/group.c
--- linux.prev/fs/sysfs/group.c	2004-11-03 15:09:12.000000000 +0800
+++ linux/fs/sysfs/group.c	2004-12-02 13:16:06.000000000 +0800
@@ -46,11 +46,11 @@
 	int error;
 
 	BUG_ON(!kobj || !kobj->dentry);
 
 	if (grp->name) {
-		error = sysfs_create_subdir(kobj,grp->name,&dir);
+		error = sysfs_create_subdir(kobj, grp, &dir);
 		if (error)
 			return error;
 	} else
 		dir = kobj->dentry;
 	dir = dget(dir);
diff -u5 linux.prev/fs/sysfs/inode.c linux/fs/sysfs/inode.c
--- linux.prev/fs/sysfs/inode.c	2004-12-02 10:14:06.000000000 +0800
+++ linux/fs/sysfs/inode.c	2004-12-02 13:18:22.000000000 +0800
@@ -92,18 +92,24 @@
 const unsigned char * sysfs_get_name(struct sysfs_dirent *sd)
 {
 	struct attribute * attr;
 	struct bin_attribute * bin_attr;
 	struct sysfs_symlink  * sl;
+	struct attribute_group *grp;
+	struct kobject *kobj;
 
 	if (!sd || !sd->s_element)
 		BUG();
 
 	switch (sd->s_type) {
 		case SYSFS_DIR:
-			/* Always have a dentry so use that */
-			return sd->s_dentry->d_name.name;
+			kobj = sd->s_element;
+			return kobj->name;
+
+		case SYSFS_ATTR_GROUP:
+			grp = sd->s_element;
+			return grp->name;
 
 		case SYSFS_KOBJ_ATTR:
 			attr = sd->s_element;
 			return attr->name;
 
diff -u5 linux.prev/fs/sysfs/sysfs.h linux/fs/sysfs/sysfs.h
--- linux.prev/fs/sysfs/sysfs.h	2004-12-02 11:55:02.000000000 +0800
+++ linux/fs/sysfs/sysfs.h	2004-12-02 13:56:51.000000000 +0800
@@ -10,11 +10,11 @@
 extern struct dentry * sysfs_get_dentry(struct dentry *, const char *);
 
 extern int sysfs_add_file(struct dentry *, const struct attribute *, int);
 extern void sysfs_hash_and_remove(struct dentry * dir, const char * name);
 
-extern int sysfs_create_subdir(struct kobject *, const char *, struct dentry **);
+extern int sysfs_create_subdir(struct kobject *, const struct attribute_group *, struct dentry **);
 extern void sysfs_remove_subdir(struct dentry *);
 
 extern const unsigned char * sysfs_get_name(struct sysfs_dirent *sd);
 extern void sysfs_drop_dentry(struct sysfs_dirent *sd, struct dentry *parent);
 
@@ -32,10 +32,15 @@
 };
 
 static inline struct kobject * to_kobj(struct dentry * dentry)
 {
 	struct sysfs_dirent * sd = dentry->d_fsdata;
+
+	if (sd->s_type == SYSFS_ATTR_GROUP)
+		sd = dentry->d_parent->d_fsdata;
+
+	BUG_ON(sd->s_type != SYSFS_DIR);
 	return ((struct kobject *) sd->s_element);
 }
 
 static inline struct attribute * to_attr(struct dentry * dentry)
 {
@@ -54,10 +59,14 @@
 	struct kobject * kobj = NULL;
 
 	spin_lock(&dcache_lock);
 	if (!d_unhashed(dentry)) {
 		struct sysfs_dirent * sd = dentry->d_fsdata;
+		if (sd->s_type == SYSFS_ATTR_GROUP) {
+			sd = dentry->d_parent->d_fsdata;
+			BUG_ON(sd->s_type != SYSFS_DIR);
+		}
 		if (sd->s_type & SYSFS_KOBJ_LINK) {
 			struct sysfs_symlink * sl = sd->s_element;
 			kobj = kobject_get(sl->target_kobj);
 		} else
 			kobj = kobject_get(sd->s_element);

Re: [Patch?] Teach sysfs_get_name not to use a dentry

[Chris Wright]

* Adam J. Richter (adam@yggdrasil.com) wrote:
> -static int create_dir(struct kobject * k, struct dentry * p,
> -		      const char * n, struct dentry ** d)
> +static int create_dir(void *element, struct dentry * p,
> +		      const char * n, struct dentry ** d, int type)

Hmm, I did not look closely, but moving from well-typed to untyped void *
doesn't look nice.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [Patch?] Teach sysfs_get_name not to use a dentry

[Al Viro]

On Wed, Dec 01, 2004 at 10:41:08PM -0800, Adam J. Richter wrote:
> Hi Maneesh,
> 
> 	The following patch changes syfs_getname to avoid using
> sysfs_dirent->s_dentry for getting the names of directories (as
> this pointer may be NULL in a future version where sysfs would
> be able to release the inode and dentry for each sysfs directory).
> It does so by defining different sysfs_dirent.s_type values depending
> on whether the diretory represents a kobject or an attribute_group.
> 
> 	This patch is ground work for unpinning the struct inode
> and struct dentry used by each sysfs directory.  The patch also may
> facilitate eliminating the sysfs_dentry for each member of an
> attribute group.  The patch has no benefits by itself, but I'm posting
> it now separately in the hopes of making it easier for people
> to spot problems, and, also, because if it is integrated, I think
> it will make the rest of the change to unpin directories (which
> I have not yet written) easier to understand.

Vetoed until you show an acceptable locking scheme for the latter.
I do not believe that it's feasible; feel free to prove me wrong,
but until then you'll have to carry the patchset on your own.

Re: [Patch?] Teach sysfs_get_name not to use a dentry

[Adam J. Richter]

Chris Wright wrote:
>* Adam J. Richter (adam@yggdrasil.com) wrote:
>> -static int create_dir(struct kobject * k, struct dentry * p,
>> -		      const char * n, struct dentry ** d)
>> +static int create_dir(void *element, struct dentry * p,
>> +		      const char * n, struct dentry ** d, int type)

>Hmm, I did not look closely, but moving from well-typed to untyped void *
>doesn't look nice.

	The reason for the change comes from the fact that there are
two different kinds of directories in a sysfs tree: one representing
a kobject, and one representing an attribute_group.  Previously, both
types of directories saved a pointer to the kobject (in the case of
the attribute group, the parent directory is a kobject).  Now, however,
the attribute group saves a pointer to the struct attribute group,
so that the name of the attribute group can be accessed without
referencing the dentry.  So, the value that create_dir sets
sysfs_dirent.s_element to really can be to more than one type
depending on who is calling create_dir (and, by the way,
sysfs_dirent.s_element is used to point to a few different
data types, not just these two, and setting sysfs_dirent.s_element
is the only thing that create_dir does with that parameter).

	You might wonder how an attribute group's kobject is
determined within sysfs, if we are no longer saving that pointer in
sysfs_dirent.s_element.  It turns out that cases where the parent
kobject needs to be determined from the attribute group in sysfs
only occur in when the attribute group's dentry is available, so
the kobject is available as sysfs_dirent->s_dentry->d_parent->s_fsdata.

                    __     ______________
Adam J. Richter        \ /
adam@yggdrasil.com      | g g d r a s i l

Re: [Patch?] Teach sysfs_get_name not to use a dentry

[Adam J. Richter]

I wrote:
>the kobject is available as sysfs_dirent->s_dentry->d_parent->s_fsdata.

Argh.  I meant the attribute group's kobject is available as

((struct sysfs_dirent *) sysfs_dirent->s_dentry->d_parent->s_fsdata)->s_element

	...although this is normally calculated with some intermediate
steps and variables for clarity.

                    __     ______________
Adam J. Richter        \ /
adam@yggdrasil.com      | g g d r a s i l

Re: [Patch?] Teach sysfs_get_name not to use a dentry

[Adam J. Richter]

On Thu, 2 Dec 2004 07:00:22 +0000 Al Viro wrote:
>On Wed, Dec 01, 2004 at 10:41:08PM -0800, Adam J. Richter wrote:
>> Hi Maneesh,
>> 
>> 	The following patch changes syfs_getname to avoid using
>> sysfs_dirent->s_dentry for getting the names of directories (as
>> this pointer may be NULL in a future version where sysfs would
>> be able to release the inode and dentry for each sysfs directory).
>> It does so by defining different sysfs_dirent.s_type values depending
>> on whether the diretory represents a kobject or an attribute_group.
>> 
>> 	This patch is ground work for unpinning the struct inode
>> and struct dentry used by each sysfs directory.  The patch also may
>> facilitate eliminating the sysfs_dentry for each member of an
>> attribute group.  The patch has no benefits by itself, but I'm posting
>> it now separately in the hopes of making it easier for people
>> to spot problems, and, also, because if it is integrated, I think
>> it will make the rest of the change to unpin directories (which
>> I have not yet written) easier to understand.

>Vetoed until you show an acceptable locking scheme for the latter.
>I do not believe that it's feasible; feel free to prove me wrong,
>but until then you'll have to carry the patchset on your own.

	I will try to code it up, but first I expect to post a couple
more incremental changes along the way that,  unlike this change,
are useful in their own right even if the unpinning the sysfs
directories are not implemented.  In particular, I expect to post
a patch to move the sysfs_dirent and the sysfs_dirent.s_type values
from the public include/linux/sysfs.h to fs/sysfs/sysfs.h, and
a patch to eliminate sysfs_dirent.s_children for non-directories.

	When I do get around to posting a patch to unpin the sysfs
directories, I think it would be incumbent upon anyone claiming
that they are "vetoed" to describing the problem including at least
a hypothetical example, as you haven't really defined "acceptable"
if you haven't shown there to be problem.  By the same token, it's
reasonable if you want to wait and see an implementation before
complaining, although if there is some problem scenario you can
already think of, I'd be interested in hearing about it.

	Here are a few basic points to check if you already see
a locking problem that you think would really be hard to address.

	1. The only renames in sysfs (as far as I know) are within
	   the same directory for an unusual use by the network
	   code, which I suspect could probably be eliminated anyhow.

	2. sysfs_dirent does not maintain a parallel "parent" pointer.

	3. I intended to replace kobject->dentry with kobject->sysfs_dir.

	4. Is the race you see unique to directories?

                    __     ______________
Adam J. Richter        \ /
adam@yggdrasil.com      | g g d r a s i l

Re: [Patch?] Teach sysfs_get_name not to use a dentry

[Maneesh Soni]

On Wed, Dec 01, 2004 at 10:41:08PM -0800, Adam J. Richter wrote:
> Hi Maneesh,
> 
> 	The following patch changes syfs_getname to avoid using
> sysfs_dirent->s_dentry for getting the names of directories (as
> this pointer may be NULL in a future version where sysfs would
> be able to release the inode and dentry for each sysfs directory).
> It does so by defining different sysfs_dirent.s_type values depending
> on whether the diretory represents a kobject or an attribute_group.
> 
> 	This patch is ground work for unpinning the struct inode
> and struct dentry used by each sysfs directory.  The patch also may
> facilitate eliminating the sysfs_dentry for each member of an
> attribute group.  The patch has no benefits by itself, but I'm posting
> it now separately in the hopes of making it easier for people
> to spot problems, and, also, because if it is integrated, I think
> it will make the rest of the change to unpin directories (which
> I have not yet written) easier to understand.
> 
> 	This patch is against 2.6.10-rc2-bk13 with both Chris Wright's
> patch to allocate sysfs_dirent's from a kmem_cache and my patch
> for removing sysfs_dirent.s_count already applied (in other words,
> bk13 with two patches --> bk13 with three patches).
> 
> 	I have verified that "find /sys" produces the same output
> before and after applying this patch, and I have also been successfully
> runing the tests that you previously suggested (loading and unloading
> the dummy network device modules with running "ls -lR" on /sys and
> cat'ing the files in /sys/class/net, all in a repeating loop).
> 
> 	Please let me know what you think.  If you could get this
> patch integrated and forwarded downstream, that would be great, and
> would be a step toward encouraging small incremental patches if you
> prefer those when feasible, but it can also wait for the patch to
> unpin sysfs directories that is your preference.


Lots of changes, which donot make sense if we don't do unpinning 
of directories. Lets see this if unpinning of directories can be
done. Better to post with a patchset containing unpinning directory
code. 

Thanks
Maneesh

> 
>                     __     ______________
> Adam J. Richter        \ /
> adam@yggdrasil.com      | g g d r a s i l
> --- linux.prev/include/linux/sysfs.h	2004-12-02 10:14:06.000000000 +0800
> +++ linux/include/linux/sysfs.h	2004-12-02 13:27:04.000000000 +0800
> @@ -70,10 +70,11 @@
>  
>  #define SYSFS_ROOT		0x0001
>  #define SYSFS_DIR		0x0002
>  #define SYSFS_KOBJ_ATTR 	0x0004
>  #define SYSFS_KOBJ_BIN_ATTR	0x0008
> +#define SYSFS_ATTR_GROUP	0x0010
>  #define SYSFS_KOBJ_LINK 	0x0020
>  #define SYSFS_NOT_PINNED	(SYSFS_KOBJ_ATTR | SYSFS_KOBJ_BIN_ATTR | SYSFS_KOBJ_LINK)
>  
>  #ifdef CONFIG_SYSFS
>  
> diff -u5 linux.prev/fs/sysfs/dir.c linux/fs/sysfs/dir.c
> --- linux.prev/fs/sysfs/dir.c	2004-12-02 11:55:02.000000000 +0800
> +++ linux/fs/sysfs/dir.c	2004-12-02 13:14:51.000000000 +0800
> @@ -89,23 +89,23 @@
>  {
>  	inode->i_op = &sysfs_symlink_inode_operations;
>  	return 0;
>  }
>  
> -static int create_dir(struct kobject * k, struct dentry * p,
> -		      const char * n, struct dentry ** d)
> +static int create_dir(void *element, struct dentry * p,
> +		      const char * n, struct dentry ** d, int type)
>  {
>  	int error;
>  	umode_t mode = S_IFDIR| S_IRWXU | S_IRUGO | S_IXUGO;
>  
>  	down(&p->d_inode->i_sem);
>  	*d = sysfs_get_dentry(p,n);
>  	if (!IS_ERR(*d)) {
>  		error = sysfs_create(*d, mode, init_dir);
>  		if (!error) {
> -			error = sysfs_make_dirent(p->d_fsdata, *d, k, mode,
> -						SYSFS_DIR);
> +			error = sysfs_make_dirent(p->d_fsdata, *d, element,
> +						  mode, type);
>  			if (!error) {
>  				p->d_inode->i_nlink++;
>  				(*d)->d_op = &sysfs_dentry_ops;
>  				d_rehash(*d);
>  			}
> @@ -118,13 +118,15 @@
>  	up(&p->d_inode->i_sem);
>  	return error;
>  }
>  
>  
> -int sysfs_create_subdir(struct kobject * k, const char * n, struct dentry ** d)
> +int sysfs_create_subdir(struct kobject * k,
> +			const struct attribute_group *grp,
> +			struct dentry ** d)
>  {
> -	return create_dir(k,k->dentry,n,d);
> +	return create_dir(grp, k->dentry, grp->name, d, SYSFS_ATTR_GROUP);
>  }
>  
>  /**
>   *	sysfs_create_dir - create a directory for an object.
>   *	@parent:	parent parent object.
> @@ -144,11 +146,11 @@
>  	else if (sysfs_mount && sysfs_mount->mnt_sb)
>  		parent = sysfs_mount->mnt_sb->s_root;
>  	else
>  		return -EFAULT;
>  
> -	error = create_dir(kobj,parent,kobject_name(kobj),&dentry);
> +	error = create_dir(kobj,parent,kobject_name(kobj),&dentry, SYSFS_DIR);
>  	if (!error)
>  		kobj->dentry = dentry;
>  	return error;
>  }
>  
> diff -u5 linux.prev/fs/sysfs/group.c linux/fs/sysfs/group.c
> --- linux.prev/fs/sysfs/group.c	2004-11-03 15:09:12.000000000 +0800
> +++ linux/fs/sysfs/group.c	2004-12-02 13:16:06.000000000 +0800
> @@ -46,11 +46,11 @@
>  	int error;
>  
>  	BUG_ON(!kobj || !kobj->dentry);
>  
>  	if (grp->name) {
> -		error = sysfs_create_subdir(kobj,grp->name,&dir);
> +		error = sysfs_create_subdir(kobj, grp, &dir);
>  		if (error)
>  			return error;
>  	} else
>  		dir = kobj->dentry;
>  	dir = dget(dir);
> diff -u5 linux.prev/fs/sysfs/inode.c linux/fs/sysfs/inode.c
> --- linux.prev/fs/sysfs/inode.c	2004-12-02 10:14:06.000000000 +0800
> +++ linux/fs/sysfs/inode.c	2004-12-02 13:18:22.000000000 +0800
> @@ -92,18 +92,24 @@
>  const unsigned char * sysfs_get_name(struct sysfs_dirent *sd)
>  {
>  	struct attribute * attr;
>  	struct bin_attribute * bin_attr;
>  	struct sysfs_symlink  * sl;
> +	struct attribute_group *grp;
> +	struct kobject *kobj;
>  
>  	if (!sd || !sd->s_element)
>  		BUG();
>  
>  	switch (sd->s_type) {
>  		case SYSFS_DIR:
> -			/* Always have a dentry so use that */
> -			return sd->s_dentry->d_name.name;
> +			kobj = sd->s_element;
> +			return kobj->name;
> +
> +		case SYSFS_ATTR_GROUP:
> +			grp = sd->s_element;
> +			return grp->name;
>  
>  		case SYSFS_KOBJ_ATTR:
>  			attr = sd->s_element;
>  			return attr->name;
>  
> diff -u5 linux.prev/fs/sysfs/sysfs.h linux/fs/sysfs/sysfs.h
> --- linux.prev/fs/sysfs/sysfs.h	2004-12-02 11:55:02.000000000 +0800
> +++ linux/fs/sysfs/sysfs.h	2004-12-02 13:56:51.000000000 +0800
> @@ -10,11 +10,11 @@
>  extern struct dentry * sysfs_get_dentry(struct dentry *, const char *);
>  
>  extern int sysfs_add_file(struct dentry *, const struct attribute *, int);
>  extern void sysfs_hash_and_remove(struct dentry * dir, const char * name);
>  
> -extern int sysfs_create_subdir(struct kobject *, const char *, struct dentry **);
> +extern int sysfs_create_subdir(struct kobject *, const struct attribute_group *, struct dentry **);
>  extern void sysfs_remove_subdir(struct dentry *);
>  
>  extern const unsigned char * sysfs_get_name(struct sysfs_dirent *sd);
>  extern void sysfs_drop_dentry(struct sysfs_dirent *sd, struct dentry *parent);
>  
> @@ -32,10 +32,15 @@
>  };
>  
>  static inline struct kobject * to_kobj(struct dentry * dentry)
>  {
>  	struct sysfs_dirent * sd = dentry->d_fsdata;
> +
> +	if (sd->s_type == SYSFS_ATTR_GROUP)
> +		sd = dentry->d_parent->d_fsdata;
> +
> +	BUG_ON(sd->s_type != SYSFS_DIR);
>  	return ((struct kobject *) sd->s_element);
>  }
>  
>  static inline struct attribute * to_attr(struct dentry * dentry)
>  {
> @@ -54,10 +59,14 @@
>  	struct kobject * kobj = NULL;
>  
>  	spin_lock(&dcache_lock);
>  	if (!d_unhashed(dentry)) {
>  		struct sysfs_dirent * sd = dentry->d_fsdata;
> +		if (sd->s_type == SYSFS_ATTR_GROUP) {
> +			sd = dentry->d_parent->d_fsdata;
> +			BUG_ON(sd->s_type != SYSFS_DIR);
> +		}
>  		if (sd->s_type & SYSFS_KOBJ_LINK) {
>  			struct sysfs_symlink * sl = sd->s_element;
>  			kobj = kobject_get(sl->target_kobj);
>  		} else
>  			kobj = kobject_get(sd->s_element);
> 

-- 
Maneesh Soni
Linux Technology Center, 
IBM India Software Labs,
Bangalore, India
email: maneesh@in.ibm.com
Phone: 91-80-25044990

Re: [Patch?] Teach sysfs_get_name not to use a dentry

[Adam J. Richter]

On Thu, 30 Dec 2004 19:05:41 +0530, Maneesh Soni wrote:
>On Wed, Dec 01, 2004 at 10:41:08PM -0800, Adam J. Richter wrote:
>> Hi Maneesh,
>> 
>> 	The following patch changes syfs_getname to avoid using
>> sysfs_dirent->s_dentry for getting the names of directories (as
>> this pointer may be NULL in a future version where sysfs would
>> be able to release the inode and dentry for each sysfs directory).
>> It does so by defining different sysfs_dirent.s_type values depending
>> on whether the diretory represents a kobject or an attribute_group.
>> 
>> 	This patch is ground work for unpinning the struct inode
>> and struct dentry used by each sysfs directory.  The patch also may
>> facilitate eliminating the sysfs_dentry for each member of an
>> attribute group.  The patch has no benefits by itself, but I'm posting
>> it now separately in the hopes of making it easier for people
>> to spot problems, and, also, because if it is integrated, I think
>> it will make the rest of the change to unpin directories (which
>> I have not yet written) easier to understand.
>> 
>> 	This patch is against 2.6.10-rc2-bk13 with both Chris Wright's
>> patch to allocate sysfs_dirent's from a kmem_cache and my patch
>> for removing sysfs_dirent.s_count already applied (in other words,
>> bk13 with two patches --> bk13 with three patches).
>> 
>> 	I have verified that "find /sys" produces the same output
>> before and after applying this patch, and I have also been successfully
>> runing the tests that you previously suggested (loading and unloading
>> the dummy network device modules with running "ls -lR" on /sys and
>> cat'ing the files in /sys/class/net, all in a repeating loop).
>> 
>> 	Please let me know what you think.  If you could get this
>> patch integrated and forwarded downstream, that would be great, and
>> would be a step toward encouraging small incremental patches if you
>> prefer those when feasible, but it can also wait for the patch to
>> unpin sysfs directories that is your preference.


>Lots of changes, which donot make sense if we don't do unpinning 
>of directories. Lets see this if unpinning of directories can be
>done. Better to post with a patchset containing unpinning directory
>code. 

	Thanks for looking at my patches.

	I did post the patch unpinning sysfs directories, two and
a half weeks ago (the second URL is for a header file that I left out):

	http://marc.theaimsgroup.com/?l=linux-kernel&m=110257739705683&w=2
	http://marc.theaimsgroup.com/?l=linux-kernel&m=110257894222719&w=2

	The patches work fine for me.  I have been running with
all of these patches applied on every kernel that I've run up to
now (2.6.10-bk2) with zero problems.  I am not a heavy sysfs user,
but I have tried running your tests (loading and unloading the
dummy network module while reading the sysfs files that it creates),
including right now on 2.6.10-bk2.

	I am keen to know what you think of the patch unpinning
sysfs directories.

                    __     ______________
Adam J. Richter        \ /
adam@yggdrasil.com      | g g d r a s i l