Hi Boris,
On 1/28/26 15:52, Boris Brezillon wrote:
Hi Akash,
On Wed, 28 Jan 2026 11:21:43 +0000
Akash Goel <[email protected]> wrote:
Sorry I have a doubt.
Should we update the panthor_gem_sync() function to ensure pages can't
get reclaimed whlilst they are being synced ?.
panthor_ioctl_bo_sync() takes a reference on the BO before calling
panthor_gem_sync(). But I think due to this patch, the backking pages
can get released in the middle of sync loop.
sgt = panthor_gem_get_dev_sgt(bo);
if (IS_ERR(sgt))
return PTR_ERR(sgt);
for_each_sgtable_dma_sg(sgt, sgl, count) {
dma_addr_t paddr = sg_dma_address(sgl);
size_t len = sg_dma_len(sgl);
dma_sync_single_for_device();
dma_sync_single_for_cpu();
}
Please can you confirm.
I think you're right. We either need to pin/unpin around a sync
operation, or we need to take the resv lock to make sure the GEM is not
reclaimed while we do that. It probably makes sense to do the latter,
Fully agree with you. The latter option is better.
since we don't want to prematurely pull back the data from the swap
just to synchronize CPU accesses (the
dma_unmap_sgtable() in panthor_gem_dev_map_cleanup_locked() should have
synchronized things for us on a swapout).
Right, the sync would happen inside dma_unmap_sgtable() on swapout.
I think we may hit the following warning, inside
panthor_gem_dev_map_get_sgt_locked(), if BO_SYNC ioctl is invoked for an
evicted BO.
if (drm_WARN_ON_ONCE(bo->base.dev, !bo->backing.pages))
return ERR_PTR(-EINVAL);
So we would need to take resv lock inside panthor_gem_sync() and also
return early if bo->backing.pages is NULL.
I'll fix that before posting a v2.
Thank you.
Best regards
Akash
Thanks,
Boris
