All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Jeff Layton <jlayton@redhat.com>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	akpm@osdl.org
Subject: Re: [PATCH 1/3] new_inode_autonum: add per-sb lastino counter and add new_inode_autonum function that guarantees i_ino uniqueness
Date: Thu, 9 Nov 2006 17:33:17 +0000	[thread overview]
Message-ID: <20061109173317.GA11406@infradead.org> (raw)
In-Reply-To: <1163085879.21469.45.camel@dantu.rdu.redhat.com>

On Thu, Nov 09, 2006 at 10:24:39AM -0500, Jeff Layton wrote:
> +/**
> + *	new_inode_autonum 	- obtain an inode with a unique i_ino value
> + *	@sb: superblock
> + *
> + *	Allocates a new inode for given superblock. Ensures that i_ino is
> + *	unique on the filesystem.
> + */
> +struct inode *new_inode_autonum(struct super_block *sb)
> +{
> +	struct inode *inode;
> +
> +	inode = __new_inode(sb);
> +	inode->i_ino = iunique(sb, 0);
> +	return inode;

Why do we need this wrapper?  The callers could aswell just do the
iunique call themselves.  It's already exported aswell.

> +/**
> + *	new_inode 	- obtain an inode -- i_ino not guaranteed unique
> + *	@sb: superblock
> + *
> + *	Allocates a new inode for given superblock. i_ino is not guaranteed to
> + *	be unique. Should only be used when i_ino is going to be clobbered.
> + */
> +struct inode *new_inode(struct super_block *sb)
> +{
> +	struct inode *inode;
> +
> +	inode = __new_inode(sb);
> +	inode->i_ino = 0; /* 0 to try to catch callers that don't reset it */
> +	return inode;

And this wrapper is rather pointless as inode_init_once already zeroes the
whole inode.  Just keep the name new_inode for the basic don't assigned
inode number thing.

> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -961,6 +961,14 @@ #endif
>  	/* Granularity of c/m/atime in ns.
>  	   Cannot be worse than a second */
>  	u32		   s_time_gran;
> +
> +	/* per-sb inode counter for new_inode. Make it a 32-bit counter when
> +	   we have the possibility of dealing with 32-bit apps */
> +#ifdef CONFIG_COMPAT
> +	unsigned int		s_nextino;
> +#else
> +	unsigned long		s_nextino;
> +#endif

This is more thanb ugly.  CONFIG_COMPAT should not modify core kernel
behaviour.  It's also short sightened as we once again plan to support
64bit inode numbers on 32bit hosts.


      parent reply	other threads:[~2006-11-09 17:33 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-11-09 15:24 [PATCH 1/3] new_inode_autonum: add per-sb lastino counter and add new_inode_autonum function that guarantees i_ino uniqueness Jeff Layton
2006-11-09 17:12 ` Eric Sandeen
2006-11-09 17:33 ` Christoph Hellwig [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20061109173317.GA11406@infradead.org \
    --to=hch@infradead.org \
    --cc=akpm@osdl.org \
    --cc=jlayton@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.