[RFC PATCH 0/2] A new way of attaching information to inodes

[RFC PATCH 0/2] A new way of attaching information to inodes

[Jan Kara]

  Hi,

  currently our VFS inode structure is quite big. It contains quite some
members that are useful only in some cases (e.g. device pointers and list head
used only when the inode represents a block/character device, quota pointers
used only when the filesystem actually supports quota, etc.). And it would
be helpful to add some more so that we can handle ACL's in generic code, or
we can do some kind of block reservation for mmaped writes, and there are
other cases.
  So I though it may be worth a try to come up with a way to attach info to
inode structure so that generic code can look it up (or find it's not there),
it's space effective and on the other hand the access does not cost us too
much.
  What I've come up with is that each inode could have a pointer to a (usually
static) table of offsets of structures associated with the inode. Filesystem
would then carry the structures it is interested in in it's private inode.
  As an example, I've converted quota pointers from inode to use this feature
and ext2 filesystem so that we have some rough idea how the resulting code
looks like. IMO from the filesystem's POV it's quite fine, the generic code
gets a bit less obvious but it's bearable as well. Regarding the speed, we
impose additinal lookup in the table and an addition of the offset from the
table so it should be IMHO acceptable cost for things that are not really fast
path.
  What do you think? Your opinions and suggestions are welcome...

								Honza

[PATCH 1/2] Implement trivial struct feature macros

[Jan Kara]

Sometimes it is useful to have a generic main structure (like struct inode)
and then with some of these structures you want to associate additional
information (like quota information, block device information etc.).
Traditionally, we solved this by having a pointer to associated data in
the generic structure. This gets inefficient when there are lots of
various kinds of data that can be associated with the generic structure
(we need one pointer per type of associated information). This is an
attempt to address the issue.

The idea is simple. There is some chunk of allocated memory that contains
the generic structure and all the associated information. The generic
structure has a pointer to the (usually static) table of offsets (from
the beginning of the generic structure) of associated structures for
each possible associated structure type.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 include/linux/fs.h             |    5 +++++
 include/linux/struct_feature.h |   12 ++++++++++++
 2 files changed, 17 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/struct_feature.h

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 5bed436..2404b16 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -710,6 +710,10 @@ static inline int mapping_writably_mapped(struct address_space *mapping)
 #define i_size_ordered_init(inode) do { } while (0)
 #endif
 
