2006-05-24 11:19:01

by Wu Fengguang

[permalink] [raw]
Subject: [PATCH 04/33] readahead: page flag PG_readahead

An new page flag PG_readahead is introduced as a look-ahead mark, which
reminds the caller to give the adaptive read-ahead logic a chance to do
read-ahead ahead of time for I/O pipelining.

It roughly corresponds to `ahead_start' of the stock read-ahead logic.

Signed-off-by: Wu Fengguang <[email protected]>
---

include/linux/page-flags.h | 5 +++++
mm/page_alloc.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)

--- linux-2.6.17-rc4-mm3.orig/include/linux/page-flags.h
+++ linux-2.6.17-rc4-mm3/include/linux/page-flags.h
@@ -89,6 +89,7 @@
#define PG_reclaim 17 /* To be reclaimed asap */
#define PG_nosave_free 18 /* Free, should not be written */
#define PG_buddy 19 /* Page is free, on buddy lists */
+#define PG_readahead 20 /* Reminder to do readahead */


#if (BITS_PER_LONG > 32)
@@ -372,6 +373,10 @@ extern void __mod_page_state_offset(unsi
#define SetPageUncached(page) set_bit(PG_uncached, &(page)->flags)
#define ClearPageUncached(page) clear_bit(PG_uncached, &(page)->flags)

+#define PageReadahead(page) test_bit(PG_readahead, &(page)->flags)
+#define __SetPageReadahead(page) __set_bit(PG_readahead, &(page)->flags)
+#define TestClearPageReadahead(page) test_and_clear_bit(PG_readahead, &(page)->flags)
+
struct page; /* forward declaration */

int test_clear_page_dirty(struct page *page);
--- linux-2.6.17-rc4-mm3.orig/mm/page_alloc.c
+++ linux-2.6.17-rc4-mm3/mm/page_alloc.c
@@ -564,7 +564,7 @@ static int prep_new_page(struct page *pa
if (PageReserved(page))
return 1;

- page->flags &= ~(1 << PG_uptodate | 1 << PG_error |
+ page->flags &= ~(1 << PG_uptodate | 1 << PG_error | 1 << PG_readahead |
1 << PG_referenced | 1 << PG_arch_1 |
1 << PG_checked | 1 << PG_mappedtodisk);
set_page_private(page, 0);

--


2006-05-24 12:27:52

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 04/33] readahead: page flag PG_readahead

On Wed, 2006-05-24 at 19:12 +0800, Wu Fengguang wrote:
> plain text document attachment
> (readahead-page-flag-PG_readahead.patch)
> An new page flag PG_readahead is introduced as a look-ahead mark, which
> reminds the caller to give the adaptive read-ahead logic a chance to do
> read-ahead ahead of time for I/O pipelining.
>
> It roughly corresponds to `ahead_start' of the stock read-ahead logic.
>
> Signed-off-by: Wu Fengguang <[email protected]>
> ---
>
> include/linux/page-flags.h | 5 +++++
> mm/page_alloc.c | 2 +-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> --- linux-2.6.17-rc4-mm3.orig/include/linux/page-flags.h
> +++ linux-2.6.17-rc4-mm3/include/linux/page-flags.h
> @@ -89,6 +89,7 @@
> #define PG_reclaim 17 /* To be reclaimed asap */
> #define PG_nosave_free 18 /* Free, should not be written */
> #define PG_buddy 19 /* Page is free, on buddy lists */
> +#define PG_readahead 20 /* Reminder to do readahead */
>

Page flags are gouped by four, 20 would start a new set.
Also in my tree (git from a few days ago), 20 is taken by PG_unchached.
What code is this patch-set against?



2006-05-24 12:37:44

by Wu Fengguang

[permalink] [raw]
Subject: Re: [PATCH 04/33] readahead: page flag PG_readahead

On Wed, May 24, 2006 at 02:27:36PM +0200, Peter Zijlstra wrote:
> On Wed, 2006-05-24 at 19:12 +0800, Wu Fengguang wrote:
> > plain text document attachment
> > (readahead-page-flag-PG_readahead.patch)
> > An new page flag PG_readahead is introduced as a look-ahead mark, which
> > reminds the caller to give the adaptive read-ahead logic a chance to do
> > read-ahead ahead of time for I/O pipelining.
> >
> > It roughly corresponds to `ahead_start' of the stock read-ahead logic.
> >
> > Signed-off-by: Wu Fengguang <[email protected]>
> > ---
> >
> > include/linux/page-flags.h | 5 +++++
> > mm/page_alloc.c | 2 +-
> > 2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > --- linux-2.6.17-rc4-mm3.orig/include/linux/page-flags.h
> > +++ linux-2.6.17-rc4-mm3/include/linux/page-flags.h
> > @@ -89,6 +89,7 @@
> > #define PG_reclaim 17 /* To be reclaimed asap */
> > #define PG_nosave_free 18 /* Free, should not be written */
> > #define PG_buddy 19 /* Page is free, on buddy lists */
> > +#define PG_readahead 20 /* Reminder to do readahead */
> >
>
> Page flags are gouped by four, 20 would start a new set.
> Also in my tree (git from a few days ago), 20 is taken by PG_unchached.

Thanks, grouped and renumbered it as 21.

> What code is this patch-set against?

It's against the latest -mm tree: linux-2.6.17-rc4-mm3.

Wu
---

Subject: readahead: page flag PG_readahead

An new page flag PG_readahead is introduced as a look-ahead mark, which
reminds the caller to give the adaptive read-ahead logic a chance to do
read-ahead ahead of time for I/O pipelining.

It roughly corresponds to `ahead_start' of the stock read-ahead logic.

