[PATCH 0/2] readahead update on splice reads

[PATCH 0/2] readahead update on splice reads

[Fengguang Wu]

Andrew,

The two patches optimizes readahead invocations in splice reads:

	readahead: move synchronous readahead call out of splice loop
	readahead: pass real splice size

They can be appended to readahead-convert-splice-invocations.patch in -mm tree.


diffstat:

 fs/splice.c |   21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

Regards,
Fengguang
--

[PATCH 0/2] readahead update on splice reads

[Fengguang Wu]

Andrew,

The two patches optimizes readahead invocations in splice reads:

	readahead: move synchronous readahead call out of splice loop
	readahead: pass real splice size

They can be appended to readahead-convert-splice-invocations.patch in -mm tree.


diffstat:

 fs/splice.c |   21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

Regards,
Fengguang
--

[PATCH 1/2] readahead: move synchronous readahead call out of splice loop

[Fengguang Wu]

[-- Attachment #1: readahead-splice-move-out-of-loop.patch --]
[-- Type: text/plain, Size: 1580 bytes --]

Move synchronous page_cache_readahead_ondemand() call out of splice loop.

This avoids one pointless page allocation/insertion in case of non-zero
ra_pages, or many pointless readahead calls in case of zero ra_pages.

Note that if a user sets ra_pages to less than PIPE_BUFFERS=16 pages, he will
not get expected readahead behavior anyway.  The splice code works in batches
of 16 pages, which can be taken as another form of synchronous readahead.

Signed-off-by: Fengguang Wu <wfg@mail.ustc.edu.cn>
---
 fs/splice.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

--- linux-2.6.22-rc3-mm1.orig/fs/splice.c
+++ linux-2.6.22-rc3-mm1/fs/splice.c
@@ -299,12 +299,16 @@ __generic_file_splice_read(struct file *
 	 * Lookup the (hopefully) full range of pages we need.
 	 */
 	spd.nr_pages = find_get_pages_contig(mapping, index, nr_pages, pages);
+	index += spd.nr_pages;
 
 	/*
 	 * If find_get_pages_contig() returned fewer pages than we needed,
-	 * allocate the rest.
+	 * readahead/allocate the rest.
 	 */
-	index += spd.nr_pages;
+	if (spd.nr_pages < nr_pages)
+		page_cache_readahead_ondemand(mapping, &in->f_ra, in,
+				NULL, index, nr_pages - spd.nr_pages);
+
 	while (spd.nr_pages < nr_pages) {
 		/*
 		 * Page could be there, find_get_pages_contig() breaks on
@@ -312,9 +316,6 @@ __generic_file_splice_read(struct file *
 		 */
 		page = find_get_page(mapping, index);
 		if (!page) {
-			page_cache_readahead_ondemand(mapping, &in->f_ra, in,
-					NULL, index, nr_pages - spd.nr_pages);
-
 			/*
 			 * page didn't exist, allocate one.
 			 */

--

[PATCH 1/2] readahead: move synchronous readahead call out of splice loop

[Fengguang Wu]

[-- Attachment #1: readahead-splice-move-out-of-loop.patch --]
[-- Type: text/plain, Size: 1580 bytes --]

Move synchronous page_cache_readahead_ondemand() call out of splice loop.

This avoids one pointless page allocation/insertion in case of non-zero
ra_pages, or many pointless readahead calls in case of zero ra_pages.

Note that if a user sets ra_pages to less than PIPE_BUFFERS=16 pages, he will
not get expected readahead behavior anyway.  The splice code works in batches
of 16 pages, which can be taken as another form of synchronous readahead.

Signed-off-by: Fengguang Wu <wfg@mail.ustc.edu.cn>
---
 fs/splice.c |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

--- linux-2.6.22-rc3-mm1.orig/fs/splice.c
+++ linux-2.6.22-rc3-mm1/fs/splice.c
@@ -299,12 +299,16 @@ __generic_file_splice_read(struct file *
 	 * Lookup the (hopefully) full range of pages we need.
 	 */
 	spd.nr_pages = find_get_pages_contig(mapping, index, nr_pages, pages);
+	index += spd.nr_pages;
 
 	/*
 	 * If find_get_pages_contig() returned fewer pages than we needed,
-	 * allocate the rest.
+	 * readahead/allocate the rest.
 	 */
-	index += spd.nr_pages;
+	if (spd.nr_pages < nr_pages)
+		page_cache_readahead_ondemand(mapping, &in->f_ra, in,
+				NULL, index, nr_pages - spd.nr_pages);
+
 	while (spd.nr_pages < nr_pages) {
 		/*
 		 * Page could be there, find_get_pages_contig() breaks on
@@ -312,9 +316,6 @@ __generic_file_splice_read(struct file *
 		 */
 		page = find_get_page(mapping, index);
 		if (!page) {
-			page_cache_readahead_ondemand(mapping, &in->f_ra, in,
-					NULL, index, nr_pages - spd.nr_pages);
-
 			/*
 			 * page didn't exist, allocate one.
 			 */

--

[PATCH 2/2] readahead: pass real splice size

[Fengguang Wu]

[-- Attachment #1: readahead-fix-splice-size.patch --]
[-- Type: text/plain, Size: 2025 bytes --]

Pass real splice size to page_cache_readahead_ondemand().

The splice code works in chunks of 16 pages internally.
The readahead code should be told of the overall splice size, instead of
the internal chunk size. Otherwize bad things may happen. Imagine some
17-page random splice reads. The code before this patch will result in
two readahead calls: readahead(16); readahead(1); That leads to one
16-page I/O and one 32-page I/O: one extra I/O and 31 readahead miss pages.

Signed-off-by: Fengguang Wu <wfg@mail.ustc.edu.cn>
---
 fs/splice.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

--- linux-2.6.22-rc3-mm1.orig/fs/splice.c
+++ linux-2.6.22-rc3-mm1/fs/splice.c
@@ -267,7 +267,7 @@ __generic_file_splice_read(struct file *
 			   unsigned int flags)
 {
 	struct address_space *mapping = in->f_mapping;
-	unsigned int loff, nr_pages;
+	unsigned int loff, nr_pages, req_pages;
 	struct page *pages[PIPE_BUFFERS];
 	struct partial_page partial[PIPE_BUFFERS];
 	struct page *page;
@@ -284,10 +284,8 @@ __generic_file_splice_read(struct file *
 
 	index = *ppos >> PAGE_CACHE_SHIFT;
 	loff = *ppos & ~PAGE_CACHE_MASK;
-	nr_pages = (len + loff + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
-
-	if (nr_pages > PIPE_BUFFERS)
-		nr_pages = PIPE_BUFFERS;
+	req_pages = (len + loff + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
+	nr_pages = min(req_pages, (unsigned)PIPE_BUFFERS);
 
 	/*
 	 * Now fill in the holes:
@@ -307,7 +305,7 @@ __generic_file_splice_read(struct file *
 	 */
 	if (spd.nr_pages < nr_pages)
 		page_cache_readahead_ondemand(mapping, &in->f_ra, in,
-				NULL, index, nr_pages - spd.nr_pages);
+				NULL, index, req_pages - spd.nr_pages);
 
 	while (spd.nr_pages < nr_pages) {
 		/*
@@ -363,7 +361,7 @@ __generic_file_splice_read(struct file *
 
 		if (PageReadahead(page))
 			page_cache_readahead_ondemand(mapping, &in->f_ra, in,
-					page, index, nr_pages - page_nr);
+					page, index, req_pages - page_nr);
 
 		/*
 		 * If the page isn't uptodate, we may need to start io on it

--

[PATCH 2/2] readahead: pass real splice size

[Fengguang Wu]

[-- Attachment #1: readahead-fix-splice-size.patch --]
[-- Type: text/plain, Size: 2025 bytes --]

Pass real splice size to page_cache_readahead_ondemand().

The splice code works in chunks of 16 pages internally.
The readahead code should be told of the overall splice size, instead of
the internal chunk size. Otherwize bad things may happen. Imagine some
17-page random splice reads. The code before this patch will result in
two readahead calls: readahead(16); readahead(1); That leads to one
16-page I/O and one 32-page I/O: one extra I/O and 31 readahead miss pages.

Signed-off-by: Fengguang Wu <wfg@mail.ustc.edu.cn>
---
 fs/splice.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

--- linux-2.6.22-rc3-mm1.orig/fs/splice.c
+++ linux-2.6.22-rc3-mm1/fs/splice.c
@@ -267,7 +267,7 @@ __generic_file_splice_read(struct file *
 			   unsigned int flags)
 {
 	struct address_space *mapping = in->f_mapping;
-	unsigned int loff, nr_pages;
+	unsigned int loff, nr_pages, req_pages;
 	struct page *pages[PIPE_BUFFERS];
 	struct partial_page partial[PIPE_BUFFERS];
 	struct page *page;
@@ -284,10 +284,8 @@ __generic_file_splice_read(struct file *
 
 	index = *ppos >> PAGE_CACHE_SHIFT;
 	loff = *ppos & ~PAGE_CACHE_MASK;
-	nr_pages = (len + loff + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
-
-	if (nr_pages > PIPE_BUFFERS)
-		nr_pages = PIPE_BUFFERS;
+	req_pages = (len + loff + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
+	nr_pages = min(req_pages, (unsigned)PIPE_BUFFERS);
 
 	/*
 	 * Now fill in the holes:
@@ -307,7 +305,7 @@ __generic_file_splice_read(struct file *
 	 */
 	if (spd.nr_pages < nr_pages)
 		page_cache_readahead_ondemand(mapping, &in->f_ra, in,
-				NULL, index, nr_pages - spd.nr_pages);
+				NULL, index, req_pages - spd.nr_pages);
 
 	while (spd.nr_pages < nr_pages) {
 		/*
@@ -363,7 +361,7 @@ __generic_file_splice_read(struct file *
 
 		if (PageReadahead(page))
 			page_cache_readahead_ondemand(mapping, &in->f_ra, in,
-					page, index, nr_pages - page_nr);
+					page, index, req_pages - page_nr);
 
 		/*
 		 * If the page isn't uptodate, we may need to start io on it

--

Re: [PATCH 0/2] readahead update on splice reads

[Jens Axboe]

On Tue, Jun 12 2007, Fengguang Wu wrote:
> Andrew,
> 
> The two patches optimizes readahead invocations in splice reads:
> 
> 	readahead: move synchronous readahead call out of splice loop
> 	readahead: pass real splice size
> 
> They can be appended to readahead-convert-splice-invocations.patch in
> -mm tree.

Patches look nice, but unfortunately only -mm stuff and will need to be
rebased once 2.6.23 opens anyway.

Where are we on the ondemand read-ahead merging? -mm has continually had
different read-ahead code than mainline now for seemingly years, seems a
bit pointless to me. Either we get the stuff merged, or toss it out.
Nothing needs to simmer that long.

I suspect we wont see any real potential regressions before it hits
mainline anyway.

-- 
Jens Axboe