linux-ext4 - Re: [PATCH 04/12] fs: Generic infrastructure for optional inode fields

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <3BFF1023-E198-4797-A96A-EA158137157E@dilger.ca>
Date:	Wed, 1 Oct 2014 15:05:46 -0600
From:	Andreas Dilger <adilger@...ger.ca>
To:	Jan Kara <jack@...e.cz>
Cc:	linux-fsdevel@...r.kernel.org, linux-ext4@...r.kernel.org,
	Dave Chinner <david@...morbit.com>, xfs@....sgi.com,
	cluster-devel@...hat.com, Steven Whitehouse <swhiteho@...hat.com>,
	Mark Fasheh <mfasheh@...e.com>,
	Joel Becker <jlbec@...lplan.org>, ocfs2-devel@....oracle.com,
	reiserfs-devel@...r.kernel.org, Jeff Mahoney <jeffm@...e.de>,
	Dave Kleikamp <shaggy@...nel.org>,
	jfs-discussion@...ts.sourceforge.net, tytso@....edu
Subject: Re: [PATCH 04/12] fs: Generic infrastructure for optional inode fields

On Oct 1, 2014, at 1:31 PM, Jan Kara <jack@...e.cz> wrote:
> There are parts of struct inode which are used only by a few filesystems
> (e.g. i_dquot pointers, i_mapping->private_list, ...). Thus all the
> other filesystems are just wasting memory with these fields. On the
> other hand it isn't simple to just move these fields to filesystem
> specific part of inode because there is generic code which needs to peek
> into the fields and it is cumbersome to provide helpers into which fs
> has to stuff the field it is storing elsewhere.
> 
> We create a simple infrastructure which allows for optional inode fields
> stored in the fs-specific part of the inode. Accessing these fields has
> a slightly worse performance as we have to lookup their offset in the
> offset table stored in the superblock but in most cases this is
> acceptable. Notably, this offset-table mechanism is faster than having
> fs-specific hook functions which would need to be called to provide
> pointers to desired fields.
> 
> Signed-off-by: Jan Kara <jack@...e.cz>
> ---
> include/linux/fs.h | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 94187721ad41..977f8fb6ca88 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -615,6 +615,11 @@ struct inode {
> 	void			*i_private; /* fs or device private pointer */
> };
> 
> +/* Optional inode fields (stored in filesystems inode if the fs needs them) */
> +enum {

This should be a named enum, like "enum inode_field" or similar, so it
can be referenced below.

> +	IF_FIELD_NR	/* Number of optional inode fields */
> +};
> +
> static inline int inode_unhashed(struct inode *inode)
> {
> 	return hlist_unhashed(&inode->i_hash);
> @@ -1236,6 +1241,11 @@ struct super_block {
> 	void 			*s_fs_info;	/* Filesystem private info */
> 	unsigned int		s_max_links;
> 	fmode_t			s_mode;
> +	/*
> +	 * We could have here just a pointer to the offsets array but this
> +	 * way we save one dereference when looking up field offsets
> +	 */
> +	int			s_inode_fields[IF_FIELD_NR];
> 
> 	/* Granularity of c/m/atime in ns.
> 	   Cannot be worse than a second */
> @@ -1286,6 +1296,20 @@ struct super_block {
> 	struct rcu_head		rcu;
> };
> 
> +static inline void *inode_field(const struct inode *inode, int field)

This should use "enum inode_field" instead of int, so the compiler could
warn about invalid parameter values.  It might make sense to add a check:

        if (field < IF_FIELD_NR)

but I'm not sure if the overhead is worthwhile, unless it can always be
resolved at compile time.  That might be possible since this is a static
inline function.

Cheers, Andreas

> +{
> +	int offset = inode->i_sb->s_inode_fields[field];
> +
> +	if (!offset)	/* Field not present? */
> +		return NULL;

> +	return ((char *)inode) + offset;
> +}
> +
> +static inline void sb_init_inode_fields(struct super_block *sb, int *fields)
> +{
> +	memcpy(sb->s_inode_fields, fields, sizeof(int) * IF_FIELD_NR);
> +}
> +
> extern struct timespec current_fs_time(struct super_block *sb);
> 
> /*
> -- 
> 1.8.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)