Re: [PATCH v2] selftests/mm: hugetlb_madv_vs_map: add underflow test

[Guillaume Morin <guillaume@morinfr.org>]

On 31 Aug 12:01, Mike Rapoport wrote:
> > +		return;
> > +	}
> > +
> > +	/* Parent */
> > +	close(pipe_fds[0]);
> > +
> > +	/* First unmap, this will close the vma */
> > +	if (munmap(huge_ptr, mmap_size) != 0) {
> > +		kill(pid, SIGKILL);
> > +		ksft_exit_fail_msg("munmap failed");
> 
> Won't this leave the child as a zombie?

It will, for a brief amount of time then re-parented to init which will
wait(). Happy to add a waitpid() here but I was concerned it added more
complexity for not much benefit (it's very very unlikely munmap will
fail). Let me know.

> > +	}
> > +	close(pipe_fds[1]);
> > +	if (waitpid(pid, NULL, 0) <= 0)
> > +		ksft_exit_fail_msg("write failed");
> > +
> > +	nr_reserved = hugetlb_nr_resv_pages(default_huge_page_size());
> > +	if (nr_reserved == 0) {
> > +		ksft_test_result_pass("After the child dies, HugePages_Rsvd is properly set to 0\n");
> 
> This makes this count the test as two, right? 

Yes, it checks both that there is no underflow but that the count is
restorred as well. is that a problem? 

> And do you need to re-check what happens after the child exits if you
> already detected the underflow?

Need is a strong word. It can be done either way really.

> 
> > +	} else {
> > +		ksft_exit_fail_msg("Unexpected HugePages_Rsvd=%ld after the child termination munmap, should be 0 or -1.  Repeat the test\n",
> > +				   nr_reserved);
> > +	}
> > +}
> > +
> > +int main(void)
> > +{
> > +	ksft_print_header();
> > +	ksft_set_plan(3);
> 
> The comment at the header implies we've added the second test, so they plan
> should be bumped to 2.

So you're saying that every test function should only fail or pass once?


-- 
Guillaume Morin <guillaume@morinfr.org>