[RFC 0/5] Backing Store for sysfs (Overhauled)

[RFC 0/5] Backing Store for sysfs (Overhauled)

[Maneesh Soni]

Hi All,

The following patch set has the overhauled prototype for sysfs backing store
for comments. I have tried to keep all the comments and suggestions from the 
last time in mind.

The main complaint was of over bloating kobject structure which becomes more
painful when kobject is not part of sysfs. So now I have changed the data
structures entirely. There is _no_ increase in the size of kobject structure.
The kobject hierarchy is represented in the form of a new structure called
sysfs_dirent (size 48 - bytes). sysfs_dirent will be there only for kobject
elements (kobject, attribute, attribute group, symlink) which are represented 
in sysfs. kobject structre has just one change. Now kobject has a field
pointing to its sysfs_dirent instead of dentry.

struct sysfs_dirent {
        struct list_head        s_sibling;
        struct list_head        s_children;
        void                    * s_element;
        struct dentry           * s_dentry;
        int                     s_type;
        struct rw_semaphore     s_rwsem;
};

The concept is still the same that in this prototype also we create dentry and 
inode on the fly when they are first looked up. This is done for both leaf or 
non-leaf dentries. The generic nature of sysfs_dirent makes it easy to do for 
both leaf or non-leaf dentries. 

Please review the patches following this posting. For testing apply all
the patches as they are splitted just for review.

Thanks
Maneesh

-- 
Maneesh Soni
Linux Technology Center, 
IBM Software Lab, Bangalore, India
email: maneesh@in.ibm.com
Phone: 91-80-5044999 Fax: 91-80-5268553
T/L : 9243696

[RFC 1/5] sysfs-backing-store.patch

[Maneesh Soni]

o This patch describes the necessary data structures needed for sysfs backing
  store and the changes related to initialing sysfs filesystem. sysfs_dirent
  represents any element from kobject infrastructure. The element could be
  kobject, attribute, binary attribute, attribute group or symlink. kobject
  hierarchy is represented by using sysfs_dirent's s_children & s_sibling
  links. 

o Root of sysfs filesystem is represented by statically allocated sysfs_dirent
  structure. Initialisation now does not mount sysfs filesystem and just
  does registration.

o The patch also has some support routines internal to sysfs code.


 fs/sysfs/mount.c        |   41 ++++++++++---------
 fs/sysfs/sysfs.h        |  103 ++++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/kobject.h |    2 
 include/linux/sysfs.h   |   19 ++++++++
 4 files changed, 142 insertions(+), 23 deletions(-)

diff -puN include/linux/kobject.h~sysfs-backing-store include/linux/kobject.h
--- linux-2.6.0-test9/include/linux/kobject.h~sysfs-backing-store	2003-11-12 16:02:27.000000000 +0530
+++ linux-2.6.0-test9-maneesh/include/linux/kobject.h	2003-11-12 16:02:27.000000000 +0530
@@ -31,7 +31,7 @@ struct kobject {
 	struct kobject		* parent;
 	struct kset		* kset;
 	struct kobj_type	* ktype;
-	struct dentry		* dentry;
+	struct sysfs_dirent	* s_dirent;
 };
 
 extern int kobject_set_name(struct kobject *, const char *, ...)
diff -puN include/linux/sysfs.h~sysfs-backing-store include/linux/sysfs.h
--- linux-2.6.0-test9/include/linux/sysfs.h~sysfs-backing-store	2003-11-12 16:02:27.000000000 +0530
+++ linux-2.6.0-test9-maneesh/include/linux/sysfs.h	2003-11-12 16:14:56.000000000 +0530
@@ -9,6 +9,8 @@
 #ifndef _SYSFS_H_
 #define _SYSFS_H_
 
+#include <linux/rwsem.h>
+
 struct kobject;
 struct module;
 
@@ -63,6 +65,23 @@ struct attribute_group {
 	struct attribute	** attrs;
 };
 
+struct sysfs_dirent {
+	struct list_head	s_sibling;  
+	struct list_head	s_children;
+	void 			* s_element;
+	struct dentry		* s_dentry;
+	int			s_type;
+	struct rw_semaphore	s_rwsem;
+};
+
+/* Types of kobject elements represented by sysfs_dirent */
+#define SYSFS_ROOT		0x0001
+#define	SYSFS_KOBJECT		0x0002
+#define	SYSFS_KOBJ_ATTR		0x0004
+#define	SYSFS_KOBJ_BIN_ATTR	0x0008
+#define	SYSFS_KOBJ_ATTR_GROUP	0x0010
+#define	SYSFS_KOBJ_LINK		0x0020
+
 int sysfs_create_group(struct kobject *, const struct attribute_group *);
 void sysfs_remove_group(struct kobject *, const struct attribute_group *);
 
diff -puN fs/sysfs/mount.c~sysfs-backing-store fs/sysfs/mount.c
--- linux-2.6.0-test9/fs/sysfs/mount.c~sysfs-backing-store	2003-11-12 16:02:27.000000000 +0530
+++ linux-2.6.0-test9-maneesh/fs/sysfs/mount.c	2003-11-12 16:02:27.000000000 +0530
@@ -14,7 +14,6 @@
 /* Random magic number */
 #define SYSFS_MAGIC 0x62656572
 
-struct vfsmount *sysfs_mount;
 struct super_block * sysfs_sb = NULL;
 
 static struct super_operations sysfs_ops = {
@@ -22,6 +21,15 @@ static struct super_operations sysfs_ops
 	.drop_inode	= generic_delete_inode,
 };
 
