RE: [RFC v1 1/3] mm/mmu_notifier: Add a new notifier for mapping updates (new pages)

["Kasireddy, Vivek" <vivek.kasireddy@intel.com>]

Hi Jason,

> 
> > > Later the importer decides it needs the memory again so it again asks
> > > for the dmabuf to be present, which does hmm_range_fault and gets
> > > whatever is appropriate at the time.
> > Unless I am missing something, I think just doing the above still won't solve
> > the problem. Consider this sequence:
> >      write_to_memfd(addr1, size, 'a');
> >      buf = create_udmabuf_list(devfd, memfd, size);
> >      addr2 = mmap_fd(buf, NUM_PAGES * NUM_ENTRIES * getpagesize());
> >      read(addr2);
> >      write_to_memfd(addr1, size, 'b');
> >      punch_hole(memfd, MEMFD_SIZE / 2);
> > -> Since we can process the invalidate at this point, as per your suggestion,
> >      we can trigger dmabuf move to let the importers know that the
> dmabuf's
> >      backing memory has changed (or moved).
> >
> >      read(addr2);
> > -> Because there is a hole, we can handle the read by either providing the
> >      old pages or zero pages (if using hmm_range_fault()) to the
> > importers.
> 
> You never provide the old pages. After trunctate the only correct
> value to read is zero.
> 
> >      Maybe it is against convention, but I think it makes sense to provide old
> >      pages (that were mapped before the hole punch) because the importers
> >      have not read the data in these pages ('b' above) yet.
> 
> Nope.
> 
> >      And, another reason to provide old pages is because the data in
> >      these pages is shown in a window on the Host's screen so it
> >      doesn't make sense to show zero page data.
> 
> So why did you trucate it if you want to keep the data?
> 
> 
> > -> write_to_memfd(addr1, size, 'c');
> >      As the hole gets refilled (with new pages) after the above write, AFAIU,
> we
> >      have to tell the importers again that since the backing memory has
> changed,
> >      (new pages) they need to recreate their mappings. But herein lies the
> problem:
> >      from inside the udmabuf driver, we cannot know when this write occurs,
> so we
> >      would not be able to notify the importers of the dmabuf move.
> 
> You get another invalidate because the memfd removes the zero pages
> that hmm_range_fault installed in the PTEs before replacing them with
> actual writable pages. Then you do the move, and another
> hmm_range_fault, and basically the whole thing over again. Except this
> time instead of returning zero pages it returns actual writable page.
Ok, when I tested earlier (by registering an invalidate callback) but without
hmm_range_fault(), I did not find this additional invalidate getting triggered.
Let me try with hmm_range_fault() and see if everything works as expected.
Thank you for your help.


Thanks,
Vivek

> 
> Jason