[Lustre-devel] readahead performance improvement

[Lustre-devel] readahead performance improvement

[adilger at sun.com]

On Nov 24, 2008  15:39 -0500, Wallior, Julien wrote:
> we are using Samba servers to share our Lustre out to the Windows world.
> We can make a few assumptions about our work load from Windows:
>
> - all the reads are sequential
> - we use 1MB IOs (which gets broken up into 64k IO by CIFS)
> - we read the whole file
> 
> I wrote a patch for lustre to help use get better performance. Basically,
> I disable the readahead algorithm and replace the initialization of the
> read-ahead window in ll_file_read by the following logic:
>
> If the read is aligned (to a 1MB boundary):
>         Bead.lrr_count=1MB
> Else
>         Do nothing

One of the problems with this change is that the first read of the file
is at offset 0, but it should not invoke readahead.  Only later reads from
the file will invoke the readahead, once a sequential access pattern is seen.
The problem is that it would take another (1MB/64kB - 1) = 15 64kB reads to
get to the next 1MB boundary.  The other problem is that this assumes the
file striping is 1MB, but that isn't always the case.


Looking at the existing Lustre ll_readahead code I see:

        /* Enlarge the RA window to encompass the full read */
	if (bead != NULL && ras->ras_window_start +
	    ras->ras_window_len < bead->lrr_start + bead->lrr_count) {
		ras->ras_window_len = bead->lrr_start + bead->lrr_count -
			ras->ras_window_start;
	}

This is incorrect in my opinion.  The readahead window should be grown
to the end of the first stripe boundary (usually 1MB, but it depends
on striping), and then continue with aligned stripe sized reads (or
multiples thereof to make full RPCs).  That can be determined by the
readahead code correctly using the obd_extent_calc() method.

It doesn't really make sense for Lustre clients to issue reads that are
less than a full stripe and/or full RPC in size.

I also see in two places the identical code:

		bead.lrr_start = *ppos >> CFS_PAGE_SHIFT;
		bead.lrr_count = (count + CFS_PAGE_SIZE - 1) >> CFS_PAGE_SHIFT;
		ll_ra_read_in(&bead);

It also makes sense that this bead initialization is done in ll_ra_read_in(),
and it should probably be renamed ll_ra_read_init().

> Therefore, when ll_readpage is called, it will prepopulate the cache
> and the LUNs will see 1MB IOs.
> 
> Would you be interested in patches like that? Should I start another
> bugzilla, or send an email to lustre-devel?

I'd be interested to see the patches, and I've CC'd lustre-devel for
further discussion.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

[Lustre-devel] readahead performance improvement

[Wallior, Julien]

> One of the problems with this change is that the first read of the file
> is at offset 0, but it should not invoke readahead.

In our case, we can do readahead starting at offset 0 because we assume
we'll be reading the whole file.

> The other problem is that this assumes the
> file striping is 1MB, but that isn't always the case.

I'm using the PTLRPC_MAX_BRW_SIZE for 1MB.

> Looking at the existing Lustre ll_readahead code I see:
>
>         /* Enlarge the RA window to encompass the full read */
>         if (bead != NULL && ras->ras_window_start +
>             ras->ras_window_len < bead->lrr_start + bead->lrr_count) {
>                 ras->ras_window_len = bead->lrr_start + bead->lrr_count -
>                         ras->ras_window_start;
>         }
>
> This is incorrect in my opinion.  The readahead window should be grown
> to the end of the first stripe boundary (usually 1MB, but it depends
> on striping), and then continue with aligned stripe sized reads (or
> multiples thereof to make full RPCs).  That can be determined by the
> readahead code correctly using the obd_extent_calc() method.

That would probably be a much better fix than mine. When we have a few
reading thread we get only 1MB IOs on the OSS. But when we have a lot
of thread (over 20), we see small IOs hitting the LUNs and that's what I'm
trying to prevent.

> I also see in two places the identical code:
>
>                 bead.lrr_start = *ppos >> CFS_PAGE_SHIFT;
>                 bead.lrr_count = (count + CFS_PAGE_SIZE - 1) >> CFS_PAGE_SHIFT;
>                 ll_ra_read_in(&bead);
>
> It also makes sense that this bead initialization is done in ll_ra_read_in(),
> and it should probably be renamed ll_ra_read_init().

That is probably a good idea. We are not using the path in ll_file_sendfile, so
I didn't change that.

> I'd be interested to see the patches, and I've CC'd lustre-devel for
> further discussion.

The patch is attached.

Julien

IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: alignedio.patch
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20081125/9edf3620/attachment.asc>