+struct sysfs_dirent sysfs_root = {
+	.s_sibling	= LIST_HEAD_INIT(sysfs_root.s_sibling),
+	.s_children	= LIST_HEAD_INIT(sysfs_root.s_children),
+	.s_element	= NULL,
+	.s_dentry	= NULL,
+	.s_type		= SYSFS_ROOT,
+	.s_rwsem	= __RWSEM_INITIALIZER(sysfs_root.s_rwsem),
+};
+
 static int sysfs_fill_super(struct super_block *sb, void *data, int silent)
 {
 	struct inode *inode;
@@ -35,10 +43,9 @@ static int sysfs_fill_super(struct super
 
 	inode = sysfs_new_inode(S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO);
 	if (inode) {
-		inode->i_op = &simple_dir_inode_operations;
-		inode->i_fop = &simple_dir_operations;
-		/* directory inodes start off with i_nlink == 2 (for "." entry) */
-		inode->i_nlink++;	
+		inode->i_op = &sysfs_dir_inode_operations;
+		inode->i_fop = &sysfs_dir_operations;
+		inode->i_nlink += sysfs_get_link_count(&sysfs_root);
 	} else {
 		pr_debug("sysfs: could not get root inode\n");
 		return -ENOMEM;
@@ -50,7 +57,10 @@ static int sysfs_fill_super(struct super
 		iput(inode);
 		return -ENOMEM;
 	}
+	root->d_fsdata = &sysfs_root;
+	sysfs_root.s_dentry = root;
 	sb->s_root = root;
+
 	return 0;
 }
 
@@ -60,24 +70,19 @@ static struct super_block *sysfs_get_sb(
 	return get_sb_single(fs_type, flags, data, sysfs_fill_super);
 }
 
+static void sysfs_kill_super(struct super_block * sb)
+{
+	sysfs_root.s_dentry = NULL;
+	kill_anon_super(sb);
+}
+
 static struct file_system_type sysfs_fs_type = {
 	.name		= "sysfs",
 	.get_sb		= sysfs_get_sb,
-	.kill_sb	= kill_litter_super,
+	.kill_sb	= sysfs_kill_super,
 };
 
 int __init sysfs_init(void)
 {
-	int err;
-
-	err = register_filesystem(&sysfs_fs_type);
-	if (!err) {
-		sysfs_mount = kern_mount(&sysfs_fs_type);
-		if (IS_ERR(sysfs_mount)) {
-			printk(KERN_ERR "sysfs: could not mount!\n");
-			err = PTR_ERR(sysfs_mount);
-			sysfs_mount = NULL;
-		}
-	}
-	return err;
+	return register_filesystem(&sysfs_fs_type);
 }
diff -puN fs/sysfs/sysfs.h~sysfs-backing-store fs/sysfs/sysfs.h
--- linux-2.6.0-test9/fs/sysfs/sysfs.h~sysfs-backing-store	2003-11-12 16:02:27.000000000 +0530
+++ linux-2.6.0-test9-maneesh/fs/sysfs/sysfs.h	2003-11-12 16:02:27.000000000 +0530
@@ -1,13 +1,108 @@
 
-extern struct vfsmount * sysfs_mount;
+#include <linux/fs.h>
+
+extern struct sysfs_dirent sysfs_root;
 
 extern struct inode * sysfs_new_inode(mode_t mode);
 extern int sysfs_create(struct dentry *, int mode, int (*init)(struct inode *));
 
 extern struct dentry * sysfs_get_dentry(struct dentry *, const char *);
 
-extern int sysfs_add_file(struct dentry * dir, const struct attribute * attr);
-extern void sysfs_hash_and_remove(struct dentry * dir, const char * name);
+extern int sysfs_add_file(struct sysfs_dirent * , const struct attribute * );
+extern void sysfs_hash_and_remove(struct sysfs_dirent *, const char *);
 
-extern int sysfs_create_subdir(struct kobject *, const char *, struct dentry **);
 extern void sysfs_remove_subdir(struct dentry *);
+extern char * sysfs_get_name(struct sysfs_dirent *);
+extern int sysfs_get_link_count(struct sysfs_dirent *);
+extern struct dentry * sysfs_get_new_dentry(struct dentry *, const char *);
+extern int sysfs_init_file(struct inode * inode);
+extern int sysfs_symlink(struct inode *, struct dentry *, const char *);
+
+extern struct inode_operations sysfs_dir_inode_operations;
+extern struct file_operations sysfs_dir_operations;
+extern struct file_operations bin_fops;
+
+struct dentry * sysfs_lookup(struct inode *, struct dentry *, struct nameidata *);
+int sysfs_dir_open(struct inode *inode, struct file *file);
+int sysfs_dir_close(struct inode *inode, struct file *file);
+loff_t sysfs_dir_lseek(struct file *file, loff_t offset, int origin);
+int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir);
+
+static inline struct kobject * sysfs_get_file_kobject(struct dentry * dentry)
+{
+	struct dentry * parent;
+	struct sysfs_dirent * sd;
+	
+	parent = dentry->d_parent;
+	sd = parent->d_fsdata;
+	if (sd->s_type == SYSFS_KOBJ_ATTR_GROUP)
+		sd = parent->d_parent->d_fsdata;
+
+	return (struct kobject *) sd->s_element;
+		
+}
+
+static inline struct attribute * sysfs_get_file_attr(struct file * filp)
+{
+	struct sysfs_dirent * sd = filp->f_dentry->d_fsdata;
+	return sd->s_element;
+		
+}
+
+static inline struct sysfs_dirent * 
+sysfs_alloc_dirent(struct sysfs_dirent * parent_sd, void * element, int type)
+{
+	struct sysfs_dirent * sd;
+					
+	sd = kmalloc(sizeof(*sd), GFP_KERNEL);
+	if (!sd)
+		return ERR_PTR(-ENOMEM);
+
+	init_rwsem(&sd->s_rwsem);
+	INIT_LIST_HEAD(&sd->s_children);
+	sd->s_element = element;
+	sd->s_type = type;
+	sd->s_dentry = NULL;
+
+	down_write(&parent_sd->s_rwsem);
+	list_add(&sd->s_sibling, &parent_sd->s_children);
+	up_write(&parent_sd->s_rwsem);
+
+	return sd;
+}
+
+static inline void
+sysfs_free_dirent(struct sysfs_dirent * parent_sd, const char * name)
+{
+	struct sysfs_dirent * sd;
+	struct list_head * tmp;
+	
+	down_write(&parent_sd->s_rwsem);
+	tmp = parent_sd->s_children.next;
+	while (tmp != &parent_sd->s_children) {
+		sd = list_entry(tmp, struct sysfs_dirent, s_sibling);
+		tmp = tmp->next;
+		if (!strcmp(name, sysfs_get_name(sd))) {
+			list_del(&sd->s_sibling);
+			kfree(sd);
+		}
+	}
+	up_write(&parent_sd->s_rwsem);
+}
+
+static inline void
+sysfs_free_children(struct sysfs_dirent * parent_sd)
+{
+	struct sysfs_dirent * sd;
+	struct list_head * tmp;
+	
+	down_write(&parent_sd->s_rwsem);
+	tmp = parent_sd->s_children.next;
+	while (tmp != &parent_sd->s_children) {
+		sd = list_entry(tmp, struct sysfs_dirent, s_sibling);
+		tmp = tmp->next;
+		list_del(&sd->s_sibling);
+		kfree(sd);
+	}
+	up_write(&parent_sd->s_rwsem);
+}

_
-- 
Maneesh Soni
Linux Technology Center, 
IBM Software Lab, Bangalore, India
email: maneesh@in.ibm.com
Phone: 91-80-5044999 Fax: 91-80-5268553
T/L : 9243696

Re: [RFC 2/5] sysfs-dir.patch

[Maneesh Soni]

o This is the main part of the sysfs backing store patch set. It provides the
  lookup routine, and open, release, readdir, lseek routines for sysfs 
  directories. As we don't create sysfs entires in sysfs_create_xxxx() calls, 
  we create the dentries  when we actually look for them in sysfs_lookup and 
  We parse the kobject hierachy for the required object and if found and not
  already created we go ahead and create the sysfs entry for it.


 fs/sysfs/dir.c |  396 +++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 341 insertions(+), 55 deletions(-)

diff -puN fs/sysfs/dir.c~sysfs-dir fs/sysfs/dir.c
--- linux-2.6.0-test9/fs/sysfs/dir.c~sysfs-dir	2003-11-12 16:20:20.000000000 +0530
+++ linux-2.6.0-test9-maneesh/fs/sysfs/dir.c	2003-11-12 16:20:20.000000000 +0530
@@ -10,84 +10,205 @@
 #include <linux/kobject.h>
 #include "sysfs.h"
 
+struct inode_operations sysfs_dir_inode_operations = {
+	.lookup		= sysfs_lookup,
+};
+
+struct file_operations sysfs_dir_operations = {
+	.open		= sysfs_dir_open,
+	.release	= sysfs_dir_close,
+	.llseek		= sysfs_dir_lseek,
+	.read		= generic_read_dir,
+	.readdir	= sysfs_readdir,
+};
+
+static void sysfs_d_iput(struct dentry *dentry, struct inode *inode)
+{
+	struct sysfs_dirent * sd = dentry->d_fsdata;
+
+	sd->s_dentry = NULL;
+	iput(inode);
+}
+
+static struct dentry_operations sysfs_dops = {
+	.d_iput		= sysfs_d_iput,
+};
+
+char * sysfs_get_name(struct sysfs_dirent *sd)
+{
+	struct kobject * k;
+	struct attribute * a;
+	struct bin_attribute * bin_attr;
+	struct attribute_group * grp;
+        char ** link_names;
+                                                                                
+	if (!sd || !sd->s_element) {
+		return NULL;
+	}
+
+	switch(sd->s_type) {
+		case SYSFS_KOBJECT:	
+				k = (struct kobject *) sd->s_element;
+				return kobject_name(k);
+
+		case SYSFS_KOBJ_ATTR:
+				a = (struct attribute *) sd->s_element;
+				return a->name;
+
+		case SYSFS_KOBJ_ATTR_GROUP:
+				grp = (struct attribute_group *) sd->s_element;
+				return grp->name;
+
+                case SYSFS_KOBJ_BIN_ATTR:
+                                bin_attr = (struct bin_attribute *) sd->s_element;
+                                return bin_attr->attr.name;
+		case SYSFS_KOBJ_LINK:
+                                link_names = sd->s_element;
+                                return link_names[0];
+		default:
+				return "/";
+
+	}
+}
+
 static int init_dir(struct inode * inode)
 {
-	inode->i_op = &simple_dir_inode_operations;
-	inode->i_fop = &simple_dir_operations;
+	inode->i_op = &sysfs_dir_inode_operations;
+	inode->i_fop = &sysfs_dir_operations;
 
-	/* directory inodes start off with i_nlink == 2 (for "." entry) */
-	inode->i_nlink++;
 	return 0;
 }
 
-
-static int create_dir(struct kobject * k, struct dentry * p,
-		      const char * n, struct dentry ** d)
+static int create_dir(struct sysfs_dirent * sd, struct dentry * dentry)
 {
 	int error;
+       
+	error = sysfs_create(dentry, S_IFDIR| S_IRWXU | S_IRUGO | S_IXUGO,
+				 init_dir);
+	if (!error) {
+		dentry->d_inode->i_nlink += sysfs_get_link_count(sd);
+		return 0;
+	}
+	dput(dentry);
 
-	down(&p->d_inode->i_sem);
-	*d = sysfs_get_dentry(p,n);
-	if (!IS_ERR(*d)) {
-		error = sysfs_create(*d,
-					 S_IFDIR| S_IRWXU | S_IRUGO | S_IXUGO,
-					 init_dir);
-		if (!error) {
-			(*d)->d_fsdata = k;
-			p->d_inode->i_nlink++;
-		}
-		dput(*d);
-	} else
-		error = PTR_ERR(*d);
-	up(&p->d_inode->i_sem);
 	return error;
 }
 
+static  int create_attr_file(struct sysfs_dirent * sd, struct dentry * dentry)
+{
+	const struct attribute * attr = NULL;
+	struct bin_attribute * bin_attr = NULL;
+	int (* init) (struct inode *) = NULL;
+	int err = 0;
+
+	if (sd->s_type == SYSFS_KOBJ_ATTR) {
+		attr = sd->s_element;
+		init = sysfs_init_file;
+	} else {
+		bin_attr = sd->s_element;
+		attr = &bin_attr->attr;
+	}
+	
+	err = sysfs_create(dentry, (attr->mode & S_IALLUGO) | S_IFREG, init);
+	if (err)
+		return err;
+
+	if (sd->s_type == SYSFS_KOBJ_BIN_ATTR) {
+		dentry->d_inode->i_size = bin_attr->size;
+		dentry->d_inode->i_fop = &bin_fops;
+	}
 
-int sysfs_create_subdir(struct kobject * k, const char * n, struct dentry ** d)
+	return 0;
+}
+
+static int sysfs_create_dirent(struct sysfs_dirent * sd, struct dentry * dentry)
 {
-	return create_dir(k,k->dentry,n,d);
+	int err = 0;
+        char ** link_names;
+
+	switch(sd->s_type) {
+		case SYSFS_KOBJ_ATTR:
+		case SYSFS_KOBJ_BIN_ATTR:
+			err = create_attr_file(sd, dentry);
+			break;
+                case SYSFS_KOBJ_LINK:
+                        link_names = sd->s_element;
+                        err = sysfs_symlink(dentry->d_parent->d_inode, dentry,
+                                       link_names[1]);
+                        break;
+		default:
+			err = create_dir(sd, dentry);
+	}
+	if (!err) {
+		dentry->d_fsdata = sd;
+		dentry->d_op = &sysfs_dops;
+		sd->s_dentry = dentry;
+	}
+
+	return err;
+}
+
+struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
+{
+	struct sysfs_dirent * parent_sd = dentry->d_parent->d_fsdata;
+	struct sysfs_dirent * sd;
+	char * name;
+	int err = -ENOENT;
+
+	down_read(&parent_sd->s_rwsem);
+	list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
+		if (!sd->s_element)
+			continue;
+		name = sysfs_get_name(sd);
+		if (strcmp(name, dentry->d_name.name))
+			continue;
+		err = sysfs_create_dirent(sd, dentry);
+		break;
+	}
+	up_read(&parent_sd->s_rwsem);
+
+	if (err == -ENOENT) {
+		d_add(dentry, NULL);
+		err = 0;
+	}
+
+	return ERR_PTR(err);
 }
 
 /**
  *	sysfs_create_dir - create a directory for an object.
- *	@parent:	parent parent object.
  *	@kobj:		object we're creating directory for. 
  */
