lkml.org 
[lkml]   [2019]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3] page cache: Store only head pages in i_pages
On Wed, Mar 06, 2019 at 02:36:35AM -0700, William Kucharski wrote:
>
> Other than the bug Song found in memfd_tag_pins(), I'd like to suggest two quick
> but pedantic changes to mm/filemap.c:
>
> Though not modified in this patch, in line 284, the parenthesis should be moved
> to after the period:
>
> * modified.) The function expects only THP head pages to be present in the

https://english.stackexchange.com/questions/6632/where-does-the-period-go-when-using-parentheses disagrees with you

> > + * Move to the next page in the vector if this is a small page
> > + * or the index is of the last page in this compound page).
>
> A few lines later, there is an extraneous parenthesis, and the comment could be a bit
> clearer.
>
> Might I suggest:
>
> * Move to the next page in the vector if this is a PAGESIZE
> * page or if the index is of the last PAGESIZE page within
> * this compound page.
>
> You can say "base" instead of "PAGESIZE," but "small" seems open to interpretation.

Agreed on the spurios close paren. The THP documentation prefers the
term 'regular page', so I went with:

* Move to the next page in the vector if this is a regular
* page or the index is of the last sub-page of this compound
* page.

> I haven't run across any problems and have been hammering the code for over five days
> without issue; all my testing was with transparent_hugepage/enabled set to
> "always."
>
> Tested-by: William Kucharski <william.kucharski@oracle.com>
> Reviewed-by: William Kucharski <william.kucharski@oracle.com>

Thanks!

\
 
 \ /
  Last update: 2019-03-07 16:06    [W:0.096 / U:0.432 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site