2011-06-27 13:31:24

by Andrea Righi

[permalink] [raw]
Subject: [PATCH v4 0/2] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

There were some reported problems in the past about trashing page cache when a
backup software (i.e., rsync) touches a huge amount of pages (see for example
[1]).

This problem has been almost fixed by the Minchan Kim's patch [2] and a proper
use of fadvise() in the backup software. For example this patch set [3] has
been proposed for inclusion in rsync.

However, there can be still other similar trashing problems: when the backup
software reads all the source files, some of them may be part of the actual
working set of the system. When a POSIX_FADV_DONTNEED is performed _all_ pages
are evicted from pagecache, both the working set and the use-once pages touched
only by the backup software.

With the following solution when POSIX_FADV_DONTNEED is called for an active
page instead of removing it from the page cache it is added to the tail of the
inactive list. Otherwise, if it's already in the inactive list the page is
removed from the page cache. Pages mapped by other processes or unevictable
pages are not touched at all.

In this way if the backup was the only user of a page, that page will be
immediately removed from the page cache by calling POSIX_FADV_DONTNEED. If the
page was also touched by other processes it'll be moved to the inactive list,
having another chance of being re-added to the working set, or simply reclaimed
when memory is needed.

Previous discussion about this topic can be found in [4].

Testcase:

- create a 1GB file called "zero"
- run md5sum zero to read all the pages in page cache (this is to
simulate the user activity on this file)
- run rsync zero zero_copy
- re-run md5sum zero (user activity on the working set) and measure
the time to complete this command

The test has been performed using 3.0.0-rc4 vanilla and with this patch applied
(3.0.0-rc4-fadvise); rsync has been patched with [3].

Results:

- after the backup run:
# perf stat -e block:block_bio_queue md5sum zero

avg elapsed time block:block_bio_queue
3.0.0-rc4 4.20s 8,228
3.0.0-rc4-fadvise 2.19s 0

[1] http://marc.info/?l=rsync&m=128885034930933&w=2
[2] https://lkml.org/lkml/2011/2/20/57
[3] http://lists.samba.org/archive/rsync/2010-November/025827.html
[4] http://marc.info/?l=linux-kernel&m=130877950220314&w=2

ChangeLog v3 -> v4:
- map the "drop if page was used once" policy to POSIX_FADV_DONTNEED, like the
first implementation (POSIX_FADV_NOREUSE is designed to apply a drop-behind
invalidation, not after data access, so it's not suitable to represent this
logic)
- do not change the behavior of other invalidate_mapping_pages() usage (only
POSIX_FADV_DONTNEED is changed)
- change the name of the additional __invalidate_mapping_pages() parameter
from "force" to "invalidate" (as suggested by Rik)

[PATCH v4 1/2] mm: introduce __invalidate_mapping_pages()
[PATCH v4 2/2] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

include/linux/fs.h | 8 ++++++--
mm/fadvise.c | 13 ++++++++++---
mm/swap.c | 2 +-
mm/truncate.c | 42 +++++++++++++++++++++++++++++++-----------
4 files changed, 48 insertions(+), 17 deletions(-)


2011-06-27 13:32:12

by Andrea Righi

[permalink] [raw]
Subject: [PATCH v4 1/2] mm: introduce __invalidate_mapping_pages()

This new function accepts an additional parameter respect to the old
invalidate_mapping_pages() that allows to specify when we want to apply
an aggressive policy to drop file cache pages or when we just want to
reduce cache eligibility.

The new prototype is the following:

unsigned long __invalidate_mapping_pages(struct address_space *mapping,
pgoff_t start, pgoff_t end, bool invalidate)

When invalidate is true pages are always dropped if possible. When
invalidate is false inactive pages are dropped and active pages are
moved to the inactive list.

This can be used to apply different levels of page cache invalidation
(e.g, by fadvise).

The old invalidate_mapping_pages() behavior can be mapped to
__invalidate_mapping_pages(..., true) using a C-preprocessor macro.

