[Cluster-devel] [PATCH 0/3] gfs2: Add new resource group header fields

[Cluster-devel] [PATCH 0/3] gfs2: Add new resource group header fields

[Andrew Price]

These patches add new fields to struct gfs2_rgrp. The intention is to improve fsck performance and error checking while reducing reliance on the rindex to be read in order to scan through the resource groups in tools such as fsck.gfs2. This is part of a wider goal to obsolete gfs2's metafs, hence the duplication of fields from the rindex. In testing performance, these patches carry very little-to-no overhead.

This posting will be followed by a patch set for gfs2-utils to make use of the new fields, and set and check them enough for testing purposes.

Andrew Price (3):
  gfs2: Add a next-resource-group pointer to resource groups
  gfs2: Add rindex fields to rgrp headers
  gfs2: Add a crc field to resource group headers

 fs/gfs2/rgrp.c                   | 14 +++++++++++++-
 include/uapi/linux/gfs2_ondisk.h | 15 ++++++++++++---
 2 files changed, 25 insertions(+), 4 deletions(-)

-- 
2.13.6

[Cluster-devel] [PATCH 1/3] gfs2: Add a next-resource-group pointer to resource groups

[Andrew Price]

Add a new rg_skip field to struct gfs2_rgrp, replacing __pad. The
rg_skip field has the following meaning:

- If rg_skip is zero, it is considered unset and not useful.
- If rg_skip is non-zero, its value will be the number of blocks between
  this rgrp's address and the next rgrp's address. This can be used as a
  hint by fsck.gfs2 when rebuilding a bad rindex, for example.

This will provide less dependency on the rindex in future, and allow
tools such as fsck.gfs2 to iterate the resource groups without keeping
the rindex around.

The field is updated in gfs2_rgrp_out() so that existing file systems
will have it set. This means that any resource groups that aren't ever
written will not be updated. The final rgrp is a special case as there
is no next rgrp, so it will always have a rg_skip of 0 (unless the fs is
extended).

Before this patch, gfs2_rgrp_out() zeroes the __pad field explicitly, so
the rg_skip field can get set back to 0 in cases where nodes with and
without this patch are mixed in a cluster. In some cases, the field may
bounce between being set by one node and then zeroed by another which
may harm performance slightly, e.g. when two nodes create many small
files. In testing this situation is rare but it becomes more likely as
the filesystem fills up and there are fewer resource groups to choose
from. The problem goes away when all nodes are running with this patch.
Dipping into the space currently occupied by the rg_reserved field would
have resulted in the same problem as it is also explicitly zeroed, so
unfortunately there is no other way around it.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 fs/gfs2/rgrp.c                   | 4 +++-
 include/uapi/linux/gfs2_ondisk.h | 5 ++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 95b2a57ded33..372203a5e655 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -1044,12 +1044,14 @@ static void gfs2_rgrp_in(struct gfs2_rgrpd *rgd, const void *buf)
 
 static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
 {
+	struct gfs2_rgrpd *next = gfs2_rgrpd_get_next(rgd);
 	struct gfs2_rgrp *str = buf;
 
 	str->rg_flags = cpu_to_be32(rgd->rd_flags & ~GFS2_RDF_MASK);
 	str->rg_free = cpu_to_be32(rgd->rd_free);
 	str->rg_dinodes = cpu_to_be32(rgd->rd_dinodes);
-	str->__pad = cpu_to_be32(0);
+	if (next != NULL && next->rd_addr > rgd->rd_addr)
+		str->rg_skip = cpu_to_be32(next->rd_addr - rgd->rd_addr);
 	str->rg_igeneration = cpu_to_be64(rgd->rd_igeneration);
 	memset(&str->rg_reserved, 0, sizeof(str->rg_reserved));
 }
