[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Cluster-devel] [PATCH][TRY 5] GFS2: kernel changes to support new gfs2_grow command



Hi,

Applied to the GFS2 -nmw git tree. Thanks,

Steve.

On Thu, 2007-05-10 at 16:54 -0500, Robert Peterson wrote:
> David Teigland wrote:
> > this needs a cast to avoid warnings on some architectures:
> > 	fs_warn(sdp, "File system extended by %llu blocks.\n",
> > 		(unsigned long long)new_free);
> 
> Okay.
>  
> > Correct me if I'm wrong, but I thought all three of these new checks above
> > wouldn't be needed if it weren't for...
> > this one call to gfs2_ri_update().  Which is the very special case where
> > the rindex is being written before gfs has even read it?  I'm trying to
> > figure out if we can get by without adding those three new checks above
> > somehow.  If in fact they're only needed due to this one call, it may be
> > nice to write a new special-purpose function to call here to do the reads
> > (instead of overloading the normal read function with the special checks),
> > or add a new arg so the checks can be narrowly applied to this case, or...
> > read it all at mount time so the problem goes away?
> 
> I misunderstood you: I thought you were talking about the 
> previously-added-but-unnecessary function gfs2_check_rindex_version
> that I took out with the last revision.
> 
> Yes, these functions can be separated out to prune the code of the
> extra conditions for this special case.  
> 
> Since Steve Whitehouse already applied the previous patch to his git
> tree, I implemented your suggestions as a new patch, given below.
> This is what I did:
> 
> To avoid code redundancy, I separated out the operational "guts" into 
> a new function called read_rindex_entry.  Then I made two functions: 
> the closer-to-original gfs2_ri_update (without the special condition 
> checks) and gfs2_ri_update_special that's designed with that condition 
> in mind.  (I don't like the name, but if you have a suggestion, I'm
> all ears).
> 
> Oh, and there's an added benefit:  we don't need all the ugly gotos
> anymore.  ;)
> 
> This patch has been tested with gfs2_fsck_hellfire (which runs for
> three and a half hours, btw).
> 
> Regards,
> 
> Bob Peterson
> Red Hat Cluster Suite
> 
> Signed-off-By: Bob Peterson <rpeterso redhat com> 
> --
>  fs/gfs2/ops_address.c |    3 +-
>  fs/gfs2/rgrp.c        |  139 ++++++++++++++++++++++++++++++++-----------------
>  2 files changed, 93 insertions(+), 49 deletions(-)
> 
> diff --git a/fs/gfs2/ops_address.c b/fs/gfs2/ops_address.c
> index 846c0ff..e0b4e8c 100644
> --- a/fs/gfs2/ops_address.c
> +++ b/fs/gfs2/ops_address.c
> @@ -469,7 +469,8 @@ static void adjust_fs_space(struct inode *inode)
>  	else
>  		new_free = 0;
>  	spin_unlock(&sdp->sd_statfs_spin);
> -	fs_warn(sdp, "File system extended by %llu blocks.\n", new_free);
> +	fs_warn(sdp, "File system extended by %llu blocks.\n",
> +		(unsigned long long)new_free);
>  	gfs2_statfs_change(sdp, new_free, new_free, 0);
>  }
>  
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index e857f40..48a6461 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -463,9 +463,62 @@ u64 gfs2_ri_total(struct gfs2_sbd *sdp)
>  }
>  
>  /**
> - * gfs2_ri_update - Pull in a new resource index from the disk
> + * read_rindex_entry - Pull in a new resource index entry from the disk
>   * @gl: The glock covering the rindex inode
>   *
> + * Returns: 0 on success, error code otherwise
> + */
> +
> +static int read_rindex_entry(struct gfs2_inode *ip,
> +			     struct file_ra_state *ra_state)
> +{
> +	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> +	loff_t pos = sdp->sd_rgrps * sizeof(struct gfs2_rindex);
> +	char buf[sizeof(struct gfs2_rindex)];
> +	int error;
> +	struct gfs2_rgrpd *rgd;
> +
> +	error = gfs2_internal_read(ip, ra_state, buf, &pos,
> +				   sizeof(struct gfs2_rindex));
> +	if (!error)
> +		return 0;
> +	if (error != sizeof(struct gfs2_rindex)) {
> +		if (error > 0)
> +			error = -EIO;
> +		return error;
> +	}
> +
> +	rgd = kzalloc(sizeof(struct gfs2_rgrpd), GFP_NOFS);
> +	error = -ENOMEM;
> +	if (!rgd)
> +		return error;
> +
> +	mutex_init(&rgd->rd_mutex);
> +	lops_init_le(&rgd->rd_le, &gfs2_rg_lops);
> +	rgd->rd_sbd = sdp;
> +
> +	list_add_tail(&rgd->rd_list, &sdp->sd_rindex_list);
> +	list_add_tail(&rgd->rd_list_mru, &sdp->sd_rindex_mru_list);
> +
> +	gfs2_rindex_in(&rgd->rd_ri, buf);
> +	error = compute_bitstructs(rgd);
> +	if (error)
> +		return error;
> +
> +	error = gfs2_glock_get(sdp, rgd->rd_ri.ri_addr,
> +			       &gfs2_rgrp_glops, CREATE, &rgd->rd_gl);
> +	if (error)
> +		return error;
> +
> +	rgd->rd_gl->gl_object = rgd;
> +	rgd->rd_rg_vn = rgd->rd_gl->gl_vn - 1;
> +	return error;
> +}
> +
> +/**
> + * gfs2_ri_update - Pull in a new resource index from the disk
> + * @ip: pointer to the rindex inode
> + *
>   * Returns: 0 on successful update, error code otherwise
>   */
>  
> @@ -473,18 +526,11 @@ static int gfs2_ri_update(struct gfs2_inode *ip)
>  {
>  	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
>  	struct inode *inode = &ip->i_inode;
> -	struct gfs2_rgrpd *rgd;
> -	char buf[sizeof(struct gfs2_rindex)];
>  	struct file_ra_state ra_state;
>  	u64 junk = ip->i_di.di_size;
>  	int error;
>  
> -	/* If someone is holding the rindex file with a glock, they must
> -	   be updating it, in which case we may have partial entries.
> -	   In this case, we ignore the partials. */
> -	if (!gfs2_glock_is_held_excl(ip->i_gl) &&
> -	    !gfs2_glock_is_held_shrd(ip->i_gl) &&
> -	    do_div(junk, sizeof(struct gfs2_rindex))) {
> +	if (do_div(junk, sizeof(struct gfs2_rindex))) {
>  		gfs2_consist_inode(ip);
>  		return -EIO;
>  	}
> @@ -493,52 +539,49 @@ static int gfs2_ri_update(struct gfs2_inode *ip)
>  
>  	file_ra_state_init(&ra_state, inode->i_mapping);
>  	for (sdp->sd_rgrps = 0;; sdp->sd_rgrps++) {
> -		loff_t pos = sdp->sd_rgrps * sizeof(struct gfs2_rindex);
> -
> -		if (pos + sizeof(struct gfs2_rindex) >= ip->i_di.di_size)
> -			break;
> -		error = gfs2_internal_read(ip, &ra_state, buf, &pos,
> -					    sizeof(struct gfs2_rindex));
> -		if (!error)
> -			break;
> -		if (error != sizeof(struct gfs2_rindex)) {
> -			if (error > 0)
> -				error = -EIO;
> -			goto fail;
> +		error = read_rindex_entry(ip, &ra_state);
> +		if (error) {
> +			clear_rgrpdi(sdp);
> +			return error;
>  		}
> +	}
>  
> -		rgd = kzalloc(sizeof(struct gfs2_rgrpd), GFP_NOFS);
> -		error = -ENOMEM;
> -		if (!rgd)
> -			goto fail;
> -
> -		mutex_init(&rgd->rd_mutex);
> -		lops_init_le(&rgd->rd_le, &gfs2_rg_lops);
> -		rgd->rd_sbd = sdp;
> -
> -		list_add_tail(&rgd->rd_list, &sdp->sd_rindex_list);
> -		list_add_tail(&rgd->rd_list_mru, &sdp->sd_rindex_mru_list);
> -
> -		gfs2_rindex_in(&rgd->rd_ri, buf);
> -		error = compute_bitstructs(rgd);
> -		if (error)
> -			goto fail;
> +	sdp->sd_rindex_vn = ip->i_gl->gl_vn;
> +	return 0;
> +}
>  
> -		error = gfs2_glock_get(sdp, rgd->rd_ri.ri_addr,
> -				       &gfs2_rgrp_glops, CREATE, &rgd->rd_gl);
> -		if (error)
> -			goto fail;
> +/**
> + * gfs2_ri_update_special - Pull in a new resource index from the disk
> + *
> + * This is a special version that's safe to call from gfs2_inplace_reserve_i.
> + * In this case we know that we don't have any resource groups in memory yet.
> + *
> + * @ip: pointer to the rindex inode
> + *
> + * Returns: 0 on successful update, error code otherwise
> + */
> +static int gfs2_ri_update_special(struct gfs2_inode *ip)
> +{
> +	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> +	struct inode *inode = &ip->i_inode;
> +	struct file_ra_state ra_state;
> +	int error;
>  
> -		rgd->rd_gl->gl_object = rgd;
> -		rgd->rd_rg_vn = rgd->rd_gl->gl_vn - 1;
> +	file_ra_state_init(&ra_state, inode->i_mapping);
> +	for (sdp->sd_rgrps = 0;; sdp->sd_rgrps++) {
> +		/* Ignore partials */
> +		if ((sdp->sd_rgrps + 1) * sizeof(struct gfs2_rindex) >
> +		    ip->i_di.di_size)
> +			break;
> +		error = read_rindex_entry(ip, &ra_state);
> +		if (error) {
> +			clear_rgrpdi(sdp);
> +			return error;
> +		}
>  	}
>  
>  	sdp->sd_rindex_vn = ip->i_gl->gl_vn;
>  	return 0;
> -
> -fail:
> -	clear_rgrpdi(sdp);
> -	return error;
>  }
>  
>  /**
> @@ -1028,7 +1071,7 @@ int gfs2_inplace_reserve_i(struct gfs2_inode *ip, char *file, unsigned int line)
>  	if (ip != GFS2_I(sdp->sd_rindex))
>  		error = gfs2_rindex_hold(sdp, &al->al_ri_gh);
>  	else if (!sdp->sd_rgrps) /* We may not have the rindex read in, so: */
> -		error = gfs2_ri_update(ip);
> +		error = gfs2_ri_update_special(ip);
>  
>  	if (error)
>  		return error;
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]