[PATCH 0/5] sysfs backing store (Re-splitted)

[PATCH 0/5] sysfs backing store (Re-splitted)

[Maneesh Soni]

Hello Andrew,

As per Viro's suggestions I have re-splitted the sysfs backing store 
patch set. This splitting is more meaningful and it should make the 
long pending review more interesting. There are no significant changes 
in the code except some cosmetic code re-arrangement and a couple of 
real changes in the error paths.

1) In fs/sysfs/dir.c:create_dir(), dput() was missing if we fail in
   sysfs_new_dirent()

2) Check for kmalloc() return value in fs/sysfs/symlink.c:sysfs_add_link().

I hope Viro will do the review soon. In the mean time I thought it will be 
nice if you could _replace_ the sysfs patches (six patches with names 
sysfs-leaves-xxx.patch) with the following patches (mailed separately). 
I will update them again based on the comments I get. 

Thanks
Maneesh

-- 
Maneesh Soni
Linux Technology Center, 
IBM Austin
email: maneesh@in.ibm.com
Phone: 1-512-838-1896 Fax: 
T/L : 6781896

[PATCH 1/5] Add sysfs_dirent to sysfs dentry

[Maneesh Soni]

o This the first patch in the series of patches implementing sysfs backing
  store. It introduces the new sysfs_dirent data structure. The sysfs_dirent 
  is added to each of the element which can be represented in sysfs like, 
  kobject (directory), text or binary attributes (files), attribute groups 
  (directory) and symlinks.

o It uses dentry's d_fsdata field to attach the corresponding sysfs_dirent.

o The sysfs_dirents are maintained in a tree of all the sysfs entries using the
  s_children and s_sibling list pointers. 

o The patch just adds the sysfs_dirent structure and hence should not be used
  independently but system can boot with it.


 fs/sysfs/bin.c        |   17 +++++++++++----
 fs/sysfs/dir.c        |   27 +++++++++++++++++++-----
 fs/sysfs/file.c       |   25 ++++++++++++++++------
 fs/sysfs/group.c      |    2 -
 fs/sysfs/mount.c      |    8 +++++++
 fs/sysfs/symlink.c    |   56 +++++++++++++++++++++++++++++++++++++++++++++-----
 fs/sysfs/sysfs.h      |   37 ++++++++++++++++++++++++++++++---
 include/linux/sysfs.h |   21 ++++++++++++++++++
 8 files changed, 169 insertions(+), 24 deletions(-)

diff -puN include/linux/sysfs.h~sysfs-backing-store-add-sysfs_dirent include/linux/sysfs.h
--- linux-2.6.8-rc2-mm1/include/linux/sysfs.h~sysfs-backing-store-add-sysfs_dirent	2004-07-29 15:30:37.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/include/linux/sysfs.h	2004-07-29 15:30:37.000000000 -0500
@@ -9,6 +9,8 @@
 #ifndef _SYSFS_H_
 #define _SYSFS_H_
 
+#include <asm/atomic.h>
+
 struct kobject;
 struct module;
 
@@ -57,6 +59,25 @@ struct sysfs_ops {
 	ssize_t	(*store)(struct kobject *,struct attribute *,const char *, size_t);
 };
 
+struct sysfs_dirent {
+	atomic_t		s_count;
+	struct list_head	s_sibling;
+	struct list_head	s_children;
+	void 			* s_element;
+	int			s_type;
+	umode_t			s_mode;
+	struct dentry		* s_dentry;
+};
+
+#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
+#define SYSFS_NOT_PINNED	(SYSFS_KOBJ_ATTR | SYSFS_KOBJ_BIN_ATTR | SYSFS_KOBJ_LINK)
+
+
 #ifdef CONFIG_SYSFS
 
 extern int
diff -puN fs/sysfs/dir.c~sysfs-backing-store-add-sysfs_dirent fs/sysfs/dir.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/dir.c~sysfs-backing-store-add-sysfs_dirent	2004-07-29 15:30:37.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/dir.c	2004-07-29 15:31:16.000000000 -0500
@@ -27,17 +27,34 @@ static int create_dir(struct kobject * k
 		      const char * n, struct dentry ** d)
 {
 	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,
-					 S_IFDIR| S_IRWXU | S_IRUGO | S_IXUGO,
-					 init_dir);
+		error = sysfs_create(*d, mode, init_dir);
 		if (!error) {
-			(*d)->d_fsdata = k;
-			p->d_inode->i_nlink++;
+			struct sysfs_dirent * sd, * parent_sd;
+			parent_sd = p->d_fsdata;
+			sd = sysfs_new_dirent(parent_sd, k,
+						(parent_sd->s_element == k) ?
+						SYSFS_KOBJ_ATTR_GROUP :
+						SYSFS_KOBJECT);
+			if (sd) {
+				(*d)->d_fsdata = sd;
+				p->d_inode->i_nlink++;
+				sd->s_dentry = *d;
+				sd->s_mode = mode;
+			} else {
+				/* error, release the ref taken in
+				 * sysfs_create()
+				 */
+				dput(*d);  
+				error = -ENOMEM;
+			}
 		}
+		/* This will free the dentry in case of error 
+		 */
 		dput(*d);
 	} else
 		error = PTR_ERR(*d);
diff -puN fs/sysfs/file.c~sysfs-backing-store-add-sysfs_dirent fs/sysfs/file.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/file.c~sysfs-backing-store-add-sysfs_dirent	2004-07-29 15:30:37.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/file.c	2004-07-29 15:31:10.000000000 -0500
@@ -346,10 +346,12 @@ static struct file_operations sysfs_file
 };
 
 
-int sysfs_add_file(struct dentry * dir, const struct attribute * attr)
+int sysfs_add_file(struct dentry * dir, const struct attribute * attr, int type)
 {
 	struct dentry * dentry;
-	int error;
+	struct sysfs_dirent * sd;
+	struct sysfs_dirent * parent_sd = dir->d_fsdata;
+	int error = 0;
 
 	down(&dir->d_inode->i_sem);
 	dentry = sysfs_get_dentry(dir,attr->name);
@@ -357,8 +359,18 @@ int sysfs_add_file(struct dentry * dir, 
 		error = sysfs_create(dentry,
 				     (attr->mode & S_IALLUGO) | S_IFREG,
 				     init_file);
-		if (!error)
-			dentry->d_fsdata = (void *)attr;
+		if (!error) {
+
+			sd = sysfs_new_dirent(parent_sd, (void *) attr, type);
+			if (sd) {
+				sd->s_mode = (attr->mode & S_IALLUGO) | S_IFREG;
+				dentry->d_fsdata = sd;
+				sd->s_dentry = dentry;
+			} else {
+				dput(dentry);
+				error = -ENOMEM;
+			}
+		}
 		dput(dentry);
 	} else
 		error = PTR_ERR(dentry);
@@ -375,8 +387,9 @@ 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);
+	if (kobj && kobj->dentry && attr)
+		return sysfs_add_file(kobj->dentry, attr, SYSFS_KOBJ_ATTR);
+
 	return -EINVAL;
 }
 