Signed-off-by: Andrea Righi <[email protected]>
---
include/linux/fs.h | 8 ++++++--
mm/swap.c | 2 +-
mm/truncate.c | 42 +++++++++++++++++++++++++++++++-----------
3 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6e73e2e..5869dd4e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2149,8 +2149,12 @@ extern int check_disk_change(struct block_device *);
extern int __invalidate_device(struct block_device *, bool);
extern int invalidate_partition(struct gendisk *, int);
#endif
-unsigned long invalidate_mapping_pages(struct address_space *mapping,
- pgoff_t start, pgoff_t end);
+
+#define invalidate_mapping_pages(__mapping, __start, __end) \
+ __invalidate_mapping_pages(__mapping, __start, __end, true)
+unsigned long __invalidate_mapping_pages(struct address_space *mapping,
+ pgoff_t start, pgoff_t end,
+ bool invalidate);

static inline void invalidate_remote_inode(struct inode *inode)
{
diff --git a/mm/swap.c b/mm/swap.c
index 3a442f1..a8fe6ac 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -413,7 +413,7 @@ void add_page_to_unevictable_list(struct page *page)
* 2. active, dirty/writeback page -> inactive, head, PG_reclaim
* 3. inactive, mapped page -> none
* 4. inactive, dirty/writeback page -> inactive, head, PG_reclaim
- * 5. inactive, clean -> inactive, tail
+ * 5. [in]active, clean -> inactive, tail
* 6. Others -> none
*
* In 4, why it moves inactive's head, the VM expects the page would
diff --git a/mm/truncate.c b/mm/truncate.c
index 3a29a61..fced0b4 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -312,20 +312,27 @@ void truncate_inode_pages(struct address_space *mapping, loff_t lstart)
EXPORT_SYMBOL(truncate_inode_pages);

/**
- * invalidate_mapping_pages - Invalidate all the unlocked pages of one inode
+ * __invalidate_mapping_pages - Invalidate all the unlocked pages of one inode
* @mapping: the address_space which holds the pages to invalidate
* @start: the offset 'from' which to invalidate
* @end: the offset 'to' which to invalidate (inclusive)
+ * @invalidate: aggressive cache invalidation when true
*
* This function only removes the unlocked pages, if you want to
* remove all the pages of one inode, you must call truncate_inode_pages.
*
- * invalidate_mapping_pages() will not block on IO activity. It will not
- * invalidate pages which are dirty, locked, under writeback or mapped into
- * pagetables.
+ * The @invalidate parameter can be used to apply a more aggressive policy
+ * (when true) that will always drop pages from page cache when possible, or to
+ * just reduce cache eligibility (when false). In the last case active pages
+ * will be moved to the tail of the inactive list by deactivate_page();
+ * inactive pages will be dropped in both cases.
+ *
+ * __invalidate_mapping_pages() will not block on IO activity. It will not
+ * invalidate pages which are dirty, locked, under writeback, mapped into
+ * pagetables, or on active lru when @invalidate is false.
*/
-unsigned long invalidate_mapping_pages(struct address_space *mapping,
- pgoff_t start, pgoff_t end)
+unsigned long __invalidate_mapping_pages(struct address_space *mapping,
+ pgoff_t start, pgoff_t end, bool invalidate)
{
struct pagevec pvec;
pgoff_t next = start;
@@ -356,12 +363,25 @@ unsigned long invalidate_mapping_pages(struct address_space *mapping,
next++;
if (lock_failed)
continue;
-
- ret = invalidate_inode_page(page);
+ /*
+ * Invalidation of active page is rather aggressive as
+ * we can't make sure it's not a working set of other
+ * processes.
+ *
+ * When "invalidate" is false, deactivate_page() would
+ * move active page into inactive's tail so the page
+ * will have a chance to activate again if other
+ * processes touch it.
+ */
+ if (!invalidate && PageActive(page))
+ ret = 0;
+ else
+ ret = invalidate_inode_page(page);
unlock_page(page);
/*
- * Invalidation is a hint that the page is no longer
- * of interest and try to speed up its reclaim.
+ * Invalidation of an inactive page (or any page when
+ * invalidate is true) is a hint that the page is no
+ * longer of interest and try to speed up its reclaim.
*/
if (!ret)
deactivate_page(page);
@@ -375,7 +395,7 @@ unsigned long invalidate_mapping_pages(struct address_space *mapping,
}
return count;
}
-EXPORT_SYMBOL(invalidate_mapping_pages);
+EXPORT_SYMBOL(__invalidate_mapping_pages);

/*
* This is like invalidate_complete_page(), except it ignores the page's
--
1.7.4.1

2011-06-27 13:32:19

by Andrea Righi

[permalink] [raw]
Subject: [PATCH v4 2/2] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

There were some reported problems in the past about trashing page cache
when a backup software (i.e., rsync) touches a huge amount of pages (see
for example [1]).

This problem has been almost fixed by the Minchan Kim's patch [2] and a
proper use of fadvise() in the backup software. For example this patch
set [3] has been proposed for inclusion in rsync.

However, there can be still other similar trashing problems: when the
backup software reads all the source files, some of them may be part of
the actual working set of the system. When a POSIX_FADV_DONTNEED is
performed _all_ pages are evicted from pagecache, both the working set
and the use-once pages touched only by the backup software.

With the following solution when POSIX_FADV_DONTNEED is called for an
active page instead of removing it from the page cache it is added to
the tail of the inactive list. Otherwise, if it's already in the
inactive list the page is removed from the page cache. Pages mapped by
other processes or unevictable pages are not touched at all.

In this way if the backup was the only user of a page, that page will be
immediately removed from the page cache by calling POSIX_FADV_DONTNEED.
If the page was also touched by other processes it'll be moved to the
inactive list, having another chance of being re-added to the working
set, or simply reclaimed when memory is needed.

Previous discussion about this topic can be found in [4].

[1] http://marc.info/?l=rsync&m=128885034930933&w=2
[2] https://lkml.org/lkml/2011/2/20/57
[3] http://lists.samba.org/archive/rsync/2010-November/025827.html
[4] http://marc.info/?l=linux-kernel&m=130877950220314&w=2

Signed-off-by: Andrea Righi <[email protected]>
---
mm/fadvise.c | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/mm/fadvise.c b/mm/fadvise.c
index 8d723c9..a59c1af 100644
--- a/mm/fadvise.c
+++ b/mm/fadvise.c
@@ -106,7 +106,7 @@ SYSCALL_DEFINE(fadvise64_64)(int fd, loff_t offset, loff_t len, int advice)
nrpages = end_index - start_index + 1;
if (!nrpages)
nrpages = ~0UL;
-
+
ret = force_page_cache_readahead(mapping, file,
start_index,
nrpages);
@@ -123,9 +123,16 @@ SYSCALL_DEFINE(fadvise64_64)(int fd, loff_t offset, loff_t len, int advice)
start_index = (offset+(PAGE_CACHE_SIZE-1)) >> PAGE_CACHE_SHIFT;
end_index = (endbyte >> PAGE_CACHE_SHIFT);

+ /*
+ * Reduce cache eligibility.
+ *
+ * This does not guarantee that pages are always dropped from
+ * page cache: active pages will be moved to the tail of the
+ * inactive list; inactive pages will be dropped if possible.
+ */
if (end_index >= start_index)
- invalidate_mapping_pages(mapping, start_index,
- end_index);
+ __invalidate_mapping_pages(mapping, start_index,
+ end_index, false);
break;
default:
ret = -EINVAL;
--
1.7.4.1

2011-06-28 22:13:36

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v4 0/2] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

On Mon, 27 Jun 2011 15:29:19 +0200
Andrea Righi <[email protected]> wrote:

> There were some reported problems in the past about trashing page cache when a
> backup software (i.e., rsync) touches a huge amount of pages (see for example
> [1]).
>
> This problem has been almost fixed by the Minchan Kim's patch [2] and a proper
> use of fadvise() in the backup software. For example this patch set [3] has
> been proposed for inclusion in rsync.
>
> However, there can be still other similar trashing problems: when the backup
> software reads all the source files, some of them may be part of the actual
> working set of the system. When a POSIX_FADV_DONTNEED is performed _all_ pages
> are evicted from pagecache, both the working set and the use-once pages touched
> only by the backup software.
>
> With the following solution when POSIX_FADV_DONTNEED is called for an active
> page instead of removing it from the page cache it is added to the tail of the
> inactive list. Otherwise, if it's already in the inactive list the page is
> removed from the page cache. Pages mapped by other processes or unevictable
> pages are not touched at all.
>
> In this way if the backup was the only user of a page, that page will be
> immediately removed from the page cache by calling POSIX_FADV_DONTNEED. If the
> page was also touched by other processes it'll be moved to the inactive list,
> having another chance of being re-added to the working set, or simply reclaimed
> when memory is needed.

So if an application touches a page twice and then runs
POSIX_FADV_DONTNEED, that page will now not be freed.

That's a big behaviour change. For many existing users
POSIX_FADV_DONTNEED simply doesn't work any more!

I'd have thought that adding a new POSIX_FADV_ANDREA would be safer
than this.


The various POSIX_FADV_foo's are so ill-defined that it was a mistake
to ever use them. We should have done something overtly linux-specific
and given userspace more explicit and direct pagecache control.

2011-06-28 22:56:57

by Andrea Righi

[permalink] [raw]
Subject: Re: [PATCH v4 0/2] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

On Tue, Jun 28, 2011 at 03:12:33PM -0700, Andrew Morton wrote:
> On Mon, 27 Jun 2011 15:29:19 +0200
> Andrea Righi <[email protected]> wrote:
>
> > There were some reported problems in the past about trashing page cache when a
> > backup software (i.e., rsync) touches a huge amount of pages (see for example
> > [1]).
> >
> > This problem has been almost fixed by the Minchan Kim's patch [2] and a proper
> > use of fadvise() in the backup software. For example this patch set [3] has
> > been proposed for inclusion in rsync.
> >
> > However, there can be still other similar trashing problems: when the backup
> > software reads all the source files, some of them may be part of the actual
> > working set of the system. When a POSIX_FADV_DONTNEED is performed _all_ pages
> > are evicted from pagecache, both the working set and the use-once pages touched
> > only by the backup software.
> >
> > With the following solution when POSIX_FADV_DONTNEED is called for an active
> > page instead of removing it from the page cache it is added to the tail of the
> > inactive list. Otherwise, if it's already in the inactive list the page is
> > removed from the page cache. Pages mapped by other processes or unevictable
> > pages are not touched at all.
> >
> > In this way if the backup was the only user of a page, that page will be
> > immediately removed from the page cache by calling POSIX_FADV_DONTNEED. If the
> > page was also touched by other processes it'll be moved to the inactive list,
> > having another chance of being re-added to the working set, or simply reclaimed
> > when memory is needed.
>
> So if an application touches a page twice and then runs
> POSIX_FADV_DONTNEED, that page will now not be freed.
>
> That's a big behaviour change. For many existing users
> POSIX_FADV_DONTNEED simply doesn't work any more!

Yes. This is the main concern that was raised by P?draig.

>
> I'd have thought that adding a new POSIX_FADV_ANDREA would be safer
> than this.

Actually Jerry (in cc) proposed
POSIX_FADV_IDONTNEEDTHISBUTIFSOMEBODYELSEDOESTHENDONTTOUCHIT in a
private email. :)

