> On Jan 31, 2022, at 10:47 AM, Mike Rapoport <rppt@xxxxxxxxxx> wrote:
>
> On Mon, Jan 31, 2022 at 03:41:05PM +0100, David Hildenbrand wrote:
>> On 31.01.22 15:28, Mike Rapoport wrote:
>>> On Mon, Jan 31, 2022 at 03:12:36PM +0100, David Hildenbrand wrote:
>>>> On 31.01.22 15:05, Mike Rapoport wrote:
>>>>> On Mon, Jan 31, 2022 at 11:48:27AM +0100, David Hildenbrand wrote:
>>>>>> On 31.01.22 11:42, Mike Rapoport wrote:
>>>>>>> Hi Nadav,
>>>>>>>
>>>>>>> On Sat, Jan 29, 2022 at 10:23:55PM -0800, Nadav Amit wrote:
>>>>>>>> Using userfautlfd and looking at the kernel code, I encountered a usability
>>>>>>>> issue that complicates userspace UFFD-monitor implementation. I obviosuly
>>>>>>>> might be wrong, so I would appreciate a (polite?) feedback. I do have a
>>>>>>>> userspace workaround, but I thought it is worthy to share and to hear your
>>>>>>>> opinion, as well as feedback from other UFFD users.
>>>>>>>>
>>>>>>>> The issue I encountered regards the ordering of UFFD events tbat might not
>>>>>>>> reflect the actual order in which events took place.
>>>>>>>>
>>>>>>>> In more detail, UFFD events (e.g., unmap, fork) are not ordered against
>>>>>>>> themselves [*]. The mm-lock is dropped before notifying the userspace
>>>>>>>> UFFD-monitor, and therefore there is no guarantee as to whether the order of
>>>>>>>> the events actually reflects the order in which the events took place.
>>>>>>>> This can prevent a UFFD-monitor from using the events to track which
>>>>>>>> ranges are mapped. Specifically, UFFD_EVENT_FORK message and a
>>>>>>>> UFFD_EVENT_UNMAP message (which reflects unmap in the parent process) can
>>>>>>>> be reordered, if the events are triggered by two different threads. In
>>>>>>>> this case the UFFD-monitor cannot figure from the events whether the
>>>>>>>> child process has the unmapped memory range still mapped (because fork
>>>>>>>> happened first) or not.
>>>>>>>
>>>>>>> Yeah, it seems that something like this is possible:
>>>>>>>
>>>>>>>
>>>>>>> fork() munmap()
>>>>>>> mmap_write_unlock();
>>>>>>> mmap_write_lock_killable();
>>>>>>> do_things();
>>>>>>> mmap_{read,write}_unlock();
>>>>>>> userfaultfd_unmap_complete();
>>>>>>> dup_userfaultfd_complete();
>>>>>>>
>>>>>>
>>>>>> I was thinking about other possible races, e.g., MADV_DONTNEED/MADV_FREE
>>>>>> racing with UFFD_EVENT_PAGEFAULT -- where we only hold the mmap_lock in
>>>>>> read mode. But not sure if they apply.
>>>>>
>>>>> The userspace can live with these, at least for uffd missing page faults.
>>>>> If the monitor will try to resolve a page fault for a removed area, the
>>>>> errno from UFFDIO_COPY/ZERO can be used to detect such races.
>>>>
>>>> I was wondering if the monitor could get confused if he just resolved a
>>>> page fault via UFFDIO_COPY/ZERO and then receives a REMOVE event.
>>>
>>> And why would it be confused?
>>
>> My thinking was that the monitor might use REMOVE events to track which
>> pages are actually populated. If you receive REMOVE after
>> UFFDIO_COPY/ZERO the monitor would conclude that the page is not
>> populated, just like if we'd get the MADV_DONTNEED/MADV_REMOVE
>> immediately after placing a page.
>
> I still don't follow your usecase.
>
> In CRIU we simply discard whatever content we had to fill when there is
> REMOVE event. If a page fault occurs in that region we use UFFDIO_ZEROPAGE,
> just as it would happen in "normal" page fault processing
> (note, CRIU does not support uffd with hugetlb or shmem)
I think that the point that David makes is valid.
There are use-cases in which you do need to know the order between
user-initiated MADV_DONTNEED and page-faults. For instance, if you
build a userspace paging mechanism, you need to know whether the
page content is zero or whatever is held in the disk.
I presume mmap_changing was designed for a similar purpose, assuming
that if you had a page-fault that started before MADV_DONTNEED, and
you try to serve it using copy-ioctl, the copy would fail. I think
that this works only if you assume that there is a single UFFD
monitor thread (that reads the uffd and issues appropriate ioctl’s),
and that all operations are performed synchronously (which I am
trying to avoid using io-uring).
Otherwise, a copy ioctl that is initiated before MADV_DONTNEED
(to resolve page-fault) can take place after the userspace was
already notified of UFFD_EVENT_REMOVE (i.e., mmap_changing==0),
and there is no way to cancel the copy that was initiated. As
a result, following MADV_DONTNEED, the memory would not be zeroed.
As for me, I decided that due to the lack of ordering, I just
cannot use the UFFD events, and I have to rely on ptrace to obtain
order of these events. I might be wrong, but any solution is not
trivial and is likely to require API changes.
