From mboxrd@z Thu Jan  1 00:00:00 1970
From: Steven Whitehouse <swhiteho@redhat.com>
Date: Wed, 23 Mar 2011 09:31:04 +0000
Subject: [Cluster-devel] [GFS2 PATCH] gfs2: remove *leaf_call_t and
 simplify leaf_dealloc
In-Reply-To: <558808392.551691.1300816364723.JavaMail.root@zmail06.collab.prod.int.phx2.redhat.com>
References: <558808392.551691.1300816364723.JavaMail.root@zmail06.collab.prod.int.phx2.redhat.com>
Message-ID: <1300872664.2590.26.camel@dolmen>
List-Id: <cluster-devel.redhat.com>
To: cluster-devel.redhat.com
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

Hi,

Looks good, but a couple of comments...

On Tue, 2011-03-22 at 13:52 -0400, Bob Peterson wrote:
> Hi,
> 
> Since foreach_leaf is only called with leaf_dealloc as its only possible
> call function, we can simplify the code by making it call leaf_dealloc
> directly.  This simplifies the code and eliminates the need for
> leaf_call_t, the generic call method.  This is a first small step in
> simplifying the directory leaf deallocation code.
> 
> Regards,
> 
> Bob Peterson
> Red Hat File Systems
> 
> Signed-off-by: Bob Peterson <rpeterso@redhat.com> 
> --
> diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
> index 5c356d0..10a4bbe 100644
> --- a/fs/gfs2/dir.c
> +++ b/fs/gfs2/dir.c
> @@ -82,11 +82,11 @@
>  struct qstr gfs2_qdot __read_mostly;
>  struct qstr gfs2_qdotdot __read_mostly;
>  
> -typedef int (*leaf_call_t) (struct gfs2_inode *dip, u32 index, u32 len,
> -			    u64 leaf_no, void *data);
>  typedef int (*gfs2_dscan_t)(const struct gfs2_dirent *dent,
>  			    const struct qstr *name, void *opaque);
>  
> +static int leaf_dealloc(struct gfs2_inode *dip, u32 index, u32 len,
> +			u64 leaf_no);
>  
>  int gfs2_dir_get_new_buffer(struct gfs2_inode *ip, u64 block,
>  			    struct buffer_head **bhp)
> @@ -1770,13 +1770,11 @@ int gfs2_dir_mvino(struct gfs2_inode *dip, const struct qstr *filename,
>  /**
>   * foreach_leaf - call a function for each leaf in a directory
>   * @dip: the directory
> - * @lc: the function to call for each each
> - * @data: private data to pass to it
>   *
>   * Returns: errno
>   */
>  
> -static int foreach_leaf(struct gfs2_inode *dip, leaf_call_t lc, void *data)
> +static int foreach_leaf(struct gfs2_inode *dip)

Maybe rename this to prune_leaves() or something like that now it is no
longer generic?

>  {
>  	struct gfs2_sbd *sdp = GFS2_SB(&dip->i_inode);
>  	struct buffer_head *bh;
> @@ -1823,7 +1821,7 @@ static int foreach_leaf(struct gfs2_inode *dip, leaf_call_t lc, void *data)
>  			len = 1 << (dip->i_depth - be16_to_cpu(leaf->lf_depth));
>  			brelse(bh);
>  
> -			error = lc(dip, index, len, leaf_no, data);
> +			error = leaf_dealloc(dip, index, len, leaf_no);
>  			if (error)
>  				goto out;
>  
> @@ -1855,7 +1853,7 @@ out:
>   */
>  
>  static int leaf_dealloc(struct gfs2_inode *dip, u32 index, u32 len,
> -			u64 leaf_no, void *data)
> +			u64 leaf_no)
>  {
>  	struct gfs2_sbd *sdp = GFS2_SB(&dip->i_inode);
>  	struct gfs2_leaf *tmp_leaf;
> @@ -1978,7 +1976,7 @@ int gfs2_dir_exhash_dealloc(struct gfs2_inode *dip)
>  	int error;
>  
>  	/* Dealloc on-disk leaves to FREEMETA state */
This comment looks like it is left over from GFS1. It should just be the
free state now that we don't differentiate free data from free meta.

Steve.

> -	error = foreach_leaf(dip, leaf_dealloc, NULL);
> +	error = foreach_leaf(dip);
>  	if (error)
>  		return error;
>  
>