+enum {
+	INODE_FEATURES
+};
+
 struct inode {
 	struct hlist_node	i_hash;
 	struct list_head	i_list;
@@ -775,6 +779,7 @@ struct inode {
 	void			*i_security;
 #endif
 	void			*i_private; /* fs or device private pointer */
+	int			*feature_table;
 };
 
 /*
diff --git a/include/linux/struct_feature.h b/include/linux/struct_feature.h
new file mode 100644
index 0000000..0849d3d
--- /dev/null
+++ b/include/linux/struct_feature.h
@@ -0,0 +1,12 @@
+#ifndef _LINUX_STRUCT_FEATURE_H
+#define _LINUX_STRUCT_FEATURE_H
+
+
+#define FEATURE_OFFSET(str, generic, feature) (offsetof(str, feature) - offsetof(str, generic))
+
+#define GET_FEATURE(generic, feature) \
+	((generic)->feature_table && (generic)->feature_table[feature] ? \
+	((void *)(((char *)(generic)) + (generic)->feature_table[feature])) : \
+	NULL)
+
+#endif
-- 
1.6.0.2

Re: [PATCH 1/2] Implement trivial struct feature macros

[Christoph Hellwig]

On Thu, Apr 30, 2009 at 07:31:17PM +0200, Jan Kara wrote:
> Sometimes it is useful to have a generic main structure (like struct inode)
> and then with some of these structures you want to associate additional
> information (like quota information, block device information etc.).
> Traditionally, we solved this by having a pointer to associated data in
> the generic structure. This gets inefficient when there are lots of
> various kinds of data that can be associated with the generic structure
> (we need one pointer per type of associated information). This is an
> attempt to address the issue.
> 
> The idea is simple. There is some chunk of allocated memory that contains
> the generic structure and all the associated information. The generic
> structure has a pointer to the (usually static) table of offsets (from
> the beginning of the generic structure) of associated structures for
> each possible associated structure type.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  include/linux/fs.h             |    5 +++++
>  include/linux/struct_feature.h |   12 ++++++++++++
>  2 files changed, 17 insertions(+), 0 deletions(-)
>  create mode 100644 include/linux/struct_feature.h
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 5bed436..2404b16 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -710,6 +710,10 @@ static inline int mapping_writably_mapped(struct address_space *mapping)
>  #define i_size_ordered_init(inode) do { } while (0)
>  #endif
>  
> +enum {
> +	INODE_FEATURES
> +};
> +
>  struct inode {
>  	struct hlist_node	i_hash;
>  	struct list_head	i_list;
> @@ -775,6 +779,7 @@ struct inode {
>  	void			*i_security;
>  #endif
>  	void			*i_private; /* fs or device private pointer */
> +	int			*feature_table;
>  };
>  
>  /*
> diff --git a/include/linux/struct_feature.h b/include/linux/struct_feature.h
> new file mode 100644
> index 0000000..0849d3d
> --- /dev/null
> +++ b/include/linux/struct_feature.h
> @@ -0,0 +1,12 @@
> +#ifndef _LINUX_STRUCT_FEATURE_H
> +#define _LINUX_STRUCT_FEATURE_H
> +
> +
> +#define FEATURE_OFFSET(str, generic, feature) (offsetof(str, feature) - offsetof(str, generic))
> +
> +#define GET_FEATURE(generic, feature) \
> +	((generic)->feature_table && (generic)->feature_table[feature] ? \
> +	((void *)(((char *)(generic)) + (generic)->feature_table[feature])) : \
> +	NULL)

These should be inlines.

Also I don't think we should make it super-generic.  If we'll eventually
need it it should be specific to struct inode, something like:

static void *get_inode_extension(struct inode *inode, emum inode_extension ext)
{
	if (!inode->i_extensions[ext])
		return NULL;
	return (char *)inode + inode->i_extensions[ext];
}

[PATCH 2/2] Use feature code to eliminate quota pointers from inodes for filesystems that don't need it.

[Jan Kara]

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext2/ext2.h     |    1 +
 fs/ext2/super.c    |    5 ++
 fs/inode.c         |    7 ++-
 fs/nilfs2/mdt.c    |    3 -
 fs/quota/dquot.c   |  134 ++++++++++++++++++++++++++++++----------------------
 include/linux/fs.h |   10 +++-
 6 files changed, 97 insertions(+), 63 deletions(-)

diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index 3203042..1e19e17 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -62,6 +62,7 @@ struct ext2_inode_info {
 	struct mutex truncate_mutex;
 	struct inode	vfs_inode;
 	struct list_head i_orphan;	/* unlinked but open inodes */
