lkml.org 
[lkml]   [2004]   [Aug]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: filemap_fdatawait() wait_on_page_writeback_range(mapping, 0, -1)?
On Thu, Aug 19, 2004 at 02:49:47PM -0700, Andrew Morton wrote:
> Marcelo Tosatti <marcelo.tosatti@cyclades.com> wrote:
> >
> > Hi Andrew,
> >
> > I dont understand why we do call wait_on_page_writeback_range() with -1
> > as the "end" argument.
>
> "every page in the file".
>
> > -1 sounds pretty stupid at first, it does unnecessary calls to
> > the radix lookup code.
>
> I guess it could cause one extra call into the lookup code. There's an
> additional check in -mm's wait_on_page_writeback_range() which would prevent
> that.

this?

+ /* until radix tree lookup accepts end_index */
+ if (page->index > end)
+ continue;

I dont see why that would make a difference.

What seem to happen is that when we get near the EOF, the min calculation
which could make sense, doesnt:

min(end - index, (pgoff_t)PAGEVEC_SIZE-1)

and some unnecessary cycles will be spent doing search at __lookup_tag(). And
using i_size_read() is more meaningful anyway.

What I'm trying to do is make wait_on_page_writeback_range() do reverse
search instead ascending. Since we write pages in ascending order, doing
the wait on reverse order makes sense and will avoid possibly tons of
wakeups.

Naive me tried to implement that using pagevec_lookup_tag(), but I'm
convinced we need pagevec_reverse_lookup_tag() do reverse search
on the radix tree. I'll try getting that done on the weekend.

What you think about it?

> > --- a/mm/filemap.c.orig 2004-08-19 14:36:02.000000000 -0300
> > +++ b/mm/filemap.c.isize 2004-08-19 18:17:14.000000000 -0300
> > @@ -231,7 +231,7 @@
> > */
> > int filemap_fdatawait(struct address_space *mapping)
> >
> > - return wait_on_page_writeback_range(mapping, 0, -1);
> > + return wait_on_page_writeback_range(mapping, 0, i_size_read(mapping->host));
> > }
>
> That would need a >> PAGE_CACHE_SHIFT

Oh yes, right. I was too excited :)

Fixed version with changelog, against 2.6.8.1-mm2:

filemap_fdatawait() calls wait_on_page_writeback_range() with -1 as "end" parameter.
This is not needed since we know the EOF from the inode. Use that instead.

Signed-off-by: Marcelo Tosatti <marcelo.tosatti@cyclades.com>

diff -u linux-2.6.8/mm/filemap.c.orig linux-2.6.8/mm/filemap.c
--- linux-2.6.8/mm/filemap.c.orig 2004-08-19 20:11:52.000000000 -0300
+++ linux-2.6.8/mm/filemap.c 2004-08-19 20:12:40.000000000 -0300
@@ -288,7 +288,8 @@
*/
int filemap_fdatawait(struct address_space *mapping)
{
- return wait_on_page_writeback_range(mapping, 0, -1);
+ return wait_on_page_writeback_range(mapping, 0,
+ i_size_read(mapping->host) >> PAGE_CACHE_SHIFT);
}
EXPORT_SYMBOL(filemap_fdatawait);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:05    [W:0.050 / U:2.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site