diff -puN fs/sysfs/bin.c~sysfs-backing-store-add-sysfs_dirent fs/sysfs/bin.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/bin.c~sysfs-backing-store-add-sysfs_dirent	2004-07-29 15:30:37.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/bin.c	2004-07-29 15:31:10.000000000 -0500
@@ -160,6 +160,8 @@ int sysfs_create_bin_file(struct kobject
 {
 	struct dentry * dentry;
 	struct dentry * parent;
+	struct sysfs_dirent * sd;
+	umode_t mode = (attr->attr.mode & S_IALLUGO) | S_IFREG;
 	int error = 0;
 
 	if (!kobj || !attr)
@@ -170,13 +172,20 @@ int sysfs_create_bin_file(struct kobject
 	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);
+		error = sysfs_create(dentry, mode, NULL);
 		if (!error) {
 			dentry->d_inode->i_size = attr->size;
 			dentry->d_inode->i_fop = &bin_fops;
+			sd = sysfs_new_dirent(parent->d_fsdata, (void *) attr, 
+						SYSFS_KOBJ_BIN_ATTR);
+			if (!sd) {
+				sd->s_mode = mode;
+				dentry->d_fsdata = sd;
+				sd->s_dentry = dentry;
+			} else {
+				dput(dentry);
+				error =  -ENOMEM;
+			}
 		}
 		dput(dentry);
 	} else
diff -puN fs/sysfs/sysfs.h~sysfs-backing-store-add-sysfs_dirent fs/sysfs/sysfs.h
--- linux-2.6.8-rc2-mm1/fs/sysfs/sysfs.h~sysfs-backing-store-add-sysfs_dirent	2004-07-29 15:30:37.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/sysfs.h	2004-07-29 15:31:16.000000000 -0500
@@ -5,8 +5,9 @@ extern struct inode * sysfs_new_inode(mo
 extern int sysfs_create(struct dentry *, int mode, int (*init)(struct inode *));
 
 extern struct dentry * sysfs_get_dentry(struct dentry *, const char *);
+extern void sysfs_drop_dentry(struct sysfs_dirent *, struct dentry *);
 
-extern int sysfs_add_file(struct dentry * dir, const struct attribute * attr);
+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 **);
@@ -16,14 +17,44 @@ extern int sysfs_readlink(struct dentry 
 extern int sysfs_follow_link(struct dentry *, struct nameidata *);
 extern struct rw_semaphore sysfs_rename_sem;
 
+struct sysfs_symlink {
+	char * link_name;
+	struct kobject * target_kobj;
+};
+
 static inline struct kobject *sysfs_get_kobject(struct dentry *dentry)
 {
 	struct kobject * kobj = NULL;
 
 	spin_lock(&dcache_lock);
-	if (!d_unhashed(dentry))
-		kobj = kobject_get(dentry->d_fsdata);
+	if (!d_unhashed(dentry)) {
+		struct sysfs_dirent * sd = dentry->d_fsdata;
+		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);
+	}
 	spin_unlock(&dcache_lock);
 
 	return kobj;
 }
+
+static inline
+struct sysfs_dirent * sysfs_new_dirent(struct sysfs_dirent * p, void * e, int t)
+{
+	struct sysfs_dirent * sd;
+
+	sd = kmalloc(sizeof(*sd), GFP_KERNEL);
+	if (!sd)
+		return NULL;
+	memset(sd, 0, sizeof(*sd));
+	atomic_set(&sd->s_count, 1);
+	sd->s_element = e;
+	sd->s_type = t;
+	sd->s_dentry = NULL;
+	INIT_LIST_HEAD(&sd->s_children);
+	list_add(&sd->s_sibling, &p->s_children);
+
+	return sd;
+}
diff -puN fs/sysfs/group.c~sysfs-backing-store-add-sysfs_dirent fs/sysfs/group.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/group.c~sysfs-backing-store-add-sysfs_dirent	2004-07-29 15:30:37.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/group.c	2004-07-29 15:30:37.000000000 -0500
@@ -31,7 +31,7 @@ static int create_files(struct dentry * 
 	int error = 0;
 
 	for (attr = grp->attrs; *attr && !error; attr++) {
-		error = sysfs_add_file(dir,*attr);
+		error = sysfs_add_file(dir, *attr, SYSFS_KOBJ_ATTR);
 	}
 	if (error)
 		remove_files(dir,grp);
diff -puN fs/sysfs/symlink.c~sysfs-backing-store-add-sysfs_dirent fs/sysfs/symlink.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/symlink.c~sysfs-backing-store-add-sysfs_dirent	2004-07-29 15:30:37.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/symlink.c	2004-07-29 15:31:07.000000000 -0500
@@ -53,6 +53,39 @@ static void fill_object_path(struct kobj
 	}
 }
 
+static struct sysfs_dirent * 
+sysfs_add_link(struct sysfs_dirent * parent_sd, char * name, 
+		struct kobject * target)
+{
+	struct sysfs_dirent * sd;
+	struct sysfs_symlink * sl;
+	int error = 0;
+
+	error = -ENOMEM;
+	sl = kmalloc(sizeof(*sl), GFP_KERNEL);
+	if (!sl)
+		goto exit1;
+
+	sl->link_name = kmalloc(strlen(name) + 1, GFP_KERNEL);
+	if (!sl->link_name)
+		goto exit2;
+
+	strcpy(sl->link_name, name);
+	sl->target_kobj = kobject_get(target);
+
+	sd = sysfs_new_dirent(parent_sd, sl, SYSFS_KOBJ_LINK);
+	if (sd) {
+		sd->s_mode = S_IFLNK|S_IRWXUGO;
+		return sd;
+	}
+
+	kfree(sl->link_name);
+exit2:
+	kfree(sl);
+exit1:
+	return ERR_PTR(error);
+}
+
 /**
  *	sysfs_create_link - create symlink between two objects.
  *	@kobj:	object whose directory we're creating the link in.
@@ -65,15 +98,28 @@ int sysfs_create_link(struct kobject * k
 	struct dentry * d;
 	int error = 0;
 
+	if (!name)
+		return -EINVAL;
+
 	down(&dentry->d_inode->i_sem);
 	d = sysfs_get_dentry(dentry,name);
 	if (!IS_ERR(d)) {
 		error = sysfs_create(d, S_IFLNK|S_IRWXUGO, init_symlink);
-		if (!error)
-			/* 
-			 * associate the link dentry with the target kobject 
-			 */
-			d->d_fsdata = kobject_get(target);
+		if (!error) {
+			struct sysfs_dirent * sd;
+			sd = sysfs_add_link(dentry->d_fsdata, name, target);
+			if (!IS_ERR(sd)) {
+				/* 
+			 	* associate the link dentry with the target  
+			 	* through the corresponding sysfs_dirent.
+			 	*/
+				d->d_fsdata = sd;
+				sd->s_dentry = dentry;
+			} else {
+				dput(d);
+				error = PTR_ERR(sd);
+			}
+		}
 		dput(d);
 	} else 
 		error = PTR_ERR(d);