+	struct inode_quota i_quota;
 };
 
 /*
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index f983225..6c3dcc7 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -32,6 +32,7 @@
 #include <linux/mount.h>
 #include <linux/log2.h>
 #include <linux/quotaops.h>
+#include <linux/struct_feature.h>
 #include <asm/uaccess.h>
 #include "ext2.h"
 #include "xattr.h"
@@ -139,6 +140,9 @@ static void ext2_put_super (struct super_block * sb)
 }
 
 static struct kmem_cache * ext2_inode_cachep;
+static int ext2_i_features[INODE_FEATURES] = {
+	[INODE_QUOTA] = FEATURE_OFFSET(struct ext2_inode_info, vfs_inode, i_quota),
+};
 
 static struct inode *ext2_alloc_inode(struct super_block *sb)
 {
@@ -152,6 +156,7 @@ static struct inode *ext2_alloc_inode(struct super_block *sb)
 #endif
 	ei->i_block_alloc_info = NULL;
 	ei->vfs_inode.i_version = 1;
+	ei->vfs_inode.feature_table = ext2_i_features;
 	return &ei->vfs_inode;
 }
 
diff --git a/fs/inode.c b/fs/inode.c
index 6ad14a1..0761e5b 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -24,6 +24,7 @@
 #include <linux/inotify.h>
 #include <linux/mount.h>
 #include <linux/async.h>
+#include <linux/struct_feature.h>
 
 /*
  * This is needed for the following functions:
@@ -141,7 +142,11 @@ struct inode *inode_init_always(struct super_block *sb, struct inode *inode)
 	inode->i_bytes = 0;
 	inode->i_generation = 0;
 #ifdef CONFIG_QUOTA
-	memset(&inode->i_dquot, 0, sizeof(inode->i_dquot));
+	{
+		struct inode_quota *iq = GET_FEATURE(inode, INODE_QUOTA);
+		if (iq)
+			memset(iq, 0, sizeof(*iq));
+	}
 #endif
 	inode->i_pipe = NULL;
 	inode->i_bdev = NULL;
diff --git a/fs/nilfs2/mdt.c b/fs/nilfs2/mdt.c
index 47dd815..725cd80 100644
--- a/fs/nilfs2/mdt.c
+++ b/fs/nilfs2/mdt.c
@@ -478,9 +478,6 @@ nilfs_mdt_new_common(struct the_nilfs *nilfs, struct super_block *sb,
 		inode->i_blocks = 0;
 		inode->i_bytes = 0;
 		inode->i_generation = 0;
-#ifdef CONFIG_QUOTA
-		memset(&inode->i_dquot, 0, sizeof(inode->i_dquot));
-#endif
 		inode->i_pipe = NULL;
 		inode->i_bdev = NULL;
 		inode->i_cdev = NULL;
diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 607c579..d1ce6b8 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -77,6 +77,7 @@
 #include <linux/capability.h>
 #include <linux/quotaops.h>
 #include <linux/writeback.h> /* for inode_lock, oddly enough.. */
+#include <linux/struct_feature.h>
 #ifdef CONFIG_QUOTA_NETLINK_INTERFACE
 #include <net/netlink.h>
 #include <net/genetlink.h>
@@ -804,14 +805,15 @@ EXPORT_SYMBOL(dqget);
 
 static int dqinit_needed(struct inode *inode, int type)
 {
+	struct inode_quota *iq = GET_FEATURE(inode, INODE_QUOTA);
 	int cnt;
 
-	if (IS_NOQUOTA(inode))
+	if (IS_NOQUOTA(inode) || !iq)
 		return 0;
 	if (type != -1)
-		return !inode->i_dquot[type];
+		return !iq->dquot[type];
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++)
-		if (!inode->i_dquot[cnt])
+		if (!iq->dquot[cnt])
 			return 1;
 	return 0;
 }
