2011-06-22 21:51:32

by Andrea Righi

[permalink] [raw]
Subject: [PATCH RFC] 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_fadvise(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_fadvise(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.

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_fadvise(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.

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" (rsync is patched with [3])
- 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).

Results:
avg elapsed time block:block_bio_queue
3.0.0-rc4 4.127s 8,214
3.0.0-rc4-fadvise 2.146s 0

In the first case the file is evicted from page cache completely and we
must re-read it from the disk. In the second case the file is still in
page cache (in the inactive list) and we don't need any other additional
I/O operation.

[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

Signed-off-by: Andrea Righi <[email protected]>
---
mm/swap.c | 9 +++++----
mm/truncate.c | 5 ++++-
2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/mm/swap.c b/mm/swap.c
index 3a442f1..fc8bb76 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -411,10 +411,11 @@ void add_page_to_unevictable_list(struct page *page)
*
* 1. active, mapped page -> none
* 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
- * 6. Others -> none
+ * 3. active, clean -> inactive, tail
+ * 4. inactive, mapped page -> none
+ * 5. inactive, dirty/writeback page -> inactive, head, PG_reclaim
+ * 6. inactive, clean -> inactive, tail
+ * 7. Others -> none
*
* In 4, why it moves inactive's head, the VM expects the page would
* be write it out by flusher threads as this is much more effective
diff --git a/mm/truncate.c b/mm/truncate.c
index 3a29a61..043aabd 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -357,7 +357,10 @@ unsigned long invalidate_mapping_pages(struct address_space *mapping,
if (lock_failed)
continue;

- ret = invalidate_inode_page(page);
+ if (PageActive(page))
+ ret = 0;
+ else
+ ret = invalidate_inode_page(page);
unlock_page(page);
/*
* Invalidation is a hint that the page is no longer
--
1.7.4.1


2011-06-22 23:05:21

by Rik van Riel

[permalink] [raw]
Subject: Re: [PATCH RFC] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

On 06/22/2011 05:51 PM, Andrea Righi 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_fadvise(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_fadvise(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.

> Signed-off-by: Andrea Righi<[email protected]>

Acked-by: Rik van Riel <[email protected]>

--
All rights reversed

2011-06-23 03:15:53

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH RFC] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

(2011/06/23 6:51), Andrea Righi 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_fadvise(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_fadvise(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.
>
> 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_fadvise(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.
>
> 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" (rsync is patched with [3])
> - 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).
>
> Results:
> avg elapsed time block:block_bio_queue
> 3.0.0-rc4 4.127s 8,214
> 3.0.0-rc4-fadvise 2.146s 0
>
> In the first case the file is evicted from page cache completely and we
> must re-read it from the disk. In the second case the file is still in
> page cache (in the inactive list) and we don't need any other additional
> I/O operation.
>
> [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
>
> Signed-off-by: Andrea Righi <[email protected]>
> ---
> mm/swap.c | 9 +++++----
> mm/truncate.c | 5 ++++-
> 2 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/mm/swap.c b/mm/swap.c
> index 3a442f1..fc8bb76 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -411,10 +411,11 @@ void add_page_to_unevictable_list(struct page *page)
> *
> * 1. active, mapped page -> none
> * 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
> - * 6. Others -> none
> + * 3. active, clean -> inactive, tail
> + * 4. inactive, mapped page -> none
> + * 5. inactive, dirty/writeback page -> inactive, head, PG_reclaim
> + * 6. inactive, clean -> inactive, tail
> + * 7. Others -> none
> *
> * In 4, why it moves inactive's head, the VM expects the page would
> * be write it out by flusher threads as this is much more effective
> diff --git a/mm/truncate.c b/mm/truncate.c
> index 3a29a61..043aabd 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -357,7 +357,10 @@ unsigned long invalidate_mapping_pages(struct address_space *mapping,
> if (lock_failed)
> continue;
>
> - ret = invalidate_inode_page(page);
> + if (PageActive(page))
> + ret = 0;
> + else
> + ret = invalidate_inode_page(page);

So, after this patch, following comment is a bit outdated. we deactivate
the page even if it's not invalidated.

/*
* Invalidation is a hint that the page is no longer
* of interest and try to speed up its reclaim.
*/

Can you please fix the comment too? Other than that,
Reviewed-by: KOSAKI Motohiro <[email protected]>







2011-06-23 06:18:09

by Andrea Righi

[permalink] [raw]
Subject: Re: [PATCH RFC] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

On Thu, Jun 23, 2011 at 12:15:26PM +0900, KOSAKI Motohiro wrote:
> (2011/06/23 6:51), Andrea Righi 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_fadvise(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_fadvise(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.
> >
> > 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_fadvise(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.
> >
> > 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" (rsync is patched with [3])
> > - 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).
> >
> > Results:
> > avg elapsed time block:block_bio_queue
> > 3.0.0-rc4 4.127s 8,214
> > 3.0.0-rc4-fadvise 2.146s 0
> >
> > In the first case the file is evicted from page cache completely and we
> > must re-read it from the disk. In the second case the file is still in
> > page cache (in the inactive list) and we don't need any other additional
> > I/O operation.
> >
> > [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
> >
> > Signed-off-by: Andrea Righi <[email protected]>
> > ---
> > mm/swap.c | 9 +++++----
> > mm/truncate.c | 5 ++++-
> > 2 files changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/mm/swap.c b/mm/swap.c
> > index 3a442f1..fc8bb76 100644
> > --- a/mm/swap.c
> > +++ b/mm/swap.c
> > @@ -411,10 +411,11 @@ void add_page_to_unevictable_list(struct page *page)
> > *
> > * 1. active, mapped page -> none
> > * 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
> > - * 6. Others -> none
> > + * 3. active, clean -> inactive, tail
> > + * 4. inactive, mapped page -> none
> > + * 5. inactive, dirty/writeback page -> inactive, head, PG_reclaim
> > + * 6. inactive, clean -> inactive, tail
> > + * 7. Others -> none
> > *
> > * In 4, why it moves inactive's head, the VM expects the page would
> > * be write it out by flusher threads as this is much more effective
> > diff --git a/mm/truncate.c b/mm/truncate.c
> > index 3a29a61..043aabd 100644
> > --- a/mm/truncate.c
> > +++ b/mm/truncate.c
> > @@ -357,7 +357,10 @@ unsigned long invalidate_mapping_pages(struct address_space *mapping,
> > if (lock_failed)
> > continue;
> >
> > - ret = invalidate_inode_page(page);
> > + if (PageActive(page))
> > + ret = 0;
> > + else
> > + ret = invalidate_inode_page(page);
>
> So, after this patch, following comment is a bit outdated. we deactivate
> the page even if it's not invalidated.
>
> /*
> * Invalidation is a hint that the page is no longer
> * of interest and try to speed up its reclaim.
> */
>
> Can you please fix the comment too? Other than that,
> Reviewed-by: KOSAKI Motohiro <[email protected]>

OK, will fix the comment and post a new version soon.

Thanks for reviewing,
-Andrea

2011-06-23 11:14:35

by Pádraig Brady

[permalink] [raw]
Subject: Re: [PATCH RFC] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

On 22/06/11 22:51, Andrea Righi 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_fadvise(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_fadvise(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.
>
> 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_fadvise(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.
>
> 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" (rsync is patched with [3])
> - 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).
>
> Results:
> avg elapsed time block:block_bio_queue
> 3.0.0-rc4 4.127s 8,214
> 3.0.0-rc4-fadvise 2.146s 0
>
> In the first case the file is evicted from page cache completely and we
> must re-read it from the disk. In the second case the file is still in
> page cache (in the inactive list) and we don't need any other additional
> I/O operation.
>
> [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
>
> Signed-off-by: Andrea Righi <[email protected]>

Hmm, What if you do want to evict it from the cache for testing purposes?
Perhaps this functionality should be associated with POSIX_FADV_NOREUSE?
dd has been recently modified to support invalidating the cache for a file,
and it uses POSIX_FADV_DONTNEED for that.
http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=5f311553

cheers,
P?draig.

2011-06-23 13:57:19

by Andrea Righi

[permalink] [raw]
Subject: Re: [PATCH RFC] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

On Thu, Jun 23, 2011 at 12:14:21PM +0100, P?draig Brady wrote:
> On 22/06/11 22:51, Andrea Righi 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_fadvise(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_fadvise(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.
> >
> > 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_fadvise(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.
> >
> > 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" (rsync is patched with [3])
> > - 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).
> >
> > Results:
> > avg elapsed time block:block_bio_queue
> > 3.0.0-rc4 4.127s 8,214
> > 3.0.0-rc4-fadvise 2.146s 0
> >
> > In the first case the file is evicted from page cache completely and we
> > must re-read it from the disk. In the second case the file is still in
> > page cache (in the inactive list) and we don't need any other additional
> > I/O operation.
> >
> > [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
> >
> > Signed-off-by: Andrea Righi <[email protected]>
>
> Hmm, What if you do want to evict it from the cache for testing purposes?
> Perhaps this functionality should be associated with POSIX_FADV_NOREUSE?
> dd has been recently modified to support invalidating the cache for a file,
> and it uses POSIX_FADV_DONTNEED for that.
> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=5f311553

I don't have any objection to associate POSIX_FADV_NOREUSE to this
functionality. Actually maintaining a specific functionality to drop
file cache pages can be useful, indeed.

However, I'm not sure if POSIX_FADV_NOREUSE or POSIX_FADV_DONTNEED
either are suitable.

According to the standard:

POSIX_FADV_NOREUSE = data will be accessed only once
POSIX_FADV_DONTNEED = data will not be accessed in the near future

So, associating the "drop the page cache" semantic sounds like an
implementation detail and applications shouldn't implicitly rely on this
behaviour.

Let's wait a bit to see also other opinions.

-Andrea

2011-06-23 14:15:09

by Pádraig Brady

[permalink] [raw]
Subject: Re: [PATCH RFC] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

On 23/06/11 14:57, Andrea Righi wrote:
> On Thu, Jun 23, 2011 at 12:14:21PM +0100, P?draig Brady wrote:
>> On 22/06/11 22:51, Andrea Righi 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_fadvise(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_fadvise(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.
>>>
>>> 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_fadvise(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.
>>>
>>> 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" (rsync is patched with [3])
>>> - 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).
>>>
>>> Results:
>>> avg elapsed time block:block_bio_queue
>>> 3.0.0-rc4 4.127s 8,214
>>> 3.0.0-rc4-fadvise 2.146s 0
>>>
>>> In the first case the file is evicted from page cache completely and we
>>> must re-read it from the disk. In the second case the file is still in
>>> page cache (in the inactive list) and we don't need any other additional
>>> I/O operation.
>>>
>>> [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
>>>
>>> Signed-off-by: Andrea Righi <[email protected]>
>>
>> Hmm, What if you do want to evict it from the cache for testing purposes?
>> Perhaps this functionality should be associated with POSIX_FADV_NOREUSE?
>> dd has been recently modified to support invalidating the cache for a file,
>> and it uses POSIX_FADV_DONTNEED for that.
>> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=5f311553
>
> I don't have any objection to associate POSIX_FADV_NOREUSE to this
> functionality. Actually maintaining a specific functionality to drop
> file cache pages can be useful, indeed.
>
> However, I'm not sure if POSIX_FADV_NOREUSE or POSIX_FADV_DONTNEED
> either are suitable.
>
> According to the standard:
>
> POSIX_FADV_NOREUSE = data will be accessed only once
> POSIX_FADV_DONTNEED = data will not be accessed in the near future


> So, associating the "drop the page cache" semantic sounds like an
> implementation detail and applications shouldn't implicitly rely on this
> behaviour.

Well the "standard" really is what has been implemented up to now.
POSIX_FADV_NOREUSE currently does nothing so, associating this
new behavior with it seems less problematic for user space.
Also the names fit pretty well I think.

POSIX_FADV_DONTNEED = drop if possible
POSIX_FADV_NOREUSE = current app won't reuse so reduce cache eligibility

cheers,
P?draig.

2011-06-23 15:32:35

by Andrea Righi

[permalink] [raw]
Subject: Re: [PATCH RFC] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

On Thu, Jun 23, 2011 at 03:14:57PM +0100, P?draig Brady wrote:
> On 23/06/11 14:57, Andrea Righi wrote:
> > On Thu, Jun 23, 2011 at 12:14:21PM +0100, P?draig Brady wrote:
> >> On 22/06/11 22:51, Andrea Righi 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_fadvise(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_fadvise(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.
> >>>
> >>> 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_fadvise(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.
> >>>
> >>> 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" (rsync is patched with [3])
> >>> - 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).
> >>>
> >>> Results:
> >>> avg elapsed time block:block_bio_queue
> >>> 3.0.0-rc4 4.127s 8,214
> >>> 3.0.0-rc4-fadvise 2.146s 0
> >>>
> >>> In the first case the file is evicted from page cache completely and we
> >>> must re-read it from the disk. In the second case the file is still in
> >>> page cache (in the inactive list) and we don't need any other additional
> >>> I/O operation.
> >>>
> >>> [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
> >>>
> >>> Signed-off-by: Andrea Righi <[email protected]>
> >>
> >> Hmm, What if you do want to evict it from the cache for testing purposes?
> >> Perhaps this functionality should be associated with POSIX_FADV_NOREUSE?
> >> dd has been recently modified to support invalidating the cache for a file,
> >> and it uses POSIX_FADV_DONTNEED for that.
> >> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=5f311553
> >
> > I don't have any objection to associate POSIX_FADV_NOREUSE to this
> > functionality. Actually maintaining a specific functionality to drop
> > file cache pages can be useful, indeed.
> >
> > However, I'm not sure if POSIX_FADV_NOREUSE or POSIX_FADV_DONTNEED
> > either are suitable.
> >
> > According to the standard:
> >
> > POSIX_FADV_NOREUSE = data will be accessed only once
> > POSIX_FADV_DONTNEED = data will not be accessed in the near future
>
>
> > So, associating the "drop the page cache" semantic sounds like an
> > implementation detail and applications shouldn't implicitly rely on this
> > behaviour.
>
> Well the "standard" really is what has been implemented up to now.
> POSIX_FADV_NOREUSE currently does nothing so, associating this
> new behavior with it seems less problematic for user space.

mmmh.. yes, we would also respect backward compatibility. The behaviour
of invalidate_mapping_pages() would remain the same: drop page cache if
possible.

> Also the names fit pretty well I think.
>
> POSIX_FADV_DONTNEED = drop if possible
> POSIX_FADV_NOREUSE = current app won't reuse so reduce cache eligibility

Agreed.

-Andrea

2011-06-27 05:41:22

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH RFC] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

> Hmm, What if you do want to evict it from the cache for testing purposes?
> Perhaps this functionality should be associated with POSIX_FADV_NOREUSE?
> dd has been recently modified to support invalidating the cache for a file,
> and it uses POSIX_FADV_DONTNEED for that.
> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=5f311553

This change don't break dd. dd don't have a special privilege of file cache
dropping if it's also used by other processes.

if you want to drop a cache forcely (maybe for testing), you need to use
/proc/sys/vm/drop_caches. It's ok to ignore other processes activity because
it's privilege operation.

Thanks.

2011-06-27 09:19:34

by Pádraig Brady

[permalink] [raw]
Subject: Re: [PATCH RFC] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

On 27/06/11 06:38, KOSAKI Motohiro wrote:
>> Hmm, What if you do want to evict it from the cache for testing purposes?
>> Perhaps this functionality should be associated with POSIX_FADV_NOREUSE?
>> dd has been recently modified to support invalidating the cache for a file,
>> and it uses POSIX_FADV_DONTNEED for that.
>> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=5f311553
>
> This change don't break dd. dd don't have a special privilege of file cache
> dropping if it's also used by other processes.
>
> if you want to drop a cache forcely (maybe for testing), you need to use
> /proc/sys/vm/drop_caches. It's ok to ignore other processes activity because
> it's privilege operation.

Well the function and privileges are separate things.
I think we've agreed that the new functionality is
best associated with POSIX_FADV_NOREUSE,
and the existing functionality with POSIX_FADV_DONTNEED.

BTW, I don't think privileges are currently enforced
as I got root to cache a file here with:
# (time md5sum; sleep 100) < big.file
And a normal user was able to uncache with:
$ dd iflag=nocache if=big.file count=0
Anyway as said, this is a separate "issue".

cheers,
P?draig.

2011-06-27 09:47:32

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH RFC] fadvise: move active pages to inactive list with POSIX_FADV_DONTNEED

(2011/06/27 18:17), P?draig Brady wrote:
> On 27/06/11 06:38, KOSAKI Motohiro wrote:
>>> Hmm, What if you do want to evict it from the cache for testing purposes?
>>> Perhaps this functionality should be associated with POSIX_FADV_NOREUSE?
>>> dd has been recently modified to support invalidating the cache for a file,
>>> and it uses POSIX_FADV_DONTNEED for that.
>>> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=5f311553
>>
>> This change don't break dd. dd don't have a special privilege of file cache
>> dropping if it's also used by other processes.
>>
>> if you want to drop a cache forcely (maybe for testing), you need to use
>> /proc/sys/vm/drop_caches. It's ok to ignore other processes activity because
>> it's privilege operation.
>
> Well the function and privileges are separate things.
> I think we've agreed that the new functionality is
> best associated with POSIX_FADV_NOREUSE,
> and the existing functionality with POSIX_FADV_DONTNEED.
>
> BTW, I don't think privileges are currently enforced
> as I got root to cache a file here with:
> # (time md5sum; sleep 100) < big.file
> And a normal user was able to uncache with:
> $ dd iflag=nocache if=big.file count=0
> Anyway as said, this is a separate "issue".

I'm failed to see your point. Why does dd need to ignore other
process activity? If no other process, this patch doesn't change
any behavior. Isn't it?