>
>
> The various POSIX_FADV_foo's are so ill-defined that it was a mistake
> to ever use them. We should have done something overtly linux-specific
> and given userspace more explicit and direct pagecache control.

That would give us the possibility to implement a wide range of
different operations (drop, drop if used once, add to the active list,
add to the inactive list, etc..). Some users always complain that they
would like to have a better control over the page cache from userspace.

-Andrea

2011-06-28 23:04:18

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v4 0/2] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

On Wed, 29 Jun 2011 00:56:45 +0200
Andrea Righi <[email protected]> wrote:

> > >
> > > In this way if the backup was the only user of a page, that page will be
> > > immediately removed from the page cache by calling POSIX_FADV_DONTNEED. If the
> > > page was also touched by other processes it'll be moved to the inactive list,
> > > having another chance of being re-added to the working set, or simply reclaimed
> > > when memory is needed.
> >
> > So if an application touches a page twice and then runs
> > POSIX_FADV_DONTNEED, that page will now not be freed.
> >
> > That's a big behaviour change. For many existing users
> > POSIX_FADV_DONTNEED simply doesn't work any more!
>
> Yes. This is the main concern that was raised by P__draig.
>
> >
> > I'd have thought that adding a new POSIX_FADV_ANDREA would be safer
> > than this.
>
> Actually Jerry (in cc) proposed
> POSIX_FADV_IDONTNEEDTHISBUTIFSOMEBODYELSEDOESTHENDONTTOUCHIT in a
> private email. :)