@@ -866,9 +868,13 @@ static inline int dqput_blocks(struct dquot *dquot)
 static int remove_inode_dquot_ref(struct inode *inode, int type,
 				  struct list_head *tofree_head)
 {
-	struct dquot *dquot = inode->i_dquot[type];
+	struct inode_quota *iq = GET_FEATURE(inode, INODE_QUOTA);
+	struct dquot *dquot;
 
-	inode->i_dquot[type] = NULL;
+	if (!iq)
+		return 0;
+	dquot = iq->dquot[type];
+	iq->dquot[type] = NULL;
 	if (dquot) {
 		if (dqput_blocks(dquot)) {
 #ifdef __DQUOT_PARANOIA
@@ -1298,10 +1304,11 @@ int dquot_initialize(struct inode *inode, int type)
 	int cnt, ret = 0;
 	struct dquot *got[MAXQUOTAS] = { NULL, NULL };
 	struct super_block *sb = inode->i_sb;
+	struct inode_quota *iq = GET_FEATURE(inode, INODE_QUOTA);
 
 	/* First test before acquiring mutex - solves deadlocks when we
          * re-enter the quota code and are already holding the mutex */
-	if (IS_NOQUOTA(inode))
+	if (IS_NOQUOTA(inode) || !iq)
 		return 0;
 
 	/* First get references to structures we might need. */
@@ -1329,8 +1336,8 @@ int dquot_initialize(struct inode *inode, int type)
 		/* Avoid races with quotaoff() */
 		if (!sb_has_quota_active(sb, cnt))
 			continue;
-		if (!inode->i_dquot[cnt]) {
-			inode->i_dquot[cnt] = got[cnt];
+		if (!iq->dquot[cnt]) {
+			iq->dquot[cnt] = got[cnt];
 			got[cnt] = NULL;
 		}
 	}
@@ -1350,11 +1357,15 @@ int dquot_drop(struct inode *inode)
 {
 	int cnt;
 	struct dquot *put[MAXQUOTAS];
+	struct inode_quota *iq = GET_FEATURE(inode, INODE_QUOTA);
+
+	if (!iq)
+		return 0;
 
 	down_write(&sb_dqopt(inode->i_sb)->dqptr_sem);
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-		put[cnt] = inode->i_dquot[cnt];
-		inode->i_dquot[cnt] = NULL;
+		put[cnt] = iq->dquot[cnt];
+		iq->dquot[cnt] = NULL;
 	}
 	up_write(&sb_dqopt(inode->i_sb)->dqptr_sem);
 
@@ -1373,13 +1384,17 @@ void vfs_dq_drop(struct inode *inode)
 	if (!IS_NOQUOTA(inode) && inode->i_sb && inode->i_sb->dq_op
 	    && inode->i_sb->dq_op->drop) {
 		int cnt;
+		struct inode_quota *iq = GET_FEATURE(inode, INODE_QUOTA);
+
 		/* Test before calling to rule out calls from proc and such
                  * where we are not allowed to block. Note that this is
 		 * actually reliable test even without the lock - the caller
 		 * must assure that nobody can come after the DQUOT_DROP and
 		 * add quota pointers back anyway */
+		if (!iq)
+			return;
 		for (cnt = 0; cnt < MAXQUOTAS; cnt++)
-			if (inode->i_dquot[cnt])
+			if (iq->dquot[cnt])
 				break;
 		if (cnt < MAXQUOTAS)
 			inode->i_sb->dq_op->drop(inode);
@@ -1399,50 +1414,52 @@ EXPORT_SYMBOL(vfs_dq_drop);
 /*
  * This operation can block, but only after everything is updated
  */
-int __dquot_alloc_space(struct inode *inode, qsize_t number,
-			int warn, int reserve)
+static int __dquot_alloc_space(struct inode *inode, qsize_t number,
+			       int warn, int reserve)
 {
 	int cnt, ret = QUOTA_OK;
 	char warntype[MAXQUOTAS];
+	struct inode_quota *iq = GET_FEATURE(inode, INODE_QUOTA);
 
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++)
 		warntype[cnt] = QUOTA_NL_NOWARN;
 
 	spin_lock(&dq_data_lock);
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-		if (!inode->i_dquot[cnt])
+		if (!iq->dquot[cnt])
 			continue;
-		if (check_bdq(inode->i_dquot[cnt], number, warn, warntype+cnt)
+		if (check_bdq(iq->dquot[cnt], number, warn, warntype+cnt)
 		    == NO_QUOTA) {
 			ret = NO_QUOTA;
 			goto out_unlock;
 		}
 	}
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-		if (!inode->i_dquot[cnt])
+		if (!iq->dquot[cnt])
 			continue;
 		if (reserve)
-			dquot_resv_space(inode->i_dquot[cnt], number);
+			dquot_resv_space(iq->dquot[cnt], number);
 		else
-			dquot_incr_space(inode->i_dquot[cnt], number);
+			dquot_incr_space(iq->dquot[cnt], number);
 	}
 	if (!reserve)
 		inode_add_bytes(inode, number);
 out_unlock:
 	spin_unlock(&dq_data_lock);
-	flush_warnings(inode->i_dquot, warntype);
+	flush_warnings(iq->dquot, warntype);
 	return ret;
 }
 
 int dquot_alloc_space(struct inode *inode, qsize_t number, int warn)
 {
 	int cnt, ret = QUOTA_OK;
+	struct inode_quota *iq = GET_FEATURE(inode, INODE_QUOTA);
 
 	/*
 	 * First test before acquiring mutex - solves deadlocks when we
 	 * re-enter the quota code and are already holding the mutex
 	 */
-	if (IS_NOQUOTA(inode)) {
+	if (IS_NOQUOTA(inode) || !iq) {
 		inode_add_bytes(inode, number);
 		goto out;
 	}
@@ -1459,8 +1476,8 @@ int dquot_alloc_space(struct inode *inode, qsize_t number, int warn)
 
 	/* Dirtify all the dquots - this can block when journalling */
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++)
-		if (inode->i_dquot[cnt])
-			mark_dquot_dirty(inode->i_dquot[cnt]);
+		if (iq->dquot[cnt])
+			mark_dquot_dirty(iq->dquot[cnt]);
 out_unlock:
 	up_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
 out:
@@ -1494,10 +1511,11 @@ int dquot_alloc_inode(const struct inode *inode, qsize_t number)
 {
 	int cnt, ret = NO_QUOTA;
 	char warntype[MAXQUOTAS];
+	struct inode_quota *iq = GET_FEATURE(inode, INODE_QUOTA);
 
 	/* First test before acquiring mutex - solves deadlocks when we
          * re-enter the quota code and are already holding the mutex */
-	if (IS_NOQUOTA(inode))
+	if (IS_NOQUOTA(inode) || !iq)
 		return QUOTA_OK;
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++)
 		warntype[cnt] = QUOTA_NL_NOWARN;
@@ -1508,17 +1526,17 @@ int dquot_alloc_inode(const struct inode *inode, qsize_t number)
 	}
 	spin_lock(&dq_data_lock);
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-		if (!inode->i_dquot[cnt])
+		if (!iq->dquot[cnt])
 			continue;
-		if (check_idq(inode->i_dquot[cnt], number, warntype+cnt)
+		if (check_idq(iq->dquot[cnt], number, warntype+cnt)
 		    == NO_QUOTA)
 			goto warn_put_all;
 	}
 
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-		if (!inode->i_dquot[cnt])
+		if (!iq->dquot[cnt])
 			continue;
-		dquot_incr_inodes(inode->i_dquot[cnt], number);
+		dquot_incr_inodes(iq->dquot[cnt], number);
 	}
 	ret = QUOTA_OK;
 warn_put_all:
@@ -1526,9 +1544,9 @@ warn_put_all:
 	if (ret == QUOTA_OK)
 		/* Dirtify all the dquots - this can block when journalling */
 		for (cnt = 0; cnt < MAXQUOTAS; cnt++)
-			if (inode->i_dquot[cnt])
-				mark_dquot_dirty(inode->i_dquot[cnt]);
-	flush_warnings(inode->i_dquot, warntype);
+			if (iq->dquot[cnt])
+				mark_dquot_dirty(iq->dquot[cnt]);
+	flush_warnings(iq->dquot, warntype);
 	up_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
 	return ret;
 }
