[Cluster-devel] GFS2: Fix one RG corner case

[Cluster-devel] GFS2: Fix one RG corner case

[Steven Whitehouse]

From aa8920c96897dd82f0520f9e7db7311b42547ce6 Mon Sep 17 00:00:00 2001
From: Steven Whitehouse <swhiteho@redhat.com>
Date: Tue, 13 Nov 2012 14:50:35 +0000
Subject: GFS2: Fix one RG corner case

For filesystems with only a single resource group, we need to be careful
that the allocation loop will not land up with a NULL resource group. This
fixes a bug in a previous patch where the gfs2_rgrpd_get_next() function
was being used instead of gfs2_rgrpd_get_first()

Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>

diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 99a6197..5625e93 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -1776,10 +1776,11 @@ static u32 gfs2_orlov_skip(const struct gfs2_inode *ip)
 static bool gfs2_select_rgrp(struct gfs2_rgrpd **pos, const struct gfs2_rgrpd *begin)
 {
 	struct gfs2_rgrpd *rgd = *pos;
+	struct gfs2_sbd *sdp = rgd->rd_sbd;
 
 	rgd = gfs2_rgrpd_get_next(rgd);
 	if (rgd == NULL)
-		rgd = gfs2_rgrpd_get_next(NULL);
+		rgd = gfs2_rgrpd_get_first(sdp);
 	*pos = rgd;
 	if (rgd != begin) /* If we didn't wrap */
 		return true;
-- 
1.7.4

[Cluster-devel] GFS2: Fix one RG corner case

[Bob Peterson]

----- Original Message -----
| >From aa8920c96897dd82f0520f9e7db7311b42547ce6 Mon Sep 17 00:00:00
| >2001
| From: Steven Whitehouse <swhiteho@redhat.com>
| Date: Tue, 13 Nov 2012 14:50:35 +0000
| Subject: GFS2: Fix one RG corner case
| 
| For filesystems with only a single resource group, we need to be
| careful
| that the allocation loop will not land up with a NULL resource group.
| This
| fixes a bug in a previous patch where the gfs2_rgrpd_get_next()
| function
| was being used instead of gfs2_rgrpd_get_first()
| 
| Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
| 
| diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
| index 99a6197..5625e93 100644
| --- a/fs/gfs2/rgrp.c
| +++ b/fs/gfs2/rgrp.c
| @@ -1776,10 +1776,11 @@ static u32 gfs2_orlov_skip(const struct
| gfs2_inode *ip)
|  static bool gfs2_select_rgrp(struct gfs2_rgrpd **pos, const struct
|  gfs2_rgrpd *begin)
|  {
|  	struct gfs2_rgrpd *rgd = *pos;
| +	struct gfs2_sbd *sdp = rgd->rd_sbd;
|  
|  	rgd = gfs2_rgrpd_get_next(rgd);
|  	if (rgd == NULL)
| -		rgd = gfs2_rgrpd_get_next(NULL);
| +		rgd = gfs2_rgrpd_get_first(sdp);
|  	*pos = rgd;
|  	if (rgd != begin) /* If we didn't wrap */
|  		return true;
| --
| 1.7.4

Hi,

If we do this, can't we get into an infinite loop if the one and only
rgrp is full? In that case, rgrp will be == begin, so it won't return
true, so the caller won't ever realize it's wrapped.

Regards,

Bob Peterson
Red Hat File Systems

[Cluster-devel] GFS2: Fix one RG corner case

[Steven Whitehouse]

Hi,

On Tue, 2012-11-13 at 10:41 -0500, Bob Peterson wrote:
> ----- Original Message -----
> | >From aa8920c96897dd82f0520f9e7db7311b42547ce6 Mon Sep 17 00:00:00
> | >2001
> | From: Steven Whitehouse <swhiteho@redhat.com>
> | Date: Tue, 13 Nov 2012 14:50:35 +0000
> | Subject: GFS2: Fix one RG corner case
> | 
> | For filesystems with only a single resource group, we need to be
> | careful
> | that the allocation loop will not land up with a NULL resource group.
> | This
> | fixes a bug in a previous patch where the gfs2_rgrpd_get_next()
> | function
> | was being used instead of gfs2_rgrpd_get_first()
> | 
> | Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
> | 
> | diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> | index 99a6197..5625e93 100644
> | --- a/fs/gfs2/rgrp.c
> | +++ b/fs/gfs2/rgrp.c
> | @@ -1776,10 +1776,11 @@ static u32 gfs2_orlov_skip(const struct
> | gfs2_inode *ip)
> |  static bool gfs2_select_rgrp(struct gfs2_rgrpd **pos, const struct
> |  gfs2_rgrpd *begin)
> |  {
> |  	struct gfs2_rgrpd *rgd = *pos;
> | +	struct gfs2_sbd *sdp = rgd->rd_sbd;
> |  
> |  	rgd = gfs2_rgrpd_get_next(rgd);
> |  	if (rgd == NULL)
> | -		rgd = gfs2_rgrpd_get_next(NULL);
> | +		rgd = gfs2_rgrpd_get_first(sdp);
> |  	*pos = rgd;
> |  	if (rgd != begin) /* If we didn't wrap */
> |  		return true;
> | --
> | 1.7.4
> 
> Hi,
> 
> If we do this, can't we get into an infinite loop if the one and only
> rgrp is full? In that case, rgrp will be == begin, so it won't return
> true, so the caller won't ever realize it's wrapped.
> 
> Regards,
> 
If every rgrp == begin, then it rgd != begin will never be true, so
we'll return false and thats ok since that gives us our wrap counter.
Plus I also tested it, using dd to fill the filesystem and we get
-ENOSPC as expected,

Steve.