[Patch] Do not allocate sysfs_dirent.s_children for non-directories

From: Adam J. Richter
Date: Thu Dec 02 2004 - 22:33:25 EST


The following patch, against a heavily hacked 2.6.10-rc2-bk15
sysfs tree, removes the s_children field from sysfs_dirent and
creates a new structure just for directories named sysfs_dir, which
embeds a sysfs_dirent and also adds s_children. Directories allocate
a sysfs_dir; non-directories allocate a sysfs_dirent. There are
two separate kmem caches for the different data types.

Not allocating s_children from each non-directory saves
two pointers (8 bytes) for each of the 2573 non-directory nodes
in my sysfs tree, or about 20kB on unswappable memory, but
having another kmem cache probably wastes an average of half
a page in memory fragmentation and then there is are few
bytes from the new code and the additional kmem_cache_t
structure, so I would guess it probably saves about 16kB in
practice.

In the future, I hope to make a similar change for symbolic
links.

By the way, this patch will also make it easier for me to
try to unpin sysfs directories because there are a few other
fields specific to directories that I would want to store
in sysfs_dir.

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

diff -u5 linux.prev/fs/sysfs/dir.c linux/fs/sysfs/dir.c
--- linux.prev/fs/sysfs/dir.c 2004-12-02 14:51:01.000000000 +0800
+++ linux/fs/sysfs/dir.c 2004-12-03 10:36:20.000000000 +0800
@@ -29,38 +29,46 @@
};

/*
* Allocates a new sysfs_dirent and links it to the parent sysfs_dirent
*/
-static struct sysfs_dirent * sysfs_new_dirent(struct sysfs_dirent * parent_sd,
- void * element)
+static struct sysfs_dirent * sysfs_new_dirent(struct sysfs_dir * parent_sd,
+ void * element, int type)
{
struct sysfs_dirent * sd;

- sd = kmem_cache_alloc(sysfs_dir_cachep, GFP_KERNEL);
- if (!sd)
- return NULL;
+ if (sysfs_type_dir(type)) {
+ struct sysfs_dir * sdir;
+ sdir = kmem_cache_alloc(sysfs_dir_cachep, GFP_KERNEL);
+ if (!sdir)
+ return NULL;
+ INIT_LIST_HEAD(&sdir->s_children);
+ sd = &sdir->s_ent;
+ } else {
+ sd = kmem_cache_alloc(sysfs_dirent_cachep, GFP_KERNEL);
+ if (!sd)
+ return NULL;
+ }

memset(sd, 0, sizeof(*sd));
- INIT_LIST_HEAD(&sd->s_children);
list_add(&sd->s_sibling, &parent_sd->s_children);
sd->s_element = element;
+ sd->s_type = type;

return sd;
}