@@ -1538,8 +1556,9 @@ int dquot_claim_space(struct inode *inode, qsize_t number)
 {
 	int cnt;
 	int ret = QUOTA_OK;
+	struct inode_quota *iq = GET_FEATURE(inode, INODE_QUOTA);
 
-	if (IS_NOQUOTA(inode)) {
+	if (IS_NOQUOTA(inode) || !iq) {
 		inode_add_bytes(inode, number);
 		goto out;
 	}
@@ -1554,17 +1573,16 @@ int dquot_claim_space(struct inode *inode, qsize_t number)
 	spin_lock(&dq_data_lock);
 	/* Claim reserved quotas to allocated quotas */
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-		if (inode->i_dquot[cnt])
-			dquot_claim_reserved_space(inode->i_dquot[cnt],
-							number);
+		if (iq->dquot[cnt])
+			dquot_claim_reserved_space(iq->dquot[cnt], number);
 	}
 	/* Update inode bytes */
 	inode_add_bytes(inode, number);
 	spin_unlock(&dq_data_lock);
 	/* Dirtify all the dquots - this can block when journalling */
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++)
-		if (inode->i_dquot[cnt])
-			mark_dquot_dirty(inode->i_dquot[cnt]);
+		if (iq->dquot[cnt])
+			mark_dquot_dirty(iq->dquot[cnt]);
 	up_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
 out:
 	return ret;