diff --git a/include/uapi/linux/gfs2_ondisk.h b/include/uapi/linux/gfs2_ondisk.h
index 5156bad77b47..da7a30ddef72 100644
--- a/include/uapi/linux/gfs2_ondisk.h
+++ b/include/uapi/linux/gfs2_ondisk.h
@@ -187,7 +187,10 @@ struct gfs2_rgrp {
 	__be32 rg_flags;
 	__be32 rg_free;
 	__be32 rg_dinodes;
-	__be32 __pad;
+	union {
+		__be32 __pad;
+		__be32 rg_skip; /* Distance to the next rgrp in fs blocks */
+	};
 	__be64 rg_igeneration;
 
 	__u8 rg_reserved[80]; /* Several fields from gfs1 now reserved */
-- 
2.13.6

[Cluster-devel] [PATCH 1/3] gfs2: Add a next-resource-group pointer to resource groups

[Bob Peterson]

Hi,

----- Original Message -----
| Add a new rg_skip field to struct gfs2_rgrp, replacing __pad. The
| -	str->__pad = cpu_to_be32(0);
| +	if (next != NULL && next->rd_addr > rgd->rd_addr)
| +		str->rg_skip = cpu_to_be32(next->rd_addr - rgd->rd_addr);

Without having looked at the original:

Shouldn't we still initialize it to 0 or put in an else clause?
I'm concerned that we might end up with an uninitialized random value
at the last rgrp (which may not be a big concern until gfs2_grow or
fsck time).

Bob Peterson
Red Hat File Systems

[Cluster-devel] [PATCH 1/3] gfs2: Add a next-resource-group pointer to resource groups

[Andrew Price]

On 07/12/17 13:14, Bob Peterson wrote:
> Hi,
> 
> ----- Original Message -----
> | Add a new rg_skip field to struct gfs2_rgrp, replacing __pad. The
> | -	str->__pad = cpu_to_be32(0);
> | +	if (next != NULL && next->rd_addr > rgd->rd_addr)
> | +		str->rg_skip = cpu_to_be32(next->rd_addr - rgd->rd_addr);
> 
> Without having looked at the original:
> 
> Shouldn't we still initialize it to 0 or put in an else clause?
> I'm concerned that we might end up with an uninitialized random value
> at the last rgrp (which may not be a big concern until gfs2_grow or
> fsck time).

Hmm I think we're safe here because mkfs.gfs2 will have set it to zero 
and the value is propagated from that point. It's not *obviously* safe 
though so I think zeroing it is a good idea. Updated the patch in my 
tree with:


-       if (next != NULL && next->rd_addr > rgd->rd_addr)
+       if (next == NULL)
+               str->rg_skip = 0;
+       else if (next->rd_addr > rgd->rd_addr)
                 str->rg_skip = cpu_to_be32(next->rd_addr - rgd->rd_addr);

Thanks,
Andy

[Cluster-devel] [PATCH 2/3] gfs2: Add rindex fields to rgrp headers

[Andrew Price]

Add rg_data0, rg_data and rg_bitbytes to struct gfs2_rgrp. The fields
are identical to their counterparts in struct gfs2_rindex and are
intended to reduce the use of the rindex. For now the fields are only
written back as the in-memory equivalents in struct gfs2_rgrpd are set
using values from the rindex. However, they are needed at this point so
that userspace can make use of them, allowing a migration away from the
rindex over time.

The new fields take up previously reserved space which was explicitly
zeroed on write so, in clusters with mixed kernels, these fields could
get zeroed after being set and this should not be treated as an error.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 fs/gfs2/rgrp.c                   | 5 +++++
 include/uapi/linux/gfs2_ondisk.h | 7 ++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 372203a5e655..9a6fa21a2f5a 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -1040,6 +1040,7 @@ static void gfs2_rgrp_in(struct gfs2_rgrpd *rgd, const void *buf)
 	rgd->rd_free = be32_to_cpu(str->rg_free);
 	rgd->rd_dinodes = be32_to_cpu(str->rg_dinodes);
 	rgd->rd_igeneration = be64_to_cpu(str->rg_igeneration);
+	/* rd_data0, rd_data and rd_bitbytes already set from rindex */
 }
 
 static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
@@ -1053,6 +1054,10 @@ static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
 	if (next != NULL && next->rd_addr > rgd->rd_addr)
 		str->rg_skip = cpu_to_be32(next->rd_addr - rgd->rd_addr);
 	str->rg_igeneration = cpu_to_be64(rgd->rd_igeneration);