diff -puN fs/sysfs/mount.c~sysfs-backing-store-add-sysfs_dirent fs/sysfs/mount.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/mount.c~sysfs-backing-store-add-sysfs_dirent	2004-07-29 15:30:37.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/mount.c	2004-07-29 15:31:16.000000000 -0500
@@ -22,6 +22,13 @@ 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_type		= SYSFS_ROOT,
+};
+
 static int sysfs_fill_super(struct super_block *sb, void *data, int silent)
 {
 	struct inode *inode;
@@ -50,6 +57,7 @@ static int sysfs_fill_super(struct super
 		iput(inode);
 		return -ENOMEM;
 	}
+	root->d_fsdata = &sysfs_root;
 	sb->s_root = root;
 	return 0;
 }

_
-- 
Maneesh Soni
Linux Technology Center, 
IBM Austin
email: maneesh@in.ibm.com
Phone: 1-512-838-1896 Fax: 
T/L : 6781896

[PATCH 2/5] Use sysfs_dirent tree for ->readdir etc.

[Maneesh Soni]

o This patch implements the sysfs_dir_operations file_operations strucutre for 
  sysfs directories. It uses the sysfs_dirent based tree for ->readdir() and 
  ->lseek() methods instead of simple_dir_operations which use dentry based 
  tree.

o This also has the code for sysfs_get() and sysfs_put() which manipulates
  the reference counting for for sysfs_dirents.


 fs/sysfs/dir.c   |  173 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 fs/sysfs/mount.c |    2 
 fs/sysfs/sysfs.h |   27 ++++++++
 3 files changed, 200 insertions(+), 2 deletions(-)

diff -puN fs/sysfs/dir.c~sysfs-backing-store-sysfs_readdir fs/sysfs/dir.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/dir.c~sysfs-backing-store-sysfs_readdir	2004-07-29 15:30:38.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/dir.c	2004-07-29 15:31:13.000000000 -0500
@@ -15,7 +15,7 @@ DECLARE_RWSEM(sysfs_rename_sem);
 static int init_dir(struct inode * inode)
 {
 	inode->i_op = &simple_dir_inode_operations;
-	inode->i_fop = &simple_dir_operations;
+	inode->i_fop = &sysfs_dir_operations;
 
 	/* directory inodes start off with i_nlink == 2 (for "." entry) */
 	inode->i_nlink++;
@@ -210,6 +210,177 @@ int sysfs_rename_dir(struct kobject * ko
 	return error;
 }
 
+/* get the name for corresponding element represented by the given sysfs_dirent
+ */
+const unsigned char * sysfs_get_name(struct sysfs_dirent *sd)
+{
+	struct attribute * attr;
+	struct bin_attribute * bin_attr;
+	struct sysfs_symlink  * sl;
+
+	if (!sd || !sd->s_element)
+		BUG();
+
+	switch (sd->s_type) {
+		case SYSFS_KOBJECT:
+		case SYSFS_KOBJ_ATTR_GROUP:
+			/* Always have a dentry so use that */
+			return sd->s_dentry->d_name.name;
+
+		case SYSFS_KOBJ_ATTR:
+			attr = sd->s_element;
+			return attr->name;
+
+		case SYSFS_KOBJ_BIN_ATTR:
+			bin_attr = sd->s_element;
+			return bin_attr->attr.name;
+
+		case SYSFS_KOBJ_LINK:
+			sl = sd->s_element;
+			return sl->link_name;
+	}
+	return NULL;
+}
+
+static int sysfs_dir_open(struct inode *inode, struct file *file)
+{
+	struct dentry * dentry = file->f_dentry;
+	struct sysfs_dirent * parent_sd = dentry->d_fsdata;
+
+	down(&dentry->d_inode->i_sem);
+	file->private_data = sysfs_new_dirent(parent_sd, NULL, 0);
+	up(&dentry->d_inode->i_sem);
+
+	return file->private_data ? 0 : -ENOMEM;
+
+}
+
+static int sysfs_dir_close(struct inode *inode, struct file *file)
+{
+	struct dentry * dentry = file->f_dentry;
+	struct sysfs_dirent * cursor = file->private_data;
+
+	down(&dentry->d_inode->i_sem);
+	list_del_init(&cursor->s_sibling);
+	up(&dentry->d_inode->i_sem);
+	sysfs_put(file->private_data);
+
+	return 0;
+}
+
+/* Relationship between s_mode and the DT_xxx types */
+static inline unsigned char dt_type(struct sysfs_dirent *sd)
+{
+	return (sd->s_mode >> 12) & 15;
+}
+
+static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir)
+{
+	struct dentry *dentry = filp->f_dentry;
+	struct sysfs_dirent * parent_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;
+
+	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:
+			if (filp->f_pos == 2) {
+				list_del(q);
+				list_add(q, &parent_sd->s_children);
+			}
+			for (p=q->next; p!= &parent_sd->s_children; p=p->next) {
+				struct sysfs_dirent *next;
+				const char * name;
+				int len;
+
+				next = list_entry(p, struct sysfs_dirent,
+						   s_sibling);
+				if (!next->s_element)
+					continue;
+
+				name = sysfs_get_name(next);
+				len = strlen(name);
+				if (next->s_dentry)
+					ino = next->s_dentry->d_inode->i_ino;
+				else
+					ino = iunique(sysfs_sb, 2);
+
+				if (filldir(dirent, name, len, filp->f_pos, ino,
+						 dt_type(next)) < 0)
+					return 0;
+
+				list_del(q);
+				list_add(q, p);
+				p = q;
+				filp->f_pos++;
+			}
+	}
+	return 0;
+}
+
+static 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(&file->f_dentry->d_inode->i_sem);
+			return -EINVAL;
+	}
+	if (offset != file->f_pos) {
+		file->f_pos = offset;
+		if (file->f_pos >= 2) {
+			struct sysfs_dirent *sd = dentry->d_fsdata;
+			struct sysfs_dirent *cursor = file->private_data;
+			struct list_head *p;
+			loff_t n = file->f_pos - 2;
+
+			list_del(&cursor->s_sibling);
+			p = sd->s_children.next;
+			while (n && p != &sd->s_children) {
+				struct sysfs_dirent *next;
+				next = list_entry(p, struct sysfs_dirent,
+						   s_sibling);
+				if (next->s_element)
+					n--;
+				p = p->next;
+			}
+			list_add_tail(&cursor->s_sibling, p);
+		}
+	}
+	up(&dentry->d_inode->i_sem);
+	return offset;
+}
+
+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,
+};
+
 EXPORT_SYMBOL(sysfs_create_dir);
 EXPORT_SYMBOL(sysfs_remove_dir);
 EXPORT_SYMBOL(sysfs_rename_dir);
diff -puN fs/sysfs/sysfs.h~sysfs-backing-store-sysfs_readdir fs/sysfs/sysfs.h
--- linux-2.6.8-rc2-mm1/fs/sysfs/sysfs.h~sysfs-backing-store-sysfs_readdir	2004-07-29 15:30:38.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/sysfs.h	2004-07-29 15:31:07.000000000 -0500
@@ -4,6 +4,8 @@ extern struct vfsmount * sysfs_mount;
 extern struct inode * sysfs_new_inode(mode_t mode);
 extern int sysfs_create(struct dentry *, int mode, int (*init)(struct inode *));
 
