2.6.1 sendfile regression

DISCUSSION Felix von Leitner 5 participants 7 messages Jan 10, 2004 – Jan 12, 2004

Subsystems (2)

Memory management Page cache

lore

Share & tools

lore

Felix von Leitner

author

2004-01-10 00:00 UTC #0 lore raw

I'm getting a huge sendfile regression in 2.6.1; when serving a large file on an IPv4 TCP socket via sendfile64, the transfer starts at about 4 MB (2.6.0: >7 MB) and the hard disk like stays on 100% (which normally only happens if the file is badly fragmented, fragmentation of this file is 0%). Then, suddenly, the network performance drops dramatically, getting worse and worse. strace shows that the process is hanging inside sendfile64 (which should not happen since the socket is non-blocking). The process then stays inside sendfile for up to a minute or so and can't be interrupted or killed in that time.

Please fix!

Felix

Andrew Morton

2004-01-10 00:21 UTC #1 lore raw

Quoted 11 lines · Felix von Leitner

I'm getting a huge sendfile regression in 2.6.1; when serving a large file on an IPv4 TCP socket via sendfile64, the transfer starts at about 4 MB (2.6.0: >7 MB) and the hard disk like stays on 100% (which normally only happens if the file is badly fragmented, fragmentation of this file is 0%). Then, suddenly, the network performance drops dramatically, getting worse and worse. strace shows that the process is hanging inside sendfile64 (which should not happen since the socket is non-blocking). The process then stays inside sendfile for up to a minute or so and can't be interrupted or killed in that time.

Probably it is the ill-advised readahead tweak. Does the below patch fix it up?

mm/filemap.c renamed from mm/filemap.c~a +0 -6