Signed-off-by: Wu Fengguang <[email protected]>
---
include/linux/page-flags.h | 5 +++++
mm/page_alloc.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)

--- linux-2.6.17-rc4-mm3.orig/include/linux/page-flags.h
+++ linux-2.6.17-rc4-mm3/include/linux/page-flags.h
@@ -90,6 +90,8 @@
#define PG_nosave_free 18 /* Free, should not be written */
#define PG_buddy 19 /* Page is free, on buddy lists */

+#define PG_readahead 21 /* Reminder to do readahead */
+

#if (BITS_PER_LONG > 32)
/*
@@ -372,6 +374,10 @@ extern void __mod_page_state_offset(unsi
#define SetPageUncached(page) set_bit(PG_uncached, &(page)->flags)
#define ClearPageUncached(page) clear_bit(PG_uncached, &(page)->flags)

+#define PageReadahead(page) test_bit(PG_readahead, &(page)->flags)
+#define __SetPageReadahead(page) __set_bit(PG_readahead, &(page)->flags)
+#define TestClearPageReadahead(page) test_and_clear_bit(PG_readahead, &(page)->flags)
+
struct page; /* forward declaration */

int test_clear_page_dirty(struct page *page);
--- linux-2.6.17-rc4-mm3.orig/mm/page_alloc.c
+++ linux-2.6.17-rc4-mm3/mm/page_alloc.c
@@ -564,7 +564,7 @@ static int prep_new_page(struct page *pa
if (PageReserved(page))
return 1;

- page->flags &= ~(1 << PG_uptodate | 1 << PG_error |
+ page->flags &= ~(1 << PG_uptodate | 1 << PG_error | 1 << PG_readahead |
1 << PG_referenced | 1 << PG_arch_1 |
1 << PG_checked | 1 << PG_mappedtodisk);
set_page_private(page, 0);

2006-05-24 12:49:05

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 04/33] readahead: page flag PG_readahead

On Wed, 2006-05-24 at 20:37 +0800, Wu Fengguang wrote:
> On Wed, May 24, 2006 at 02:27:36PM +0200, Peter Zijlstra wrote:
> > On Wed, 2006-05-24 at 19:12 +0800, Wu Fengguang wrote:


> > > --- linux-2.6.17-rc4-mm3.orig/include/linux/page-flags.h
> > > +++ linux-2.6.17-rc4-mm3/include/linux/page-flags.h
> > > @@ -89,6 +89,7 @@
> > > #define PG_reclaim 17 /* To be reclaimed asap */
> > > #define PG_nosave_free 18 /* Free, should not be written */
> > > #define PG_buddy 19 /* Page is free, on buddy lists */
> > > +#define PG_readahead 20 /* Reminder to do readahead */
> > >
> >
> > Page flags are gouped by four, 20 would start a new set.
> > Also in my tree (git from a few days ago), 20 is taken by PG_unchached.
>
> Thanks, grouped and renumbered it as 21.
>
> > What code is this patch-set against?
>
> It's against the latest -mm tree: linux-2.6.17-rc4-mm3.

Ah, now I see, -mm has got a trick up its sleeve for PG_uncached.

20 would indeed be the correct number for -mm. Then my sole comment
would be the grouping, which is a stylish nit really.

Sorry for the confusion.

Peter

2006-05-25 16:23:46

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 04/33] readahead: page flag PG_readahead

Wu Fengguang <[email protected]> wrote:
>
> An new page flag PG_readahead is introduced as a look-ahead mark, which
> reminds the caller to give the adaptive read-ahead logic a chance to do
> read-ahead ahead of time for I/O pipelining.
>
> It roughly corresponds to `ahead_start' of the stock read-ahead logic.
>

This isn't a very revealing description of what this flag does.

> +#define __SetPageReadahead(page) __set_bit(PG_readahead, &(page)->flags)

uh-oh. This is extremly risky. Needs extensive justification, please.

2006-05-26 07:06:46

by Wu Fengguang

[permalink] [raw]
Subject: Re: [PATCH 04/33] readahead: page flag PG_readahead

On Thu, May 25, 2006 at 09:23:11AM -0700, Andrew Morton wrote:
> Wu Fengguang <[email protected]> wrote:
> >
> > An new page flag PG_readahead is introduced as a look-ahead mark, which
> > reminds the caller to give the adaptive read-ahead logic a chance to do
> > read-ahead ahead of time for I/O pipelining.
> >
> > It roughly corresponds to `ahead_start' of the stock read-ahead logic.
> >
>
> This isn't a very revealing description of what this flag does.

Updated to:

An new page flag PG_readahead is introduced.

It acts as a look-ahead mark, which tells the page reader:
Hey, it's time to invoke the adaptive read-ahead logic!
For the sake of I/O pipelining, don't wait until it runs out of
cached pages. ;-)

> > +#define __SetPageReadahead(page) __set_bit(PG_readahead, &(page)->flags)
>
> uh-oh. This is extremly risky. Needs extensive justification, please.

Ok, removed the ugly __ :-)

Wu