Hi David,
Thank you very much for reviewing this.
On Mon, Feb 26, 2024 at 9:58 AM David Hildenbrand <david@xxxxxxxxxx> wrote:
>
> On 26.02.24 09:58, Fuad Tabba wrote:
> > Hi David,
> >
> > On Thu, Feb 22, 2024 at 4:28 PM David Hildenbrand <david@xxxxxxxxxx> wrote:
> >>
> >>> +static vm_fault_t kvm_gmem_fault(struct vm_fault *vmf)
> >>> +{
> >>> + struct folio *folio;
> >>> +
> >>> + folio = kvm_gmem_get_folio(file_inode(vmf->vma->vm_file), vmf->pgoff);
> >>> + if (!folio)
> >>> + return VM_FAULT_SIGBUS;
> >>> +
> >>> + /*
> >>> + * Check if the page is allowed to be faulted to the host, with the
> >>> + * folio lock held to ensure that the check and incrementing the page
> >>> + * count are protected by the same folio lock.
> >>> + */
> >>> + if (!kvm_gmem_isfaultable(vmf)) {
> >>> + folio_unlock(folio);
> >>> + return VM_FAULT_SIGBUS;
> >>> + }
> >>> +
> >>> + vmf->page = folio_file_page(folio, vmf->pgoff);
> >>
> >> We won't currently get hugetlb (or even THP) here. It mimics what shmem
> >> would do.
> >
> > At the moment there isn't hugetlb support in guest_memfd(), and
> > neither in pKVM. Although we do plan on supporting it.
> >
> >> finish_fault->set_pte_range() will call folio_add_file_rmap_ptes(),
> >> getting the rmap involved.
> >>
> >> Do we have some tests in place that make sure that
> >> fallocate(FALLOC_FL_PUNCH_HOLE|FALLOC_FL_KEEP_SIZE) will properly unmap
> >> the page again (IOW, that the rmap does indeed work?).
> >
> > I'm not sure if you mean kernel tests, or if I've tested it. There are
> > guest_memfd() tests for
> > fallocate(FALLOC_FL_PUNCH_HOLE|FALLOC_FL_KEEP_SIZE) , which I have
> > run. I've also tested it manually with sample programs, and it behaves
> > as expected.
>
> Can you point me at the existing tests? I'm interested in
> mmap()-specific guest_memfd tests.
>
> A test that would make sense to me:
>
> 1) Create guest_memfd() and size it to contain at least one page.
>
> 2) mmap() it
>
> 3) Write some pattern (e.g., all 1's) to the first page using the mmap
>
> 4) fallocate(FALLOC_FL_PUNCH_HOLE|FALLOC_FL_KEEP_SIZE) the first page
>
> 5) Make sure reading from the first page using the mmap reads all 0's
>
> IOW, during 4) we properly unmapped (via rmap) and discarded the page,
> such that 5) will populate a fresh page in the page cache filled with
> 0's and map that one.
The existing tests don't test mmap (or rather, they test the inability
to mmap). They do test FALLOC_FL_PUNCH_HOLE. [1]
The tests for mmap() are ones that I wrote myself. I will write a test
like the one you mentioned, and send it with V2, or earlier if you
prefer.
Thanks again,
/fuad
> --
> Cheers,
>
> David / dhildenb
>
