Re: [PATCH 01/61] vfs: widen inode hash/lookup functions to u64

[Jan Kara]

On Thu 26-02-26 10:55:03, Jeff Layton wrote:
> Change the inode hash/lookup VFS API functions to accept u64 parameters
> instead of unsigned long for inode numbers and hash values. This is
> preparation for widening i_ino itself to u64, which will allow
> filesystems to store full 64-bit inode numbers on 32-bit architectures.
> 
> Since unsigned long implicitly widens to u64 on all architectures, this
> change is backward-compatible with all existing callers.
> 
> Functions updated:
>   - hash(), find_inode_fast(), find_inode_by_ino_rcu(), test_inode_iunique()
>   - __insert_inode_hash(), iget_locked(), iget5_locked(), iget5_locked_rcu()
>   - ilookup(), ilookup5(), ilookup5_nowait()
>   - find_inode_nowait(), find_inode_rcu()
>   - inode_insert5(), insert_inode_locked4()
>   - insert_inode_locked() (local variable)
>   - dump_mapping() (local variable and format string)
> 
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  fs/inode.c         | 46 +++++++++++++++++++++++-----------------------
>  include/linux/fs.h | 26 +++++++++++++-------------
>  2 files changed, 36 insertions(+), 36 deletions(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index cc12b68e021b2c97cc88a46ddc736334ecb8edfa..2cabec9043e8176d20aecc5ce7e0f276c114f122 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -672,7 +672,7 @@ static inline void inode_sb_list_del(struct inode *inode)
>  	}
>  }
>  
> -static unsigned long hash(struct super_block *sb, unsigned long hashval)
> +static unsigned long hash(struct super_block *sb, u64 hashval)
>  {
>  	unsigned long tmp;
>  
> @@ -685,12 +685,12 @@ static unsigned long hash(struct super_block *sb, unsigned long hashval)
>  /**
>   *	__insert_inode_hash - hash an inode
>   *	@inode: unhashed inode
> - *	@hashval: unsigned long value used to locate this object in the
> + *	@hashval: u64 value used to locate this object in the
>   *		inode_hashtable.
>   *
>   *	Add an inode to the inode hash for this superblock.
>   */
> -void __insert_inode_hash(struct inode *inode, unsigned long hashval)
> +void __insert_inode_hash(struct inode *inode, u64 hashval)
>  {
>  	struct hlist_head *b = inode_hashtable + hash(inode->i_sb, hashval);
>  
> @@ -726,7 +726,7 @@ void dump_mapping(const struct address_space *mapping)
>  	struct dentry *dentry_ptr;
>  	struct dentry dentry;
>  	char fname[64] = {};
> -	unsigned long ino;
> +	u64 ino;
>  
>  	/*
>  	 * If mapping is an invalid pointer, we don't want to crash
> @@ -750,14 +750,14 @@ void dump_mapping(const struct address_space *mapping)
>  	}
>  
>  	if (!dentry_first) {
> -		pr_warn("aops:%ps ino:%lx\n", a_ops, ino);
> +		pr_warn("aops:%ps ino:%llx\n", a_ops, ino);
>  		return;
>  	}
>  
>  	dentry_ptr = container_of(dentry_first, struct dentry, d_u.d_alias);
>  	if (get_kernel_nofault(dentry, dentry_ptr) ||
>  	    !dentry.d_parent || !dentry.d_name.name) {
> -		pr_warn("aops:%ps ino:%lx invalid dentry:%px\n",
> +		pr_warn("aops:%ps ino:%llx invalid dentry:%px\n",
>  				a_ops, ino, dentry_ptr);
>  		return;
>  	}
> @@ -768,7 +768,7 @@ void dump_mapping(const struct address_space *mapping)
>  	 * Even if strncpy_from_kernel_nofault() succeeded,
>  	 * the fname could be unreliable
>  	 */
> -	pr_warn("aops:%ps ino:%lx dentry name(?):\"%s\"\n",
> +	pr_warn("aops:%ps ino:%llx dentry name(?):\"%s\"\n",
>  		a_ops, ino, fname);
>  }
>  
> @@ -1087,7 +1087,7 @@ static struct inode *find_inode(struct super_block *sb,
>   * iget_locked for details.
>   */
>  static struct inode *find_inode_fast(struct super_block *sb,
> -				struct hlist_head *head, unsigned long ino,
> +				struct hlist_head *head, u64 ino,
>  				bool hash_locked, bool *isnew)
>  {
>  	struct inode *inode = NULL;
> @@ -1301,7 +1301,7 @@ EXPORT_SYMBOL(unlock_two_nondirectories);
>   * Note that both @test and @set are called with the inode_hash_lock held, so
>   * they can't sleep.
>   */
> -struct inode *inode_insert5(struct inode *inode, unsigned long hashval,
> +struct inode *inode_insert5(struct inode *inode, u64 hashval,
>  			    int (*test)(struct inode *, void *),
>  			    int (*set)(struct inode *, void *), void *data)
>  {
> @@ -1378,7 +1378,7 @@ EXPORT_SYMBOL(inode_insert5);
>   * Note that both @test and @set are called with the inode_hash_lock held, so
>   * they can't sleep.
>   */
> -struct inode *iget5_locked(struct super_block *sb, unsigned long hashval,
> +struct inode *iget5_locked(struct super_block *sb, u64 hashval,
>  		int (*test)(struct inode *, void *),
>  		int (*set)(struct inode *, void *), void *data)
>  {
> @@ -1408,7 +1408,7 @@ EXPORT_SYMBOL(iget5_locked);
>   * This is equivalent to iget5_locked, except the @test callback must
>   * tolerate the inode not being stable, including being mid-teardown.
>   */
> -struct inode *iget5_locked_rcu(struct super_block *sb, unsigned long hashval,
> +struct inode *iget5_locked_rcu(struct super_block *sb, u64 hashval,
>  		int (*test)(struct inode *, void *),
>  		int (*set)(struct inode *, void *), void *data)
>  {
> @@ -1455,7 +1455,7 @@ EXPORT_SYMBOL_GPL(iget5_locked_rcu);
>   * hashed, and with the I_NEW flag set.  The file system gets to fill it in
>   * before unlocking it via unlock_new_inode().
>   */
> -struct inode *iget_locked(struct super_block *sb, unsigned long ino)
> +struct inode *iget_locked(struct super_block *sb, u64 ino)
>  {
>  	struct hlist_head *head = inode_hashtable + hash(sb, ino);
>  	struct inode *inode;
> @@ -1527,7 +1527,7 @@ EXPORT_SYMBOL(iget_locked);
>   *
>   * Returns 1 if the inode number is unique, 0 if it is not.
>   */
> -static int test_inode_iunique(struct super_block *sb, unsigned long ino)
> +static int test_inode_iunique(struct super_block *sb, u64 ino)
>  {
>  	struct hlist_head *b = inode_hashtable + hash(sb, ino);
>  	struct inode *inode;
> @@ -1616,7 +1616,7 @@ EXPORT_SYMBOL(igrab);
>   *
>   * Note2: @test is called with the inode_hash_lock held, so can't sleep.
>   */
> -struct inode *ilookup5_nowait(struct super_block *sb, unsigned long hashval,
> +struct inode *ilookup5_nowait(struct super_block *sb, u64 hashval,
>  		int (*test)(struct inode *, void *), void *data, bool *isnew)
>  {
>  	struct hlist_head *head = inode_hashtable + hash(sb, hashval);
> @@ -1647,7 +1647,7 @@ EXPORT_SYMBOL(ilookup5_nowait);
>   *
>   * Note: @test is called with the inode_hash_lock held, so can't sleep.
>   */
> -struct inode *ilookup5(struct super_block *sb, unsigned long hashval,
> +struct inode *ilookup5(struct super_block *sb, u64 hashval,
>  		int (*test)(struct inode *, void *), void *data)
>  {
>  	struct inode *inode;
> @@ -1677,7 +1677,7 @@ EXPORT_SYMBOL(ilookup5);
>   * Search for the inode @ino in the inode cache, and if the inode is in the
>   * cache, the inode is returned with an incremented reference count.
>   */
> -struct inode *ilookup(struct super_block *sb, unsigned long ino)
> +struct inode *ilookup(struct super_block *sb, u64 ino)
>  {
>  	struct hlist_head *head = inode_hashtable + hash(sb, ino);
>  	struct inode *inode;
> @@ -1726,8 +1726,8 @@ EXPORT_SYMBOL(ilookup);
>   * very carefully implemented.
>   */
>  struct inode *find_inode_nowait(struct super_block *sb,
> -				unsigned long hashval,
> -				int (*match)(struct inode *, unsigned long,
> +				u64 hashval,
> +				int (*match)(struct inode *, u64,
>  					     void *),
>  				void *data)
>  {
> @@ -1773,7 +1773,7 @@ EXPORT_SYMBOL(find_inode_nowait);
>   *
>   * The caller must hold the RCU read lock.
>   */
> -struct inode *find_inode_rcu(struct super_block *sb, unsigned long hashval,
> +struct inode *find_inode_rcu(struct super_block *sb, u64 hashval,
>  			     int (*test)(struct inode *, void *), void *data)
>  {
>  	struct hlist_head *head = inode_hashtable + hash(sb, hashval);
> @@ -1812,7 +1812,7 @@ EXPORT_SYMBOL(find_inode_rcu);
>   * The caller must hold the RCU read lock.
>   */
>  struct inode *find_inode_by_ino_rcu(struct super_block *sb,
> -				    unsigned long ino)
> +				    u64 ino)
>  {
>  	struct hlist_head *head = inode_hashtable + hash(sb, ino);
>  	struct inode *inode;
> @@ -1833,7 +1833,7 @@ EXPORT_SYMBOL(find_inode_by_ino_rcu);
>  int insert_inode_locked(struct inode *inode)
>  {
>  	struct super_block *sb = inode->i_sb;
> -	ino_t ino = inode->i_ino;
> +	u64 ino = inode->i_ino;
>  	struct hlist_head *head = inode_hashtable + hash(sb, ino);
>  	bool isnew;
>  
> @@ -1884,7 +1884,7 @@ int insert_inode_locked(struct inode *inode)
>  }
>  EXPORT_SYMBOL(insert_inode_locked);
>  
> -int insert_inode_locked4(struct inode *inode, unsigned long hashval,
> +int insert_inode_locked4(struct inode *inode, u64 hashval,
>  		int (*test)(struct inode *, void *), void *data)
>  {
>  	struct inode *old;
> @@ -2642,7 +2642,7 @@ void init_special_inode(struct inode *inode, umode_t mode, dev_t rdev)
>  		break;
>  	default:
>  		printk(KERN_DEBUG "init_special_inode: bogus i_mode (%o) for"
> -				  " inode %s:%lu\n", mode, inode->i_sb->s_id,
> +				  " inode %s:%llu\n", mode, inode->i_sb->s_id,
>  				  inode->i_ino);
>  		break;
>  	}
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 8b3dd145b25ec12b00ac1df17a952d9116b88047..dfa1f475b1c480c503ab6f00e891aa9b051607fa 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2935,32 +2935,32 @@ static inline int inode_generic_drop(struct inode *inode)
>  extern void d_mark_dontcache(struct inode *inode);
>  
>  extern struct inode *ilookup5_nowait(struct super_block *sb,
> -		unsigned long hashval, int (*test)(struct inode *, void *),
> +		u64 hashval, int (*test)(struct inode *, void *),
>  		void *data, bool *isnew);
> -extern struct inode *ilookup5(struct super_block *sb, unsigned long hashval,
> +extern struct inode *ilookup5(struct super_block *sb, u64 hashval,
>  		int (*test)(struct inode *, void *), void *data);
> -extern struct inode *ilookup(struct super_block *sb, unsigned long ino);
> +extern struct inode *ilookup(struct super_block *sb, u64 ino);
>  
> -extern struct inode *inode_insert5(struct inode *inode, unsigned long hashval,
> +extern struct inode *inode_insert5(struct inode *inode, u64 hashval,
>  		int (*test)(struct inode *, void *),
>  		int (*set)(struct inode *, void *),
>  		void *data);
> -struct inode *iget5_locked(struct super_block *, unsigned long,
> +struct inode *iget5_locked(struct super_block *, u64,
>  			   int (*test)(struct inode *, void *),
>  			   int (*set)(struct inode *, void *), void *);
> -struct inode *iget5_locked_rcu(struct super_block *, unsigned long,
> +struct inode *iget5_locked_rcu(struct super_block *, u64,
>  			       int (*test)(struct inode *, void *),
>  			       int (*set)(struct inode *, void *), void *);
> -extern struct inode * iget_locked(struct super_block *, unsigned long);
> +extern struct inode *iget_locked(struct super_block *, u64);
>  extern struct inode *find_inode_nowait(struct super_block *,
> -				       unsigned long,
> +				       u64,
>  				       int (*match)(struct inode *,
> -						    unsigned long, void *),
> +						    u64, void *),
>  				       void *data);
> -extern struct inode *find_inode_rcu(struct super_block *, unsigned long,
> +extern struct inode *find_inode_rcu(struct super_block *, u64,
>  				    int (*)(struct inode *, void *), void *);
> -extern struct inode *find_inode_by_ino_rcu(struct super_block *, unsigned long);
> -extern int insert_inode_locked4(struct inode *, unsigned long, int (*test)(struct inode *, void *), void *);
> +extern struct inode *find_inode_by_ino_rcu(struct super_block *, u64);
> +extern int insert_inode_locked4(struct inode *, u64, int (*test)(struct inode *, void *), void *);
>  extern int insert_inode_locked(struct inode *);
>  #ifdef CONFIG_DEBUG_LOCK_ALLOC
>  extern void lockdep_annotate_inode_mutex_key(struct inode *inode);
> @@ -3015,7 +3015,7 @@ int setattr_should_drop_sgid(struct mnt_idmap *idmap,
>   */
>  #define alloc_inode_sb(_sb, _cache, _gfp) kmem_cache_alloc_lru(_cache, &_sb->s_inode_lru, _gfp)
>  
> -extern void __insert_inode_hash(struct inode *, unsigned long hashval);
> +extern void __insert_inode_hash(struct inode *, u64 hashval);
>  static inline void insert_inode_hash(struct inode *inode)
>  {
>  	__insert_inode_hash(inode, inode->i_ino);
> 
> -- 
> 2.53.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR