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

Kasireddy, Vivek
2023-08-01T05:32:49Z

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