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

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

[Robert Peterson]

Hi,

Sorry about this, but Dave informed me that I still had some __u64's
that I needed to change back to u64's, so this is a respin.

Regards,

Bob Peterson
Red Hat Cluster Suite

Signed-off-By: Bob Peterson <rpeterso@redhat.com>
--

 fs/gfs2/ops_address.c |   29 ++++++++++++++++++++++-
 fs/gfs2/ops_address.h |    5 +++-
 fs/gfs2/rgrp.c        |   60 ++++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 86 insertions(+), 8 deletions(-)

diff --git a/fs/gfs2/ops_address.c b/fs/gfs2/ops_address.c
index 30c1562..846c0ff 100644
--- a/fs/gfs2/ops_address.c
+++ b/fs/gfs2/ops_address.c
@@ -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
@@ -450,6 +450,30 @@ out_uninit:
 }
 
 /**
+ * adjust_fs_space - Adjusts the free space available due to gfs2_grow
+ * @inode: the rindex inode
+ */
+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);
+	gfs2_statfs_change(sdp, new_free, new_free, 0);
+}
+
+/**
  * gfs2_commit_write - Commit write to a file
  * @file: The file to write to
  * @page: The page containing the data
@@ -511,6 +535,9 @@ static int gfs2_commit_write(struct file *file, struct page *page,
 		di->di_size = cpu_to_be64(inode->i_size);
 	}
 
+	if (inode == sdp->sd_rindex)
+		adjust_fs_space(inode);
+
 	brelse(dibh);
 	gfs2_trans_end(sdp);
 	if (al->al_requested) {
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);
 
 #endif /* __OPS_ADDRESS_DOT_H__ */
diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 1727f50..e857f40 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -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
@@ -431,6 +431,38 @@ static int compute_bitstructs(struct gfs2_rgrpd *rgd)
 }
 
 /**
+ * gfs2_ri_total - Total up the file system space, according to the rindex.
+ *
+ */
+u64 gfs2_ri_total(struct gfs2_sbd *sdp)
+{
+	u64 total_data = 0;	
+	struct inode *inode = sdp->sd_rindex;
+	struct gfs2_inode *ip = GFS2_I(inode);
+	struct gfs2_rindex_host ri;
+	char buf[sizeof(struct gfs2_rindex)];
+	struct file_ra_state ra_state;
+	int error, rgrps;
+
+	mutex_lock(&sdp->sd_rindex_mutex);
+	file_ra_state_init(&ra_state, inode->i_mapping);
+	for (rgrps = 0;; rgrps++) {
+		loff_t pos = 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 != sizeof(struct gfs2_rindex))
+			break;
+		gfs2_rindex_in(&ri, buf);
+		total_data += ri.ri_data;
+	}
+	mutex_unlock(&sdp->sd_rindex_mutex);
+	return total_data;
+}
+
+/**
  * gfs2_ri_update - Pull in a new resource index from the disk
  * @gl: The glock covering the rindex inode
  *
@@ -447,7 +479,12 @@ static int gfs2_ri_update(struct gfs2_inode *ip)
 	u64 junk = ip->i_di.di_size;
 	int error;
 
-	if (do_div(junk, sizeof(struct gfs2_rindex))) {
+	/* 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))) {
 		gfs2_consist_inode(ip);
 		return -EIO;
 	}
@@ -457,6 +494,9 @@ 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)
@@ -978,18 +1018,25 @@ int gfs2_inplace_reserve_i(struct gfs2_inode *ip, char *file, unsigned int line)
 {
 	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
 	struct gfs2_alloc *al = &ip->i_alloc;
-	int error;
+	int error = 0;
 
 	if (gfs2_assert_warn(sdp, al->al_requested))
 		return -EINVAL;
 
-	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);
+
 	if (error)
 		return error;
 
 	error = get_local_rgrp(ip);
 	if (error) {
-		gfs2_glock_dq_uninit(&al->al_ri_gh);
+		if (ip != GFS2_I(sdp->sd_rindex))
+			gfs2_glock_dq_uninit(&al->al_ri_gh);
 		return error;
 	}
 
@@ -1019,7 +1066,8 @@ void gfs2_inplace_release(struct gfs2_inode *ip)
 
 	al->al_rgd = NULL;
 	gfs2_glock_dq_uninit(&al->al_rgd_gh);
-	gfs2_glock_dq_uninit(&al->al_ri_gh);
+	if (ip != GFS2_I(sdp->sd_rindex))
+		gfs2_glock_dq_uninit(&al->al_ri_gh);
 }
 
 /**
diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h

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

[Steven Whitehouse]

Hi,

Now applied to the GFS2 -nmw git tree. Thanks,

Steve.

On Wed, 2007-05-09 at 09:37 -0500, Robert Peterson wrote:
> Hi,
> 
> Sorry about this, but Dave informed me that I still had some __u64's
> that I needed to change back to u64's, so this is a respin.
> 
> Regards,
> 
> Bob Peterson
> Red Hat Cluster Suite
> 
> Signed-off-By: Bob Peterson <rpeterso@redhat.com>
> --
> 
>  fs/gfs2/ops_address.c |   29 ++++++++++++++++++++++-
>  fs/gfs2/ops_address.h |    5 +++-
>  fs/gfs2/rgrp.c        |   60 ++++++++++++++++++++++++++++++++++++++++++++-----
>  3 files changed, 86 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/gfs2/ops_address.c b/fs/gfs2/ops_address.c
> index 30c1562..846c0ff 100644
> --- a/fs/gfs2/ops_address.c
> +++ b/fs/gfs2/ops_address.c
> @@ -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
> @@ -450,6 +450,30 @@ out_uninit:
>  }
>  
>  /**
> + * adjust_fs_space - Adjusts the free space available due to gfs2_grow
> + * @inode: the rindex inode
> + */
> +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);
> +	gfs2_statfs_change(sdp, new_free, new_free, 0);
> +}
> +
> +/**
>   * gfs2_commit_write - Commit write to a file
>   * @file: The file to write to
>   * @page: The page containing the data
> @@ -511,6 +535,9 @@ static int gfs2_commit_write(struct file *file, struct page *page,
>  		di->di_size = cpu_to_be64(inode->i_size);
>  	}
>  
> +	if (inode == sdp->sd_rindex)
> +		adjust_fs_space(inode);
> +
>  	brelse(dibh);
>  	gfs2_trans_end(sdp);
>  	if (al->al_requested) {
> 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);
>  
>  #endif /* __OPS_ADDRESS_DOT_H__ */
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 1727f50..e857f40 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -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
> @@ -431,6 +431,38 @@ static int compute_bitstructs(struct gfs2_rgrpd *rgd)
>  }
>  
>  /**
> + * gfs2_ri_total - Total up the file system space, according to the rindex.
> + *
> + */
> +u64 gfs2_ri_total(struct gfs2_sbd *sdp)
> +{
> +	u64 total_data = 0;	
> +	struct inode *inode = sdp->sd_rindex;
> +	struct gfs2_inode *ip = GFS2_I(inode);
> +	struct gfs2_rindex_host ri;
> +	char buf[sizeof(struct gfs2_rindex)];
> +	struct file_ra_state ra_state;
> +	int error, rgrps;
> +
> +	mutex_lock(&sdp->sd_rindex_mutex);
> +	file_ra_state_init(&ra_state, inode->i_mapping);
> +	for (rgrps = 0;; rgrps++) {
> +		loff_t pos = 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 != sizeof(struct gfs2_rindex))
> +			break;
> +		gfs2_rindex_in(&ri, buf);
> +		total_data += ri.ri_data;
> +	}
> +	mutex_unlock(&sdp->sd_rindex_mutex);
> +	return total_data;
> +}
> +
> +/**
>   * gfs2_ri_update - Pull in a new resource index from the disk
>   * @gl: The glock covering the rindex inode
>   *
> @@ -447,7 +479,12 @@ static int gfs2_ri_update(struct gfs2_inode *ip)
>  	u64 junk = ip->i_di.di_size;
>  	int error;
>  
> -	if (do_div(junk, sizeof(struct gfs2_rindex))) {
> +	/* 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))) {
>  		gfs2_consist_inode(ip);
>  		return -EIO;
>  	}
> @@ -457,6 +494,9 @@ 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)
> @@ -978,18 +1018,25 @@ int gfs2_inplace_reserve_i(struct gfs2_inode *ip, char *file, unsigned int line)
>  {
>  	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
>  	struct gfs2_alloc *al = &ip->i_alloc;
> -	int error;
> +	int error = 0;
>  
>  	if (gfs2_assert_warn(sdp, al->al_requested))
>  		return -EINVAL;
>  
> -	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);
> +
>  	if (error)
>  		return error;
>  
>  	error = get_local_rgrp(ip);
>  	if (error) {
> -		gfs2_glock_dq_uninit(&al->al_ri_gh);
> +		if (ip != GFS2_I(sdp->sd_rindex))
> +			gfs2_glock_dq_uninit(&al->al_ri_gh);
>  		return error;
>  	}
>  
> @@ -1019,7 +1066,8 @@ void gfs2_inplace_release(struct gfs2_inode *ip)
>  
>  	al->al_rgd = NULL;
>  	gfs2_glock_dq_uninit(&al->al_rgd_gh);
> -	gfs2_glock_dq_uninit(&al->al_ri_gh);
> +	if (ip != GFS2_I(sdp->sd_rindex))
> +		gfs2_glock_dq_uninit(&al->al_ri_gh);
>  }
>  
>  /**
> diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h
> 

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

[David Teigland]

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

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

[Robert Peterson]

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;

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

[Steven Whitehouse]

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