[Cluster-devel] [PATCH] GFS2: Allow the number of committed revokes to temporarily be negative

[Cluster-devel] [PATCH] GFS2: Allow the number of committed revokes to temporarily be negative

[Benjamin Marzinski]

GFS2 tracks the number of revokes and unrevokes that are part of committed
transactions via sd_log_commited_revoke. It is possible for one process to add
revokes during its transaction, while another process unrevokes them during its
transaction. If the second process finishes its transaction first,
sd_log_commited_revoke will be decremented by the number of unrevokes that the
second process did, without first being incremented by the number of revokes
the first process did. This is fine, since all started transactions must be
completed before the journal can be flushed.  However, sd_log_commited_revoke
is an unsigned integer, and log_refund() causes an assertion failure if it
would go negative at the end of a transaction.  This patch makes
sd_log_commited_revoke a signed integer and allows it to go negative.
__gfs2_log_flush() still checks that it mataches the actual number of revokes. 

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 fs/gfs2/incore.h |    2 +-
 fs/gfs2/log.c    |    3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

Index: gfs2-2.6-nmw/fs/gfs2/log.c
===================================================================
--- gfs2-2.6-nmw.orig/fs/gfs2/log.c
+++ gfs2-2.6-nmw/fs/gfs2/log.c
@@ -417,7 +417,7 @@ static unsigned int calc_reserved(struct
 	databufhdrs_needed = (sdp->sd_log_commited_databuf +
 			      (dbuf_limit - 1)) / dbuf_limit;
 
-	if (sdp->sd_log_commited_revoke)
+	if (sdp->sd_log_commited_revoke > 0)
 		revokes = gfs2_struct2blk(sdp, sdp->sd_log_commited_revoke,
 					  sizeof(u64));
 
@@ -790,7 +790,6 @@ static void log_refund(struct gfs2_sbd *
 	gfs2_assert_withdraw(sdp, (((int)sdp->sd_log_commited_buf) >= 0) ||
 			     (((int)sdp->sd_log_commited_databuf) >= 0));
 	sdp->sd_log_commited_revoke += tr->tr_num_revoke - tr->tr_num_revoke_rm;
-	gfs2_assert_withdraw(sdp, ((int)sdp->sd_log_commited_revoke) >= 0);
 	reserved = calc_reserved(sdp);
 	gfs2_assert_withdraw(sdp, sdp->sd_log_blks_reserved + tr->tr_reserved >= reserved);
 	unused = sdp->sd_log_blks_reserved - reserved + tr->tr_reserved;
Index: gfs2-2.6-nmw/fs/gfs2/incore.h
===================================================================
--- gfs2-2.6-nmw.orig/fs/gfs2/incore.h
+++ gfs2-2.6-nmw/fs/gfs2/incore.h
@@ -616,7 +616,7 @@ struct gfs2_sbd {
 	unsigned int sd_log_blks_reserved;
 	unsigned int sd_log_commited_buf;
 	unsigned int sd_log_commited_databuf;
-	unsigned int sd_log_commited_revoke;
+	int sd_log_commited_revoke;
 
 	unsigned int sd_log_num_buf;
 	unsigned int sd_log_num_revoke;

[Cluster-devel] [PATCH] GFS2: Allow the number of committed revokes to temporarily be negative

[Steven Whitehouse]

Hi,

Now in the -nmw git tree. Thanks,

Steve.

On Wed, 2010-03-10 at 18:10 -0600, Benjamin Marzinski wrote:
> GFS2 tracks the number of revokes and unrevokes that are part of committed
> transactions via sd_log_commited_revoke. It is possible for one process to add
> revokes during its transaction, while another process unrevokes them during its
> transaction. If the second process finishes its transaction first,
> sd_log_commited_revoke will be decremented by the number of unrevokes that the
> second process did, without first being incremented by the number of revokes
> the first process did. This is fine, since all started transactions must be
> completed before the journal can be flushed.  However, sd_log_commited_revoke
> is an unsigned integer, and log_refund() causes an assertion failure if it
> would go negative at the end of a transaction.  This patch makes
> sd_log_commited_revoke a signed integer and allows it to go negative.
> __gfs2_log_flush() still checks that it mataches the actual number of revokes. 
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  fs/gfs2/incore.h |    2 +-
>  fs/gfs2/log.c    |    3 +--
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> Index: gfs2-2.6-nmw/fs/gfs2/log.c
> ===================================================================
> --- gfs2-2.6-nmw.orig/fs/gfs2/log.c
> +++ gfs2-2.6-nmw/fs/gfs2/log.c
> @@ -417,7 +417,7 @@ static unsigned int calc_reserved(struct
>  	databufhdrs_needed = (sdp->sd_log_commited_databuf +
>  			      (dbuf_limit - 1)) / dbuf_limit;
>  
> -	if (sdp->sd_log_commited_revoke)
> +	if (sdp->sd_log_commited_revoke > 0)
>  		revokes = gfs2_struct2blk(sdp, sdp->sd_log_commited_revoke,
>  					  sizeof(u64));
>  
> @@ -790,7 +790,6 @@ static void log_refund(struct gfs2_sbd *
>  	gfs2_assert_withdraw(sdp, (((int)sdp->sd_log_commited_buf) >= 0) ||
>  			     (((int)sdp->sd_log_commited_databuf) >= 0));
>  	sdp->sd_log_commited_revoke += tr->tr_num_revoke - tr->tr_num_revoke_rm;
> -	gfs2_assert_withdraw(sdp, ((int)sdp->sd_log_commited_revoke) >= 0);
>  	reserved = calc_reserved(sdp);
>  	gfs2_assert_withdraw(sdp, sdp->sd_log_blks_reserved + tr->tr_reserved >= reserved);
>  	unused = sdp->sd_log_blks_reserved - reserved + tr->tr_reserved;
> Index: gfs2-2.6-nmw/fs/gfs2/incore.h
> ===================================================================
> --- gfs2-2.6-nmw.orig/fs/gfs2/incore.h
> +++ gfs2-2.6-nmw/fs/gfs2/incore.h
> @@ -616,7 +616,7 @@ struct gfs2_sbd {
>  	unsigned int sd_log_blks_reserved;
>  	unsigned int sd_log_commited_buf;
>  	unsigned int sd_log_commited_databuf;
> -	unsigned int sd_log_commited_revoke;
> +	int sd_log_commited_revoke;
>  
>  	unsigned int sd_log_num_buf;
>  	unsigned int sd_log_num_revoke;
> 

[Cluster-devel] [PATCH] GFS2: Allow the number of committed revokes to temporarily be negative

[Bob Peterson]

----- "Benjamin Marzinski" <bmarzins@redhat.com> wrote:
| GFS2 tracks the number of revokes and unrevokes that are part of
| committed
| transactions via sd_log_commited_revoke. It is possible for one
| process to add
| revokes during its transaction, while another process unrevokes them
| during its
| transaction. If the second process finishes its transaction first,
| sd_log_commited_revoke will be decremented by the number of unrevokes
| that the
| second process did, without first being incremented by the number of
| revokes
| the first process did. This is fine, since all started transactions
| must be
| completed before the journal can be flushed.  However,
| sd_log_commited_revoke
| is an unsigned integer, and log_refund() causes an assertion failure
| if it
| would go negative at the end of a transaction.  This patch makes
| sd_log_commited_revoke a signed integer and allows it to go negative.
| __gfs2_log_flush() still checks that it mataches the actual number of
| revokes. 
| 
| Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
| ---
|  fs/gfs2/incore.h |    2 +-
|  fs/gfs2/log.c    |    3 +--
|  2 files changed, 2 insertions(+), 3 deletions(-)

Hi,

ACKed by Bob Peterson <rpeterso@redhat.com>

Regards,

Bob Peterson
Red Hat File Systems