+	str->rg_data0 = cpu_to_be64(rgd->rd_data0);
+	str->rg_data = cpu_to_be64(rgd->rd_data);
+	str->rg_bitbytes = cpu_to_be64(rgd->rd_bitbytes);
+
 	memset(&str->rg_reserved, 0, sizeof(str->rg_reserved));
 }
 
diff --git a/include/uapi/linux/gfs2_ondisk.h b/include/uapi/linux/gfs2_ondisk.h
index da7a30ddef72..648e0cbca574 100644
--- a/include/uapi/linux/gfs2_ondisk.h
+++ b/include/uapi/linux/gfs2_ondisk.h
@@ -192,8 +192,13 @@ struct gfs2_rgrp {
 		__be32 rg_skip; /* Distance to the next rgrp in fs blocks */
 	};
 	__be64 rg_igeneration;
+	/* The following 3 fields are duplicated from gfs2_rindex to reduce
+	   reliance on the rindex */
+	__be64 rg_data0;     /* First data location */
+	__be32 rg_data;      /* Number of data blocks in rgrp */
+	__be32 rg_bitbytes;  /* Number of bytes in data bitmaps */
 
-	__u8 rg_reserved[80]; /* Several fields from gfs1 now reserved */
+	__u8 rg_reserved[64]; /* Several fields from gfs1 now reserved */
 };
 
 /*
-- 
2.13.6

[Cluster-devel] [PATCH 2/3] gfs2: Add rindex fields to rgrp headers

[Bob Peterson]

Hi,

----- Original Message -----
| Add rg_data0, rg_data and rg_bitbytes to struct gfs2_rgrp. The fields
| are identical to their counterparts in struct gfs2_rindex and are
| intended to reduce the use of the rindex. For now the fields are only
| written back as the in-memory equivalents in struct gfs2_rgrpd are set
| using values from the rindex. However, they are needed at this point so
| that userspace can make use of them, allowing a migration away from the
| rindex over time.
| 
| The new fields take up previously reserved space which was explicitly
| zeroed on write so, in clusters with mixed kernels, these fields could
| get zeroed after being set and this should not be treated as an error.
| 
| Signed-off-by: Andrew Price <anprice@redhat.com>
(snip)
| @@ -1053,6 +1054,10 @@ static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void
| *buf)
|  	if (next != NULL && next->rd_addr > rgd->rd_addr)
|  		str->rg_skip = cpu_to_be32(next->rd_addr - rgd->rd_addr);
|  	str->rg_igeneration = cpu_to_be64(rgd->rd_igeneration);
| +	str->rg_data0 = cpu_to_be64(rgd->rd_data0);
| +	str->rg_data = cpu_to_be64(rgd->rd_data);
| +	str->rg_bitbytes = cpu_to_be64(rgd->rd_bitbytes);

These last two should be cpu_to_be32() not 64()

Regards,

Bob Peterson
Red Hat File Systems

[Cluster-devel] [PATCH 2/3] gfs2: Add rindex fields to rgrp headers

[Andrew Price]

On 07/12/17 13:18, Bob Peterson wrote:
> Hi,
> 
> ----- Original Message -----
> | Add rg_data0, rg_data and rg_bitbytes to struct gfs2_rgrp. The fields
> | are identical to their counterparts in struct gfs2_rindex and are
> | intended to reduce the use of the rindex. For now the fields are only
> | written back as the in-memory equivalents in struct gfs2_rgrpd are set
> | using values from the rindex. However, they are needed at this point so
> | that userspace can make use of them, allowing a migration away from the
> | rindex over time.
> |
> | The new fields take up previously reserved space which was explicitly
> | zeroed on write so, in clusters with mixed kernels, these fields could
> | get zeroed after being set and this should not be treated as an error.
> |
> | Signed-off-by: Andrew Price <anprice@redhat.com>
> (snip)
> | @@ -1053,6 +1054,10 @@ static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void
> | *buf)
> |  	if (next != NULL && next->rd_addr > rgd->rd_addr)
> |  		str->rg_skip = cpu_to_be32(next->rd_addr - rgd->rd_addr);
> |  	str->rg_igeneration = cpu_to_be64(rgd->rd_igeneration);
> | +	str->rg_data0 = cpu_to_be64(rgd->rd_data0);
> | +	str->rg_data = cpu_to_be64(rgd->rd_data);
> | +	str->rg_bitbytes = cpu_to_be64(rgd->rd_bitbytes);
> 
> These last two should be cpu_to_be32() not 64()

D'oh! Fixed.

Thanks,
Andy

[Cluster-devel] [PATCH 3/3] gfs2: Add a crc field to resource group headers

[Andrew Price]

Add the rg_crc field to store a crc32 of the gfs2_rgrp structure. This
allows us to check resource group headers' integrity and removes the
requirement to check them against the rindex entries in fsck. If this
field is found to be zero, it should be ignored (or updated with an
accurate value).

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 fs/gfs2/rgrp.c                   | 5 +++++
 include/uapi/linux/gfs2_ondisk.h | 3 ++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 9a6fa21a2f5a..80e020fc117f 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -34,6 +34,7 @@
 #include "log.h"
 #include "inode.h"
 #include "trace_gfs2.h"
+#include "dir.h"
 
 #define BFITNOENT ((u32)~0)
 #define NO_BLOCK ((u64)~0)
@@ -1047,6 +1048,7 @@ static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
 {
 	struct gfs2_rgrpd *next = gfs2_rgrpd_get_next(rgd);
 	struct gfs2_rgrp *str = buf;
+	u32 crc;
 
 	str->rg_flags = cpu_to_be32(rgd->rd_flags & ~GFS2_RDF_MASK);
 	str->rg_free = cpu_to_be32(rgd->rd_free);
@@ -1057,6 +1059,9 @@ static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
 	str->rg_data0 = cpu_to_be64(rgd->rd_data0);
 	str->rg_data = cpu_to_be64(rgd->rd_data);
 	str->rg_bitbytes = cpu_to_be64(rgd->rd_bitbytes);
+	str->rg_crc = 0;
+	crc = gfs2_disk_hash(buf, sizeof(struct gfs2_rgrp));
+	str->rg_crc = cpu_to_be32(crc);
 
 	memset(&str->rg_reserved, 0, sizeof(str->rg_reserved));
 }
diff --git a/include/uapi/linux/gfs2_ondisk.h b/include/uapi/linux/gfs2_ondisk.h
index 648e0cbca574..09f0920f07e9 100644
--- a/include/uapi/linux/gfs2_ondisk.h
+++ b/include/uapi/linux/gfs2_ondisk.h
@@ -197,8 +197,9 @@ struct gfs2_rgrp {
 	__be64 rg_data0;     /* First data location */
 	__be32 rg_data;      /* Number of data blocks in rgrp */
 	__be32 rg_bitbytes;  /* Number of bytes in data bitmaps */
