On 05/10/2026 12:53, Boris Brezillon wrote: > On Mon, 5 Oct 2026 12:03:26 +0100 > Steven Price <[email protected]> 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 <[email protected]> >>> Reviewed-by: Akash Goel <[email protected]> >>> Signed-off-by: Boris Brezillon <[email protected]> >> >> In itself I can't see anything wrong with this change, so: >> >> Reviewed-by: Steven Price <[email protected]> >> >> 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
