Re: [PATCH v6 04/26] drm/gpuvm: Introduce DRM_GPUVM_SM_MAP_OPS_FLAG_SPLIT_MADVISE flag

["Danilo Krummrich" <dakr@kernel.org>]

On Mon Aug 11, 2025 at 8:52 AM CEST, Himal Prasad Ghimiray wrote:
> On 09-08-2025 18:53, Danilo Krummrich wrote:
> Possible scenarios for ops functionality based on input start and end 
> address from user:
>
> a) User-provided range is a subset of an existing drm_gpuva
> Expected Result: Same behavior as the default sm_map logic.
> Reference: Case 1 from [1].
>
> b) Either start or end (but not both) is not aligned with a drm_gpuva 
> boundary
> Expected Result: One REMAP and one MAP operation.
> Reference: Case 3 from [1].
>
> Existing GPUVMAs:
>
>           drm_gpuva1                        drm_gpuva2
> 	[a----------------------------b-1][b-------------------c-1]
>
> User Input to ops:
>    start = inside drm_gpuva1
>    end   = exactly at c-1 (end of drm_gpuva2)
>
> Resulting Mapping:
> 	drm_gpuva1:pre       drm_gpuva:New map     drm_gpuva2
> 	[a---------start-1][start------- b-1] [b------------c-1]
>
> Ops Created:
>    REMAP:UNMAP drm_gpuva1 a to b
>    REMAP:PREV a to start - 1
>    MAP: start to b-1
>
> Note: No unmap of drm_gpuvma2 and no merging of New map and drm_gpuva2.
>
> c) Both start and end are not aligned with drm_gpuva boundaries, and 
> they fall within different drm_gpuva regions
> Expected Result: Two REMAP operations and two MAP operations.
> Reference: Case 2 from [1].
>
>
> d) User-provided range does not overlap with any existing drm_gpuva
> Expected Result: No operations.
> start and end exactly match the boundaries of one or more existing 
> drm_gpuva regions
>
> e) This includes cases where start is at the beginning of drm_gpuva1 and 
> end is at the end of drm_gpuva2 (drm_gpuva1 and drm_gpuva2 can be same 
> or different).
> Expected Result: No operations
>
> [1] 
> https://lore.kernel.org/intel-xe/4203f450-4b49-401d-81a8-cdcca02035f9@intel.com/ 

<snip>

> I’ve tried to explain the behavior/usecase with madvise and expected 
> outcomes of the ops logic in detail in [1]. Could you please take a 
> moment to review that and let me know if the explanation is sufficient 
> or if any part needs further clarification?

Thanks a lot for writing this up!

I think this clarifies everything, the examples from [1] are good (sorry that
your reply from the RFC got lost somehow on my end).

>> Please add a separate section about madvise operations to the documentation at
>> the beginning of the drivers/gpu/drm/drm_gpuvm.c file.
>
> Sure will do that.

Great, this will help users (as well as reviewers) a lot. Please also add your
examples from [1] to this entry, similar to the existing examples for sm_map.

>>> v2
>>> - use drm_gpuvm_sm_map_ops_create with flags instead of defining new
>>>    ops_create (Danilo)
>> 
>> If this turns out not to be what I thought semantically and we still agree it's
>> the correct approach, I think I have to take this back and it should indeed be
>> an entirely separate code path. But let's wait for your answers above.

Having the correct understanding of how this is supposed to work (and seeing how
the code turns out) I think it's still OK to integrate it into sm_map().

However, it probably makes sense to factor out the code into a common function
and then build the madvise() and sm_map() functions on top of it.

Please also find some more comments on the patch itself.

>> Again, I really think this needs some proper documentation like in the
>> "DOC: Split and Merge" documentation section.
>
> Sure

Thanks!