Re: [PATCH v4 13/19] selftests/mm: cover a shared-source collapse write race

[Mike Rapoport]

> collapse_fork() checks that a fork-shared range collapses in the process
> that asks for it while the co-sharer keeps its own page, but the co-sharer
> sits still while that happens.

co-sharer sounds awkward, can we use parent and child?

> Add a case where the co-sharer writes to the shared range throughout the
> collapse.  CoW has to keep the two sides apart under those writes: the
> collapsing child must see the content from before the fork, and the
> writing parent must see only its own writes.
> 
> The co-sharer unshares one page every 10ms, and only once the collapsing
> side says it is about to start.  Writing the range in a burst breaks CoW
> on all of it before the collapse begins, leaving the child to collapse
> pages that are already exclusive to it.
> 
> Nothing here is new behaviour: the case passes on mainline, and locks in
> isolation that collapse already provides.

And that's imporant because?

> Assisted-by: Claude-Code:claude-opus-5
> Tested-by: Muhammad Usama Anjum <usama.anjum@xxxxxxx>
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>
>
> diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c
> index 392fd65fcc97..1d357143258a 100644
> --- a/tools/testing/selftests/mm/khugepaged.c
> +++ b/tools/testing/selftests/mm/khugepaged.c
> @@ -1218,6 +1218,97 @@ static void collapse_max_ptes_shared(struct collapse_context *c, struct mem_ops
>  	ksft_test_result_report(exit_status, "%s\n", __func__);
>  }
>  
> +/*
> + * Content stays isolated while a co-sharer writes concurrently. A shared
> + * source is copied live (not frozen), relying on it being CoW - immutable
> + * for the duration of the copy; a co-sharer's write goes to a CoW copy. The
> + * collapsing child must see the pre-fork content, the writing parent only
> + * its own writes.
> + */
> +static void collapse_fork_cow_race(struct collapse_context *c, struct mem_ops *ops)
> +{
> +	const unsigned long shared = 64 * page_size;
> +	const int stride = page_size / sizeof(int);
> +	int wstatus, child_status, i, n = shared / page_size;
> +	/* volatile: the loop below must really store, on every iteration */
> +	volatile int *ip;
> +	pid_t child;
> +	int sync[2];
> +	char go = 1;
> +	void *p;
> +
> +	p = ops->setup_area(1);
> +	ip = p;
> +	ops->fault(p, 0, shared);		/* shared prefix, pre-fork pattern */
> +	if (pipe(sync))
> +		ksft_exit_fail_perror("pipe()");
> +
> +	ksft_print_msg("Fork, collapse in the child while the parent rewrites...");
> +	child = fork();
> +	if (!child) {
> +		int collapse_status;
> +
> +		close(sync[0]);
> +		ops->fault(p, shared, hpage_pmd_size);	/* private remainder */
> +		/* Start the parent unsharing, and give it a head start. */
> +		if (write(sync[1], &go, 1) != 1)
> +			_exit(KSFT_FAIL);
> +		usleep(5000);
> +		c->collapse("Collapse a range shared with a writing co-sharer",
> +			    p, 1, ops, true);
> +		collapse_status = exit_status;
> +		for (i = 0; i < n; i++)
> +			if (ip[i * stride] != i + 0xdead0000)
> +				break;
> +		if (i == n)
> +			success("OK");
> +		else
> +			fail("Fail: child content");
> +		/* The content check must not bury a failed collapse. */
> +		if (exit_status != KSFT_FAIL)
> +			exit_status = collapse_status;
> +		ops->cleanup_area(p, hpage_pmd_size);
> +		_exit(exit_status);
> +	}
> +
> +	close(sync[1]);
> +	if (read(sync[0], &go, 1) != 1)
> +		ksft_exit_fail_msg("child never reached the collapse\n");
> +
> +	/*
> +	 * Unshare one page at a time.  A burst would break CoW on all of them
> +	 * in microseconds -- wait_for_scan() does not even poll for TICK --
> +	 * and the child would collapse pages already exclusive to it.
> +	 */
> +	i = 0;
> +	do {
> +		if (i < n)
> +			ip[i * stride] = i + 0xbeef0000;
> +		i++;
> +		usleep(10 * 1000);
> +	} while (waitpid(child, &wstatus, WNOHANG) == 0);

Can we please not put waitpid() inside while condition?

-- 
Sincerely yours,
Mike.