Re: [PATCH v2 4/5] drm/panthor: Actually check huge-page mapping on sparse regions

[Steven Price]

On 05/10/2026 12:53, Boris Brezillon wrote:
> On Mon, 5 Oct 2026 12:03:26 +0100
> Steven Price <steven.price@xxxxxxx> wrote:
> 
>> On 24/09/2026 12:04, Boris Brezillon wrote:
>>> With the recent changes to iova_mapped_as_huge_page(), the check for
>>> huge-page mapping of sparse BOs is actually simple:
>>>
>>> - for a sparse mapping, we know the BO offset any VA in this regions is
>>>   va & (SZ_2M - 1)
>>> - the VA we're searching the BO offset for is the 2M-aligned
>>>   aligned_va value
>>>
>>> This guarantees that the BO offset to check is always zero in that case.
>>>
>>> This is simple enough to let the code check if page 0 is a huge page
>>> and save the unmap+map dance when the dummy BO is not backed by a
>>> a huge page. So let's do that and kill the comment that says it's too
>>> complicated.
>>>
>>> Reviewed-by: Liviu Dudau <liviu.dudau@xxxxxxx>
>>> Reviewed-by: Akash Goel <akash.goel@xxxxxxx>
>>> Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>  
>>
>> In itself I can't see anything wrong with this change, so:
>>
>> Reviewed-by: Steven Price <steven.price@xxxxxxx>
>>
>> However...
>>
>>> ---
>>>  drivers/gpu/drm/panthor/panthor_mmu.c | 15 ++++++++-------
>>>  1 file changed, 8 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
>>> index d2897099763e..01564d250adf 100644
>>> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
>>> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
>>> @@ -2337,18 +2337,18 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
>>>  
>>>  		return false;
>>>  	} else {
>>> -		const struct page *pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
>>>  		struct panthor_vma *vma = container_of(mapping, struct panthor_vma, base);
>>>  		bool is_sparse = vma->flags & DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE;
>>> +		const struct page *pg;
>>>  
>>> -		/* If the unmapped VMA stands for a sparse mapping, always
>>> -		 * assume the backing storage is a THP, since the overhead of
>>> -		 * unmapping 2MiB worth of 4KiB pages and remapping some of
>>> -		 * them is offset by the logic of working out whether it's
>>> -		 * the opposite case right below.
>>> +		/* BO offset on a sparse mapping is chosen so that 2M-aligned
>>> +		 * VAs point to the start of the BO. Since aligned_va (the
>>> +		 * address we check huge-page against) is 2M-aligned, the BO
>>> +		 * offset is guaranteed to be zero.
>>> +		 * Check panthor_fix_sparse_map_offset() for more details.
>>>  		 */
>>>  		if (is_sparse)
>>> -			return true;
>>> +			bo_offset = 0;
>>>  
>>>  		/* In case of shmem backing, we know we can only have a huge
>>>  		 * mapping if the bo_offset is 2M aligned, meaning we can skip
>>> @@ -2357,6 +2357,7 @@ iova_mapped_as_huge_page(struct drm_gpuva *mapping, u64 va)
>>>  		if (!IS_ALIGNED(bo_offset, SZ_2M))
>>>  			return false;
>>>  
>>> +		pg = bo->backing.pages[bo_offset >> PAGE_SHIFT];
>>>  		return folio_size(page_folio(pg)) >= SZ_2M;  
>>
>> ... this seems like it could be problematic. On the mapping side we use
>> the scatter list to decide whether the region is huge page mapped or
>> not. The scatter list code can merge segments that are contiguous (see
>> pages_are_mergeable()), so if we have a region which has small folios we
>> fail this check even though the pages might have been mapped as huge pages.
>>
>> This is a problem on the non-sparse path as well (hence not really
>> related to this patch). I'm not really sure how to test this though - I
>> may well have overlooked something here.
> 
> So, this is based on the assumption that shmem backing is allocated
> with the buddy allocator, and because of how this allocator splits
> bigger order blocks to service smaller allocations, it's my
> understanding that two consecutive folios of the same size/order can't
> be physically contiguous.

Yes, you'd expect the buddy allocator to combine the folios back into a
larger one if they were contiguous. I guess we should be safe, at least
for now. I still feel it's unnecessarily fragile and complex trying to
work out whether we've mapped as a huge page or not. But I don't
actually have a better solution at the moment, and this series is at
least improving things. I'll put it on my todo list to look at later.

Thanks,
Steve