[PATCH] readahead: add comments on PG_readahead

[PATCH] readahead: add comments on PG_readahead

[Wu Fengguang]

Add comments to clarify the easily misunderstood PG_readahead timing.

PG_readahead is a trigger to say, when you get this far, it's time to
think about kicking off the _next_ readahead.            -- Hugh

CC: Hugh Dickins <hughd@google.com>
CC: Matthew Wilcox <willy@linux.intel.com>
Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 mm/readahead.c |   27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

--- linux-next.orig/mm/readahead.c	2011-08-05 10:53:01.000000000 +0800
+++ linux-next/mm/readahead.c	2011-08-05 11:40:06.000000000 +0800
@@ -185,6 +185,14 @@ __do_page_cache_readahead(struct address
 			break;
 		page->index = page_offset;
 		list_add(&page->lru, &page_pool);
+		/*
+		 * set PG_readahead to trigger the _next_ ASYNC readahead.
+		 *
+		 *     |----------------- nr_to_read ---------------->|
+		 *     |==================#===========================|
+		 *                        |<---- lookahead_size ------|
+		 *       PG_readahead mark^
+		 */
 		if (page_idx == nr_to_read - lookahead_size)
 			SetPageReadahead(page);
 		ret++;
@@ -321,6 +329,25 @@ static unsigned long get_next_ra_size(st
  * indicator. The flag won't be set on already cached pages, to avoid the
  * readahead-for-nothing fuss, saving pointless page cache lookups.
  *
+ * A typical readahead time chart for a sequential read stream. Note that when
+ * read(2) hits the PG_readahead mark, a new readahead will be started and the
+ * PG_readahead mark will be "pushed forward" by clearing the old PG_readahead
+ * and setting a new PG_readahead in the new readahead window.
+ *
+ * t0
+ * t1 +#__                         ==>  SYNC readahead triggered by page miss
+ * t2 -+__#_______                 ==> ASYNC readahead triggered by PG_readahead
+ * t3 --+_#_______
+ * t4 ---+#_______
+ * t5 ----+_______#_______________ ==> ASYNC readahead triggered by PG_readahead
+ * t6 -----+______#_______________
+ * t7 ------+_____#_______________
+ *
+ * [-] accessed page
+ * [+] the page read(2) is accessing
+ * [#] the PG_readahead mark
+ * [_] readahead page (newly brought into page cache but not yet accessed)
+ *
  * prev_pos tracks the last visited byte in the _previous_ read request.
  * It should be maintained by the caller, and will be used for detecting
  * small random reads. Note that the readahead algorithm checks loosely

Re: [PATCH] readahead: add comments on PG_readahead

[Minchan Kim]

On Fri, Aug 05, 2011 at 11:50:40AM +0800, Wu Fengguang wrote:
> Add comments to clarify the easily misunderstood PG_readahead timing.
> 
> PG_readahead is a trigger to say, when you get this far, it's time to
> think about kicking off the _next_ readahead.            -- Hugh
> 
> CC: Hugh Dickins <hughd@google.com>
> CC: Matthew Wilcox <willy@linux.intel.com>
> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

I expect it would be very helpful.

-- 
Kind regards,
Minchan Kim