-int sysfs_make_dirent(struct sysfs_dirent * parent_sd, struct dentry * dentry,
+int sysfs_make_dirent(struct sysfs_dir * parent_sd, struct dentry * dentry,
void * element, umode_t mode, int type)
{
struct sysfs_dirent * sd;

- sd = sysfs_new_dirent(parent_sd, element);
+ sd = sysfs_new_dirent(parent_sd, element, type);
if (!sd)
return -ENOMEM;

sd->s_mode = mode;
- sd->s_type = type;
sd->s_dentry = dentry;
if (dentry) {
dentry->d_fsdata = sd;
dentry->d_op = &sysfs_dentry_ops;
}
@@ -94,17 +102,19 @@
static int create_dir(void *element, struct dentry * p,
const char * n, struct dentry ** d, int type)
{
int error;
umode_t mode = S_IFDIR| S_IRWXU | S_IRUGO | S_IXUGO;
+ struct sysfs_dir *parent_sd;

down(&p->d_inode->i_sem);
*d = sysfs_get_dentry(p,n);
if (!IS_ERR(*d)) {
error = sysfs_create(*d, mode, init_dir);
if (!error) {
- error = sysfs_make_dirent(p->d_fsdata, *d, element,
+ parent_sd = dentry_to_sysfs_dir(p);
+ error = sysfs_make_dirent(parent_sd, *d, element,
mode, type);
if (!error) {
p->d_inode->i_nlink++;
(*d)->d_op = &sysfs_dentry_ops;
d_rehash(*d);
@@ -122,11 +132,12 @@

int sysfs_create_subdir(struct kobject * k,
const struct attribute_group *grp,
struct dentry ** d)
{
- return create_dir(grp, k->dentry, grp->name, d, SYSFS_ATTR_GROUP);
+ return create_dir((void*)grp, k->dentry, grp->name, d,
+ SYSFS_ATTR_GROUP);
}

/**
* sysfs_create_dir - create a directory for an object.
* @parent: parent parent object.
@@ -203,11 +214,11 @@
}

static 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_dir * parent_sd = dentry_to_sysfs_dir(dentry->d_parent);
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) {
@@ -267,20 +278,20 @@
*/

void sysfs_remove_dir(struct kobject * kobj)
{
struct dentry * dentry = dget(kobj->dentry);
- struct sysfs_dirent * parent_sd;
+ struct sysfs_dir * parent_sd;
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);
- parent_sd = dentry->d_fsdata;
- list_for_each_entry_safe(sd, tmp, &parent_sd->s_children, s_sibling) {
+ parent_sd = dentry_to_sysfs_dir(dentry);
+ list_for_each_entry_safe(sd,tmp,&parent_sd->s_children,s_sibling) {
if (!sd->s_element || !(sd->s_type & SYSFS_NOT_PINNED))
continue;
sysfs_drop_dentry(sd, dentry);
list_del_init(&sd->s_sibling);
sysfs_put(sd);
@@ -331,14 +342,14 @@
}

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;
+ struct sysfs_dir * parent_sd = dentry_to_sysfs_dir(dentry);

down(&dentry->d_inode->i_sem);
- file->private_data = sysfs_new_dirent(parent_sd, NULL);
+ file->private_data = sysfs_new_dirent(parent_sd, NULL, SYSFS_CURSOR);
up(&dentry->d_inode->i_sem);

return file->private_data ? 0 : -ENOMEM;

}
@@ -350,10 +361,13 @@

down(&dentry->d_inode->i_sem);
list_del_init(&cursor->s_sibling);
up(&dentry->d_inode->i_sem);

+ BUG_ON(cursor->s_dentry != NULL);
+ release_sysfs_dirent(cursor);
+
return 0;
}

/* Relationship between s_mode and the DT_xxx types */
static inline unsigned char dt_type(struct sysfs_dirent *sd)
@@ -362,11 +376,11 @@
}

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_dir * parent_sd = dentry_to_sysfs_dir(dentry);
struct sysfs_dirent *cursor = filp->private_data;
struct list_head *p, *q = &cursor->s_sibling;
ino_t ino;
int i = filp->f_pos;

@@ -395,10 +409,11 @@
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);
@@ -436,11 +451,11 @@
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_dir *sd = dentry_to_sysfs_dir(dentry);
struct sysfs_dirent *cursor = file->private_data;
struct list_head *p;
loff_t n = file->f_pos - 2;

list_del(&cursor->s_sibling);
diff -u5 linux.prev/fs/sysfs/file.c linux/fs/sysfs/file.c
--- linux.prev/fs/sysfs/file.c 2004-11-07 13:02:53.000000000 +0800
+++ linux/fs/sysfs/file.c 2004-12-02 23:19:56.000000000 +0800
@@ -355,11 +355,11 @@
};


int sysfs_add_file(struct dentry * dir, const struct attribute * attr, int type)
{
- struct sysfs_dirent * parent_sd = dir->d_fsdata;
+ struct sysfs_dir * parent_sd = dentry_to_sysfs_dir(dir);
umode_t mode = (attr->mode & S_IALLUGO) | S_IFREG;
int error = 0;

down(&dir->d_inode->i_sem);
error = sysfs_make_dirent(parent_sd, NULL, (void *) attr, mode, type);
diff -u5 linux.prev/fs/sysfs/inode.c linux/fs/sysfs/inode.c
--- linux.prev/fs/sysfs/inode.c 2004-12-02 14:51:01.000000000 +0800
+++ linux/fs/sysfs/inode.c 2004-12-03 00:43:39.000000000 +0800
@@ -146,11 +146,11 @@
}

void sysfs_hash_and_remove(struct dentry * dir, const char * name)
{
struct sysfs_dirent * sd;
- struct sysfs_dirent * parent_sd = dir->d_fsdata;
+ struct sysfs_dir * parent_sd = dentry_to_sysfs_dir(dir);

down(&dir->d_inode->i_sem);
list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
if (!sd->s_element)
continue;
diff -u5 linux.prev/fs/sysfs/mount.c linux/fs/sysfs/mount.c
--- linux.prev/fs/sysfs/mount.c 2004-12-02 11:55:02.000000000 +0800
+++ linux/fs/sysfs/mount.c 2004-12-02 23:27:39.000000000 +0800
@@ -15,21 +15,24 @@
#define SYSFS_MAGIC 0x62656572

struct vfsmount *sysfs_mount;
struct super_block * sysfs_sb = NULL;
kmem_cache_t *sysfs_dir_cachep;
+kmem_cache_t *sysfs_dirent_cachep;

static struct super_operations sysfs_ops = {
.statfs = simple_statfs,
.drop_inode = generic_delete_inode,
};

-static struct sysfs_dirent sysfs_root = {
- .s_sibling = LIST_HEAD_INIT(sysfs_root.s_sibling),
+static struct sysfs_dir sysfs_root = {
.s_children = LIST_HEAD_INIT(sysfs_root.s_children),
- .s_element = NULL,
- .s_type = SYSFS_ROOT,
+ .s_ent = {
+ .s_sibling = LIST_HEAD_INIT(sysfs_root.s_ent.s_sibling),
+ .s_element = NULL,
+ .s_type = SYSFS_ROOT,
+ }
};

static int sysfs_fill_super(struct super_block *sb, void *data, int silent)
{
struct inode *inode;
@@ -56,11 +59,11 @@
if (!root) {
pr_debug("%s: could not get root dentry!\n",__FUNCTION__);
iput(inode);
return -ENOMEM;
}
- root->d_fsdata = &sysfs_root;
+ root->d_fsdata = &sysfs_root.s_ent;
sb->s_root = root;
return 0;
}

static struct super_block *sysfs_get_sb(struct file_system_type *fs_type,
@@ -77,16 +80,22 @@

int __init sysfs_init(void)
{
int err = -ENOMEM;

- sysfs_dir_cachep = kmem_cache_create("sysfs_dir_cache",
- sizeof(struct sysfs_dirent),
+ sysfs_dir_cachep = kmem_cache_create("sysfs_dir",
+ sizeof(struct sysfs_dir),
0, 0, NULL, NULL);
if (!sysfs_dir_cachep)
goto out;

+ sysfs_dirent_cachep = kmem_cache_create("sysfs_dirent",
+ sizeof(struct sysfs_dirent),
+ 0, 0, NULL, NULL);
+ if (!sysfs_dirent_cachep)
+ goto out_no_dirent_cachep;
+
err = register_filesystem(&sysfs_fs_type);
if (!err) {
sysfs_mount = kern_mount(&sysfs_fs_type);
if (IS_ERR(sysfs_mount)) {
printk(KERN_ERR "sysfs: could not mount!\n");
@@ -97,8 +106,10 @@
} else
goto out_err;
out:
return err;
out_err:
+ kmem_cache_destroy(sysfs_dirent_cachep);
+out_no_dirent_cachep:
kmem_cache_destroy(sysfs_dir_cachep);
goto out;
}
diff -u5 linux.prev/fs/sysfs/symlink.c linux/fs/sysfs/symlink.c
--- linux.prev/fs/sysfs/symlink.c 2004-11-07 13:02:53.000000000 +0800
+++ linux/fs/sysfs/symlink.c 2004-12-02 23:57:01.000000000 +0800
@@ -43,11 +43,11 @@
}
}

static int sysfs_add_link(struct dentry * parent, char * name, struct kobject * target)
{
- struct sysfs_dirent * parent_sd = parent->d_fsdata;
+ struct sysfs_dir * parent_sd = dentry_to_sysfs_dir(parent);
struct sysfs_symlink * sl;
int error = 0;

error = -ENOMEM;
sl = kmalloc(sizeof(*sl), GFP_KERNEL);
diff -u5 linux.prev/fs/sysfs/sysfs.h linux/fs/sysfs/sysfs.h
--- linux.prev/fs/sysfs/sysfs.h 2004-12-02 15:29:39.000000000 +0800
+++ linux/fs/sysfs/sysfs.h 2004-12-03 00:51:44.000000000 +0800
@@ -1,32 +1,38 @@
#include <linux/time.h>
#include <linux/sysfs.h>

+#define SYSFS_CURSOR 0
#define SYSFS_ROOT 0x0001
#define SYSFS_DIR 0x0002
#define SYSFS_KOBJ_ATTR 0x0004
#define SYSFS_KOBJ_BIN_ATTR 0x0008
#define SYSFS_ATTR_GROUP 0x0010
#define SYSFS_KOBJ_LINK 0x0020
#define SYSFS_NOT_PINNED (SYSFS_KOBJ_ATTR | SYSFS_KOBJ_BIN_ATTR | SYSFS_KOBJ_LINK)

struct sysfs_dirent {
struct list_head s_sibling;
- struct list_head s_children;
void * s_element;
- int s_type;
+ unsigned short s_type;
umode_t s_mode;
struct dentry * s_dentry;
};

+struct sysfs_dir {
+ struct list_head s_children;
+ struct sysfs_dirent s_ent;
+};
+
extern struct vfsmount * sysfs_mount;
extern kmem_cache_t *sysfs_dir_cachep;
+extern kmem_cache_t *sysfs_dirent_cachep;

extern struct inode * sysfs_new_inode(mode_t mode);
extern int sysfs_create(struct dentry *, int mode, int (*init)(struct inode *));

-extern int sysfs_make_dirent(struct sysfs_dirent *, struct dentry *, void *,
+extern int sysfs_make_dirent(struct sysfs_dir *, struct dentry *, void *,
umode_t, int);
extern struct dentry * sysfs_get_dentry(struct dentry *, const char *);

extern int sysfs_add_file(struct dentry *, const struct attribute *, int);
extern void sysfs_hash_and_remove(struct dentry * dir, const char * name);
@@ -48,10 +54,29 @@
struct sysfs_symlink {
char * link_name;
struct kobject * target_kobj;
};

+static inline int sysfs_type_dir(int s_type)
+{
+ return (s_type == SYSFS_DIR ||
+ s_type == SYSFS_ATTR_GROUP ||
+ s_type == SYSFS_ROOT);
+}
+
+static inline struct sysfs_dir * to_sysfs_dir(struct sysfs_dirent *ent)
+{
+ BUG_ON(!sysfs_type_dir(ent->s_type));
+ return container_of(ent, struct sysfs_dir, s_ent);
+}
+
+static inline struct sysfs_dir *dentry_to_sysfs_dir(struct dentry * dentry)
+{
+ struct sysfs_dirent * sd = dentry->d_fsdata;
+ return to_sysfs_dir(sd);
+}
+
static inline struct kobject * to_kobj(struct dentry * dentry)
{
struct sysfs_dirent * sd = dentry->d_fsdata;

if (sd->s_type == SYSFS_ATTR_GROUP)
@@ -101,11 +126,14 @@
struct sysfs_symlink * sl = sd->s_element;
kfree(sl->link_name);
kobject_put(sl->target_kobj);
kfree(sl);
}
- kmem_cache_free(sysfs_dir_cachep, sd);
+ if (sysfs_type_dir(sd->s_type))
+ kmem_cache_free(sysfs_dir_cachep, to_sysfs_dir(sd));
+ else
+ kmem_cache_free(sysfs_dirent_cachep, sd);
}

static inline void sysfs_put(struct sysfs_dirent * sd)
{
if (list_empty(&sd->s_sibling) && sd->s_dentry == NULL)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/