On 9 Sep 2020, at 9:46, Kirill A. Shutemov wrote: > On Mon, Sep 07, 2020 at 11:11:05AM -0400, Zi Yan wrote: >> 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 >>>> >>>> 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 >>> >>> 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? > > Okay, fair enough. > > Although, I think you can get away without a new data structure. We don't > need double-linked list to deposit page tables. You can rework PTE tables > deposit code to have single-linked list and use one pointer of ->lru (with > proper name) and make PMD tables deposit to use the other one. This way > you can avoid conflict for ->lru. > > Does it make sense? Yes. Thanks. Will do this in the next version. I think the single linked list from llist.h can be used. — Best Regards, Yan Zi