-
 int sysfs_create_dir(struct kobject * kobj)
 {
-	struct dentry * dentry = NULL;
-	struct dentry * parent;
-	int error = 0;
+	struct sysfs_dirent * sd; 
+	struct sysfs_dirent * parent_sd;
 
 	if (!kobj)
 		return -EINVAL;
 
-	if (kobj->parent)
-		parent = kobj->parent->dentry;
-	else if (sysfs_mount && sysfs_mount->mnt_sb)
-		parent = sysfs_mount->mnt_sb->s_root;
+	if (kobj->parent) 
+		parent_sd = kobj->parent->s_dirent;
 	else
-		return -EFAULT;
+		parent_sd = &sysfs_root;
 
-	error = create_dir(kobj,parent,kobject_name(kobj),&dentry);
-	if (!error)
-		kobj->dentry = dentry;
-	return error;
-}
+	sd = sysfs_alloc_dirent(parent_sd, kobj, SYSFS_KOBJECT);
+	if (IS_ERR(sd))
+		return PTR_ERR(sd);
+	kobj->s_dirent = sd;
 
+	return 0;
+}
 
 static void remove_dir(struct dentry * d)
 {
 	struct dentry * parent = dget(d->d_parent);
+
 	down(&parent->d_inode->i_sem);
-	d_delete(d);
+	d_drop(d);
 	simple_rmdir(parent->d_inode,d);
-
 	pr_debug(" o %s removing done (%d)\n",d->d_name.name,
 		 atomic_read(&d->d_count));
-
 	up(&parent->d_inode->i_sem);
 	dput(parent);
 }
