Re: [PATCH v10 3/8] selftests/mm: add HMM test for mmap lock-dropping faults

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `dmirror_range_fault_unlocked` fails to enforce an absolute timeout for its `mmu_interval_read_retry` loop, leading to potential livelocks.
--

--- Patch [3]: [PATCH v10 3/8] selftests/mm: add HMM test for mmap lock-dropping faults ---
commit e4ef68d1c56c2f5bfba882ccad54a4d7284e5403
Author: Stanislav Kinsburskii <[email protected]>

selftests/mm: add HMM test for mmap lock-dropping faults

This commit adds test_hmm coverage for the HMM lock-dropping fault path.
It introduces a new ioctl that exercises the unbounded retry mode while
allowing the mmap lock to be dropped during fault handling, alongside a
corresponding userfaultfd read selftest.

> diff --git a/lib/test_hmm.c b/lib/test_hmm.c
> index 45c0cb9922184..6205fb313bd05 100644
> --- a/lib/test_hmm.c
> +++ b/lib/test_hmm.c

[ ... ]

> @@ -389,6 +389,67 @@ static int dmirror_range_fault(struct dmirror *dmirror,
>  	return ret;
>  }
>  
> +static int dmirror_range_fault_unlocked(struct dmirror *dmirror,
> +					struct hmm_range *range,
> +					unsigned long timeout)
> +{
> +	int ret;
> +
> +	while (true) {
> +		ret = hmm_range_fault_unlocked_timeout(range, timeout);
> +		if (ret)
> +			goto out;
> +
> +		mutex_lock(&dmirror->mutex);
> +		if (mmu_interval_read_retry(range->notifier,
> +					    range->notifier_seq)) {
> +			mutex_unlock(&dmirror->mutex);
> +			continue;

[Severity: Medium]
Does this loop lack an absolute timeout enforcement?

When mmu_interval_read_retry() triggers a retry, the same relative timeout
value is passed back into hmm_range_fault_unlocked_timeout(). Because that
function calculates its internal deadline as jiffies + timeout, the
deadline gets pushed forward on every retry.

Could this lead to a livelock if this function is ever called with a non-zero
timeout, since the timeout resets infinitely during concurrent page table
invalidations?

> +		}
> +		break;
> +	}
> +
> +	ret = dmirror_do_fault(dmirror, range);
> +
> +	mutex_unlock(&dmirror->mutex);
> +out:
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3