Messages in this thread | | | Subject | Re: [PATCH -V2] mm: Code cleanup for MADV_FREE | From | Vlastimil Babka <> | Date | Tue, 17 Mar 2020 09:51:04 +0100 |
| |
On 3/17/20 9:10 AM, Huang, Ying wrote: > 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
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); >>> } >
| |