* [RFC PATCH 0/5] Put struct inode on a diet
@ 2009-05-21 20:00 Theodore Ts'o
2009-05-21 20:00 ` [PATCH 1/5] fs: i_flags and i_state in struct inode only need to be unsigned short Theodore Ts'o
0 siblings, 1 reply; 9+ messages in thread
From: Theodore Ts'o @ 2009-05-21 20:00 UTC (permalink / raw)
To: linux-kernel; +Cc: linux-fsdevel, Theodore Ts'o
These patches attempt to slim down struct inode to reduce the overhead
of storing cached inodes. A typical Linux system has hundreds of
thousands of inodes cached, if not more, so reducing the inode size is a
high leverage thing to do. For example these patches save 24 bytes, or
a little over 4% of an inode (with no debugging enabled) on an x86_64
machine. But on a system with 160,000 inodes in its inode cache, this
might mean a difference of six and half megabytes of physical memory
(once SLAB rounding issues are taken into account).
Theodore Ts'o (5):
fs: i_flags and i_state in struct inode only need to be unsigned
short
fs: Remove i_cindex from struct inode
fs: Slim down inode by only using an unsigned int for i_dnotify_mask
fs: Rearrange inode structure elements to avoid waste due to padding
Dump the inode structure (for debugging purposes only)
drivers/ieee1394/dv1394.c | 5 +-
drivers/ieee1394/ieee1394_core.h | 5 ++-
fs/Makefile | 1 +
fs/char_dev.c | 14 ++++++-
fs/inode-struct-dumper.c | 87 ++++++++++++++++++++++++++++++++++++++
include/linux/cdev.h | 2 +
include/linux/dnotify.h | 2 +-
include/linux/fs.h | 9 ++--
8 files changed, 115 insertions(+), 10 deletions(-)
create mode 100644 fs/inode-struct-dumper.c
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/5] fs: i_flags and i_state in struct inode only need to be unsigned short 2009-05-21 20:00 [RFC PATCH 0/5] Put struct inode on a diet Theodore Ts'o @ 2009-05-21 20:00 ` Theodore Ts'o 2009-05-21 20:01 ` [PATCH 2/5] fs: Remove i_cindex from struct inode Theodore Ts'o 0 siblings, 1 reply; 9+ messages in thread From: Theodore Ts'o @ 2009-05-21 20:00 UTC (permalink / raw) To: linux-kernel; +Cc: linux-fsdevel, Theodore Ts'o Currently i_flags and i_state do not need to be an unsigned int and an unsigned long, respectively. (We currently use 9 i_flags bits, and 8 i_state bits.) Changing them to be an unsigned short saves 4 bytes per inode on an x86 platform, and 8 bytes on an x86_64 platform. Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> Cc: linux-fsdevel@vger.kernel.org --- include/linux/fs.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/fs.h b/include/linux/fs.h index 3b534e5..9b20f13 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -765,10 +765,10 @@ struct inode { struct mutex inotify_mutex; /* protects the watches list */ #endif - unsigned long i_state; unsigned long dirtied_when; /* jiffies of first dirtying */ - unsigned int i_flags; + unsigned short i_state; + unsigned short i_flags; atomic_t i_writecount; #ifdef CONFIG_SECURITY -- 1.6.3.1.1.g75fc.dirty ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/5] fs: Remove i_cindex from struct inode 2009-05-21 20:00 ` [PATCH 1/5] fs: i_flags and i_state in struct inode only need to be unsigned short Theodore Ts'o @ 2009-05-21 20:01 ` Theodore Ts'o 2009-05-21 20:01 ` [PATCH 3/5] fs: Slim down inode by only using an unsigned int for i_dnotify_mask Theodore Ts'o 2009-05-21 20:30 ` [PATCH 2/5] fs: Remove i_cindex from struct inode Stefan Richter 0 siblings, 2 replies; 9+ messages in thread From: Theodore Ts'o @ 2009-05-21 20:01 UTC (permalink / raw) To: linux-kernel; +Cc: linux-fsdevel, Theodore Ts'o, krh, stefanr The only user of the i_cindex element in the inode structure is used is by the firewire drivers. As part of an attempt to slim down the inode structure to save memory --- since a typical Linux system will have hundreds of thousands if not millions of inodes cached, a reduction in the size inode has high leverage. The firewire driver does not need i_cindex in any fast path, so it's simple enough to calculate when it is needed, instead of wasting space in the inode structure. Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> Cc: krh@redhat.com Cc: stefanr@s5r6.in-berlin.de Cc: linux-fsdevel@vger.kernel.org --- drivers/ieee1394/dv1394.c | 5 +++-- drivers/ieee1394/ieee1394_core.h | 5 ++++- fs/char_dev.c | 14 +++++++++++++- include/linux/cdev.h | 2 ++ include/linux/fs.h | 1 - 5 files changed, 22 insertions(+), 5 deletions(-) diff --git a/drivers/ieee1394/dv1394.c b/drivers/ieee1394/dv1394.c index 823a629..2cd00b5 100644 --- a/drivers/ieee1394/dv1394.c +++ b/drivers/ieee1394/dv1394.c @@ -1789,12 +1789,13 @@ static int dv1394_open(struct inode *inode, struct file *file) } else { /* look up the card by ID */ unsigned long flags; + int idx = ieee1394_file_to_instance(file); spin_lock_irqsave(&dv1394_cards_lock, flags); if (!list_empty(&dv1394_cards)) { struct video_card *p; list_for_each_entry(p, &dv1394_cards, list) { - if ((p->id) == ieee1394_file_to_instance(file)) { + if ((p->id) == idx) { video = p; break; } @@ -1803,7 +1804,7 @@ static int dv1394_open(struct inode *inode, struct file *file) spin_unlock_irqrestore(&dv1394_cards_lock, flags); if (!video) { - debug_printk("dv1394: OHCI card %d not found", ieee1394_file_to_instance(file)); + debug_printk("dv1394: OHCI card %d not found", idx); return -ENODEV; } diff --git a/drivers/ieee1394/ieee1394_core.h b/drivers/ieee1394/ieee1394_core.h index 21d50f7..fda0f5e 100644 --- a/drivers/ieee1394/ieee1394_core.h +++ b/drivers/ieee1394/ieee1394_core.h @@ -155,7 +155,10 @@ void hpsb_packet_received(struct hpsb_host *host, quadlet_t *data, size_t size, */ static inline unsigned char ieee1394_file_to_instance(struct file *file) { - return file->f_path.dentry->d_inode->i_cindex; + int idx = cdev_index(file->f_path.dentry->d_inode); + if (idx < 0) + idx = 0; + return idx; } extern int hpsb_disable_irm; diff --git a/fs/char_dev.c b/fs/char_dev.c index 38f7122..b7c9d51 100644 --- a/fs/char_dev.c +++ b/fs/char_dev.c @@ -375,7 +375,6 @@ static int chrdev_open(struct inode *inode, struct file *filp) p = inode->i_cdev; if (!p) { inode->i_cdev = p = new; - inode->i_cindex = idx; list_add(&inode->i_devices, &p->list); new = NULL; } else if (!cdev_get(p)) @@ -405,6 +404,18 @@ static int chrdev_open(struct inode *inode, struct file *filp) return ret; } +int cdev_index(struct inode *inode) +{ + int idx; + struct kobject *kobj; + + kobj = kobj_lookup(cdev_map, inode->i_rdev, &idx); + if (!kobj) + return -1; + kobject_put(kobj); + return idx; +} + void cd_forget(struct inode *inode) { spin_lock(&cdev_lock); @@ -557,6 +568,7 @@ EXPORT_SYMBOL(cdev_init); EXPORT_SYMBOL(cdev_alloc); EXPORT_SYMBOL(cdev_del); EXPORT_SYMBOL(cdev_add); +EXPORT_SYMBOL(cdev_index); EXPORT_SYMBOL(register_chrdev); EXPORT_SYMBOL(unregister_chrdev); EXPORT_SYMBOL(directly_mappable_cdev_bdi); diff --git a/include/linux/cdev.h b/include/linux/cdev.h index fb45919..f389e31 100644 --- a/include/linux/cdev.h +++ b/include/linux/cdev.h @@ -28,6 +28,8 @@ int cdev_add(struct cdev *, dev_t, unsigned); void cdev_del(struct cdev *); +int cdev_index(struct inode *inode); + void cd_forget(struct inode *); extern struct backing_dev_info directly_mappable_cdev_bdi; diff --git a/include/linux/fs.h b/include/linux/fs.h index 9b20f13..e3b8334 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -751,7 +751,6 @@ struct inode { struct block_device *i_bdev; struct cdev *i_cdev; }; - int i_cindex; __u32 i_generation; -- 1.6.3.1.1.g75fc.dirty ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/5] fs: Slim down inode by only using an unsigned int for i_dnotify_mask 2009-05-21 20:01 ` [PATCH 2/5] fs: Remove i_cindex from struct inode Theodore Ts'o @ 2009-05-21 20:01 ` Theodore Ts'o 2009-05-21 20:01 ` [PATCH 4/5] fs: Rearrange inode structure elements to avoid waste due to padding Theodore Ts'o 2009-05-21 20:30 ` [PATCH 2/5] fs: Remove i_cindex from struct inode Stefan Richter 1 sibling, 1 reply; 9+ messages in thread From: Theodore Ts'o @ 2009-05-21 20:01 UTC (permalink / raw) To: linux-kernel; +Cc: linux-fsdevel, Theodore Ts'o, sfr The dnotify mask values range from 0x0001 through 0x0020, as well as 0x80000000 (for DN_MULTISHOT). There is no need for this field to be an unsigned long (which is 8 bytes on 64-bit systems). Using an unsigned int will save 4 bytes for the inode structure on 64-bit systems. Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> Cc: sfr@canb.auug.org.au Cc: linux-fsdevel@vger.kernel.org --- include/linux/dnotify.h | 2 +- include/linux/fs.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/dnotify.h b/include/linux/dnotify.h index 102a902..d573542 100644 --- a/include/linux/dnotify.h +++ b/include/linux/dnotify.h @@ -10,7 +10,7 @@ struct dnotify_struct { struct dnotify_struct * dn_next; - unsigned long dn_mask; + unsigned int dn_mask; int dn_fd; struct file * dn_filp; fl_owner_t dn_owner; diff --git a/include/linux/fs.h b/include/linux/fs.h index e3b8334..7c73c59 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -755,7 +755,7 @@ struct inode { __u32 i_generation; #ifdef CONFIG_DNOTIFY - unsigned long i_dnotify_mask; /* Directory notify events */ + unsigned int i_dnotify_mask; /* Directory notify events */ struct dnotify_struct *i_dnotify; /* for directory notifications */ #endif -- 1.6.3.1.1.g75fc.dirty ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/5] fs: Rearrange inode structure elements to avoid waste due to padding 2009-05-21 20:01 ` [PATCH 3/5] fs: Slim down inode by only using an unsigned int for i_dnotify_mask Theodore Ts'o @ 2009-05-21 20:01 ` Theodore Ts'o 2009-05-21 20:01 ` [PATCH 5/5] Dump the inode structure (for debugging purposes only) Theodore Ts'o 0 siblings, 1 reply; 9+ messages in thread From: Theodore Ts'o @ 2009-05-21 20:01 UTC (permalink / raw) To: linux-kernel; +Cc: linux-fsdevel, Theodore Ts'o Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> Cc: linux-fsdevel@vger.kernel.org --- include/linux/fs.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/include/linux/fs.h b/include/linux/fs.h index 7c73c59..88bd16a 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -729,8 +729,8 @@ struct inode { struct timespec i_atime; struct timespec i_mtime; struct timespec i_ctime; - unsigned int i_blkbits; blkcnt_t i_blocks; + unsigned int i_blkbits; unsigned short i_bytes; umode_t i_mode; spinlock_t i_lock; /* i_blocks, i_bytes, maybe i_size */ -- 1.6.3.1.1.g75fc.dirty ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 5/5] Dump the inode structure (for debugging purposes only) 2009-05-21 20:01 ` [PATCH 4/5] fs: Rearrange inode structure elements to avoid waste due to padding Theodore Ts'o @ 2009-05-21 20:01 ` Theodore Ts'o 2009-05-21 20:09 ` Matthew Wilcox 0 siblings, 1 reply; 9+ messages in thread From: Theodore Ts'o @ 2009-05-21 20:01 UTC (permalink / raw) To: linux-kernel; +Cc: linux-fsdevel, Theodore Ts'o This is not intended for merging; but it's useful for determining how much varous fields in the struct inode space, and whether there is any padding leading to waste, especially on a 64-bit platforms. For example: Begin dump of struct inode (size 1144) i_hash : 0 16 0 i_list : 16 16 0 i_sb_list : 32 16 0 i_dentry : 48 16 0 i_ino : 64 8 0 i_count : 72 4 0 i_nlink : 76 4 0 i_uid : 80 4 0 i_gid : 84 4 0 i_rdev : 88 4 0 i_version : 96 8 4 i_size : 104 8 0 i_atime : 112 16 0 i_mtime : 128 16 0 i_ctime : 144 16 0 i_blocks : 160 8 0 i_blkbits : 168 4 0 i_bytes : 172 2 0 i_mode : 174 2 0 i_lock : 176 64 0 i_mutex : 240 152 0 i_alloc_sem: 392 128 0 i_op : 520 8 0 i_fop : 528 8 0 i_sb : 536 8 0 i_flock : 544 8 0 i_mapping : 552 8 0 i_data : 560 328 0 i_dquot : 888 16 0 i_devices : 904 16 0 i_pipe : 920 8 0 i_bdev : 920 8 -8 i_cdev : 920 8 -8 i_generation: 928 4 0 i_dnotify_mask: 932 4 0 i_dnotify : 936 8 0 inotify_watches: 944 16 0 inotify_mutex: 960 152 0 dirtied_when: 1112 8 0 i_state : 1120 2 0 i_flags : 1122 2 0 i_writecount: 1124 4 0 i_security: 1128 8 0 i_private : 1136 8 0 This shows us that i_data, an struct address_space data structure, takes 328 bytes; i_mutex takes 152 bytes, the i_alloc_sem rw_semaphore takes 128 bytes, and the i_lock spinlock takes 64 bytes. A lot of this space is due to varous LOCKDEP and spinlock debugging options. (With all debugging turned off, the inode structure shrinks from 1144 bytes to 560 bytes, with i_data being 144 bytes, i_mutex being 32 bytes, i_alloc_sem being 24 bytes, and i_lock taking 4 bytes.) --- fs/Makefile | 1 + fs/inode-struct-dumper.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 0 deletions(-) create mode 100644 fs/inode-struct-dumper.c diff --git a/fs/Makefile b/fs/Makefile index af6d047..196f3f5 100644 --- a/fs/Makefile +++ b/fs/Makefile @@ -29,6 +29,7 @@ obj-$(CONFIG_EVENTFD) += eventfd.o obj-$(CONFIG_AIO) += aio.o obj-$(CONFIG_FILE_LOCKING) += locks.o obj-$(CONFIG_COMPAT) += compat.o compat_ioctl.o +obj-y += inode-struct-dumper.o nfsd-$(CONFIG_NFSD) := nfsctl.o obj-y += $(nfsd-y) $(nfsd-m) diff --git a/fs/inode-struct-dumper.c b/fs/inode-struct-dumper.c new file mode 100644 index 0000000..c85d792 --- /dev/null +++ b/fs/inode-struct-dumper.c @@ -0,0 +1,87 @@ +/* + * inode struct dumper --- dump out the inode structure + */ + +#include <linux/module.h> +#include <linux/init.h> + +#include <linux/kernel.h> +#include <linux/fs.h> +#include <linux/stddef.h> + +#define ELEMENT(xx) { \ + offset=offsetof(struct inode, xx); \ + size=sizeof(p->xx); \ + printk(KERN_NOTICE "%-10s: %d %d %d\n", #xx, \ + offset, size, offset-(last_offset+last_size)); \ + last_offset=offset; \ + last_size = size; \ + } + +static int __init dumper_init(void) +{ + struct inode *p; + int offset, size, last_offset=0, last_size=0; + + printk(KERN_NOTICE "Begin dump of struct inode (size %lu)\n", + sizeof(struct inode)); + ELEMENT(i_hash); + ELEMENT(i_list); + ELEMENT(i_sb_list); + ELEMENT(i_dentry); + ELEMENT(i_ino); + ELEMENT(i_count); + ELEMENT(i_nlink); + ELEMENT(i_uid); + ELEMENT(i_gid); + ELEMENT(i_rdev); + ELEMENT(i_version); + ELEMENT(i_size); +#ifdef __NEED_I_SIZE_ORDERED + ELEMENT(i_size_seqcount); +#endif + ELEMENT(i_atime); + ELEMENT(i_mtime); + ELEMENT(i_ctime); + ELEMENT(i_blocks); + ELEMENT(i_blkbits); + ELEMENT(i_bytes); + ELEMENT(i_mode); + ELEMENT(i_lock); + ELEMENT(i_mutex); + ELEMENT(i_alloc_sem); + ELEMENT(i_op); + ELEMENT(i_fop); + ELEMENT(i_sb); + ELEMENT(i_flock); + ELEMENT(i_mapping); + ELEMENT(i_data); +#ifdef CONFIG_QUOTA + ELEMENT(i_dquot); +#endif + ELEMENT(i_devices); + ELEMENT(i_pipe); + ELEMENT(i_bdev); + ELEMENT(i_cdev); + ELEMENT(i_generation); +#ifdef CONFIG_DNOTIFY + ELEMENT(i_dnotify_mask); + ELEMENT(i_dnotify); +#endif +#ifdef CONFIG_INOTIFY + ELEMENT(inotify_watches); + ELEMENT(inotify_mutex); +#endif + ELEMENT(dirtied_when); + ELEMENT(i_state); + ELEMENT(i_flags); + ELEMENT(i_writecount); +#ifdef CONFIG_SECURITY + ELEMENT(i_security); +#endif + ELEMENT(i_private); + return 0; +} + +module_init(dumper_init); + -- 1.6.3.1.1.g75fc.dirty ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 5/5] Dump the inode structure (for debugging purposes only) 2009-05-21 20:01 ` [PATCH 5/5] Dump the inode structure (for debugging purposes only) Theodore Ts'o @ 2009-05-21 20:09 ` Matthew Wilcox 2009-05-21 20:28 ` Theodore Tso 0 siblings, 1 reply; 9+ messages in thread From: Matthew Wilcox @ 2009-05-21 20:09 UTC (permalink / raw) To: Theodore Ts'o; +Cc: linux-kernel, linux-fsdevel On Thu, May 21, 2009 at 04:01:03PM -0400, Theodore Ts'o wrote: > This is not intended for merging; but it's useful for determining how > much varous fields in the struct inode space, and whether there is any > padding leading to waste, especially on a 64-bit platforms. Why not just use acme's pahole? -- Matthew Wilcox Intel Open Source Technology Centre "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 5/5] Dump the inode structure (for debugging purposes only) 2009-05-21 20:09 ` Matthew Wilcox @ 2009-05-21 20:28 ` Theodore Tso 0 siblings, 0 replies; 9+ messages in thread From: Theodore Tso @ 2009-05-21 20:28 UTC (permalink / raw) To: Matthew Wilcox; +Cc: linux-kernel, linux-fsdevel On Thu, May 21, 2009 at 02:09:42PM -0600, Matthew Wilcox wrote: > On Thu, May 21, 2009 at 04:01:03PM -0400, Theodore Ts'o wrote: > > This is not intended for merging; but it's useful for determining how > > much varous fields in the struct inode space, and whether there is any > > padding leading to waste, especially on a 64-bit platforms. > > Why not just use acme's pahole? > Because I didn't know about it, and now that I've looked at it, it requires compiling the kernel with CONFIG_DEBUG_INFO, which means the kernel takes a lot longer to build. What I did is a bit more hackish, to be sure, but it's faster to drop it into the tree and then run it under kvm and then pull it out of the console logs. If you were planning on investigating a large number of structures, acme's solution probably is a better one; but if you only need one, it might be faster to whip up a custom kernel module, perhaps built out of the tree, instead of rebuilding the entire kernel with CONFIG_DEBUG_INFO. - Ted ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/5] fs: Remove i_cindex from struct inode 2009-05-21 20:01 ` [PATCH 2/5] fs: Remove i_cindex from struct inode Theodore Ts'o 2009-05-21 20:01 ` [PATCH 3/5] fs: Slim down inode by only using an unsigned int for i_dnotify_mask Theodore Ts'o @ 2009-05-21 20:30 ` Stefan Richter 1 sibling, 0 replies; 9+ messages in thread From: Stefan Richter @ 2009-05-21 20:30 UTC (permalink / raw) To: Theodore Ts'o; +Cc: linux-kernel, linux-fsdevel, krh Theodore Ts'o wrote: [...] > The firewire driver does not need i_cindex in any fast path, so it's > simple enough to calculate when it is needed, instead of wasting space > in the inode structure. > > Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> > Cc: krh@redhat.com > Cc: stefanr@s5r6.in-berlin.de > Cc: linux-fsdevel@vger.kernel.org > --- > drivers/ieee1394/dv1394.c | 5 +++-- > drivers/ieee1394/ieee1394_core.h | 5 ++++- > fs/char_dev.c | 14 +++++++++++++- > include/linux/cdev.h | 2 ++ > include/linux/fs.h | 1 - > 5 files changed, 22 insertions(+), 5 deletions(-) [...] Acked-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (No Ack needed from krh, he is concerned with drivers/firewire, not really with driver/ieee1394. :-) -- Stefan Richter -=====-=-=== -=-= -==-= http://arcgraph.de/sr/ ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-05-21 20:30 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-05-21 20:00 [RFC PATCH 0/5] Put struct inode on a diet Theodore Ts'o 2009-05-21 20:00 ` [PATCH 1/5] fs: i_flags and i_state in struct inode only need to be unsigned short Theodore Ts'o 2009-05-21 20:01 ` [PATCH 2/5] fs: Remove i_cindex from struct inode Theodore Ts'o 2009-05-21 20:01 ` [PATCH 3/5] fs: Slim down inode by only using an unsigned int for i_dnotify_mask Theodore Ts'o 2009-05-21 20:01 ` [PATCH 4/5] fs: Rearrange inode structure elements to avoid waste due to padding Theodore Ts'o 2009-05-21 20:01 ` [PATCH 5/5] Dump the inode structure (for debugging purposes only) Theodore Ts'o 2009-05-21 20:09 ` Matthew Wilcox 2009-05-21 20:28 ` Theodore Tso 2009-05-21 20:30 ` [PATCH 2/5] fs: Remove i_cindex from struct inode Stefan Richter
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.