Sounds good. Needs more underscores though.

> >
> >
> > The various POSIX_FADV_foo's are so ill-defined that it was a mistake
> > to ever use them. We should have done something overtly linux-specific
> > and given userspace more explicit and direct pagecache control.
>
> That would give us the possibility to implement a wide range of
> different operations (drop, drop if used once, add to the active list,
> add to the inactive list, etc..). Some users always complain that they
> would like to have a better control over the page cache from userspace.

Well, I'd listen to proposals ;)

One thing we must be careful about is to not expose things like "active
list" to userspace. linux-4.5 may not _have_ an active list, and its
implementors would hate us and would have to jump through hoops to
implement vaguely compatible behaviour in the new scheme.

So any primitives which are exposed should be easily implementable and
should *make sense* within any future scheme...

2011-06-29 11:21:05

by Pádraig Brady

[permalink] [raw]
Subject: Re: [PATCH v4 0/2] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

On 29/06/11 00:03, Andrew Morton wrote:
> On Wed, 29 Jun 2011 00:56:45 +0200
> Andrea Righi <[email protected]> wrote:
>
>>>>
>>>> In this way if the backup was the only user of a page, that page will be
>>>> immediately removed from the page cache by calling POSIX_FADV_DONTNEED. If the
>>>> page was also touched by other processes it'll be moved to the inactive list,
>>>> having another chance of being re-added to the working set, or simply reclaimed
>>>> when memory is needed.
>>>
>>> So if an application touches a page twice and then runs
>>> POSIX_FADV_DONTNEED, that page will now not be freed.
>>>
>>> That's a big behaviour change. For many existing users
>>> POSIX_FADV_DONTNEED simply doesn't work any more!
>>
>> Yes. This is the main concern that was raised by P__draig.
>>
>>>
>>> I'd have thought that adding a new POSIX_FADV_ANDREA would be safer
>>> than this.
>>
>> Actually Jerry (in cc) proposed
>> POSIX_FADV_IDONTNEEDTHISBUTIFSOMEBODYELSEDOESTHENDONTTOUCHIT in a
>> private email. :)
>
> Sounds good. Needs more underscores though.
>
>>>
>>>
>>> The various POSIX_FADV_foo's are so ill-defined that it was a mistake
>>> to ever use them. We should have done something overtly linux-specific
>>> and given userspace more explicit and direct pagecache control.
>>
>> That would give us the possibility to implement a wide range of
>> different operations (drop, drop if used once, add to the active list,
>> add to the inactive list, etc..). Some users always complain that they
>> would like to have a better control over the page cache from userspace.
>
> Well, I'd listen to proposals ;)
>
> One thing we must be careful about is to not expose things like "active
> list" to userspace. linux-4.5 may not _have_ an active list, and its
> implementors would hate us and would have to jump through hoops to
> implement vaguely compatible behaviour in the new scheme.
>
> So any primitives which are exposed should be easily implementable and
> should *make sense* within any future scheme...

