Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.2\))
Subject: Re: [PATCH v3] page cache: Store only head pages in i_pages
From:   William Kucharski <william.kucharski@oracle.com>
In-Reply-To: <CAPhsuW5a8=QJe2acWXQGWic1a=CJigwPR6BxSu2O2vg4W1mhzA@mail.gmail.com>
Date:   Wed, 6 Mar 2019 02:36:35 -0700
Cc:     "Kirill A. Shutemov" <kirill@shutemov.name>,
        Linux-MM <linux-mm@kvack.org>, linux-fsdevel@vger.kernel.org,
        open list <linux-kernel@vger.kernel.org>,
        Hugh Dickins <hughd@google.com>, Jan Kara <jack@suse.cz>,
        Song Liu <liu.song.a23@gmail.com>
Content-Transfer-Encoding: quoted-printable
Message-Id: <863F9255-E992-402F-827D-DA5F4661B9AB@oracle.com>
References: <20190215222525.17802-1-willy@infradead.org>
 <CAPhsuW7Hu6jBn-ti7S2cJhO1YQYg_RDZUgkqtgFO8zpBMV_9LA@mail.gmail.com>
 <CAPhsuW5a8=QJe2acWXQGWic1a=CJigwPR6BxSu2O2vg4W1mhzA@mail.gmail.com>
To:     Matthew Wilcox <willy@infradead.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


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

> +		 * 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.

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>=