Messages in this thread | | | From | "Huang\, Ying" <> | Subject | Re: [PATCH -V2] mm: Code cleanup for MADV_FREE | Date | Tue, 17 Mar 2020 16:10:30 +0800 |
| |
Vlastimil Babka <vbabka@suse.cz> writes:
> On 3/16/20 7:37 AM, Huang, Ying wrote: >> From: Huang Ying <ying.huang@intel.com> >> >> Some comments for MADV_FREE is revised and added to help people understand the >> MADV_FREE code, especially the page flag, PG_swapbacked. This makes >> page_is_file_cache() isn't consistent with its comments. So the function is >> renamed to page_is_file_lru() to make them consistent again. All these are put >> in one patch as one logical change. >> Signed-off-by: "Huang, Ying" <ying.huang@intel.com> >> Suggested-by: David Hildenbrand <david@redhat.com> >> Suggested-by: Johannes Weiner <hannes@cmpxchg.org> >> Suggested-and-acked-by: David Rientjes <rientjes@google.com> >> Acked-by: Michal Hocko <mhocko@kernel.org> >> Acked-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com> >> Cc: Dave Hansen <dave.hansen@linux.intel.com> >> Cc: Mel Gorman <mgorman@suse.de> >> Cc: Vlastimil Babka <vbabka@suse.cz> >> Cc: Minchan Kim <minchan@kernel.org> >> Cc: Hugh Dickins <hughd@google.com> >> Cc: Rik van Riel <riel@surriel.com> > > Acked-by: Vlastimil Babka <vbabka@suse.cz> > > Thanks! A grammar nit below: > >> --- a/include/linux/mm_inline.h >> +++ b/include/linux/mm_inline.h >> @@ -6,19 +6,20 @@ >> #include <linux/swap.h> >> >> /** >> - * page_is_file_cache - should the page be on a file LRU or anon LRU? >> + * page_is_file_lru - 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 > > a lazily freed anonymous page (e.g. ...
Thought again. Should we make the 2 sub-clauses consistent? That is, either
if @page is page cache page backed by a regular filesystem or anonymous page freed lazily
or
if @page is a regular filesystem backed page cache page or a lazily freed anonymous page
But I know that my English grammar isn't good enough :-(
Best Regards, Huang, Ying
>> + * 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 >> * could be as far down as __page_cache_release. >> */ >> -static inline int page_is_file_cache(struct page *page) >> +static inline int page_is_file_lru(struct page *page) >> { >> return !PageSwapBacked(page); >> }
| |