* [PATCH 0/5] sysfs backing store (Re-splitted) @ 2004-07-29 20:37 Maneesh Soni 2004-07-29 20:38 ` [PATCH 1/5] Add sysfs_dirent to sysfs dentry Maneesh Soni 0 siblings, 1 reply; 11+ messages in thread From: Maneesh Soni @ 2004-07-29 20:37 UTC (permalink / raw) To: Andrew Morton; +Cc: Greg KH, Al Viro, LKML 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/5] Add sysfs_dirent to sysfs dentry 2004-07-29 20:37 [PATCH 0/5] sysfs backing store (Re-splitted) Maneesh Soni @ 2004-07-29 20:38 ` Maneesh Soni 2004-07-29 20:39 ` [PATCH 2/5] Use sysfs_dirent tree for ->readdir etc Maneesh Soni 2004-08-03 12:43 ` [PATCH 1/5] Add sysfs_dirent to sysfs dentry viro 0 siblings, 2 replies; 11+ messages in thread From: Maneesh Soni @ 2004-07-29 20:38 UTC (permalink / raw) To: Andrew Morton; +Cc: Greg KH, Al Viro, LKML 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/5] Use sysfs_dirent tree for ->readdir etc. 2004-07-29 20:38 ` [PATCH 1/5] Add sysfs_dirent to sysfs dentry Maneesh Soni @ 2004-07-29 20:39 ` Maneesh Soni 2004-07-29 20:40 ` [PATCH 3/5] Free sysfs_dirent on file removal Maneesh Soni 2004-08-03 12:47 ` [PATCH 2/5] Use sysfs_dirent tree for ->readdir etc viro 2004-08-03 12:43 ` [PATCH 1/5] Add sysfs_dirent to sysfs dentry viro 1 sibling, 2 replies; 11+ messages in thread From: Maneesh Soni @ 2004-07-29 20:39 UTC (permalink / raw) To: Andrew Morton; +Cc: Greg KH, Al Viro, LKML 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/5] Free sysfs_dirent on file removal 2004-07-29 20:39 ` [PATCH 2/5] Use sysfs_dirent tree for ->readdir etc Maneesh Soni @ 2004-07-29 20:40 ` Maneesh Soni 2004-07-29 20:43 ` [PATCH 4/5] Change sysfs_file_operations Maneesh Soni 2004-08-03 12:52 ` [PATCH 3/5] Free sysfs_dirent on file removal viro 2004-08-03 12:47 ` [PATCH 2/5] Use sysfs_dirent tree for ->readdir etc viro 1 sibling, 2 replies; 11+ messages in thread From: Maneesh Soni @ 2004-07-29 20:40 UTC (permalink / raw) To: Andrew Morton; +Cc: Greg KH, Al Viro, LKML 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/5] Change sysfs_file_operations 2004-07-29 20:40 ` [PATCH 3/5] Free sysfs_dirent on file removal Maneesh Soni @ 2004-07-29 20:43 ` Maneesh Soni 2004-07-29 20:44 ` [PATCH 5/5] Stop pinning dentries & inodes for leaves Maneesh Soni 2004-08-03 12:57 ` [PATCH 4/5] Change sysfs_file_operations viro 2004-08-03 12:52 ` [PATCH 3/5] Free sysfs_dirent on file removal viro 1 sibling, 2 replies; 11+ messages in thread From: Maneesh Soni @ 2004-07-29 20:43 UTC (permalink / raw) To: Andrew Morton; +Cc: Greg KH, Al Viro, LKML 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 5/5] Stop pinning dentries & inodes for leaves 2004-07-29 20:43 ` [PATCH 4/5] Change sysfs_file_operations Maneesh Soni @ 2004-07-29 20:44 ` Maneesh Soni 2004-08-03 13:02 ` viro 2004-08-03 12:57 ` [PATCH 4/5] Change sysfs_file_operations viro 1 sibling, 1 reply; 11+ messages in thread From: Maneesh Soni @ 2004-07-29 20:44 UTC (permalink / raw) To: Andrew Morton; +Cc: Greg KH, Al Viro, LKML 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5/5] Stop pinning dentries & inodes for leaves 2004-07-29 20:44 ` [PATCH 5/5] Stop pinning dentries & inodes for leaves Maneesh Soni @ 2004-08-03 13:02 ` viro 0 siblings, 0 replies; 11+ messages in thread From: viro @ 2004-08-03 13:02 UTC (permalink / raw) To: Maneesh Soni; +Cc: Andrew Morton, Greg KH, LKML 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/5] Change sysfs_file_operations 2004-07-29 20:43 ` [PATCH 4/5] Change sysfs_file_operations Maneesh Soni 2004-07-29 20:44 ` [PATCH 5/5] Stop pinning dentries & inodes for leaves Maneesh Soni @ 2004-08-03 12:57 ` viro 1 sibling, 0 replies; 11+ messages in thread From: viro @ 2004-08-03 12:57 UTC (permalink / raw) To: Maneesh Soni; +Cc: Andrew Morton, Greg KH, LKML 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/5] Free sysfs_dirent on file removal 2004-07-29 20:40 ` [PATCH 3/5] Free sysfs_dirent on file removal Maneesh Soni 2004-07-29 20:43 ` [PATCH 4/5] Change sysfs_file_operations Maneesh Soni @ 2004-08-03 12:52 ` viro 1 sibling, 0 replies; 11+ messages in thread From: viro @ 2004-08-03 12:52 UTC (permalink / raw) To: Maneesh Soni; +Cc: Andrew Morton, Greg KH, LKML 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/5] Use sysfs_dirent tree for ->readdir etc. 2004-07-29 20:39 ` [PATCH 2/5] Use sysfs_dirent tree for ->readdir etc Maneesh Soni 2004-07-29 20:40 ` [PATCH 3/5] Free sysfs_dirent on file removal Maneesh Soni @ 2004-08-03 12:47 ` viro 1 sibling, 0 replies; 11+ messages in thread From: viro @ 2004-08-03 12:47 UTC (permalink / raw) To: Maneesh Soni; +Cc: Andrew Morton, Greg KH, LKML 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/5] Add sysfs_dirent to sysfs dentry 2004-07-29 20:38 ` [PATCH 1/5] Add sysfs_dirent to sysfs dentry Maneesh Soni 2004-07-29 20:39 ` [PATCH 2/5] Use sysfs_dirent tree for ->readdir etc Maneesh Soni @ 2004-08-03 12:43 ` viro 1 sibling, 0 replies; 11+ messages in thread From: viro @ 2004-08-03 12:43 UTC (permalink / raw) To: Maneesh Soni; +Cc: Andrew Morton, Greg KH, LKML 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()). ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2004-08-03 13:03 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2004-07-29 20:37 [PATCH 0/5] sysfs backing store (Re-splitted) Maneesh Soni 2004-07-29 20:38 ` [PATCH 1/5] Add sysfs_dirent to sysfs dentry Maneesh Soni 2004-07-29 20:39 ` [PATCH 2/5] Use sysfs_dirent tree for ->readdir etc Maneesh Soni 2004-07-29 20:40 ` [PATCH 3/5] Free sysfs_dirent on file removal Maneesh Soni 2004-07-29 20:43 ` [PATCH 4/5] Change sysfs_file_operations Maneesh Soni 2004-07-29 20:44 ` [PATCH 5/5] Stop pinning dentries & inodes for leaves Maneesh Soni 2004-08-03 13:02 ` viro 2004-08-03 12:57 ` [PATCH 4/5] Change sysfs_file_operations viro 2004-08-03 12:52 ` [PATCH 3/5] Free sysfs_dirent on file removal viro 2004-08-03 12:47 ` [PATCH 2/5] Use sysfs_dirent tree for ->readdir etc viro 2004-08-03 12:43 ` [PATCH 1/5] Add sysfs_dirent to sysfs dentry viro
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.