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

[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 di        0xa6e9a7 di        0xa6e97b di        0xa6e942 di
>         0xa6e911 di        0xa6e8d2 di        0xa6e898 di        0xa6e86e di
>         0xa6e840 di        0xa6e809 di        0xa6e7d7 di        0xa6e77e di
>         0xa6e747 di        0xa6e721 di        0xa6e6f8 di        0xa6e6ca di
>         0xa6e680 di        0xa6e63d di        0xa6e619 di        0xa6e5f4 di
>         0xa6e5d1 di        0xa6e598 di        0xa6e520 di        0xa6e4f4 di
>         0xa6e4c1 di        0xa6e489 di        0xa6e45c di        0xa6e427 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 di        0x4d5aa0 lf        0x4d5aa1 di        0x4d5aa2 di
>         0x4d5aa3 di        0x4d5aa4 di        0x4d5aa5 di        0x4d5aa6 di
>         0x4d5aa7 di        0x4d5aa8 di        0x4d5aa9 di        0x4d5aaa di
>         0x4d5aab di        0x4d5aac di        0x4d5aad di        0x4d5aae di
>         0x4d5aaf di        0x4d5ab0 di        0x4d5ab1 di        0x4d5ab2 di
>         0x4d5ab3 di        0x4d5ab4 di        0x4d5ab5 di        0x4d5ab6 di
>         0x4d5ab7 di        0x4d5ab8 di        0x4d5ab9 di        0x4d5aba 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 <rpeterso at 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);
>