Re: [PATCH v5 6/7] selftests: prctl: introduce tests for disabling THPs completely

[David Hildenbrand <david@redhat.com>]

> +
> +TEST_F(prctl_thp_disable_completely, fork)
> +{
> +	int ret = 0;
> +	pid_t pid;
> +
> +	/* Make sure prctl changes are carried across fork */
> +	pid = fork();
> +	ASSERT_GE(pid, 0);
> +
> +	if (!pid)
> +		prctl_thp_disable_completely_test(_metadata, self->pmdsize, variant->thp_policy);
> +

Skimming over this once more ... this raises two questions

(a) There is nothing to wait for in the child
(b) Does it work when we return in the child from this function?

I think (b) works by design of the kselftest_harness, as this function is
itself executed from a child process.

Regarding (a), it might be cleaner to just

index 77c53a91124f1..1a48bcf2e9160 100644
--- a/tools/testing/selftests/mm/prctl_thp_disable.c
+++ b/tools/testing/selftests/mm/prctl_thp_disable.c
@@ -271,9 +271,11 @@ TEST_F(prctl_thp_disable_except_madvise, fork)
         pid = fork();
         ASSERT_GE(pid, 0);
  
-       if (!pid)
+       if (!pid) {
                 prctl_thp_disable_except_madvise_test(_metadata, self->pmdsize,
                                                       variant->thp_policy);
+               return;
+       }
  
         wait(&ret);
         if (WIFEXITED(ret))



Same probably applies to patch #7. Feel free to send simple fixup patches
that Andrew can squash. No need for a full resend.

-- 
Cheers

David / dhildenb