Re: [RFC v3 4/7] drm/xe/vm: Add madvise autoreset notifier worker

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Returning false for non-blockable invalidations incorrectly prevents the OOM reaper from unmapping memory.
- [Medium] Redundant VMA splitting for partial overlaps when the VMA already has default attributes.
--

commit fe0b22bc527cb1c9caa55068f36be5982435fd31
Author: Arvind Yadav <arvind.yadav@intel.com>

drm/xe/vm: Add madvise autoreset notifier worker

This commit adds a worker to reset madvise-managed VMA attributes when
userspace unmaps CPU-only mirror ranges. It uses an MMU notifier callback
to record pending unmap events and widens the range if multiple unmaps
arrive before the worker runs. The worker then resets attributes for ranges
not covered by a CPU VMA.

> diff --git a/drivers/gpu/drm/xe/xe_vm_madvise.c b/drivers/gpu/drm/xe/xe_vm_madvise.c
> index c4fb290041956..c2abe712598a1 100644
> --- a/drivers/gpu/drm/xe/xe_vm_madvise.c
> +++ b/drivers/gpu/drm/xe/xe_vm_madvise.c
[ ... ]
> @@ -732,3 +738,501 @@ int xe_vm_madvise_ioctl(struct drm_device *dev, void *data, struct drm_file *fil
[ ... ]
> +static int xe_vm_madvise_process_unmap(struct xe_vm *vm, u64 start, u64 end)
> +{
[ ... ]
> +		has_default_attr = xe_vma_has_default_mem_attrs(vma);
> +		seg_start = max(addr, xe_vma_start(vma));
> +		seg_end = min(end, xe_vma_end(vma));
> +
> +		/* Merge adjacent default-attr VMAs when possible. */
> +		if (has_default_attr &&
> +		    xe_vma_start(vma) >= start &&
> +		    xe_vma_end(vma) <= end) {
> +			seg_start = xe_vma_start(vma);
> +			seg_end = xe_vma_end(vma);
> +			xe_vm_find_cpu_addr_mirror_vma_range(vm, &seg_start, &seg_end);
> +			if (xe_vma_start(vma) == seg_start && xe_vma_end(vma) == seg_end) {
> +				/* Nothing to merge. */
> +				addr = seg_end;
> +				continue;
> +			}
> +		} else if (xe_vma_start(vma) == seg_start && xe_vma_end(vma) == seg_end) {
> +			/* Exact VMA match, reset in place. */
> +			xe_vma_reset_to_default_attrs(vma);
> +			addr = seg_end;
> +			continue;
> +		}
> +
> +		err = xe_vm_alloc_cpu_addr_mirror_vma(vm, seg_start, seg_end - seg_start);

[Severity: Medium]
When a CPU hole partially overlaps a GPU VMA that already has default
attributes, will this fall through to xe_vm_alloc_cpu_addr_mirror_vma() and
cause redundant VMA splitting?

If the overlap is strictly partial, both the if and else if conditions above
fail. The fall-through maps the overlapping portion with default attributes,
forcing the GPUVM infrastructure to split the existing VMA. 

Since the existing VMA already possessed default attributes, this split
appears to be redundant and might needlessly fragment a single large VMA into
smaller identical pieces. Repeated partial unmaps from userspace could
artificially fragment the VMA tree and leak xe_vma objects.

[ ... ]
> +static bool xe_madvise_notifier_callback(struct mmu_interval_notifier *mni,
> +					 const struct mmu_notifier_range *range,
> +					 unsigned long cur_seq)
> +{
> +	struct xe_madvise_notifier *notifier =
> +		container_of(mni, struct xe_madvise_notifier, mmu_notifier);
> +	struct xe_vm *vm = notifier->vm;
> +	u64 adj_start, adj_end;
> +
> +	if (range->event != MMU_NOTIFY_UNMAP)
> +		return true;
> +
> +	if (!mmu_notifier_range_blockable(range))
> +		return false;

[Severity: High]
Does returning false for non-blockable invalidations incorrectly prevent the
OOM reaper from unmapping memory?

Because the callback implementation is entirely non-blocking (it relies on
down_read_trylock(), spin_lock(), and queue_work()), there seems to be no
technical limitation requiring it to block. 

Returning false from an MMU interval notifier's invalidate callback signals
that the invalidation could not be completed without blocking, causing
__mmu_notifier_invalidate_range_start() to return -EAGAIN. 

Since the OOM reaper operates in a non-blockable context, receiving -EAGAIN
means it cannot retry indefinitely and will skip the VMA, failing to reclaim
the memory and defeating the purpose of the OOM killer. Could this safely
process the event and unconditionally return true for unmap events instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604095012.367231-1-arvind.yadav@intel.com?part=4