Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Device memory mappings are permanently leaked on the error path due to deferred assignment of svm_pages->dpagemap -- commit 09830ce1d99e5366614becf544e5d523ceeb461d Author: Honglei Huang <[email protected]> drm/gpusvm: extract drm_gpusvm_dma_map_pages() helper This commit moves the per-device DMA mapping loop of drm_gpusvm_get_pages() into a helper drm_gpusvm_dma_map_pages(). The mapping logic is only moved, not changed, so there is no functional change. > diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c > index 3496cb5688365..753361b965cf9 100644 > --- a/drivers/gpu/drm/drm_gpusvm.c > +++ b/drivers/gpu/drm/drm_gpusvm.c [ ... ] > @@ -1695,17 +1631,114 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm, > /* WRITE_ONCE pairs with READ_ONCE for opportunistic checks */ > WRITE_ONCE(svm_pages->flags.__flags, flags.__flags); > > + return 0; > + > +err_unmap: > + svm_pages->flags.has_dma_mapping = true; > + __drm_gpusvm_unmap_pages(gpusvm, svm_pages, num_dma_mapped); [Severity: High] This is a pre-existing issue, but does this code permanently leak device memory mappings on the error path? When a device page is successfully mapped in drm_gpusvm_dma_map_pages(), dpagemap->ops->device_map() is used. However, svm_pages->dpagemap is only updated at the end of the function upon complete success. If an error occurs on a subsequent page (such as returning -EOPNOTSUPP due to encountering a mixed device memory mapping), the code jumps to err_unmap and calls __drm_gpusvm_unmap_pages(). Because svm_pages->dpagemap is still NULL at this point, the unmap helper will skip the cleanup callback: drivers/gpu/drm/drm_gpusvm.c:__drm_gpusvm_unmap_pages() { ... struct drm_pagemap *dpagemap = svm_pages->dpagemap; ... else if (dpagemap && dpagemap->ops->device_unmap) dpagemap->ops->device_unmap(...) ... } Should the local dpagemap be temporarily assigned to svm_pages->dpagemap before calling the unmap helper, or is there another way to ensure these partially mapped pages are properly cleaned up? > + return err; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