@@ -1577,8 +1595,9 @@ EXPORT_SYMBOL(dquot_claim_space);
 void dquot_release_reserved_space(struct inode *inode, qsize_t number)
 {
 	int cnt;
+	struct inode_quota *iq = GET_FEATURE(inode, INODE_QUOTA);
 
-	if (IS_NOQUOTA(inode))
+	if (IS_NOQUOTA(inode) || !iq)
 		goto out;
 
 	down_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
@@ -1588,8 +1607,8 @@ void dquot_release_reserved_space(struct inode *inode, qsize_t number)
 	spin_lock(&dq_data_lock);
 	/* Release reserved dquots */
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-		if (inode->i_dquot[cnt])
-			dquot_free_reserved_space(inode->i_dquot[cnt], number);
+		if (iq->dquot[cnt])
+			dquot_free_reserved_space(iq->dquot[cnt], number);
 	}
 	spin_unlock(&dq_data_lock);
 
@@ -1607,10 +1626,11 @@ int dquot_free_space(struct inode *inode, qsize_t number)
 {
 	unsigned int cnt;
 	char warntype[MAXQUOTAS];
+	struct inode_quota *iq = GET_FEATURE(inode, INODE_QUOTA);
 
 	/* First test before acquiring mutex - solves deadlocks when we
          * re-enter the quota code and are already holding the mutex */
-	if (IS_NOQUOTA(inode)) {
+	if (IS_NOQUOTA(inode) || !iq) {
 out_sub:
 		inode_sub_bytes(inode, number);
 		return QUOTA_OK;
@@ -1624,18 +1644,18 @@ out_sub:
 	}
 	spin_lock(&dq_data_lock);
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-		if (!inode->i_dquot[cnt])
+		if (!iq->dquot[cnt])
 			continue;
-		warntype[cnt] = info_bdq_free(inode->i_dquot[cnt], number);
-		dquot_decr_space(inode->i_dquot[cnt], number);
+		warntype[cnt] = info_bdq_free(iq->dquot[cnt], number);
+		dquot_decr_space(iq->dquot[cnt], number);
 	}
 	inode_sub_bytes(inode, number);
 	spin_unlock(&dq_data_lock);
 	/* Dirtify all the dquots - this can block when journalling */
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++)
-		if (inode->i_dquot[cnt])
-			mark_dquot_dirty(inode->i_dquot[cnt]);
-	flush_warnings(inode->i_dquot, warntype);
+		if (iq->dquot[cnt])
+			mark_dquot_dirty(iq->dquot[cnt]);
+	flush_warnings(iq->dquot, warntype);
 	up_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
 	return QUOTA_OK;
 }
@@ -1648,10 +1668,11 @@ int dquot_free_inode(const struct inode *inode, qsize_t number)
 {
 	unsigned int cnt;
 	char warntype[MAXQUOTAS];
+	struct inode_quota *iq = GET_FEATURE(inode, INODE_QUOTA);
 
 	/* First test before acquiring mutex - solves deadlocks when we
          * re-enter the quota code and are already holding the mutex */
-	if (IS_NOQUOTA(inode))
+	if (IS_NOQUOTA(inode) || !iq)
 		return QUOTA_OK;
 
 	down_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
@@ -1662,17 +1683,17 @@ int dquot_free_inode(const struct inode *inode, qsize_t number)
 	}
 	spin_lock(&dq_data_lock);
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-		if (!inode->i_dquot[cnt])
+		if (!iq->dquot[cnt])
 			continue;
