Re: [PATCH 2/5] SQUASHME pnfs-submit remove pnfs_alloc_init_inode

[Boaz Harrosh <bharrosh@panasas.com>]

On 07/07/2010 12:16 AM, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>
> 
> Place all layout initialization in nfs4_init_once
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  fs/nfs/inode.c |   19 ++++++-------------
>  1 files changed, 6 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 231cfa3..fa310b1 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -1361,18 +1361,6 @@ void nfs4_clear_inode(struct inode *inode)
>  }
>  #endif
>  
> -static void pnfs_alloc_init_inode(struct nfs_inode *nfsi)
> -{
> -#ifdef CONFIG_NFS_V4_1
> -	nfsi->layout.pnfs_layout_state = 0;
> -	memset(&nfsi->layout.stateid, 0, NFS4_STATEID_SIZE);
> -	nfsi->layout.roc_iomode = 0;
> -	nfsi->layout.lo_cred = NULL;
> -	nfsi->layout.pnfs_write_begin_pos = 0;
> -	nfsi->layout.pnfs_write_end_pos = 0;
> -#endif /* CONFIG_NFS_V4_1 */
> -}
> -
>  struct inode *nfs_alloc_inode(struct super_block *sb)
>  {
>  	struct nfs_inode *nfsi;
> @@ -1388,7 +1376,6 @@ struct inode *nfs_alloc_inode(struct super_block *sb)
>  #ifdef CONFIG_NFS_V4
>  	nfsi->nfs4_acl = NULL;
>  #endif /* CONFIG_NFS_V4 */
> -	pnfs_alloc_init_inode(nfsi);
>  	return &nfsi->vfs_inode;
>  }
>  
> @@ -1430,6 +1417,12 @@ static inline void nfs4_init_once(struct nfs_inode *nfsi)
>  	INIT_LIST_HEAD(&nfsi->layout.segs);
>  	nfsi->layout.refcount = 0;
>  	nfsi->layout.ld_data = NULL;
> +	nfsi->layout.pnfs_layout_state = 0;
> +	memset(&nfsi->layout.stateid, 0, NFS4_STATEID_SIZE);
> +	nfsi->layout.roc_iomode = 0;
> +	nfsi->layout.lo_cred = NULL;
> +	nfsi->layout.pnfs_write_begin_pos = 0;
> +	nfsi->layout.pnfs_write_end_pos = 0;

If we are already here. What are the rules with zeros? It is costume elsewhere in
Kernel that at construction points all zeros are just not done, if a kzalloc or memset
is guaranteed. Isn't nfs_inode zero_allocated? If not it should. Adding zero initialization
to every new member, and an extra remove line to any member remove is just maintenance
nightmare.

(This patch could be much more beautiful if it was "only removed lines")

Boaz


>  #endif /* CONFIG_NFS_V4_1 */
>  #endif
>  }