[Cluster-devel] [GFS2 PATCH] GFS2: Write the log descriptor entries in the correct order

2014-11-26 Thread Bob Peterson
Hi,

I recently discovered GFS2 was writing the log descriptor items in
the journal in reverse order. For example, if I do a bunch of file
creates, the result was a log header that looked something like this:

0x298 (j+ 242): Log descriptor, type 300 (Metadata) len:504, data1: 503
   0xa6e9ce di0xa6e9a7 di0xa6e97b di0xa6e942 di
   0xa6e911 di0xa6e8d2 di0xa6e898 di0xa6e86e di
   0xa6e840 di0xa6e809 di0xa6e7d7 di0xa6e77e di
   0xa6e747 di0xa6e721 di0xa6e6f8 di0xa6e6ca di
   0xa6e680 di0xa6e63d di0xa6e619 di0xa6e5f4 di
   0xa6e5d1 di0xa6e598 di0xa6e520 di0xa6e4f4 di
   0xa6e4c1 di0xa6e489 di0xa6e45c di0xa6e427 di

As you can see, the blocks are reverse-sorted. This patch changes the
order so that they're written in sorted order. Log headers written with
the patch look more like this:

0x36e (j+ 318): Log descriptor, type 300 (Metadata) len:504, data1: 503
   0x4d5a80 di0x4d5aa0 lf0x4d5aa1 di0x4d5aa2 di
   0x4d5aa3 di0x4d5aa4 di0x4d5aa5 di0x4d5aa6 di
   0x4d5aa7 di0x4d5aa8 di0x4d5aa9 di0x4d5aaa di
   0x4d5aab di0x4d5aac di0x4d5aad di0x4d5aae di
   0x4d5aaf di0x4d5ab0 di0x4d5ab1 di0x4d5ab2 di
   0x4d5ab3 di0x4d5ab4 di0x4d5ab5 di0x4d5ab6 di
   0x4d5ab7 di0x4d5ab8 di0x4d5ab9 di0x4d5aba di

The blocks following the log descriptor are written sequentially, so the
sort order won't affect performance at all, but it makes the journal
easier to decipher for debugging purposes.

Patch description:

This patch changes the order in which the blocks are processed by function
gfs2_before_commit. Before, it was processing them in the wrong order such
that the blocks would be layed out and written in reverse order. This
changes the order to the correct order.

Regards,

Bob Peterson
Red Hat File Systems