+	__be32 rg_crc;       /* crc32 of the structure with this field 0 */
 
-	__u8 rg_reserved[64]; /* Several fields from gfs1 now reserved */
+	__u8 rg_reserved[60]; /* Several fields from gfs1 now reserved */
 };
 
 /*
-- 
2.13.6

[Cluster-devel] [PATCH 3/3] gfs2: Add a crc field to resource group headers

[Steven Whitehouse]

On 07/12/17 11:52, Andrew Price wrote:
> Add the rg_crc field to store a crc32 of the gfs2_rgrp structure. This
> allows us to check resource group headers' integrity and removes the
> requirement to check them against the rindex entries in fsck. If this
> field is found to be zero, it should be ignored (or updated with an
> accurate value).
>
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
>   fs/gfs2/rgrp.c                   | 5 +++++
>   include/uapi/linux/gfs2_ondisk.h | 3 ++-
>   2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 9a6fa21a2f5a..80e020fc117f 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -34,6 +34,7 @@
>   #include "log.h"
>   #include "inode.h"
>   #include "trace_gfs2.h"
> +#include "dir.h"
>   
>   #define BFITNOENT ((u32)~0)
>   #define NO_BLOCK ((u64)~0)
> @@ -1047,6 +1048,7 @@ static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
>   {
>   	struct gfs2_rgrpd *next = gfs2_rgrpd_get_next(rgd);
>   	struct gfs2_rgrp *str = buf;
> +	u32 crc;
>   
>   	str->rg_flags = cpu_to_be32(rgd->rd_flags & ~GFS2_RDF_MASK);
>   	str->rg_free = cpu_to_be32(rgd->rd_free);
> @@ -1057,6 +1059,9 @@ static void gfs2_rgrp_out(struct gfs2_rgrpd *rgd, void *buf)
>   	str->rg_data0 = cpu_to_be64(rgd->rd_data0);
>   	str->rg_data = cpu_to_be64(rgd->rd_data);
>   	str->rg_bitbytes = cpu_to_be64(rgd->rd_bitbytes);
> +	str->rg_crc = 0;
> +	crc = gfs2_disk_hash(buf, sizeof(struct gfs2_rgrp));
> +	str->rg_crc = cpu_to_be32(crc);
>   
>   	memset(&str->rg_reserved, 0, sizeof(str->rg_reserved));
>   }
> diff --git a/include/uapi/linux/gfs2_ondisk.h b/include/uapi/linux/gfs2_ondisk.h
> index 648e0cbca574..09f0920f07e9 100644
> --- a/include/uapi/linux/gfs2_ondisk.h
> +++ b/include/uapi/linux/gfs2_ondisk.h
> @@ -197,8 +197,9 @@ struct gfs2_rgrp {
>   	__be64 rg_data0;     /* First data location */
>   	__be32 rg_data;      /* Number of data blocks in rgrp */
>   	__be32 rg_bitbytes;  /* Number of bytes in data bitmaps */
> +	__be32 rg_crc;       /* crc32 of the structure with this field 0 */
In this case because rg_crc is followed by another structure element, if 
that element is 64 bit aligned, then there might be a gap here. Worth 
checking that the overall structure size has not changed, and might be 
worth adding an explicit 32 bit pad field to pair up with the crc too, 
and reducing the reserved field by another 4 bytes.
>   
> -	__u8 rg_reserved[64]; /* Several fields from gfs1 now reserved */
> +	__u8 rg_reserved[60]; /* Several fields from gfs1 now reserved */
>   };
>   
>   /*

Otherwise the patches look good, and this should be a major step forward 
for fsck in due course,

Steve.

[Cluster-devel] [PATCH 3/3] gfs2: Add a crc field to resource group headers

[Andrew Price]

On 07/12/17 12:02, Steven Whitehouse wrote:
> On 07/12/17 11:52, Andrew Price wrote:
>> Add the rg_crc field to store a crc32 of the gfs2_rgrp structure. This
>> allows us to check resource group headers' integrity and removes the
>> requirement to check them against the rindex entries in fsck. If this
>> field is found to be zero, it should be ignored (or updated with an
>> accurate value).
>>
>> Signed-off-by: Andrew Price <anprice@redhat.com>
>> ---
>> ? fs/gfs2/rgrp.c?????????????????? | 5 +++++
>> ? include/uapi/linux/gfs2_ondisk.h | 3 ++-
>> ? 2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
>> index 9a6fa21a2f5a..80e020fc117f 100644
>> --- a/fs/gfs2/rgrp.c
>> +++ b/fs/gfs2/rgrp.c
>> @@ -34,6 +34,7 @@
>> ? #include "log.h"
>> ? #include "inode.h"
>> ? #include "trace_gfs2.h"
>> +#include "dir.h"
>> ? #define BFITNOENT ((u32)~0)
>> ? #define NO_BLOCK ((u64)~0)
>> @@ -1047,6 +1048,7 @@ static void gfs2_rgrp_out(struct gfs2_rgrpd 
>> *rgd, void *buf)
>> ? {
>> ????? struct gfs2_rgrpd *next = gfs2_rgrpd_get_next(rgd);
>> ????? struct gfs2_rgrp *str = buf;
>> +??? u32 crc;
>> ????? str->rg_flags = cpu_to_be32(rgd->rd_flags & ~GFS2_RDF_MASK);
>> ????? str->rg_free = cpu_to_be32(rgd->rd_free);
>> @@ -1057,6 +1059,9 @@ static void gfs2_rgrp_out(struct gfs2_rgrpd 
>> *rgd, void *buf)
>> ????? str->rg_data0 = cpu_to_be64(rgd->rd_data0);
>> ????? str->rg_data = cpu_to_be64(rgd->rd_data);
>> ????? str->rg_bitbytes = cpu_to_be64(rgd->rd_bitbytes);
>> +??? str->rg_crc = 0;
>> +??? crc = gfs2_disk_hash(buf, sizeof(struct gfs2_rgrp));
>> +??? str->rg_crc = cpu_to_be32(crc);
>> ????? memset(&str->rg_reserved, 0, sizeof(str->rg_reserved));
>> ? }
>> diff --git a/include/uapi/linux/gfs2_ondisk.h 
>> b/include/uapi/linux/gfs2_ondisk.h
>> index 648e0cbca574..09f0920f07e9 100644
>> --- a/include/uapi/linux/gfs2_ondisk.h
>> +++ b/include/uapi/linux/gfs2_ondisk.h
>> @@ -197,8 +197,9 @@ struct gfs2_rgrp {
>> ????? __be64 rg_data0;???? /* First data location */
>> ????? __be32 rg_data;????? /* Number of data blocks in rgrp */
>> ????? __be32 rg_bitbytes;? /* Number of bytes in data bitmaps */
>> +??? __be32 rg_crc;?????? /* crc32 of the structure with this field 0 */
> In this case because rg_crc is followed by another structure element, if 
> that element is 64 bit aligned, then there might be a gap here. Worth 
> checking that the overall structure size has not changed, and might be 
> worth adding an explicit 32 bit pad field to pair up with the crc too, 
> and reducing the reserved field by another 4 bytes.

