lkml.org 
[lkml]   [2020]   [Sep]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [RFC PATCH 01/16] mm: add pagechain container for storing multiple pages.
    Date
    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 <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?
    >
    > 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
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2020-09-09 18:49    [W:3.872 / U:0.092 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site