On 3/17/20 9:10 AM, Huang, Ying wrote: > Vlastimil Babka <vbabka@xxxxxxx> writes: > >> On 3/16/20 7:37 AM, Huang, Ying wrote: >>> From: Huang Ying <ying.huang@xxxxxxxxx> >>> >>> 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@xxxxxxxxx> >>> Suggested-by: David Hildenbrand <david@xxxxxxxxxx> >>> Suggested-by: Johannes Weiner <hannes@xxxxxxxxxxx> >>> Suggested-and-acked-by: David Rientjes <rientjes@xxxxxxxxxx> >>> Acked-by: Michal Hocko <mhocko@xxxxxxxxxx> >>> Acked-by: Pankaj Gupta <pankaj.gupta.linux@xxxxxxxxx> >>> Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx> >>> Cc: Mel Gorman <mgorman@xxxxxxx> >>> Cc: Vlastimil Babka <vbabka@xxxxxxx> >>> Cc: Minchan Kim <minchan@xxxxxxxxxx> >>> Cc: Hugh Dickins <hughd@xxxxxxxxxx> >>> Cc: Rik van Riel <riel@xxxxxxxxxxx> >> >> Acked-by: Vlastimil Babka <vbabka@xxxxxxx> >> >> 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 Yeah this one looks fine :) > 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); >>> } >