lkml.org 
[lkml]   [2020]   [Nov]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 3/6] mm: support THP migration to device private memory
    On Wed, Nov 11, 2020 at 03:38:42PM -0800, Ralph Campbell wrote:

    > MEMORY_DEVICE_GENERIC:
    > Struct pages are created in dev_dax_probe() and represent non-volatile memory.
    > The device can be mmap()'ed which calls dax_mmap() which sets
    > vma->vm_flags | VM_HUGEPAGE.
    > A CPU page fault will result in a PTE, PMD, or PUD sized page
    > (but not compound) to be inserted by vmf_insert_mixed() which will call either
    > insert_pfn() or insert_page().
    > Neither insert_pfn() nor insert_page() increments the page reference
    > count.

    But why was this done? It seems very strange to put a pfn with a
    struct page into a VMA and then deliberately not take the refcount for
    the duration of that pfn being in the VMA?

    What prevents memunmap_pages() from progressing while VMAs still point
    at the memory?

    > I think just leaving the page reference count at one is better than trying
    > to use the mmu_interval_notifier or changing vmf_insert_mixed() and
    > invalidations of pfn_t_devmap(pfn) to adjust the page reference count.

    Why so? The entire point of getting struct page's for this stuff was
    to be able to follow the struct page flow. I never did learn a reason
    why there is devmap stuff all over the place in the page table code...

    > MEMORY_DEVICE_FS_DAX:
    > Struct pages are created in pmem_attach_disk() and virtio_fs_setup_dax() with
    > an initial reference count of one.
    > The problem I see is that there are 3 states that are important:
    > a) memory is free and not allocated to any file (page_ref_count() == 0).
    > b) memory is allocated to a file and in the page cache (page_ref_count() == 1).
    > c) some gup() or I/O has a reference even after calling unmap_mapping_pages()
    > (page_ref_count() > 1). ext4_break_layouts() basically waits until the
    > page_ref_count() == 1 with put_page() calling wake_up_var(&page->_refcount)
    > to wake up ext4_break_layouts().
    > The current code doesn't seem to distinguish (a) and (b). If we want to use
    > the 0->1 reference count to signal (c), then the page cache would have hold
    > entries with a page_ref_count() == 0 which doesn't match the general page cache
    > assumptions.

    This explanation feels confusing. If *anything* has a reference on the
    page it cannot be recycled. I would have guess the logic is to remove
    it from the page cache then wait for a 0 reference??

    Jason

    \
     
     \ /
      Last update: 2020-11-20 21:03    [W:3.058 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site