Signed-off-by: Bob Peterson rpete...@redhat.com 
---
diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index 2c1ae86..d644470 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -444,7 +444,7 @@ static void gfs2_before_commit(struct gfs2_sbd *sdp, 
unsigned int limit,
ptr = (__be64 *)(ld + 1);
 
n = 0;
-   list_for_each_entry_continue(bd1, blist, bd_list) {
+   list_for_each_entry_continue_reverse(bd1, blist, bd_list) {
*ptr++ = cpu_to_be64(bd1-bd_bh-b_blocknr);
if (is_databuf) {
gfs2_check_magic(bd1-bd_bh);
@@ -459,7 +459,7 @@ static void gfs2_before_commit(struct gfs2_sbd *sdp, 
unsigned int limit,
gfs2_log_lock(sdp);
 
n = 0;
-   list_for_each_entry_continue(bd2, blist, bd_list) {
+   list_for_each_entry_continue_reverse(bd2, blist, bd_list) {
get_bh(bd2-bd_bh);
gfs2_log_unlock(sdp);
lock_buffer(bd2-bd_bh);



Re: [Cluster-devel] [GFS2 PATCH] GFS2: Write the log descriptor entries in the correct order

2014-11-26 Thread Steven Whitehouse

Hi,

On 26/11/14 14:09, Bob Peterson wrote:

Hi,

I recently discovered GFS2 was writing the log descriptor items in
the journal in reverse order. For example, if I do a bunch of file
creates, the result was a log header that looked something like this:

0x298 (j+ 242): Log descriptor, type 300 (Metadata) len:504, data1: 503
0xa6e9ce di0xa6e9a7 di0xa6e97b di0xa6e942 di
0xa6e911 di0xa6e8d2 di0xa6e898 di0xa6e86e di
0xa6e840 di0xa6e809 di0xa6e7d7 di0xa6e77e di
0xa6e747 di0xa6e721 di0xa6e6f8 di0xa6e6ca di
0xa6e680 di0xa6e63d di0xa6e619 di0xa6e5f4 di
0xa6e5d1 di0xa6e598 di0xa6e520 di0xa6e4f4 di
0xa6e4c1 di0xa6e489 di0xa6e45c di0xa6e427 di

As you can see, the blocks are reverse-sorted. This patch changes the
order so that they're written in sorted order. Log headers written with
the patch look more like this:

0x36e (j+ 318): Log descriptor, type 300 (Metadata) len:504, data1: 503
0x4d5a80 di0x4d5aa0 lf0x4d5aa1 di0x4d5aa2 di
0x4d5aa3 di0x4d5aa4 di0x4d5aa5 di0x4d5aa6 di
0x4d5aa7 di0x4d5aa8 di0x4d5aa9 di0x4d5aaa di
0x4d5aab di0x4d5aac di0x4d5aad di0x4d5aae di
0x4d5aaf di0x4d5ab0 di0x4d5ab1 di0x4d5ab2 di
0x4d5ab3 di0x4d5ab4 di0x4d5ab5 di0x4d5ab6 di
0x4d5ab7 di0x4d5ab8 di0x4d5ab9 di0x4d5aba di

The blocks following the log descriptor are written sequentially, so the
sort order won't affect performance at all, but it makes the journal
easier to decipher for debugging purposes.

Patch description:

This patch changes the order in which the blocks are processed by function
gfs2_before_commit. Before, it was processing them in the wrong order such
that the blocks would be layed out and written in reverse order. This
changes the order to the correct order.

Regards,

Bob Peterson
Red Hat File Systems

Does that mean that this patch got the ordering the wrong way around then?

https://git.kernel.org/cgit/linux/kernel/git/steve/gfs2-3.0-nmw.git/commit/fs/gfs2/lops.c?id=7f63257da1aeea9b2ee68f469b0f9f4a39e5dff8

Steve.



Signed-off-by: Bob Peterson rpete...@redhat.com
---
diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
index 2c1ae86..d644470 100644
--- a/fs/gfs2/lops.c
+++ b/fs/gfs2/lops.c
@@ -444,7 +444,7 @@ static void gfs2_before_commit(struct gfs2_sbd *sdp, 
unsigned int limit,
ptr = (__be64 *)(ld + 1);
  
  		n = 0;

-   list_for_each_entry_continue(bd1, blist, bd_list) {
+   list_for_each_entry_continue_reverse(bd1, blist, bd_list) {
*ptr++ = cpu_to_be64(bd1-bd_bh-b_blocknr);
if (is_databuf) {
gfs2_check_magic(bd1-bd_bh);
@@ -459,7 +459,7 @@ static void gfs2_before_commit(struct gfs2_sbd *sdp, 
unsigned int limit,
gfs2_log_lock(sdp);
  
  		n = 0;

-   list_for_each_entry_continue(bd2, blist, bd_list) {
+   list_for_each_entry_continue_reverse(bd2, blist, bd_list) {
get_bh(bd2-bd_bh);
gfs2_log_unlock(sdp);
lock_buffer(bd2-bd_bh);





Re: [Cluster-devel] [GFS2 PATCH] GFS2: Write the log descriptor entries in the correct order

2014-11-26 Thread Bob Peterson
- Original Message -
  As you can see, the blocks are reverse-sorted. This patch changes the
  order so that they're written in sorted order. Log headers written with
  the patch look more like this:

 Does that mean that this patch got the ordering the wrong way around then?
 
 https://git.kernel.org/cgit/linux/kernel/git/steve/gfs2-3.0-nmw.git/commit/fs/gfs2/lops.c?id=7f63257da1aeea9b2ee68f469b0f9f4a39e5dff8
 
 Steve.

No. It means I missed Ben's patch. Or maybe I've lost my mind. Sigh.

Bob