diff -puN mm/filemap.c~a mm/filemap.c
@@ -596,12 +596,6 @@ void do_generic_mapping_read(struct addr
 	offset = *ppos & ~PAGE_CACHE_MASK;
 	last = (*ppos + desc->count) >> PAGE_CACHE_SHIFT;
 
-	/*
-	 * Let the readahead logic know upfront about all
-	 * the pages we'll need to satisfy this request
-	 */
-	for (; index < last; index++)
-		page_cache_readahead(mapping, ra, filp, index);
 	index = *ppos >> PAGE_CACHE_SHIFT;
 
 	for (;;) {

Ram Pai

2004-01-10 00:53 UTC #2 lore raw

Quoted 10 lines · Andrew Morton

Probably it is the ill-advised readahead tweak. Does the below patch fix it up?
diff -puN mm/filemap.c~a mm/filemap.c
mm/filemap.c renamed from mm/filemap.c~a +0 -0
@@ -596,12 +596,6 @@ void do_generic_mapping_read(struct addr
 	offset = *ppos & ~PAGE_CACHE_MASK;
 	last = (*ppos + desc->count) >> PAGE_CACHE_SHIFT;

mm/filemap.c void do_generic_mapping_read(struct addr · line 601

Quoted 10 lines

mm/filemap.c renamed from mm/filemap.c~a +0 -6

 
-	/*
-	 * Let the readahead logic know upfront about all
-	 * the pages we'll need to satisfy this request
-	 */
-	for (; index < last; index++)
-		page_cache_readahead(mapping, ra, filp, index);
 	index = *ppos >> PAGE_CACHE_SHIFT;
 
 	for (;;) {

There is a small mistake in Andrew's patch. The call to page_cache_readahead() is missing.

Try this one.

*** linux-2.6.1-rc2/mm/filemap.c        Fri Jan  9 00:47:04 2004
--- linux-2.6.1-rc2/mm/filemap.c.1      Fri Jan  9 00:46:43 2004
***************
*** 587,608 ****
                             read_actor_t actor)
  {
        struct inode *inode = mapping->host;
!       unsigned long index, offset, last;
        struct page *cached_page;
        int error;

        cached_page = NULL;
        index = *ppos >> PAGE_CACHE_SHIFT;
        offset = *ppos & ~PAGE_CACHE_MASK;
-       last = (*ppos + desc->count) >> PAGE_CACHE_SHIFT;
- 
-       /*
-        * Let the readahead logic know upfront about all
-        * the pages we'll need to satisfy this request
-        */

- for (; index < last; index++)
- page_cache_readahead(mapping, ra, filp, index);
- index = *ppos >> PAGE_CACHE_SHIFT;

        for (;;) {
                struct page *page;
--- 587,599 ----
                             read_actor_t actor)
  {
        struct inode *inode = mapping->host;
!       unsigned long index, offset;
        struct page *cached_page;
        int error;

        cached_page = NULL;
        index = *ppos >> PAGE_CACHE_SHIFT;
        offset = *ppos & ~PAGE_CACHE_MASK;

        for (;;) {
                struct page *page;
***************
*** 621,626 ****
--- 612,618 ----
                }

                cond_resched();

+ page_cache_readahead(mapping, ra, filp, index);

                nr = nr - offset;

find_page:

Quoted 3 lines

_

Andrew Morton

2004-01-10 01:42 UTC #3 lore raw

Quoted 5 lines · Ram Pai

There is a small mistake in Andrew's patch. The call to
page_cache_readahead() is missing.
Try this one.

Third time lucky.

mm/filemap.c renamed from mm/filemap.c~readahead-partial-backout +2 ~~-10~~

diff -puN mm/filemap.c~readahead-partial-backout mm/filemap.c
@@ -587,22 +587,13 @@ void do_generic_mapping_read(struct addr
 			     read_actor_t actor)
 {
 	struct inode *inode = mapping->host;
-	unsigned long index, offset, last;
+	unsigned long index, offset;
 	struct page *cached_page;
 	int error;
 
 	cached_page = NULL;
 	index = *ppos >> PAGE_CACHE_SHIFT;
 	offset = *ppos & ~PAGE_CACHE_MASK;
-	last = (*ppos + desc->count) >> PAGE_CACHE_SHIFT;
-
-	/*
-	 * Let the readahead logic know upfront about all
-	 * the pages we'll need to satisfy this request
-	 */
-	for (; index < last; index++)
-		page_cache_readahead(mapping, ra, filp, index);
-	index = *ppos >> PAGE_CACHE_SHIFT;
 
 	for (;;) {
 		struct page *page;

@@ -621,6 +612,7 @@ void do_generic_mapping_read(struct addr
 		}
 
 		cond_resched();
+		page_cache_readahead(mapping, ra, filp, index);
 
 		nr = nr - offset;
 find_page:

Lennert Buytenhek

2004-01-10 05:23 UTC #4 lore raw

In reply to Felix von Leitner

Quoted 3 lines · Felix von Leitner

strace shows that the process is hanging
inside sendfile64 (which should not happen since the socket is non-blocking).

What if the data you're sending is not in the page cache?

--L

Linus Torvalds

2004-01-12 03:26 UTC #5 lore raw

Quoted 7 lines · Lennert Buytenhek

Quoted 3 lines · Felix von Leitner
strace shows that the process is hanging
inside sendfile64 (which should not happen since the socket is non-blocking).
What if the data you're sending is not in the page cache?

It will always block on the actual page cache, although we could try to change that. However, even if it blocks, it should only block at one page at a time (or "incidental" blockage due to memory allocations etc).

Blocking for long times implies a bug.

Linus

Lennert Buytenhek

2004-01-12 11:32 UTC #6 lore raw

Quoted 8 lines · Linus Torvalds

Quoted 5 lines
Quoted 3 lines
strace shows that the process is hanging
inside sendfile64 (which should not happen since the socket is non-blocking).
What if the data you're sending is not in the page cache?
It will always block on the actual page cache, although we could try to change that.

My impression is that this is the reason why AIO sendfile was attempted. Although my wild guess is that that probably would only work on raw block devices and still not on regular filesystem files.

Quoted 4 lines

However, even if it blocks, it should only block at one page at a time (or "incidental" blockage due to memory allocations etc).
Blocking for long times implies a bug.

(Even if it only blocks a page at a time, it will happily block on the next page and the one after that as long as there is write space in the destination socket?)

--L