-		warntype[cnt] = info_idq_free(inode->i_dquot[cnt], number);
-		dquot_decr_inodes(inode->i_dquot[cnt], number);
+		warntype[cnt] = info_idq_free(iq->dquot[cnt], number);
+		dquot_decr_inodes(iq->dquot[cnt], number);
 	}
 	spin_unlock(&dq_data_lock);
 	/* Dirtify all the dquots - this can block when journalling */
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++)
-		if (inode->i_dquot[cnt])
-			mark_dquot_dirty(inode->i_dquot[cnt]);
-	flush_warnings(inode->i_dquot, warntype);
+		if (iq->dquot[cnt])
+			mark_dquot_dirty(iq->dquot[cnt]);
+	flush_warnings(iq->dquot, warntype);
 	up_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
 	return QUOTA_OK;
 }
@@ -1708,10 +1729,11 @@ int dquot_transfer(struct inode *inode, struct iattr *iattr)
 	    chgid = iattr->ia_valid & ATTR_GID && inode->i_gid != iattr->ia_gid;
 	char warntype_to[MAXQUOTAS];
 	char warntype_from_inodes[MAXQUOTAS], warntype_from_space[MAXQUOTAS];
+	struct inode_quota *iq = GET_FEATURE(inode, INODE_QUOTA);
 
 	/* First test before acquiring mutex - solves deadlocks when we
          * re-enter the quota code and are already holding the mutex */
-	if (IS_NOQUOTA(inode))
+	if (IS_NOQUOTA(inode) || !iq)
 		return QUOTA_OK;
 	/* Initialize the arrays */
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
@@ -1740,7 +1762,7 @@ int dquot_transfer(struct inode *inode, struct iattr *iattr)
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
 		if (!transfer_to[cnt])
 			continue;
-		transfer_from[cnt] = inode->i_dquot[cnt];
+		transfer_from[cnt] = iq->dquot[cnt];
 		if (check_idq(transfer_to[cnt], 1, warntype_to + cnt) ==
 		    NO_QUOTA || check_bdq(transfer_to[cnt], space, 0,
 		    warntype_to + cnt) == NO_QUOTA)
@@ -1773,7 +1795,7 @@ int dquot_transfer(struct inode *inode, struct iattr *iattr)
 		dquot_incr_space(transfer_to[cnt], cur_space);
 		dquot_resv_space(transfer_to[cnt], rsv_space);
 
-		inode->i_dquot[cnt] = transfer_to[cnt];
+		iq->dquot[cnt] = transfer_to[cnt];
 	}
 	spin_unlock(&dq_data_lock);
 	up_write(&sb_dqopt(inode->i_sb)->dqptr_sem);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 2404b16..beb9aa7 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -711,9 +711,16 @@ static inline int mapping_writably_mapped(struct address_space *mapping)
 #endif
 
 enum {
+	INODE_QUOTA,
 	INODE_FEATURES
 };
 
