lkml.org 
[lkml]   [2020]   [Mar]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH -V2] mm: Code cleanup for MADV_FREE
    From
    Date
    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);
    >>> }
    >

    \
     
     \ /
      Last update: 2020-03-17 09:51    [W:10.032 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site