Re: [RFC] [PATCH 1/8] inode_diet: Replace inode.u.generic_ip with inode.i_private

Steven Whitehouse
2006-06-20T09:36:32Z

Hi,

On Mon, 2006-06-19 at 11:20 -0400, Theodore Tso wrote:
> plain text document attachment (inode-slim-1)
> The filesystem or device-specific pointer in the inode is inside a
> union, which is pretty pointless given that all 30+ users of this
> field have been using the void pointer. Get rid of the union and
> rename it to i_private, with a comment to explain who is allowed to
> use the void pointer. This is just a cleanup, but it allows us to
> reuse the union 'u' for something something where the union will
> actually be used.
>
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
>
> Index: linux-2.6.17/include/linux/fs.h
> ===================================================================
> --- linux-2.6.17.orig/include/linux/fs.h 2006-06-18 18:58:51.000000000 -0400
> +++ linux-2.6.17/include/linux/fs.h 2006-06-18 18:58:55.000000000 -0400
> @@ -534,9 +534,7 @@
>
> atomic_t i_writecount;
> void *i_security;
> - union {
> - void *generic_ip;
> - } u;
> + void *i_private; /* fs or device private pointer */
> #ifdef __NEED_I_SIZE_ORDERED
> seqcount_t i_size_seqcount;
> #endif

As a further suggestion, I wonder do we really need i_private at all?
Since we have sb->s_op->alloc_inode and inode->i_sb->s_op->destroy_inode
if all filesystems did something along the following lines:

struct myfs_inode {
struct inode i_inode;
...
};

#define MYFS_I(inode) container_of((inode), struct myfs_inode, i_inode)

then it would seem that i_private is redundant. If there is a file
system which does genuinely need a pointer here (if indeed such a
filesystem does exist, I haven't actually checked that) then a pointer
can just be added as the one single other member of (in my example)
struct myfs_inode.

Steve.