sizeof(struct gfs2_rgrp) is 128 before and after the change, which I 
would expect as rg_reserved doesn't require 64-bit alignment. I can add 
a padding field as documentation though.

>> -??? __u8 rg_reserved[64]; /* Several fields from gfs1 now reserved */
>> +??? __u8 rg_reserved[60]; /* Several fields from gfs1 now reserved */
>> ? };
>> ? /*
> 
> Otherwise the patches look good, and this should be a major step forward 
> for fsck in due course,

Thanks for the quick review!

Andy

[Cluster-devel] [PATCH 3/3] gfs2: Add a crc field to resource group headers

[Andrew Price]

On 07/12/17 12:32, Andrew Price wrote:
> On 07/12/17 12:02, Steven Whitehouse wrote:
>>> --- a/include/uapi/linux/gfs2_ondisk.h
>>> +++ b/include/uapi/linux/gfs2_ondisk.h
>>> @@ -197,8 +197,9 @@ struct gfs2_rgrp {
>>> ????? __be64 rg_data0;???? /* First data location */
>>> ????? __be32 rg_data;????? /* Number of data blocks in rgrp */
>>> ????? __be32 rg_bitbytes;? /* Number of bytes in data bitmaps */
>>> +??? __be32 rg_crc;?????? /* crc32 of the structure with this field 0 */
>> In this case because rg_crc is followed by another structure element, 
>> if that element is 64 bit aligned, then there might be a gap here. 
>> Worth checking that the overall structure size has not changed, and 
>> might be worth adding an explicit 32 bit pad field to pair up with the 
>> crc too, and reducing the reserved field by another 4 bytes.
> 
> sizeof(struct gfs2_rgrp) is 128 before and after the change, which I 
> would expect as rg_reserved doesn't require 64-bit alignment. I can add 
> a padding field as documentation though.

Thinking about this again, adding a padding field like this would need 
special casing in the rgrp_in and out functions so I'm not sure it's 
worth making that change.

Andy