On Wed, Apr 12, 2023 at 11:20:35AM -0700, Axel Rasmussen wrote:
> > - uffdio_register.range.start = (unsigned long)addr;
> > - uffdio_register.range.len = len;
> > - uffdio_register.mode = UFFDIO_REGISTER_MODE_MISSING;
> > - if (ioctl(uffd, UFFDIO_REGISTER, &uffdio_register) == -1) {
> > + if (uffd_register(uffd, addr, len, true, false, false)) {
>
> For what it's worth, I agree with Mike that the booleans here are hard
> to read. It's not clear what "true, false, false" means without going
> to look at the header file, whereas "uffd_register(uffd, addr, len,
> MINOR)" would be immediately clear.
>
> One solution I've seen outside the kernel is to comment the bools, like:
>
> uffd_register(..., /*minor=*/true, /*wp=*/false, /*minor=*/false);
>
> But, then I feel we lose most of the benefit we wanted from switching
> to bools anyway (code length). :)
>
> I do agree the macro names are unwieldy, and lots of tests already use
> booleans so we'd have to convert from bool -> flag. If it were me, I
> would resolve that by:
>
> - Define much shorter aliases for these macros in uffd-common.h
> - Consider refactoring callers to not use bools either.
>
> Then again, I also agree with Mike that it's not a deal breaker, if
> you like this way much more than that alternative we can leave it.
No strong feeling here either. I kept it just to avoid code churns and
rebases.
I'll see whether I'll need a repost, if so I can go back to use modes.
Thanks,
--
Peter Xu