+
+extern const unsigned char * sysfs_get_name(struct sysfs_dirent *sd);
 extern struct dentry * sysfs_get_dentry(struct dentry *, const char *);
 extern void sysfs_drop_dentry(struct sysfs_dirent *, struct dentry *);
 
@@ -16,6 +18,8 @@ extern void sysfs_remove_subdir(struct d
 extern int sysfs_readlink(struct dentry *, char __user *, int );
 extern int sysfs_follow_link(struct dentry *, struct nameidata *);
 extern struct rw_semaphore sysfs_rename_sem;
+extern struct super_block * sysfs_sb;
+extern struct file_operations sysfs_dir_operations;
 
 struct sysfs_symlink {
 	char * link_name;
@@ -58,3 +62,26 @@ struct sysfs_dirent * sysfs_new_dirent(s
 
 	return sd;
 }
+
+static inline struct sysfs_dirent * sysfs_get(struct sysfs_dirent * sd)
+{
+	if (sd) {
+		WARN_ON(!atomic_read(&sd->s_count));
+		atomic_inc(&sd->s_count);
+	}
+	return sd;
+}
+
+static inline void sysfs_put(struct sysfs_dirent * sd)
+{
+	if (atomic_dec_and_test(&sd->s_count)) {
+		if (sd->s_type & SYSFS_KOBJ_LINK) {
+			struct sysfs_symlink * sl = sd->s_element;
+			kfree(sl->link_name);
+			kobject_put(sl->target_kobj);
+			kfree(sl);
+		}
+		kfree(sd);
+	}
+}
+
diff -puN fs/sysfs/mount.c~sysfs-backing-store-sysfs_readdir fs/sysfs/mount.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/mount.c~sysfs-backing-store-sysfs_readdir	2004-07-29 15:30:38.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/mount.c	2004-07-29 15:31:07.000000000 -0500
@@ -43,7 +43,7 @@ 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;
+		inode->i_fop = &sysfs_dir_operations;
 		/* directory inodes start off with i_nlink == 2 (for "." entry) */
 		inode->i_nlink++;	
 	} else {

_
-- 
Maneesh Soni
Linux Technology Center, 
IBM Austin
email: maneesh@in.ibm.com
Phone: 1-512-838-1896 Fax: 
T/L : 6781896

[PATCH 3/5] Free sysfs_dirent on file removal

[Maneesh Soni]

o The following patch implements the code to free up the sysfs_dirents upon
  directory or file removal. It uses the sysfs_dirent based tree in order
  to remove the directory contents before removing the directory itself.
  It could do this without taking dcache_lock in sysfs_remove_dir() as
  it doesnot use dentry based tree.


 fs/sysfs/dir.c   |   48 ++++++++++++++----------------------------------
 fs/sysfs/inode.c |   51 ++++++++++++++++++++++++++++++---------------------
 2 files changed, 44 insertions(+), 55 deletions(-)

diff -puN fs/sysfs/inode.c~sysfs-backing-store-kill-sysfs_dirent fs/sysfs/inode.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/inode.c~sysfs-backing-store-kill-sysfs_dirent	2004-07-29 15:30:39.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/inode.c	2004-07-29 15:31:07.000000000 -0500
@@ -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 = {
@@ -88,33 +90,40 @@ struct dentry * sysfs_get_dentry(struct 
 	return lookup_hash(&qstr,parent);
 }
 
+/* Unhashes the dentry corresponding to given sysfs_dirent
+ * Called with parent inode's i_sem held.
+ */
+void sysfs_drop_dentry(struct sysfs_dirent * sd, struct dentry * parent)
+{
+	struct dentry * dentry = sd->s_dentry;
+
+	if (dentry) {
+		if (!(d_unhashed(dentry) && dentry->d_inode)) {
+			spin_lock(&dcache_lock);
+			dget_locked(dentry);
+			__d_drop(dentry);
+			spin_unlock(&dcache_lock);
+			simple_unlink(parent->d_inode, dentry);
+		} else
+			dput(dentry);
+	}
+}
+
 void sysfs_hash_and_remove(struct dentry * dir, const char * name)
 {
-	struct dentry * victim;
+	struct sysfs_dirent * sd, * parent_sd = dir->d_fsdata;
 
 	down(&dir->d_inode->i_sem);
-	victim = sysfs_get_dentry(dir,name);
-	if (!IS_ERR(victim)) {
-		/* 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,
-				 atomic_read(&victim->d_count));
-
-			d_drop(victim);
-			/* release the target kobject in case of 
-			 * a symlink
-			 */
-			if (S_ISLNK(victim->d_inode->i_mode))
-				kobject_put(victim->d_fsdata);
-			simple_unlink(dir->d_inode,victim);
+	list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
+		if (!sd->s_element)
+			continue;
+		if (!strcmp(sysfs_get_name(sd), name)) {
+			list_del_init(&sd->s_sibling);
+			sysfs_drop_dentry(sd, dir);
+			sysfs_put(sd);
+			break;
 		}
-		/*
-		 * Drop reference from sysfs_get_dentry() above.
-		 */
-		dput(victim);
 	}
 	up(&dir->d_inode->i_sem);
 }
 
-
diff -puN fs/sysfs/dir.c~sysfs-backing-store-kill-sysfs_dirent fs/sysfs/dir.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/dir.c~sysfs-backing-store-kill-sysfs_dirent	2004-07-29 15:30:39.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/dir.c	2004-07-29 15:31:07.000000000 -0500
@@ -100,8 +100,13 @@ int sysfs_create_dir(struct kobject * ko
 static void remove_dir(struct dentry * d)
 {
 	struct dentry * parent = dget(d->d_parent);
+	struct sysfs_dirent * sd;
+
 	down(&parent->d_inode->i_sem);
 	d_delete(d);
+	sd = d->d_fsdata;
+ 	list_del_init(&sd->s_sibling);
+ 	sysfs_put(d->d_fsdata);
 	if (d->d_inode)
 		simple_rmdir(parent->d_inode,d);
 
@@ -129,47 +134,22 @@ void sysfs_remove_subdir(struct dentry *
 
 void sysfs_remove_dir(struct kobject * kobj)
 {
-	struct list_head * node;
 	struct dentry * dentry = dget(kobj->dentry);
+	struct sysfs_dirent * parent_sd = dentry->d_fsdata;
+	struct sysfs_dirent * sd, * tmp;
 
 	if (!dentry)
 		return;
 
 	pr_debug("sysfs %s: removing dir\n",dentry->d_name.name);
 	down(&dentry->d_inode->i_sem);
-
-	spin_lock(&dcache_lock);
-restart:
-	node = dentry->d_subdirs.next;
-	while (node != &dentry->d_subdirs) {
-		struct dentry * d = list_entry(node,struct dentry,d_child);
-
-		node = node->next;
-		pr_debug(" o %s (%d): ",d->d_name.name,atomic_read(&d->d_count));
-		if (!d_unhashed(d) && (d->d_inode)) {
-			d = dget_locked(d);
-			pr_debug("removing");
-
-			/**
-			 * Unlink and unhash.
-			 */
-			__d_drop(d);
-			spin_unlock(&dcache_lock);
-			/* release the target kobject in case of 
-			 * a symlink
-			 */
-			if (S_ISLNK(d->d_inode->i_mode))
-				kobject_put(d->d_fsdata);
-			
-			simple_unlink(dentry->d_inode,d);
-			dput(d);
-			pr_debug(" done\n");
-			spin_lock(&dcache_lock);
-			/* re-acquired dcache_lock, need to restart */
-			goto restart;
-		}
-	}
-	spin_unlock(&dcache_lock);
+	list_for_each_entry_safe(sd, tmp, &parent_sd->s_children, s_sibling) {
+		if (!sd->s_element)
+			continue;
+		list_del_init(&sd->s_sibling);
+		sysfs_drop_dentry(sd, dentry);
+		sysfs_put(sd);
+  	}
 	up(&dentry->d_inode->i_sem);
 
 	remove_dir(dentry);

_
-- 
Maneesh Soni
Linux Technology Center, 
IBM Austin
email: maneesh@in.ibm.com
Phone: 1-512-838-1896 Fax: 
T/L : 6781896

[PATCH 4/5] Change sysfs_file_operations

[Maneesh Soni]

o This patch modifies the sysfs_file_operations methods so as to get the
  kobject and attribute pointers via corresponding sysfs_dirent.


 fs/sysfs/bin.c  |   21 ++++++++++++++-------
 fs/sysfs/file.c |   24 +++++++++++++++++-------
 2 files changed, 31 insertions(+), 14 deletions(-)

diff -puN fs/sysfs/file.c~sysfs-backing-store-file_operations fs/sysfs/file.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/file.c~sysfs-backing-store-file_operations	2004-07-29 15:30:40.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/file.c	2004-07-29 15:31:07.000000000 -0500
@@ -77,8 +77,10 @@ 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 sysfs_dirent * attr_sd = file->f_dentry->d_fsdata;
+	struct attribute * attr = attr_sd->s_element;
+	struct sysfs_dirent * kobj_sd = file->f_dentry->d_parent->d_fsdata;
+	struct kobject * kobj = kobj_sd->s_element;
 	struct sysfs_ops * ops = buffer->ops;
 	int ret = 0;
 	ssize_t count;
@@ -135,6 +137,9 @@ static int flush_read_buffer(struct sysf
  *	is in the file's ->d_fsdata. The target object is in the directory's
  *	->d_fsdata.
  *
+ *	It is safe to use ->d_parent->d_fsdata as both dentry and the kobject
+ *	are pinned in ->open().
+ *
  *	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
  *	the beginning of the file). That should fill the entire buffer with
@@ -199,8 +204,10 @@ 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 sysfs_dirent * attr_sd = file->f_dentry->d_fsdata;
+	struct attribute * attr = attr_sd->s_element;
+	struct sysfs_dirent * kobj_sd = file->f_dentry->d_parent->d_fsdata;
+	struct kobject * kobj = kobj_sd->s_element;
 	struct sysfs_ops * ops = buffer->ops;
 
 	return ops->store(kobj,attr,buffer->page,count);
@@ -240,7 +247,8 @@ sysfs_write_file(struct file *file, cons
 static int check_perm(struct inode * inode, struct file * file)
 {
 	struct kobject *kobj = sysfs_get_kobject(file->f_dentry->d_parent);
-	struct attribute * attr = file->f_dentry->d_fsdata;
+	struct sysfs_dirent * attr_sd = file->f_dentry->d_fsdata;
+	struct attribute * attr = attr_sd->s_element;
 	struct sysfs_buffer * buffer;
 	struct sysfs_ops * ops = NULL;
 	int error = 0;
@@ -321,8 +329,10 @@ 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 sysfs_dirent * attr_sd = filp->f_dentry->d_fsdata;
+	struct attribute * attr = attr_sd->s_element;
+	struct sysfs_dirent * kobj_sd = filp->f_dentry->d_parent->d_fsdata;
+	struct kobject * kobj = kobj_sd->s_element;
 	struct sysfs_buffer * buffer = filp->private_data;
 
 	if (kobj) 
diff -puN fs/sysfs/bin.c~sysfs-backing-store-file_operations fs/sysfs/bin.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/bin.c~sysfs-backing-store-file_operations	2004-07-29 15:30:40.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/bin.c	2004-07-29 15:31:07.000000000 -0500
@@ -17,8 +17,10 @@
 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 * attr_sd = dentry->d_fsdata;
+	struct bin_attribute * attr = attr_sd->s_element;
+	struct sysfs_dirent * kobj_sd = dentry->d_parent->d_fsdata;
+	struct kobject * kobj = kobj_sd->s_element;
 
 	return attr->read(kobj, buffer, off, count);
 }
@@ -60,8 +62,10 @@ 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 * attr_sd = dentry->d_fsdata;
+	struct bin_attribute * attr = attr_sd->s_element;
+	struct sysfs_dirent * kobj_sd = dentry->d_parent->d_fsdata;
+	struct kobject * kobj = kobj_sd->s_element;
 
 	return attr->write(kobj, buffer, offset, count);
 }
@@ -95,7 +99,8 @@ static ssize_t write(struct file * file,
 static int open(struct inode * inode, struct file * file)
 {
 	struct kobject *kobj = sysfs_get_kobject(file->f_dentry->d_parent);
-	struct bin_attribute * attr = file->f_dentry->d_fsdata;
+	struct sysfs_dirent * attr_sd = file->f_dentry->d_fsdata;
+	struct bin_attribute * attr = attr_sd->s_element;
 	int error = -EINVAL;
 
 	if (!kobj || !attr)
@@ -130,8 +135,10 @@ 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 bin_attribute * attr = file->f_dentry->d_fsdata;
+	struct sysfs_dirent * sd = file->f_dentry->d_parent->d_fsdata;
+	struct kobject * kobj = sd->s_element;
+	struct sysfs_dirent * attr_sd = file->f_dentry->d_fsdata;
+	struct bin_attribute * attr = attr_sd->s_element;
 	u8 * buffer = file->private_data;
 
 	if (kobj) 

_
-- 
Maneesh Soni
Linux Technology Center, 
IBM Austin
email: maneesh@in.ibm.com
Phone: 1-512-838-1896 Fax: 
T/L : 6781896

[PATCH 5/5] Stop pinning dentries & inodes for leaves

[Maneesh Soni]

o This patch stops the pinning of non-directory or leaf dentries and inodes. 
  The leaf dentries and inodes are created during lookup based on the
  entries on sysfs_dirent tree. These leaves are removed from the dcache
  through the VFS dentry ageing process during shrink dcache operations. Thus
  reducing about 80% of sysfs lowmem needs.

o This implments the ->lookup() for sysfs directory inodes and allocates
  dentry and inode if the lookup is successful and avoids the need of 
  allocating and pinning of dentry and inodes during the creation of 
  corresponding sysfs leaf entry. As of now the implementation has not 
  required negative dentry creation on failed lookup. As sysfs is still a
  RAM based filesystem, negative dentries are not of any use IMO.

o The leaf dentry allocated after successful lookup is connected to the 
  existing corresponding sysfs_dirent through the d_fsdata field. This 
  increments the ref count of sysfs_dirent. The ref count is released through 
  ->d_iput() dentry_operation when dentry dies.


 fs/sysfs/bin.c     |   41 ++----------------
 fs/sysfs/dir.c     |  115 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 fs/sysfs/file.c    |   37 ++---------------
 fs/sysfs/inode.c   |    3 -
 fs/sysfs/mount.c   |    2 
 fs/sysfs/symlink.c |   40 ++----------------
 fs/sysfs/sysfs.h   |    4 +
 7 files changed, 137 insertions(+), 105 deletions(-)

diff -puN fs/sysfs/inode.c~sysfs-backing-store-stop-pinning fs/sysfs/inode.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/inode.c~sysfs-backing-store-stop-pinning	2004-07-29 15:30:41.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/inode.c	2004-07-29 15:30:41.000000000 -0500
@@ -68,7 +68,8 @@ int sysfs_create(struct dentry * dentry,
 		error = init(inode);
 	if (!error) {
 		d_instantiate(dentry, inode);
-		dget(dentry); /* Extra count - pin the dentry in core */
+		if (S_ISDIR(mode))
+			dget(dentry);  /* pin only directory dentry in core */
 	} else
 		iput(inode);
  Done:
diff -puN fs/sysfs/dir.c~sysfs-backing-store-stop-pinning fs/sysfs/dir.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/dir.c~sysfs-backing-store-stop-pinning	2004-07-29 15:30:41.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/dir.c	2004-07-29 15:30:41.000000000 -0500
@@ -14,7 +14,7 @@ DECLARE_RWSEM(sysfs_rename_sem);
 
 static int init_dir(struct inode * inode)
 {
-	inode->i_op = &simple_dir_inode_operations;
+	inode->i_op = &sysfs_dir_inode_operations;
 	inode->i_fop = &sysfs_dir_operations;
 
 	/* directory inodes start off with i_nlink == 2 (for "." entry) */
@@ -22,6 +22,35 @@ static int init_dir(struct inode * inode
 	return 0;
 }
 
+static int init_file(struct inode * inode)
+{
+	inode->i_size = PAGE_SIZE;
+	inode->i_fop = &sysfs_file_operations;
+	return 0;
+}
+
+static int init_symlink(struct inode * inode)
+{
+	inode->i_op = &sysfs_symlink_inode_operations;
+	return 0;
+}
+
+static void sysfs_d_iput(struct dentry * dentry, struct inode * inode)
+{
+	struct sysfs_dirent * sd = dentry->d_fsdata;
+
+	if (sd) {
+		BUG_ON(sd->s_dentry != dentry);
+		sd->s_dentry = NULL;
+		sysfs_put(sd);
+	}
+	iput(inode);
+}
+
+static struct dentry_operations sysfs_dentry_ops = {
+	.d_iput		= sysfs_d_iput,
+};
+
 
 static int create_dir(struct kobject * k, struct dentry * p,
 		      const char * n, struct dentry ** d)
@@ -41,10 +70,12 @@ static int create_dir(struct kobject * k
 						SYSFS_KOBJ_ATTR_GROUP :
 						SYSFS_KOBJECT);
 			if (sd) {
-				(*d)->d_fsdata = sd;
+				(*d)->d_fsdata = sysfs_get(sd);
+				(*d)->d_op = &sysfs_dentry_ops;
 				p->d_inode->i_nlink++;
 				sd->s_dentry = *d;
 				sd->s_mode = mode;
+				d_rehash(*d);
 			} else {
 				/* error, release the ref taken in
 				 * sysfs_create()
@@ -96,6 +127,82 @@ int sysfs_create_dir(struct kobject * ko
 	return error;
 }
 
+/* attaches attribute's sysfs_dirent to the dentry corresponding to the
+ * attribute file
+ */
+static int sysfs_attach_attr(struct sysfs_dirent * sd, struct dentry * dentry)
+{
+	struct attribute * attr = NULL;
+	struct bin_attribute * bin_attr = NULL;
+	int (* init) (struct inode *) = NULL;
+	int error = 0;
+
+        if (sd->s_type & SYSFS_KOBJ_BIN_ATTR) {
+                bin_attr = sd->s_element;
+                attr = &bin_attr->attr;
+        } else {
+                attr = sd->s_element;
+                init = init_file;
+        }
+
+	error = sysfs_create(dentry, (attr->mode & S_IALLUGO) | S_IFREG, init);
+	if (error)
+		return error;
+
+        if (bin_attr) {
+		dentry->d_inode->i_size = bin_attr->size;
+		dentry->d_inode->i_fop = &bin_fops;
+	}
+	dentry->d_op = &sysfs_dentry_ops;
+	dentry->d_fsdata = sysfs_get(sd);
+	sd->s_dentry = dentry;
+	d_rehash(dentry);
+
+	return 0;
+}
+
+static int sysfs_attach_link(struct sysfs_dirent * sd, struct dentry * dentry)
+{
+	int err = 0;
+
+	err = sysfs_create(dentry, S_IFLNK|S_IRWXUGO, init_symlink);
+	if (!err) {
+		dentry->d_op = &sysfs_dentry_ops;
+		dentry->d_fsdata = sysfs_get(sd);
+		sd->s_dentry = dentry;
+		d_rehash(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;
+	int err = 0;
+
+	list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
+		if (sd->s_type & SYSFS_NOT_PINNED) {
+			const unsigned char * name = sysfs_get_name(sd);
+
+			if (strcmp(name, dentry->d_name.name))
+				continue;
+
+			if (sd->s_type & SYSFS_KOBJ_LINK)
+				err = sysfs_attach_link(sd, dentry);
+			else
+				err = sysfs_attach_attr(sd, dentry);
+			break;
+		}
+	}
+
+	return ERR_PTR(err);
+}
+
+struct inode_operations sysfs_dir_inode_operations = {
+	.lookup		= sysfs_lookup,
+};
 
 static void remove_dir(struct dentry * d)
 {
@@ -179,8 +286,10 @@ int sysfs_rename_dir(struct kobject * ko
 	if (!IS_ERR(new_dentry)) {
   		if (!new_dentry->d_inode) {
 			error = kobject_set_name(kobj,new_name);
-			if (!error)
+			if (!error) {
+				d_add(new_dentry, NULL);
 				d_move(kobj->dentry, new_dentry);
+			}
 		}
 		dput(new_dentry);
 	}
diff -puN fs/sysfs/file.c~sysfs-backing-store-stop-pinning fs/sysfs/file.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/file.c~sysfs-backing-store-stop-pinning	2004-07-29 15:30:41.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/file.c	2004-07-29 15:30:41.000000000 -0500
@@ -9,15 +9,6 @@
 
 #include "sysfs.h"
 
-static struct file_operations sysfs_file_operations;
-
-static int init_file(struct inode * inode)
-{
-	inode->i_size = PAGE_SIZE;
-	inode->i_fop = &sysfs_file_operations;
-	return 0;
-}
-
 #define to_subsys(k) container_of(k,struct subsystem,kset.kobj)
 #define to_sattr(a) container_of(a,struct subsys_attribute,attr)
 
@@ -347,7 +338,7 @@ static int sysfs_release(struct inode * 
 	return 0;
 }
 
-static struct file_operations sysfs_file_operations = {
+struct file_operations sysfs_file_operations = {
 	.read		= sysfs_read_file,
 	.write		= sysfs_write_file,
 	.llseek		= generic_file_llseek,
@@ -358,32 +349,16 @@ static struct file_operations sysfs_file
 
 int sysfs_add_file(struct dentry * dir, const struct attribute * attr, int type)
 {
-	struct dentry * dentry;
 	struct sysfs_dirent * sd;
 	struct sysfs_dirent * parent_sd = dir->d_fsdata;
 	int error = 0;
 
 	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) {
-
-			sd = sysfs_new_dirent(parent_sd, (void *) attr, type);
-			if (sd) {
-				sd->s_mode = (attr->mode & S_IALLUGO) | S_IFREG;
-				dentry->d_fsdata = sd;
-				sd->s_dentry = dentry;
-			} else {
-				dput(dentry);
-				error = -ENOMEM;
-			}
-		}
-		dput(dentry);
-	} else
-		error = PTR_ERR(dentry);
+	sd = sysfs_new_dirent(parent_sd, (void *) attr, type);
+	if (sd)
+		sd->s_mode = (attr->mode & S_IALLUGO) | S_IFREG;
+	else
+		error = -ENOMEM;
 	up(&dir->d_inode->i_sem);
 	return error;
 }
diff -puN fs/sysfs/bin.c~sysfs-backing-store-stop-pinning fs/sysfs/bin.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/bin.c~sysfs-backing-store-stop-pinning	2004-07-29 15:30:41.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/bin.c	2004-07-29 15:30:41.000000000 -0500
@@ -148,7 +148,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,
@@ -165,40 +165,11 @@ static struct file_operations bin_fops =
 
 int sysfs_create_bin_file(struct kobject * kobj, struct bin_attribute * attr)
 {
-	struct dentry * dentry;
-	struct dentry * parent;
-	struct sysfs_dirent * sd;
-	umode_t mode = (attr->attr.mode & S_IALLUGO) | S_IFREG;
-	int error = 0;
-
-	if (!kobj || !attr)
-		return -EINVAL;
-
-	parent = kobj->dentry;
-
-	down(&parent->d_inode->i_sem);
-	dentry = sysfs_get_dentry(parent,attr->attr.name);
-	if (!IS_ERR(dentry)) {
-		error = sysfs_create(dentry, mode, NULL);
-		if (!error) {
-			dentry->d_inode->i_size = attr->size;
-			dentry->d_inode->i_fop = &bin_fops;
-			sd = sysfs_new_dirent(parent->d_fsdata, (void *) attr, 
-						SYSFS_KOBJ_BIN_ATTR);
-			if (!sd) {
-				sd->s_mode = mode;
-				dentry->d_fsdata = sd;
-				sd->s_dentry = dentry;
-			} else {
-				dput(dentry);
-				error =  -ENOMEM;
-			}
-		}
-		dput(dentry);
-	} else
-		error = PTR_ERR(dentry);
-	up(&parent->d_inode->i_sem);
-	return error;
+	if (kobj && kobj->dentry && attr)
+		return sysfs_add_file(kobj->dentry, &attr->attr,
+					SYSFS_KOBJ_BIN_ATTR);
+	return -EINVAL;
+
 }
 
 
diff -puN fs/sysfs/symlink.c~sysfs-backing-store-stop-pinning fs/sysfs/symlink.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/symlink.c~sysfs-backing-store-stop-pinning	2004-07-29 15:30:41.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/symlink.c	2004-07-29 15:30:41.000000000 -0500
@@ -8,17 +8,11 @@
 
 #include "sysfs.h"
 
-static struct inode_operations sysfs_symlink_inode_operations = {
+struct inode_operations sysfs_symlink_inode_operations = {
 	.readlink = sysfs_readlink,
 	.follow_link = sysfs_follow_link,
 };
 
-static int init_symlink(struct inode * inode)
-{
-	inode->i_op = &sysfs_symlink_inode_operations;
-	return 0;
-}
-
 static int object_depth(struct kobject * kobj)
 {
 	struct kobject * p = kobj;
@@ -53,9 +47,8 @@ static void fill_object_path(struct kobj
 	}
 }
 
-static struct sysfs_dirent * 
-sysfs_add_link(struct sysfs_dirent * parent_sd, char * name, 
-		struct kobject * target)
+static int sysfs_add_link(struct sysfs_dirent * parent_sd, char * name,
+			    struct kobject * target)
 {
 	struct sysfs_dirent * sd;
 	struct sysfs_symlink * sl;
@@ -76,14 +69,14 @@ sysfs_add_link(struct sysfs_dirent * par
 	sd = sysfs_new_dirent(parent_sd, sl, SYSFS_KOBJ_LINK);
 	if (sd) {
 		sd->s_mode = S_IFLNK|S_IRWXUGO;
-		return sd;
+		return 0;
 	}
 
 	kfree(sl->link_name);
 exit2:
 	kfree(sl);
 exit1:
-	return ERR_PTR(error);
+	return error;
 }
 
 /**
@@ -95,34 +88,13 @@ exit1:
 int sysfs_create_link(struct kobject * kobj, struct kobject * target, char * name)
 {
 	struct dentry * dentry = kobj->dentry;
-	struct dentry * d;
 	int error = 0;
 
 	if (!name)
 		return -EINVAL;
 
 	down(&dentry->d_inode->i_sem);
-	d = sysfs_get_dentry(dentry,name);
-	if (!IS_ERR(d)) {
-		error = sysfs_create(d, S_IFLNK|S_IRWXUGO, init_symlink);
-		if (!error) {
-			struct sysfs_dirent * sd;
-			sd = sysfs_add_link(dentry->d_fsdata, name, target);
-			if (!IS_ERR(sd)) {
-				/* 
-			 	* associate the link dentry with the target  
-			 	* through the corresponding sysfs_dirent.
-			 	*/
-				d->d_fsdata = sd;
-				sd->s_dentry = dentry;
-			} else {
-				dput(d);
-				error = PTR_ERR(sd);
-			}
-		}
-		dput(d);
-	} else 
-		error = PTR_ERR(d);
+	error = sysfs_add_link(dentry->d_fsdata, name, target);
 	up(&dentry->d_inode->i_sem);
 	return error;
 }
diff -puN fs/sysfs/sysfs.h~sysfs-backing-store-stop-pinning fs/sysfs/sysfs.h
--- linux-2.6.8-rc2-mm1/fs/sysfs/sysfs.h~sysfs-backing-store-stop-pinning	2004-07-29 15:30:41.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/sysfs.h	2004-07-29 15:30:41.000000000 -0500
@@ -20,6 +20,10 @@ extern int sysfs_follow_link(struct dent
 extern struct rw_semaphore sysfs_rename_sem;
 extern struct super_block * sysfs_sb;
 extern struct file_operations sysfs_dir_operations;
+extern struct file_operations sysfs_file_operations;
+extern struct file_operations bin_fops;
+extern struct inode_operations sysfs_dir_inode_operations;
+extern struct inode_operations sysfs_symlink_inode_operations;
 
 struct sysfs_symlink {
 	char * link_name;
diff -puN fs/sysfs/mount.c~sysfs-backing-store-stop-pinning fs/sysfs/mount.c
--- linux-2.6.8-rc2-mm1/fs/sysfs/mount.c~sysfs-backing-store-stop-pinning	2004-07-29 15:30:41.000000000 -0500
+++ linux-2.6.8-rc2-mm1-maneesh/fs/sysfs/mount.c	2004-07-29 15:30:41.000000000 -0500
@@ -42,7 +42,7 @@ 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_op = &sysfs_dir_inode_operations;
 		inode->i_fop = &sysfs_dir_operations;
 		/* directory inodes start off with i_nlink == 2 (for "." entry) */
 		inode->i_nlink++;	

_
-- 
Maneesh Soni
Linux Technology Center, 
IBM Austin
email: maneesh@in.ibm.com
Phone: 1-512-838-1896 Fax: 
T/L : 6781896

Re: [PATCH 1/5] Add sysfs_dirent to sysfs dentry

[viro]

On Thu, Jul 29, 2004 at 03:38:21PM -0500, Maneesh Soni wrote:
> +			} else {
> +				/* error, release the ref taken in
> +				 * sysfs_create()
> +				 */
> +				dput(*d);  

Umm...  Shouldn't we unhash here?  sysfs_create() did hash it, so...
Same goes for other places where we fail to create sysfs_dirent; as
the matter of fact, how about making
	sysfs_make_dirent(dentry, mode, data, type)
that would either allocate sysfs_dirent and bind it to dentry, or
unhash and dput() dentry and return error?  AFAICS, that would make
life easier.

>  int sysfs_create_file(struct kobject * kobj, const struct attribute * attr)
>  {
> -	if (kobj && attr)
> -		return sysfs_add_file(kobj->dentry,attr);
> +	if (kobj && kobj->dentry && attr)
> +		return sysfs_add_file(kobj->dentry, attr, SYSFS_KOBJ_ATTR);

Can we legitimately get NULL kobj or kobj->dentry here?  If not, that'd
better be BUG_ON()...

> @@ -65,15 +98,28 @@ int sysfs_create_link(struct kobject * k
>  	struct dentry * d;
>  	int error = 0;
>  
> +	if (!name)
> +		return -EINVAL;

Again, can that happen legitimately?

>  	down(&dentry->d_inode->i_sem);
>  	d = sysfs_get_dentry(dentry,name);
>  	if (!IS_ERR(d)) {
>  		error = sysfs_create(d, S_IFLNK|S_IRWXUGO, init_symlink);
> -		if (!error)
> -			/* 
> -			 * associate the link dentry with the target kobject 
> -			 */
> -			d->d_fsdata = kobject_get(target);
> +		if (!error) {
> +			struct sysfs_dirent * sd;
> +			sd = sysfs_add_link(dentry->d_fsdata, name, target);
> +			if (!IS_ERR(sd)) {
> +				/* 
> +			 	* associate the link dentry with the target  
> +			 	* through the corresponding sysfs_dirent.
> +			 	*/
> +				d->d_fsdata = sd;
> +				sd->s_dentry = dentry;
> +			} else {
> +				dput(d);
> +				error = PTR_ERR(sd);
> +			}

	I'd pull that inside sysfs_add_link() (and then further into
sysfs_new_dirent()).

Re: [PATCH 2/5] Use sysfs_dirent tree for ->readdir etc.

[viro]

On Thu, Jul 29, 2004 at 03:39:19PM -0500, Maneesh Soni wrote:
> 
> o This patch implements the sysfs_dir_operations file_operations strucutre for 
>   sysfs directories. It uses the sysfs_dirent based tree for ->readdir() and 
>   ->lseek() methods instead of simple_dir_operations which use dentry based 
>   tree.
> 
> o This also has the code for sysfs_get() and sysfs_put() which manipulates
>   the reference counting for for sysfs_dirents.

ACK, except that I'd move refcounting primitives to the chunk that actually
uses them.

Re: [PATCH 3/5] Free sysfs_dirent on file removal

[viro]

On Thu, Jul 29, 2004 at 03:40:31PM -0500, Maneesh Soni wrote:
> 
> 
> o The following patch implements the code to free up the sysfs_dirents upon
>   directory or file removal. It uses the sysfs_dirent based tree in order
>   to remove the directory contents before removing the directory itself.
>   It could do this without taking dcache_lock in sysfs_remove_dir() as
>   it doesnot use dentry based tree.

ACK, but some of that (freeing sysfs_dirent upon removal, no smarts, just
enough to plug the leak) belongs in the first chunk.

Note that there we shouldn't care about refcounts - that stuff belongs here.

Re: [PATCH 4/5] Change sysfs_file_operations

[viro]

On Thu, Jul 29, 2004 at 03:43:59PM -0500, Maneesh Soni wrote:
> 
> 
> 
> o This patch modifies the sysfs_file_operations methods so as to get the
>   kobject and attribute pointers via corresponding sysfs_dirent.

Hmm...  After looking at that, I'd probably add typed inlined helpers
(dentry to attr, dentry to kobject, dentry to bin_attr) and converted
to their use *before* everything else (patch #0).  And change their
definitions to use sysfs_dirent in patch #1.  That way we avoid breakage
on intermediate stages.

Re: [PATCH 5/5] Stop pinning dentries & inodes for leaves

[viro]

On Thu, Jul 29, 2004 at 03:44:49PM -0500, Maneesh Soni wrote:
> 
> 
> 
> o This patch stops the pinning of non-directory or leaf dentries and inodes. 
>   The leaf dentries and inodes are created during lookup based on the
>   entries on sysfs_dirent tree. These leaves are removed from the dcache
>   through the VFS dentry ageing process during shrink dcache operations. Thus
>   reducing about 80% of sysfs lowmem needs.
> 
> o This implments the ->lookup() for sysfs directory inodes and allocates
>   dentry and inode if the lookup is successful and avoids the need of 
>   allocating and pinning of dentry and inodes during the creation of 
>   corresponding sysfs leaf entry. As of now the implementation has not 
>   required negative dentry creation on failed lookup. As sysfs is still a
>   RAM based filesystem, negative dentries are not of any use IMO.
> 
> o The leaf dentry allocated after successful lookup is connected to the 
>   existing corresponding sysfs_dirent through the d_fsdata field. This 
>   increments the ref count of sysfs_dirent. The ref count is released through 
>   ->d_iput() dentry_operation when dentry dies.

ACK; there's some reordering that might make sense here, but that depends on
what will fall out of changes in previous chunks.