[Cluster-devel] [PATCH 2/4] gfs2: ordered writes are backwards

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,

This looks good. There is an argument for trying to sort the buffers as
we write them (in case the application writes them out of order) but
this seems a sensible change to catch 90% of cases. I'm just about to
give this a quick test and I'll push this one in straight away if it
looks good on my test,

Steve.

On Fri, 2010-02-05 at 16:45 +1100, Dave Chinner wrote:
> When we queue data buffers for ordered write, the buffers are added
> to the head of the ordered write list. When the log needs to push
> these buffers to disk, it also walks the list from the head. The
> result is that the the ordered buffers are submitted to disk in
> reverse order.
> 
> For large writes, this means that whenever the log flushes large
> streams of reverse sequential order buffers are pushed down into the
> block layers. The elevators don't handle this particularly well, so
> IO rates tend to be significantly lower than if the IO was issued in
> ascending block order.
> 
> Queue new ordered buffers to the tail of the ordered buffer list to
> ensure that IO is dispatched in the order it was submitted. This
> should significantly improve large sequential write speeds. On a
> disk capable of 85MB/s, speeds increase from 50MB/s to 65MB/s for
> noop and from 38MB/s to 50MB/s for cfq.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/gfs2/lops.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/gfs2/lops.c b/fs/gfs2/lops.c
> index 5708edf..7278cf0 100644
> --- a/fs/gfs2/lops.c
> +++ b/fs/gfs2/lops.c
> @@ -532,9 +532,9 @@ static void databuf_lo_add(struct gfs2_sbd *sdp, struct gfs2_log_element *le)
>  		gfs2_pin(sdp, bd->bd_bh);
>  		tr->tr_num_databuf_new++;
>  		sdp->sd_log_num_databuf++;
> -		list_add(&le->le_list, &sdp->sd_log_le_databuf);
> +		list_add_tail(&le->le_list, &sdp->sd_log_le_databuf);
>  	} else {
> -		list_add(&le->le_list, &sdp->sd_log_le_ordered);
> +		list_add_tail(&le->le_list, &sdp->sd_log_le_ordered);
>  	}
>  out:
>  	gfs2_log_unlock(sdp);