linux-kernel - Re: [PATCH v3] page cache: Store only head pages in i

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20190307150623.GL13380@bombadil.infradead.org>
Date:   Thu, 7 Mar 2019 07:06:23 -0800
From:   Matthew Wilcox <willy@...radead.org>
To:     William Kucharski <william.kucharski@...cle.com>
Cc:     "Kirill A. Shutemov" <kirill@...temov.name>,
        Linux-MM <linux-mm@...ck.org>, linux-fsdevel@...r.kernel.org,
        open list <linux-kernel@...r.kernel.org>,
        Hugh Dickins <hughd@...gle.com>, Jan Kara <jack@...e.cz>,
        Song Liu <liu.song.a23@...il.com>
Subject: Re: [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@...cle.com>
> Reviewed-by: William Kucharski <william.kucharski@...cle.com>

Thanks!