mm: Add more comments for MADV_FREE
Subsystems (3)
PATCH 1/1 mm: Add more comments for MADV_FREE
From: Huang Ying <[email protected]>
To help people understand the MADV_FREE code, especially the page flag, PG_swapbacked.
--- include/linux/mm_inline.h | 9 +++++---- include/linux/page-flags.h | 4 ++++ mm/swap.c | 6 +++--- 3 files changed, 12 insertions(+), 7 deletions(-)
include/linux/mm_inline.h +5 -4
@@ -9,10 +9,11 @@* page_is_file_cache - should the page be on a file LRU or anon LRU? * @page: the page to test * -* Returns 1 if @page is page cache page backed by a regular filesystem, -* or 0 if @page is anonymous, tmpfs or otherwise ram or swap backed. -* Used by functions that manipulate the LRU lists, to sort a page -* onto the right LRU list. +* Returns 1 if @page is page cache page backed by a regular filesystem or +* anonymous page lazily freed (e.g. via MADV_FREE). Returns 0 if @page is +* normal anonymous page, tmpfs or otherwise ram or swap backed. Used by +* functions that manipulate the LRU lists, to sort a page onto the right LRU +* list. * * We would like to get this info without a page flag, but the state * needs to survive until the page is last deleted from the LRU, which
include/linux/page-flags.h +4 -0
@@ -63,6 +63,10 @@* page_waitqueue(page) is a wait queue of all tasks waiting for the page * to become unlocked. * +* PG_swapbacked is cleared if the page is page cache page backed by a regular +* file system or anonymous page lazily freed (e.g. via MADV_FREE). It is set +* if the page is normal anonymous page, tmpfs or otherwise RAM or swap backed. +* * PG_uptodate tells whether the page's contents is valid. When a read * completes, the page becomes uptodate, unless a disk I/O error happened. *
mm/swap.c +3 -3
@@ -573,9 +573,9 @@ static void lru_lazyfree_fn(struct page *page, struct lruvec *lruvec,ClearPageActive (page);ClearPageReferenced (page);/* -* lazyfree pages are clean anonymous pages. They have -* SwapBacked flag cleared to distinguish normal anonymous -* pages +* Lazyfree pages are clean anonymous pages. They +* have PG_swapbacked flag cleared, to distinguish +* them from normal anonymous pages */ ClearPageSwapBacked (page);add_page_to_lru_list (page, lruvec,LRU_INACTIVE_FILE );
Signature · 1 line
2.25.0
Quoted 7 lines · Ying
include/linux/mm_inline.h +0
-0@@ -9,10 +9,11 @@* page_is_file_cache - should the page be on a file LRU or anon LRU? * @page: the page to test
include/linux/mm_inline.h line 16
Quoted 10 lines
include/linux/mm_inline.h +5
-4* -* Returns 1 if @page is page cache page backed by a regular filesystem, -* or 0 if @page is anonymous, tmpfs or otherwise ram or swap backed. -* Used by functions that manipulate the LRU lists, to sort a page -* onto the right LRU list. +* Returns 1 if @page is page cache page backed by a regular filesystem or +* anonymous page lazily freed (e.g. via MADV_FREE). Returns 0 if @page is +* normal anonymous page, tmpfs or otherwise ram or swap backed. Used by +* functions that manipulate the LRU lists, to sort a page onto the right LRU +* list.
The function name is misleading: anonymous pages that can be lazily freed are not file cache. This returns 1 because of the question it is asking:
anonymous lazily freeable pages should be on the file lru, not the anon lru. So before adjusting the comment I'd suggest renaming the function to something like page_is_file_lru().
The rest looks fine.
Quoted 17 lines · David Rientjes
Quoted 15 lines · Ying
include/linux/mm_inline.h +3
-4@@ -9,10 +9,11 @@* page_is_file_cache - should the page be on a file LRU or anon LRU? * @page: the page to test * -* Returns 1 if @page is page cache page backed by a regular filesystem, -* or 0 if @page is anonymous, tmpfs or otherwise ram or swap backed. -* Used by functions that manipulate the LRU lists, to sort a page -* onto the right LRU list. +* Returns 1 if @page is page cache page backed by a regular filesystem or +* anonymous page lazily freed (e.g. via MADV_FREE). Returns 0 if @page is +* normal anonymous page, tmpfs or otherwise ram or swap backed. Used by
Quoted 8 lines
include/linux/mm_inline.h line 16
Quoted 2 lines
include/linux/mm_inline.h +2
-0+* functions that manipulate the LRU lists, to sort a page onto the right LRU +* list. The function name is misleading: anonymous pages that can be lazily freed are not file cache. This returns 1 because of the question it is asking:
anonymous lazily freeable pages should be on the file lru, not the anon lru. So before adjusting the comment I'd suggest renaming the function to something likepage_is_file_lru().
Yes. I think page_is_file_lru() is a better name too. And whether tmpfs pages are file cache pages is confusing too. But I think we can do that after this patch if others think this is a good idea too.
Best Regards,
Huang, Ying
Quoted 28 lines · Ying
Quoted 17 lines · David Rientjes
Quoted 15 lines · Ying
include/linux/mm_inline.h +3
-4@@ -9,10 +9,11 @@* page_is_file_cache - should the page be on a file LRU or anon LRU? * @page: the page to test * -* Returns 1 if @page is page cache page backed by a regular filesystem, -* or 0 if @page is anonymous, tmpfs or otherwise ram or swap backed. -* Used by functions that manipulate the LRU lists, to sort a page -* onto the right LRU list. +* Returns 1 if @page is page cache page backed by a regular filesystem or +* anonymous page lazily freed (e.g. via MADV_FREE). Returns 0 if @page is +* normal anonymous page, tmpfs or otherwise ram or swap backed. Used by Quoted 8 lines
include/linux/mm_inline.h line 16
Quoted 2 lines
include/linux/mm_inline.h +2
-0+* functions that manipulate the LRU lists, to sort a page onto the right LRU +* list. The function name is misleading: anonymous pages that can be lazily freed are not file cache. This returns 1 because of the question it is asking:
anonymous lazily freeable pages should be on the file lru, not the anon lru. So before adjusting the comment I'd suggest renaming the function to something likepage_is_file_lru().
Quoted 3 lines
Yes. I think
page_is_file_lru()is a better name too. And whether tmpfs pages are file cache pages is confusing too. But I think we can do that after this patch if others think this is a good idea too.
I also think the rename is a great idea.
Personally, I'd prefer it in the same patch. Right now the name and the documentation are out of date, but at least they're consistent in their view of the world. Fixing this interface - name and documentation - to reflect the existence of MADV_FREE anon pages is one logical change, not two.