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

[David Teigland <teigland@redhat.com>]

On Wed, May 09, 2007 at 09:37:57AM -0500, Robert Peterson wrote:
> +static void adjust_fs_space(struct inode *inode)
> +{
> +	struct gfs2_sbd *sdp = inode->i_sb->s_fs_info;
> +	struct gfs2_statfs_change_host *m_sc = &sdp->sd_statfs_master;
> +	struct gfs2_statfs_change_host *l_sc = &sdp->sd_statfs_local;
> +	u64 fs_total, new_free;
> +
> +	/* Total up the file system space, according to the latest rindex. */
> +	fs_total = gfs2_ri_total(sdp);
> +
> +	spin_lock(&sdp->sd_statfs_spin);
> +	if (fs_total > (m_sc->sc_total + l_sc->sc_total))
> +		new_free = fs_total - (m_sc->sc_total + l_sc->sc_total);
> +	else
> +		new_free = 0;
> +	spin_unlock(&sdp->sd_statfs_spin);
> +	fs_warn(sdp, "File system extended by %llu blocks.\n", new_free);

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);


> diff --git a/fs/gfs2/ops_address.h b/fs/gfs2/ops_address.h
> index 35aaee4..56c30da 100644
> --- a/fs/gfs2/ops_address.h
> +++ b/fs/gfs2/ops_address.h
> @@ -1,6 +1,6 @@
> /*
>  * Copyright (C) Sistina Software, Inc.  1997-2003 All rights reserved.
> - * Copyright (C) 2004-2006 Red Hat, Inc.  All rights reserved.
> + * Copyright (C) 2004-2007 Red Hat, Inc.  All rights reserved.
>  *
>  * This copyrighted material is made available to anyone wishing to use,
>  * modify, copy, or redistribute it subject to the terms and conditions
> @@ -18,5 +18,8 @@ extern const struct address_space_operations 
> gfs2_file_aops;
> extern int gfs2_get_block(struct inode *inode, sector_t lblock,
> 			  struct buffer_head *bh_result, int create);
> extern int gfs2_releasepage(struct page *page, gfp_t gfp_mask);
> +extern u64 gfs2_ri_total(struct gfs2_sbd *sdp);
> +extern void gfs2_statfs_change(struct gfs2_sbd *sdp, s64 total, s64 free,
> +			       s64 dinodes);

!?


> +		if (pos + sizeof(struct gfs2_rindex) >= ip->i_di.di_size)
> +			break;

> +	/* 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) &&

> +
> +		if (pos + sizeof(struct gfs2_rindex) >= ip->i_di.di_size)
> +			break;

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...


> -	error = gfs2_rindex_hold(sdp, &al->al_ri_gh);
> +	/* We need to hold the rindex unless the inode we're using is
> +	   the rindex itself, in which case it's already held. */
> +	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);

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?

Dave