Re: [PATCH v2 1/5] userfaultfd: introduce uffd_flags

From: Nadav Amit <namit@vmware.com>
To: David Hildenbrand <david@redhat.com>
Cc: Linux MM <linux-mm@kvack.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Mike Kravetz <mike.kravetz@oracle.com>,
	Hugh Dickins <hughd@google.com>,
	Axel Rasmussen <axelrasmussen@google.com>,
	Peter Xu <peterx@redhat.com>, Mike Rapoport <rppt@linux.ibm.com>
Subject: Re: [PATCH v2 1/5] userfaultfd: introduce uffd_flags
Date: Fri, 22 Jul 2022 18:47:49 +0000	[thread overview]
Message-ID: <F0878743-8898-4CE4-9830-9D2A0E428727@vmware.com> (raw)
In-Reply-To: <28f4cf6c-1770-8628-0fc9-5d7982ede319@redhat.com>

On Jul 22, 2022, at 12:54 AM, David Hildenbrand <david@redhat.com> wrote:

> ⚠ External Email
> 
> On 18.07.22 13:47, Nadav Amit wrote:
>> From: Nadav Amit <namit@vmware.com>
>> 
>> As the next patches are going to introduce more information that needs
>> to be propagated regarding handled user requests, introduce uffd_flags
>> that would be used to propagate this information.
>> 
>> Remove the unused UFFD_FLAGS_SET to avoid confusion in the constant
>> names.
>> 
>> Introducing uffd flags also allows to avoid mm/userfaultfd from being
>> using uapi (e.g., UFFDIO_COPY_MODE_WP).
>> 
>> Cc: Mike Kravetz <mike.kravetz@oracle.com>
>> Cc: Hugh Dickins <hughd@google.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Axel Rasmussen <axelrasmussen@google.com>
>> Cc: Peter Xu <peterx@redhat.com>
>> Cc: Mike Rapoport <rppt@linux.ibm.com>
>> Acked-by: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Nadav Amit <namit@vmware.com>
> 
> [...]
> 
>> int mwriteprotect_range(struct mm_struct *dst_mm, unsigned long start,
>> -                     unsigned long len, bool enable_wp,
>> -                     atomic_t *mmap_changing)
>> +                     unsigned long len,
>> +                     atomic_t *mmap_changing, uffd_flags_t uffd_flags)
>> {
>> +     bool enable_wp = uffd_flags & UFFD_FLAGS_WP;
> 
> Could be that this will trigger a sparse warnings, but I haven't fully
> understood yet when/how sparse will start to complain. If so, this would
> have to be
> 
> bool enable_wp = !!(uffd_flags & UFFD_FLAGS_WP);
> 
> I stumbled into something like that in
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F202202252038.ij1YGn0d-lkp%40intel.com%2FT%2F&amp;data=05%7C01%7Cnamit%40vmware.com%7Cc237032d11f04972fdb708da6bb77ada%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637940733049609220%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=qW0xe6sS7PjP3papl890GoPTbZ97iE%2Ffztt1rA9t6%2F0%3D&amp;reserved=0

Oh, damn. Thanks for pointing it out. Sparse gives me segmentation faults
for some reason, but I guess it should be addressed - just in case.