Agreed.

In fairness to posix_fadvise(), I think it's designed to
specify hints for the current process' use of data
so that it can get at it more efficiently and also be
allow the system to manipulate cache more efficiently.
I.E. it's not meant for direct control of the cache.

That being said, existing use has allowed this,
and it would be nice not to change without consideration.

I've mentioned how high level cache control functions
might map to the existing FADV knobs here:

http://marc.info/?l=linux-kernel&m=130917619416123&w=2

cheers,
P?draig.

2011-06-29 14:05:07

by Andrea Righi

[permalink] [raw]
Subject: Re: [PATCH v4 0/2] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

On Wed, Jun 29, 2011 at 12:20:22PM +0100, P?draig Brady wrote:
> On 29/06/11 00:03, Andrew Morton wrote:
> > On Wed, 29 Jun 2011 00:56:45 +0200
> > Andrea Righi <[email protected]> wrote:
> >
> >>>>
> >>>> In this way if the backup was the only user of a page, that page will be
> >>>> immediately removed from the page cache by calling POSIX_FADV_DONTNEED. If the
> >>>> page was also touched by other processes it'll be moved to the inactive list,
> >>>> having another chance of being re-added to the working set, or simply reclaimed
> >>>> when memory is needed.
> >>>
> >>> So if an application touches a page twice and then runs
> >>> POSIX_FADV_DONTNEED, that page will now not be freed.
> >>>
> >>> That's a big behaviour change. For many existing users
> >>> POSIX_FADV_DONTNEED simply doesn't work any more!
> >>
> >> Yes. This is the main concern that was raised by P__draig.
> >>
> >>>
> >>> I'd have thought that adding a new POSIX_FADV_ANDREA would be safer
> >>> than this.
> >>
> >> Actually Jerry (in cc) proposed
> >> POSIX_FADV_IDONTNEEDTHISBUTIFSOMEBODYELSEDOESTHENDONTTOUCHIT in a
> >> private email. :)
> >
> > Sounds good. Needs more underscores though.
> >
> >>>
> >>>
> >>> The various POSIX_FADV_foo's are so ill-defined that it was a mistake
> >>> to ever use them. We should have done something overtly linux-specific
> >>> and given userspace more explicit and direct pagecache control.
> >>
> >> That would give us the possibility to implement a wide range of
> >> different operations (drop, drop if used once, add to the active list,
> >> add to the inactive list, etc..). Some users always complain that they
> >> would like to have a better control over the page cache from userspace.
> >
> > Well, I'd listen to proposals ;)
> >
> > One thing we must be careful about is to not expose things like "active
> > list" to userspace. linux-4.5 may not _have_ an active list, and its
> > implementors would hate us and would have to jump through hoops to
> > implement vaguely compatible behaviour in the new scheme.
> >
> > So any primitives which are exposed should be easily implementable and
> > should *make sense* within any future scheme...
>
> Agreed.
>
> In fairness to posix_fadvise(), I think it's designed to
> specify hints for the current process' use of data
> so that it can get at it more efficiently and also be
> allow the system to manipulate cache more efficiently.
> I.E. it's not meant for direct control of the cache.
>
> That being said, existing use has allowed this,
> and it would be nice not to change without consideration.
>
> I've mentioned how high level cache control functions
> might map to the existing FADV knobs here:
>
> http://marc.info/?l=linux-kernel&m=130917619416123&w=2
>
> cheers,
> P?draig.

OK, your proposal seems a good start to implement a better cache control
interface.

Basically you're proposing to provide the following operations:
1. DROP
2. DROP if used once
3. ADD
4. ADD if there's space

I would also add for sure:
5. ADD and will use once

Some of them are already implemented by the available fadvise()
operations, like 1 (POSIX_FADV_DONTNEED) and 3 (POSIX_FADV_WILLNEED).
Option 5 can be mapped to POSIX_FADV_NOREUSE, but it's not yet
implemented.

I need to think a little bit more about all of this. I'll try to post a
new RFC, proposing the list of high-level operations to implement the
better page cache control from userspace.

Suggestions, comments, ideas are always welcome.

Thanks,
-Andrea