Re: [RFC PATCH 01/16] mm: add pagechain container for storing multiple pages.

From: Zi Yan <ziy@nvidia.com>
To: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: <linux-mm@kvack.org>, Roman Gushchin <guro@fb.com>,
	Rik van Riel <riel@surriel.com>,
	"Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
	Matthew Wilcox <willy@infradead.org>,
	Shakeel Butt <shakeelb@google.com>,
	Yang Shi <yang.shi@linux.alibaba.com>,
	David Nellans <dnellans@nvidia.com>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH 01/16] mm: add pagechain container for storing multiple pages.
Date: Mon, 7 Sep 2020 11:11:05 -0400	[thread overview]
Message-ID: <50FA95D1-9222-48C0-9223-C1267E8C7A4A@nvidia.com> (raw)
In-Reply-To: <20200907122228.4zlyfysdul3s62me@box>

[-- Attachment #1: Type: text/plain, Size: 2325 bytes --]

On 7 Sep 2020, at 8:22, Kirill A. Shutemov wrote:

> On Wed, Sep 02, 2020 at 02:06:13PM -0400, Zi Yan wrote:
>> From: Zi Yan <ziy@nvidia.com>
>>
>> When depositing page table pages for 1GB THPs, we need 512 PTE pages +
>> 1 PMD page. Instead of counting and depositing 513 pages, we can use the
>> PMD page as a leader page and chain the rest 512 PTE pages with ->lru.
>> This, however, prevents us depositing PMD pages with ->lru, which is
>> currently used by depositing PTE pages for 2MB THPs. So add a new
>> pagechain container for PMD pages.
>>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>
> Just deposit it to a linked list in the mm_struct as we do for PMD if
> split ptl disabled.
>

Thank you for checking the patches. Since we don’t have PUD split lock
yet, I store the PMD page table pages in a newly added linked list head
in mm_struct like you suggested above.

I was too vague about my pagechain design for depositing page table pages
for PUD THPs. Sorry about the confusion. Let me clarify why
I am doing this pagechain here too. I am sure there would be
some other designs and I am happy to change my code.

In my design, I did not store all page table pages in a single list.
I first deposit 512 PTE pages in one PMD page table page’s pmd_huge_pte
using pgtable_trans_huge_depsit(), then deposit the PMD page to
a newly added linked list in mm_struct. Since pmd_huge_pte shares space
with half of lru in struct page, we cannot use lru to link all PMD
pages together. As a result, I added pagechain. Also in this way,
we can avoid these things:

1. when we withdraw the PMD page during PUD THP split, we don’t need
to withdraw 513 page, set up one PMD page, then, deposit 512 PTE pages
in that PMD page.

2. we don’t mix PMD page table pages and PTE page table pages in a single
list, since they are initialized in different ways. Otherwise, we need
to maintain a subtle rule in the single page table page list that in every
513 pages, first one is PMD page table page and the rest are PTE page
table pages.

As I am typing, I also realize that my current design does not work
when PMD split lock is disabled, so I will fix it. I would store PMD pages
and PTE pages in two separate lists in mm_struct.

Any comments?

—
Best Regards,
Yan Zi

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]