@@ -110,10 +231,15 @@ void sysfs_remove_subdir(struct dentry *
 void sysfs_remove_dir(struct kobject * kobj)
 {
 	struct list_head * node;
-	struct dentry * dentry = dget(kobj->dentry);
+	struct dentry * dentry = kobj->s_dirent->s_dentry;
+	struct sysfs_dirent * parent_sd;
 
 	if (!dentry)
-		return;
+		goto exit;
+		
+	spin_lock(&dcache_lock);
+	dentry = dget_locked(dentry);
+	spin_unlock(&dcache_lock);
 
 	pr_debug("sysfs %s: removing dir\n",dentry->d_name.name);
 	down(&dentry->d_inode->i_sem);
@@ -124,7 +250,7 @@ void sysfs_remove_dir(struct kobject * k
 		struct dentry * d = list_entry(node,struct dentry,d_child);
 		list_del_init(node);
 
-		pr_debug(" o %s (%d): ",d->d_name.name,atomic_read(&d->d_count));
+		pr_debug(" o %s (%d):",d->d_name.name,atomic_read(&d->d_count));
 		if (d->d_inode) {
 			d = dget_locked(d);
 			pr_debug("removing");
@@ -132,10 +258,9 @@ void sysfs_remove_dir(struct kobject * k
 			/**
 			 * Unlink and unhash.
 			 */
+			__d_drop(d);
 			spin_unlock(&dcache_lock);
-			d_delete(d);
 			simple_unlink(dentry->d_inode,d);
-			dput(d);
 			spin_lock(&dcache_lock);
 		}
 		pr_debug(" done\n");
@@ -146,10 +271,15 @@ void sysfs_remove_dir(struct kobject * k
 	up(&dentry->d_inode->i_sem);
 
 	remove_dir(dentry);
-	/**
-	 * Drop reference from dget() on entrance.
-	 */
 	dput(dentry);
+
+exit:
+	sysfs_free_children(kobj->s_dirent);
+	if (kobj->parent)
+		parent_sd = kobj->parent->s_dirent;
+	else
+		parent_sd = &sysfs_root;
+	sysfs_free_dirent(parent_sd, kobject_name(kobj));
 }
 
 void sysfs_rename_dir(struct kobject * kobj, const char *new_name)
@@ -162,14 +292,170 @@ void sysfs_rename_dir(struct kobject * k
 	if (!kobj->parent)
 		return;
 
-	parent = kobj->parent->dentry;
+	parent = kobj->parent->s_dirent->s_dentry;
+	if (parent) {
+		spin_lock(&dcache_lock);
+		dget_locked(parent);
+		spin_unlock(&dcache_lock);
+
+		down(&parent->d_inode->i_sem);
+		new_dentry = sysfs_get_dentry(parent, new_name);
+		if (!IS_ERR(new_dentry))
+			d_move(kobj->s_dirent->s_dentry, new_dentry);
+		up(&parent->d_inode->i_sem);	
+		dput(parent);
+	}
+	kobject_set_name(kobj,new_name);
+}
 
-	down(&parent->d_inode->i_sem);
+int sysfs_dir_close(struct inode *inode, struct file *filp)
+{
+	struct dentry * dentry = filp->f_dentry;
+	struct sysfs_dirent * sd = dentry->d_fsdata;
+	struct sysfs_dirent * cursor = filp->private_data;
+	
+	down_write(&sd->s_rwsem);
+	list_del(&cursor->s_sibling);
+	kfree(cursor);
+	up_write(&sd->s_rwsem);
+	
+	return 0;
+}
 
-	new_dentry = sysfs_get_dentry(parent, new_name);
-	d_move(kobj->dentry, new_dentry);
-	kobject_set_name(kobj,new_name);
-	up(&parent->d_inode->i_sem);	
+int sysfs_dir_open(struct inode *inode, struct file *filp)
+{
+	struct dentry * parent = filp->f_dentry;
+	struct sysfs_dirent * parent_sd = parent->d_fsdata;
+	struct sysfs_dirent * cursor;
+
+	down(&inode->i_sem);
+	cursor = sysfs_alloc_dirent(parent_sd, NULL, 0);
+	up(&inode->i_sem);
+	if (IS_ERR(cursor))
+		return PTR_ERR(cursor);
+	filp->private_data = cursor;
+	return 0;
+}
+
+loff_t sysfs_dir_lseek(struct file *file, loff_t offset, int origin)
+{
+	struct dentry * dentry = file->f_dentry;
+
+	down(&dentry->d_inode->i_sem);
+	switch (origin) {
+		case 1:
+			offset += file->f_pos;
+		case 0:
+			if (offset >= 0)
+				break;
+		default:
+			up(&dentry->d_inode->i_sem);
+			return -EINVAL;
+	}
+	if (offset != file->f_pos) {
+		file->f_pos = offset;
+		if (file->f_pos >= 2) {
+			struct list_head *p;
+			struct sysfs_dirent * parent_sd = dentry->d_fsdata;
+			struct sysfs_dirent * cursor = file->private_data;
+			loff_t n = file->f_pos - 2;
+
+			down_write(&parent_sd->s_rwsem);
+			p = parent_sd->s_children.next;
+			while (n && p != &parent_sd->s_children) {
+				struct sysfs_dirent * next_sd;
+				struct dentry *next;
+				char * name;
+				next_sd = list_entry(p, struct sysfs_dirent, s_sibling);
+				if (!next_sd->s_element)
+					continue;
+				name = sysfs_get_name(next_sd);
+				up_write(&parent_sd->s_rwsem);
+				next = sysfs_get_dentry(dentry, name);
+				down_write(&parent_sd->s_rwsem);
+				if (IS_ERR(next))
+					break;
+				if (!d_unhashed(next) && next->d_inode)
+					n--;
+				p = p->next;
+				dput(next);
+			}
+			list_del(&cursor->s_sibling);
+			list_add_tail(&cursor->s_sibling, p);
+			up_write(&parent_sd->s_rwsem);
+		}
+	}
+	up(&dentry->d_inode->i_sem);
+	return offset;
+	return 0;
+}
+
+/* Relationship between i_mode and the DT_xxx types */
+static inline unsigned char dt_type(struct inode *inode)
+{
+	return (inode->i_mode >> 12) & 15;
+}
+
+int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir)
+{
+	struct dentry *dentry = filp->f_dentry;
+	struct sysfs_dirent *sd = dentry->d_fsdata;
+	struct sysfs_dirent *cursor = filp->private_data;
+	struct list_head *p, *q = &cursor->s_sibling;
+	ino_t ino;
+	int i = filp->f_pos;
+	int err = 0;
+
+	switch (i) {
+		case 0:
+			ino = dentry->d_inode->i_ino;
+			if (filldir(dirent, ".", 1, i, ino, DT_DIR) < 0)
+				break;
+			filp->f_pos++;
+			i++;
+			/* fallthrough */
+		case 1:
+			ino = parent_ino(dentry);
+			if (filldir(dirent, "..", 2, i, ino, DT_DIR) < 0)
+				break;
+			filp->f_pos++;
+			i++;
+			/* fallthrough */
+		default:
+			down_write(&sd->s_rwsem);
+			if (filp->f_pos == 2) {
+				list_del(q);
+				list_add(q, &sd->s_children);
+			}
+			for (p = q->next; p != &sd->s_children; p = p->next) {
+				struct sysfs_dirent *next_sd;
+				struct dentry * next;
+				char * name;
+
+				next_sd = list_entry(p, struct sysfs_dirent, 
+						   s_sibling);
+				if (!next_sd->s_element)
+					continue;
+				name = sysfs_get_name(next_sd);
+				up_write(&sd->s_rwsem);
+				next = sysfs_get_dentry(dentry, name);
+				down_write(&sd->s_rwsem);
+				if (IS_ERR(next))
+					break;
+				if (!next || !next->d_inode)
+					continue;
+				err = filldir(dirent, next->d_name.name, next->d_name.len, filp->f_pos, next->d_inode->i_ino, dt_type(next->d_inode));
+				dput(next);
+				if (err < 0) 
+					break;
+				list_del(q);
+				list_add(q, p);
+				p = q;
+				filp->f_pos++;
+			}
+			up_write(&sd->s_rwsem);
+	}
+	return 0;
 }
 
 EXPORT_SYMBOL(sysfs_create_dir);

_
-- 
Maneesh Soni
Linux Technology Center, 
IBM Software Lab, Bangalore, India
email: maneesh@in.ibm.com
Phone: 91-80-5044999 Fax: 91-80-5268553
T/L : 9243696

Re: [RFC 3/5] sysfs-file.patch

[Maneesh Soni]

o  This patch modifies the sysfs_create_file and sysfs_create_bin_file keeping 
   the same signature. Now these don't actually create files but allocates 
   a sysfs_dirent representing the attribute and links it to the kobject's
   sysfs_dirent

o  Now dentry->d_fsdata does not point to the attribute structure but points
   to the sysfs_dirent representing the attribute. The attribute pointer
   is saved in the sysfs_dirent's s_element field.

o  At the time of actually creating the dentry for sysfs entries we don't do
   the dreaded "Extra dget"


 fs/sysfs/bin.c   |   47 +++++++++++++++------------------------
 fs/sysfs/file.c  |   66 +++++++++++++++++++++++++------------------------------
 fs/sysfs/inode.c |   41 ++++++++++++++++++++++++----------
 3 files changed, 78 insertions(+), 76 deletions(-)

diff -puN fs/sysfs/file.c~sysfs-file fs/sysfs/file.c
--- linux-2.6.0-test9/fs/sysfs/file.c~sysfs-file	2003-11-12 16:24:20.000000000 +0530
+++ linux-2.6.0-test9-maneesh/fs/sysfs/file.c	2003-11-12 16:24:20.000000000 +0530
@@ -11,7 +11,7 @@
 
 static struct file_operations sysfs_file_operations;
 
-static int init_file(struct inode * inode)
+int sysfs_init_file(struct inode * inode)
 {
 	inode->i_size = PAGE_SIZE;
 	inode->i_fop = &sysfs_file_operations;
@@ -77,12 +77,13 @@ struct sysfs_buffer {
  */
 static int fill_read_buffer(struct file * file, struct sysfs_buffer * buffer)
 {
-	struct attribute * attr = file->f_dentry->d_fsdata;
-	struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
+	struct attribute * attr = sysfs_get_file_attr(file);
+	struct kobject * kobj;
 	struct sysfs_ops * ops = buffer->ops;
 	int ret = 0;
 	ssize_t count;
 
+	kobj = sysfs_get_file_kobject(file->f_dentry);
 	if (!buffer->page)
 		buffer->page = (char *) get_zeroed_page(GFP_KERNEL);
 	if (!buffer->page)
@@ -131,8 +132,8 @@ static int flush_read_buffer(struct sysf
  *	@ppos:	starting offset in file.
  *
  *	Userspace wants to read an attribute file. The attribute descriptor
- *	is in the file's ->d_fsdata. The target object is in the directory's
- *	->d_fsdata.
+ *	is in the file's sysfs_dirent which is present in ->d_fsdata. The 
+ *      target object is in the directory's sysfs_dirent.
  *
  *	We call fill_read_buffer() to allocate and fill the buffer from the
  *	object's show() method exactly once (if the read is happening from
@@ -198,8 +199,8 @@ fill_write_buffer(struct sysfs_buffer * 
 static int 
 flush_write_buffer(struct file * file, struct sysfs_buffer * buffer, size_t count)
 {
-	struct attribute * attr = file->f_dentry->d_fsdata;
-	struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
+	struct attribute * attr = sysfs_get_file_attr(file);
+	struct kobject * kobj = sysfs_get_file_kobject(file->f_dentry);
 	struct sysfs_ops * ops = buffer->ops;
 
 	return ops->store(kobj,attr,buffer->page,count);
@@ -238,8 +239,8 @@ sysfs_write_file(struct file *file, cons
 
 static int check_perm(struct inode * inode, struct file * file)
 {
-	struct kobject * kobj = kobject_get(file->f_dentry->d_parent->d_fsdata);
-	struct attribute * attr = file->f_dentry->d_fsdata;
+	struct kobject * kobj = kobject_get(sysfs_get_file_kobject(file->f_dentry));
+	struct attribute * attr = sysfs_get_file_attr(file);
 	struct sysfs_buffer * buffer;
 	struct sysfs_ops * ops = NULL;
 	int error = 0;
@@ -320,8 +321,8 @@ static int sysfs_open_file(struct inode 
 
 static int sysfs_release(struct inode * inode, struct file * filp)
 {
-	struct kobject * kobj = filp->f_dentry->d_parent->d_fsdata;
-	struct attribute * attr = filp->f_dentry->d_fsdata;
+	struct kobject * kobj = sysfs_get_file_kobject(filp->f_dentry);
+	struct attribute * attr = sysfs_get_file_attr(filp);
 	struct sysfs_buffer * buffer = filp->private_data;
 
 	if (kobj) 
@@ -345,24 +346,16 @@ static struct file_operations sysfs_file
 };
 
 
-int sysfs_add_file(struct dentry * dir, const struct attribute * attr)
+int sysfs_add_file(struct sysfs_dirent * parent_sd, 
+			const struct attribute * attr)
 {
-	struct dentry * dentry;
-	int error;
+	struct sysfs_dirent * sd;
 
-	down(&dir->d_inode->i_sem);
-	dentry = sysfs_get_dentry(dir,attr->name);
-	if (!IS_ERR(dentry)) {
-		error = sysfs_create(dentry,
-				     (attr->mode & S_IALLUGO) | S_IFREG,
-				     init_file);
-		if (!error)
-			dentry->d_fsdata = (void *)attr;
-		dput(dentry);
-	} else
-		error = PTR_ERR(dentry);
-	up(&dir->d_inode->i_sem);
-	return error;
+	sd = sysfs_alloc_dirent(parent_sd, (void *) attr, SYSFS_KOBJ_ATTR);
+	if (IS_ERR(sd))
+		return PTR_ERR(sd);
+
+	return 0;
 }
 
 
@@ -375,7 +368,7 @@ int sysfs_add_file(struct dentry * dir, 
 int sysfs_create_file(struct kobject * kobj, const struct attribute * attr)
 {
 	if (kobj && attr)
-		return sysfs_add_file(kobj->dentry,attr);
+		return sysfs_add_file(kobj->s_dirent, attr);
 	return -EINVAL;
 }
 
@@ -390,10 +383,17 @@ int sysfs_create_file(struct kobject * k
  */
 int sysfs_update_file(struct kobject * kobj, const struct attribute * attr)
 {
-	struct dentry * dir = kobj->dentry;
+	struct dentry * dir = kobj->s_dirent->s_dentry;
 	struct dentry * victim;
 	int res = -ENOENT;
 
+	if (!dir)
+		return res;
+
+	spin_lock(&dcache_lock);
+	dget_locked(dir);
+	spin_unlock(&dcache_lock);
+
 	down(&dir->d_inode->i_sem);
 	victim = sysfs_get_dentry(dir, attr->name);
 	if (!IS_ERR(victim)) {
@@ -402,11 +402,6 @@ int sysfs_update_file(struct kobject * k
 		    (victim->d_parent->d_inode == dir->d_inode)) {
 			victim->d_inode->i_mtime = CURRENT_TIME;
 			dnotify_parent(victim, DN_MODIFY);
-
-			/**
-			 * Drop reference from initial sysfs_get_dentry().
-			 */
-			dput(victim);
 			res = 0;
 		}
 		
@@ -416,6 +411,7 @@ int sysfs_update_file(struct kobject * k
 		dput(victim);
 	}
 	up(&dir->d_inode->i_sem);
+	dput(dir);
 
 	return res;
 }
@@ -431,7 +427,7 @@ int sysfs_update_file(struct kobject * k
 
 void sysfs_remove_file(struct kobject * kobj, const struct attribute * attr)
 {
-	sysfs_hash_and_remove(kobj->dentry,attr->name);
+	sysfs_hash_and_remove(kobj->s_dirent, attr->name);
 }
 
 
diff -puN fs/sysfs/bin.c~sysfs-file fs/sysfs/bin.c
--- linux-2.6.0-test9/fs/sysfs/bin.c~sysfs-file	2003-11-12 16:24:20.000000000 +0530
+++ linux-2.6.0-test9-maneesh/fs/sysfs/bin.c	2003-11-12 16:24:20.000000000 +0530
@@ -17,8 +17,9 @@
 static int
 fill_read(struct dentry *dentry, char *buffer, loff_t off, size_t count)
 {
-	struct bin_attribute * attr = dentry->d_fsdata;
-	struct kobject * kobj = dentry->d_parent->d_fsdata;
+	struct sysfs_dirent * sd = dentry->d_fsdata;
+	struct bin_attribute * attr = sd->s_element;
+	struct kobject * kobj = sysfs_get_file_kobject(dentry);
 
 	return attr->read(kobj, buffer, off, count);
 }
@@ -60,8 +61,9 @@ read(struct file * file, char __user * u
 static int
 flush_write(struct dentry *dentry, char *buffer, loff_t offset, size_t count)
 {
-	struct bin_attribute *attr = dentry->d_fsdata;
-	struct kobject *kobj = dentry->d_parent->d_fsdata;
+	struct sysfs_dirent * sd = dentry->d_fsdata;
+	struct bin_attribute * attr = sd->s_element;
+	struct kobject * kobj = sysfs_get_file_kobject(dentry);
 
 	return attr->write(kobj, buffer, offset, count);
 }
@@ -94,8 +96,9 @@ static ssize_t write(struct file * file,
 
 static int open(struct inode * inode, struct file * file)
 {
-	struct kobject * kobj = kobject_get(file->f_dentry->d_parent->d_fsdata);
-	struct bin_attribute * attr = file->f_dentry->d_fsdata;
+	struct kobject * kobj = kobject_get(sysfs_get_file_kobject(file->f_dentry));
+	struct sysfs_dirent * sd = file->f_dentry->d_fsdata;
+	struct bin_attribute * attr = sd->s_element;
 	int error = -EINVAL;
 
 	if (!kobj || !attr)
@@ -122,7 +125,7 @@ static int open(struct inode * inode, st
 
 static int release(struct inode * inode, struct file * file)
 {
-	struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
+	struct kobject * kobj = sysfs_get_file_kobject(file->f_dentry);
 	u8 * buffer = file->private_data;
 
 	if (kobj) 
@@ -131,7 +134,7 @@ static int release(struct inode * inode,
 	return 0;
 }
 
-static struct file_operations bin_fops = {
+struct file_operations bin_fops = {
 	.read		= read,
 	.write		= write,
 	.llseek		= generic_file_llseek,
@@ -148,31 +151,17 @@ static struct file_operations bin_fops =
 
 int sysfs_create_bin_file(struct kobject * kobj, struct bin_attribute * attr)
 {
-	struct dentry * dentry;
-	struct dentry * parent;
-	int error = 0;
+	struct sysfs_dirent * sd;
 
 	if (!kobj || !attr)
 		return -EINVAL;
 
-	parent = kobj->dentry;
+	sd = sysfs_alloc_dirent(kobj->s_dirent, attr, SYSFS_KOBJ_BIN_ATTR);
+	if (IS_ERR(sd))
+		return PTR_ERR(sd);
+
+	return 0;
 
-	down(&parent->d_inode->i_sem);
-	dentry = sysfs_get_dentry(parent,attr->attr.name);
-	if (!IS_ERR(dentry)) {
-		dentry->d_fsdata = (void *)attr;
-		error = sysfs_create(dentry,
-				     (attr->attr.mode & S_IALLUGO) | S_IFREG,
-				     NULL);
-		if (!error) {
-			dentry->d_inode->i_size = attr->size;
-			dentry->d_inode->i_fop = &bin_fops;
-		}
-		dput(dentry);
-	} else
-		error = PTR_ERR(dentry);
-	up(&parent->d_inode->i_sem);
-	return error;
 }
 
 
@@ -185,7 +174,7 @@ int sysfs_create_bin_file(struct kobject
 
 int sysfs_remove_bin_file(struct kobject * kobj, struct bin_attribute * attr)
 {
-	sysfs_hash_and_remove(kobj->dentry,attr->attr.name);
+	sysfs_hash_and_remove(kobj->s_dirent, attr->attr.name);
 	return 0;
 }
 
diff -puN fs/sysfs/inode.c~sysfs-file fs/sysfs/inode.c
--- linux-2.6.0-test9/fs/sysfs/inode.c~sysfs-file	2003-11-12 16:24:20.000000000 +0530
+++ linux-2.6.0-test9-maneesh/fs/sysfs/inode.c	2003-11-12 16:24:20.000000000 +0530
@@ -11,6 +11,8 @@
 #include <linux/pagemap.h>
 #include <linux/namei.h>
 #include <linux/backing-dev.h>
+#include "sysfs.h"
+
 extern struct super_block * sysfs_sb;
 
 static struct address_space_operations sysfs_aops = {
@@ -59,10 +61,9 @@ int sysfs_create(struct dentry * dentry,
  Proceed:
 	if (init)
 		error = init(inode);
-	if (!error) {
-		d_instantiate(dentry, inode);
-		dget(dentry); /* Extra count - pin the dentry in core */
-	} else
+	if (!error)
+		d_add(dentry, inode);
+	else
 		iput(inode);
  Done:
 	return error;
@@ -83,9 +84,17 @@ struct dentry * sysfs_get_dentry(struct 
 	return lookup_hash(&qstr,parent);
 }
 
-void sysfs_hash_and_remove(struct dentry * dir, const char * name)
+void sysfs_hash_and_remove(struct sysfs_dirent * parent_sd, const char * name)
 {
 	struct dentry * victim;
+	struct dentry * dir = parent_sd->s_dentry;
+
+	if (!dir)
+		goto exit;
+
+	spin_lock(&dcache_lock);
+	dget_locked(dir);
+	spin_unlock(&dcache_lock);
 
 	down(&dir->d_inode->i_sem);
 	victim = sysfs_get_dentry(dir,name);
@@ -93,18 +102,26 @@ void sysfs_hash_and_remove(struct dentry
 		/* make sure dentry is really there */
 		if (victim->d_inode && 
 		    (victim->d_parent->d_inode == dir->d_inode)) {
-			pr_debug("sysfs: Removing %s (%d)\n", victim->d_name.name,
+			printk("sysfs:Removing %s (%d)\n", victim->d_name.name,
 				 atomic_read(&victim->d_count));
-
-			d_delete(victim);
+			d_drop(victim);
 			simple_unlink(dir->d_inode,victim);
 		}
-		/*
-		 * Drop reference from sysfs_get_dentry() above.
-		 */
-		dput(victim);
 	}
 	up(&dir->d_inode->i_sem);
+	dput(dir);
+exit:
+	sysfs_free_dirent(parent_sd, name);
+
 }
 
+int sysfs_get_link_count(struct sysfs_dirent * sd)
+{
+	int count = 1; 
+	struct sysfs_dirent *tmp_sd;
+
+	list_for_each_entry(tmp_sd, &sd->s_children, s_sibling)
+		count++;
 
+	return count;
+}

_
-- 
Maneesh Soni
Linux Technology Center, 
IBM Software Lab, Bangalore, India
email: maneesh@in.ibm.com
Phone: 91-80-5044999 Fax: 91-80-5268553
T/L : 9243696

Re: [RFC 4/5] sysfs-attr_group.patch

[Maneesh Soni]

o This patch creates attribute group for the given kobject. Again here we don;t
  create the sysfs directory but just allocated sysfs_dirent for the attribute
  group and links it to the kobject's hierarchy.


 fs/sysfs/group.c |   75 +++++++++++++++++++++++++++++++++----------------------
 1 files changed, 45 insertions(+), 30 deletions(-)

diff -puN fs/sysfs/group.c~sysfs-attr_group fs/sysfs/group.c
--- linux-2.6.0-test9/fs/sysfs/group.c~sysfs-attr_group	2003-11-12 16:30:55.000000000 +0530
+++ linux-2.6.0-test9-maneesh/fs/sysfs/group.c	2003-11-12 16:30:55.000000000 +0530
@@ -12,70 +12,85 @@
 #include <linux/module.h>
 #include <linux/dcache.h>
 #include <linux/err.h>
+#include <linux/namei.h>
 #include "sysfs.h"
 
 
-static void remove_files(struct dentry * dir, 
+static void remove_files(struct sysfs_dirent * sd, 
 			 const struct attribute_group * grp)
 {
 	struct attribute *const* attr;
 
 	for (attr = grp->attrs; *attr; attr++)
-		sysfs_hash_and_remove(dir,(*attr)->name);
+		sysfs_hash_and_remove(sd, (*attr)->name);
 }
 
-static int create_files(struct dentry * dir,
+static int create_files(struct sysfs_dirent * parent_sd,
 			const struct attribute_group * grp)
 {
 	struct attribute *const* attr;
 	int error = 0;
 
 	for (attr = grp->attrs; *attr && !error; attr++) {
-		error = sysfs_add_file(dir,*attr);
+		error = sysfs_add_file(parent_sd, *attr);
 	}
 	if (error)
-		remove_files(dir,grp);
+		remove_files(parent_sd, grp);
 	return error;
 }
 
-
 int sysfs_create_group(struct kobject * kobj, 
 		       const struct attribute_group * grp)
 {
-	struct dentry * dir;
-	int error;
+	struct sysfs_dirent * sd;
+	int error = 0;
 
-	if (grp->name) {
-		error = sysfs_create_subdir(kobj,grp->name,&dir);
-		if (error)
-			return error;
-	} else
-		dir = kobj->dentry;
-	dir = dget(dir);
-	if ((error = create_files(dir,grp))) {
-		if (grp->name)
-			sysfs_remove_subdir(dir);
-		dput(dir);
+	if (!kobj || !grp)
+		return -EINVAL;
+
+	sd = sysfs_alloc_dirent(kobj->s_dirent, (void *) grp, SYSFS_KOBJ_ATTR_GROUP);
+	if (IS_ERR(sd))
+		return PTR_ERR(sd);
+
+	if ((error = create_files(sd, grp))) {
+		sysfs_remove_group(kobj, grp);
 	}
+
 	return error;
 }
 
 void sysfs_remove_group(struct kobject * kobj, 
 			const struct attribute_group * grp)
 {
-	struct dentry * dir;
+	struct sysfs_dirent * tmp_sd;
+	struct sysfs_dirent * grp_sd = NULL;
+	struct dentry * grp_dentry;
 
-	if (grp->name)
-		dir = sysfs_get_dentry(kobj->dentry,grp->name);
-	else
-		dir = kobj->dentry;
-
-	remove_files(dir,grp);
-	dput(dir);
-	if (grp->name)
-		sysfs_remove_subdir(dir);
-}
+	if (grp->name) {
+		down_read(&kobj->s_dirent->s_rwsem);
+		list_for_each_entry(tmp_sd, &kobj->s_dirent->s_children, s_sibling) {
+			if (!strcmp(grp->name, sysfs_get_name(tmp_sd))) {
+				grp_sd = tmp_sd;
+				break;
+			}
+		}
+		up_read(&kobj->s_dirent->s_rwsem);
+	} else 
+		grp_sd = kobj->s_dirent;
+
+	if (grp_sd->s_dentry) {
+		spin_lock(&dcache_lock);
+		grp_dentry = dget_locked(grp_sd->s_dentry);
+		spin_unlock(&dcache_lock);
+		remove_files(grp_sd, grp);
+		if (grp->name) 
+			sysfs_remove_subdir(grp_dentry);
+		dput(grp_dentry);
+	} else
+		sysfs_free_children(grp_sd);
 
+	sysfs_free_dirent(kobj->s_dirent, grp->name);
+}
 
 EXPORT_SYMBOL(sysfs_create_group);
 EXPORT_SYMBOL(sysfs_remove_group);

_
-- 
Maneesh Soni
Linux Technology Center, 
IBM Software Lab, Bangalore, India
email: maneesh@in.ibm.com
Phone: 91-80-5044999 Fax: 91-80-5268553
T/L : 9243696

Re: [RFC 5/5] sysfs-symlink.patch

[Maneesh Soni]

o This patch creates symlink kobject. We don't create the symlink in sysfs
  when sysfs_create_symlink() is called but just allocates one sysfs_dirent 
  of type SYSFS_KOBJ_LINK. The s_element for symlinks is an array of two
  strings, one prepresenting the name of the link and another patch of 
  the target of the link.


 fs/sysfs/symlink.c |   36 ++++++++++++++++++++----------------
 1 files changed, 20 insertions(+), 16 deletions(-)

diff -puN fs/sysfs/symlink.c~sysfs-symlink fs/sysfs/symlink.c
--- linux-2.6.0-test9/fs/sysfs/symlink.c~sysfs-symlink	2003-11-12 15:26:34.000000000 +0530
+++ linux-2.6.0-test9-maneesh/fs/sysfs/symlink.c	2003-11-12 15:26:53.000000000 +0530
@@ -15,7 +15,7 @@ static int init_symlink(struct inode * i
 	return 0;
 }
 
-static int sysfs_symlink(struct inode * dir, struct dentry *dentry, const char * symname)
+int sysfs_symlink(struct inode * dir, struct dentry *dentry, const char * symname)
 {
 	int error;
 
@@ -71,13 +71,12 @@ static void fill_object_path(struct kobj
  */
 int sysfs_create_link(struct kobject * kobj, struct kobject * target, char * name)
 {
-	struct dentry * dentry = kobj->dentry;
-	struct dentry * d;
-	int error = 0;
 	int size;
 	int depth;
 	char * path;
 	char * s;
+        char ** link_names;
+        struct sysfs_dirent * link;
 
 	depth = object_depth(kobj);
 	size = object_path_length(target) + depth * 3 - 1;
@@ -96,18 +95,23 @@ int sysfs_create_link(struct kobject * k
 	fill_object_path(target,path,size);
 	pr_debug("%s: path = '%s'\n",__FUNCTION__,path);
 
-	down(&dentry->d_inode->i_sem);
-	d = sysfs_get_dentry(dentry,name);
-	if (!IS_ERR(d))
-		error = sysfs_symlink(dentry->d_inode,d,path);
-	else
-		error = PTR_ERR(d);
-	dput(d);
-	up(&dentry->d_inode->i_sem);
-	kfree(path);
-	return error;
-}
+        link_names = kmalloc(2 * (sizeof(char *)), GFP_KERNEL);
+        if (!link_names) {
+                kfree(path);
+                return -ENOMEM;
+        }
+        link_names[0] = name;
+        link_names[1] = path;
+                                                                                
+	link = sysfs_alloc_dirent(kobj->s_dirent, link_names, SYSFS_KOBJ_LINK);
+	if (IS_ERR(link)) {
+                kfree(path);
+                kfree(link_names);
+		return PTR_ERR(link);
+	}
 
+	return 0;
+}
 
 /**
  *	sysfs_remove_link - remove symlink in object's directory.
@@ -117,7 +121,7 @@ int sysfs_create_link(struct kobject * k
 
 void sysfs_remove_link(struct kobject * kobj, char * name)
 {
-	sysfs_hash_and_remove(kobj->dentry,name);
+	sysfs_hash_and_remove(kobj->s_dirent, name);
 }
 
 

_
-- 
Maneesh Soni
Linux Technology Center, 
IBM Software Lab, Bangalore, India
email: maneesh@in.ibm.com
Phone: 91-80-5044999 Fax: 91-80-5268553
T/L : 9243696

Re: [RFC 2/5] sysfs-dir.patch

[viro]

On Wed, Nov 12, 2003 at 05:55:29PM +0530, Maneesh Soni wrote:
> @@ -110,10 +231,15 @@ void sysfs_remove_subdir(struct dentry *
>  void sysfs_remove_dir(struct kobject * kobj)
>  {
>  	struct list_head * node;
> -	struct dentry * dentry = dget(kobj->dentry);
> +	struct dentry * dentry = kobj->s_dirent->s_dentry;
> +	struct sysfs_dirent * parent_sd;
>  
>  	if (!dentry)
> -		return;
> +		goto exit;
> +		
> +	spin_lock(&dcache_lock);
> +	dentry = dget_locked(dentry);
> +	spin_unlock(&dcache_lock);

Racy.  Directory might've been looked up just as you've decided that it
had no dentry.

>  void sysfs_rename_dir(struct kobject * kobj, const char *new_name)
> @@ -162,14 +292,170 @@ void sysfs_rename_dir(struct kobject * k
>  	if (!kobj->parent)
>  		return;
>  
> -	parent = kobj->parent->dentry;
> +	parent = kobj->parent->s_dirent->s_dentry;
> +	if (parent) {

Ditto.

Look, the *only* benefit of ramfs as a backing store for sysfs was that we
could easily get locking right.  You want second tree - you get to fight
for coherency.

Re: [RFC 0/5] Backing Store for sysfs (Overhauled)

[Greg KH]

On Wed, Nov 12, 2003 at 05:53:44PM +0530, Maneesh Soni wrote:
> 
> The concept is still the same that in this prototype also we create dentry and 
> inode on the fly when they are first looked up. This is done for both leaf or 
> non-leaf dentries. The generic nature of sysfs_dirent makes it easy to do for 
> both leaf or non-leaf dentries. 

What happens once a dentry and inode is created?  Is there any way for
them to be forced out, or do they stay around in memory forever?

thanks,

greg k-h

Re: [RFC 0/5] Backing Store for sysfs (Overhauled)

[Dipankar Sarma]

On Wed, Nov 12, 2003 at 08:00:15AM -0800, Greg KH wrote:
> On Wed, Nov 12, 2003 at 05:53:44PM +0530, Maneesh Soni wrote:
> > 
> > The concept is still the same that in this prototype also we create dentry and 
> > inode on the fly when they are first looked up. This is done for both leaf or 
> > non-leaf dentries. The generic nature of sysfs_dirent makes it easy to do for 
> > both leaf or non-leaf dentries. 
> 
> What happens once a dentry and inode is created?  Is there any way for
> them to be forced out, or do they stay around in memory forever?

The idea atleast, is that if no one is using a dentry, it will
be put in the dentry lru list and eventually returned to the slab.
inodes too are returned alongwith. Just like how on-disk filesystems work.

Typically, an open() of a sysfs file would result in creation of the
corresponding dentry/inode and holding of the reference. close() releases
the reference. The last one to release puts the dentry in the lru list
for later pruning. The result is that we have less memory use and
smaller number of dcache hash table elements under normal circumstances.

THanks
Dipankar

Re: [RFC 0/5] Backing Store for sysfs (Overhauled)

[Greg KH]

On Wed, Nov 12, 2003 at 09:57:40PM +0530, Dipankar Sarma wrote:
> On Wed, Nov 12, 2003 at 08:00:15AM -0800, Greg KH wrote:
> > On Wed, Nov 12, 2003 at 05:53:44PM +0530, Maneesh Soni wrote:
> > > 
> > > The concept is still the same that in this prototype also we create dentry and 
> > > inode on the fly when they are first looked up. This is done for both leaf or 
> > > non-leaf dentries. The generic nature of sysfs_dirent makes it easy to do for 
> > > both leaf or non-leaf dentries. 
> > 
> > What happens once a dentry and inode is created?  Is there any way for
> > them to be forced out, or do they stay around in memory forever?
> 
> The idea atleast, is that if no one is using a dentry, it will
> be put in the dentry lru list and eventually returned to the slab.
> inodes too are returned alongwith. Just like how on-disk filesystems work.

Do you all have any numbers backing this up?

thanks,

greg k-h

Re: [RFC 0/5] Backing Store for sysfs (Overhauled)

[Patrick Mochel]

> The main complaint was of over bloating kobject structure which becomes more
> painful when kobject is not part of sysfs. So now I have changed the data
> structures entirely. There is _no_ increase in the size of kobject structure.
> The kobject hierarchy is represented in the form of a new structure called
> sysfs_dirent (size 48 - bytes). sysfs_dirent will be there only for kobject
> elements (kobject, attribute, attribute group, symlink) which are represented 
> in sysfs. kobject structre has just one change. Now kobject has a field
> pointing to its sysfs_dirent instead of dentry.
> 
> struct sysfs_dirent {
>         struct list_head        s_sibling;
>         struct list_head        s_children;
>         void                    * s_element;
>         struct dentry           * s_dentry;
>         int                     s_type;
>         struct rw_semaphore     s_rwsem;
> };
> 
> The concept is still the same that in this prototype also we create dentry and 
> inode on the fly when they are first looked up. This is done for both leaf or 
> non-leaf dentries. The generic nature of sysfs_dirent makes it easy to do for 
> both leaf or non-leaf dentries. 

I still don't like it, though I think things are gradually getting better. 

I still think that keeping the directories static and only creating the 
leaf (file) dentries dynamically is the best tradeoff between complexity 
and memory consumption. 

It will require some minor infrastructural modification to associate a
kobject with all of its leaf nodes, but the result will be cleaner, and
the worst-case memory consumption will be less than your patches with a 
per-attribute-per-kobject data structure (which there currently isn't).

Thanks,


	Pat

Re: [RFC 0/5] Backing Store for sysfs (Overhauled)

[Dipankar Sarma]

On Thu, Nov 13, 2003 at 11:34:04AM -0800, Patrick Mochel wrote:
> 
> I still think that keeping the directories static and only creating the 
> leaf (file) dentries dynamically is the best tradeoff between complexity 
> and memory consumption. 

Sounds like after the locking in the current patches fixed, this
should perhaps be implemented and compared for locking complexity
and memory consumption.

> 
> It will require some minor infrastructural modification to associate a
> kobject with all of its leaf nodes, but the result will be cleaner, and
> the worst-case memory consumption will be less than your patches with a 
> per-attribute-per-kobject data structure (which there currently isn't).

Would a smaller replacement for the sysfs_dirent structure to link
just leaf nodes to kobject be close to what infrastructural
modifications you are talking about ?

Thanks
Dipankar