lkml.org 
[lkml]   [2023]   [Apr]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 14/31] selftests/mm: uffd_[un]register()
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

\
 
 \ /
  Last update: 2023-04-12 21:35    [W:0.127 / U:0.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site