Re: [PATCH RFC] userfaultfd: introduce UFFDIO_COPY_MODE_YOUNG

[John Hubbard <jhubbard@nvidia.com>]

On 6/14/22 13:56, Nadav Amit wrote:
...
>> So if you have a choice, I implore you to prefer flags and/or enums. :)
> 
> Thanks for the feedback - I am aware it is very confusing to have booleans
> and especially multiple ones in a func call.
> 
> Just not sure how it maps to what I proposed. I thought of passing as an
> argument reference (pointer) to something similar to the following struct,
> which I think is very self-descriptive:
> 
> struct uffd_op {
> 	/* various fields */
> 	struct vm_area_struct *dst_vma;
> 	unsigned long len;
> 	atomic_t *mmap_changing;
> 
> 	...
> 	
> 	/* ... and some flags */
> 	int wp: 1;
> 	int zero: 1;
> 	int read_likely: 1;

I am more accustomed to seeing:
	unsigned int flags;

...and then some #defines or enums nearby that are used for .flags.
The bitfields are not used as much, Linus wrote some words about why,
(which I'm not hopeful I can find). Basically they are not a very
robust C feature, and the kernel has good support for dealing with
flags within a word.

> 
> 	...
> };
> 
> I think that fits what you were asking for. The only thing I am not sure of,
> is whether to include in uffd_op fields that are internal to mm/userfaultfd
> such as “page” and “newly_allocated”. I guess not.
> 

Actually, I think passing around a struct might be overkill, when you can
simply collapse the various boolean args into a single flags arg. It looked
like a lot of the new args were bools...

thanks,
-- 
John Hubbard
NVIDIA