On Tue, 4 Jan 2022 14:35:34 -0800 Axel Rasmussen <axelrasmussen@xxxxxxxxxx> wrote:
> On Mon, Jan 3, 2022 at 6:17 PM Mike Kravetz <mike.kravetz@xxxxxxxxxx> wrote:
> >
> > The message for commit f5c73297181c ("userfaultfd/selftests: fix hugetlb
> > area allocations") says there is no need to create a hugetlb file in the
> > non-shared testing case. However, the commit did not actually change
> > the code to prevent creation of the file.
> >
> > While it is technically true that there is no need to create and use a
> > hugetlb file in the case of non-shared-testing, it is useful. This is
> > because 'hole punching' of a hugetlb file has the potentially incorrect
> > side effect of also removing pages from private mappings. The
> > userfaultfd test relies on this side effect for removing pages from the
> > destination buffer during rounds of stress testing.
> >
> > Remove the incomplete code that was added to deal with no hugetlb file.
> > Just keep the code that prevents reserves from being created for the
> > destination area.
> >
> > *alloc_area = mmap(NULL, nr_pages * page_size, PROT_READ | PROT_WRITE,
> > - map_shared ? MAP_SHARED :
> > - MAP_PRIVATE | MAP_HUGETLB |
> > + (map_shared ? MAP_SHARED : MAP_PRIVATE) |
> > + MAP_HUGETLB |
> > (*alloc_area == area_src ? 0 : MAP_NORESERVE),
> > - huge_fd,
> > - *alloc_area == area_src ? 0 : nr_pages * page_size);
> > + huge_fd, *alloc_area == area_src ? 0 :
> > + nr_pages * page_size);
>
> Sorry to nitpick, but I think it was slightly more readable when the
> ternary was all on one line.
When you have that many arguments I think it's clearer to put one per
line, viz.
*alloc_area = mmap(NULL,
nr_pages * page_size,
PROT_READ | PROT_WRITE,
(map_shared ? MAP_SHARED : MAP_PRIVATE) |
MAP_HUGETLB |
(*alloc_area == area_src ? 0 : MAP_NORESERVE),
huge_fd,
*alloc_area == area_src ? 0 : nr_pages * page_size);
But whatever...