+#ifdef CONFIG_QUOTA
+struct inode_quota {
+	struct dquot		*dquot[MAXQUOTAS];
+};
+#endif
+
 struct inode {
 	struct hlist_node	i_hash;
 	struct list_head	i_list;
@@ -746,9 +753,6 @@ struct inode {
 	struct file_lock	*i_flock;
 	struct address_space	*i_mapping;
 	struct address_space	i_data;
-#ifdef CONFIG_QUOTA
-	struct dquot		*i_dquot[MAXQUOTAS];
-#endif
 	struct list_head	i_devices;
 	union {
 		struct pipe_inode_info	*i_pipe;
-- 
1.6.0.2

Re: [PATCH 2/2] Use feature code to eliminate quota pointers from inodes for filesystems that don't need it.

[Ryusuke Konishi]

On Thu, 30 Apr 2009 19:31:18 +0200, Jan Kara wrote:
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/ext2/ext2.h     |    1 +
>  fs/ext2/super.c    |    5 ++
>  fs/inode.c         |    7 ++-
>  fs/nilfs2/mdt.c    |    3 -
>  fs/quota/dquot.c   |  134 ++++++++++++++++++++++++++++++----------------------
>  include/linux/fs.h |   10 +++-
>  6 files changed, 97 insertions(+), 63 deletions(-)

<snip>

> diff --git a/fs/nilfs2/mdt.c b/fs/nilfs2/mdt.c
> index 47dd815..725cd80 100644
> --- a/fs/nilfs2/mdt.c
> +++ b/fs/nilfs2/mdt.c
> @@ -478,9 +478,6 @@ nilfs_mdt_new_common(struct the_nilfs *nilfs, struct super_block *sb,
>  		inode->i_blocks = 0;
>  		inode->i_bytes = 0;
>  		inode->i_generation = 0;
> -#ifdef CONFIG_QUOTA
> -		memset(&inode->i_dquot, 0, sizeof(inode->i_dquot));
> -#endif
>  		inode->i_pipe = NULL;
>  		inode->i_bdev = NULL;
>  		inode->i_cdev = NULL;

Ack-by: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>

<snip>

Thanks,
Ryusuke Konishi

Re: [RFC PATCH 0/2] A new way of attaching information to inodes

[Theodore Tso]

On Thu, Apr 30, 2009 at 07:31:16PM +0200, Jan Kara wrote:
> 
>   currently our VFS inode structure is quite big. It contains quite some
> members that are useful only in some cases (e.g. device pointers and list head
> used only when the inode represents a block/character device, quota pointers
> used only when the filesystem actually supports quota, etc.). And it would
> be helpful to add some more so that we can handle ACL's in generic code, or
> we can do some kind of block reservation for mmaped writes, and there are
> other cases.

One of the things on my "round tuit" list was separating out those
inode fields which are only needed when the inode is in use, and
separating them out into a separate data structure, so that the 90+%
of the inodes which are just being cached, and are not active, don't
burn space in struct inode.  In particular, we can probably move out
i_mutex, i_alloc_sem, i_size_seqcount, i_flock, and perhaps others.

The same applies for the filesystem-specific portion of the in-core
inode; and for some filesystems there might be even more opportunity
for savings there.

Just a thought,

						- Ted

Re: [RFC PATCH 0/2] A new way of attaching information to inodes

[Christoph Hellwig]

On Thu, Apr 30, 2009 at 03:43:39PM -0400, Theodore Tso wrote:
> One of the things on my "round tuit" list was separating out those
> inode fields which are only needed when the inode is in use, and
> separating them out into a separate data structure, so that the 90+%
> of the inodes which are just being cached, and are not active, don't
> burn space in struct inode.  In particular, we can probably move out
> i_mutex, i_alloc_sem, i_size_seqcount, i_flock, and perhaps others.

I think that's generally a good idea, but I don't think we should
use this mechanism for it.  Just split the inode into two structures,
as struct cached_inode and a struct active_inode and make the struct
inode an anonymous union of the two.

> The same applies for the filesystem-specific portion of the in-core
> inode; and for some filesystems there might be even more opportunity
> for savings there.

I'd rather move them out into the filesystem.  E.g. i_alloc_sem is
specific to those few filesystem that support O_DIRECT but don't
actually do good enough internal locking - we'd be much better
off just having the i_alloc_sem equivalent inside those filesystems
and get rid of the calls to it in the VFS which only hurt the others.

Similarly I'd really love to get rid of the quota calls in the VFS but
instead move calls to these helpers into the filesystems - it's not
a function of the VFS switch and that way we can easily keep the quota
inode in the FS without a feature table.