| Message ID | 20230307144621.10748-2-thomas.hellstrom@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/ttm: Small fixes / cleanups in prep for shrinking | expand |
Am 07.03.23 um 15:46 schrieb Thomas Hellström: > The LRU mechanism may look up a resource in the process of being removed > from an object. The locking rules here are a bit unclear but it looks > currently like res->bo assignment is protected by the LRU lock, whereas > bo->resource is protected by the object lock, while *clearing* of > bo->resource is also protected by the LRU lock. This means that if > we check that bo->resource points to the LRU resource under the LRU > lock we should be safe. > So perform that check before deciding to swap out a bo. That avoids > dereferencing a NULL bo->resource in ttm_bo_swapout(). Please make sure that this is pushed to drm-misc-fixes ASAP. I've getting complains for this from different sides. Thanks, Christian. > > Fixes: 6a9b02899402 ("drm/ttm: move the LRU into resource handling v4") > Cc: Christian König <christian.koenig@amd.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Christian Koenig <christian.koenig@amd.com> > Cc: Huang Rui <ray.huang@amd.com> > Cc: Alex Deucher <alexander.deucher@amd.com> > Cc: Felix Kuehling <Felix.Kuehling@amd.com> > Cc: Philip Yang <Philip.Yang@amd.com> > Cc: Qiang Yu <qiang.yu@amd.com> > Cc: Matthew Auld <matthew.auld@intel.com> > Cc: Nirmoy Das <nirmoy.das@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com> > Cc: Anshuman Gupta <anshuman.gupta@intel.com> > Cc: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com> > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> > Reviewed-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/ttm/ttm_device.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c > index c7a1862f322a..ae2f19dc9f81 100644 > --- a/drivers/gpu/drm/ttm/ttm_device.c > +++ b/drivers/gpu/drm/ttm/ttm_device.c > @@ -158,7 +158,7 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx, > struct ttm_buffer_object *bo = res->bo; > uint32_t num_pages; > > - if (!bo) > + if (!bo || bo->resource != res) > continue; > > num_pages = PFN_UP(bo->base.size);
On 3/7/23 17:55, Christian König wrote: > Am 07.03.23 um 15:46 schrieb Thomas Hellström: >> The LRU mechanism may look up a resource in the process of being removed >> from an object. The locking rules here are a bit unclear but it looks >> currently like res->bo assignment is protected by the LRU lock, whereas >> bo->resource is protected by the object lock, while *clearing* of >> bo->resource is also protected by the LRU lock. This means that if >> we check that bo->resource points to the LRU resource under the LRU >> lock we should be safe. >> So perform that check before deciding to swap out a bo. That avoids >> dereferencing a NULL bo->resource in ttm_bo_swapout(). > > Please make sure that this is pushed to drm-misc-fixes ASAP. > > I've getting complains for this from different sides. > > Thanks, > Christian. Done. /Thomas > >> >> Fixes: 6a9b02899402 ("drm/ttm: move the LRU into resource handling v4") >> Cc: Christian König <christian.koenig@amd.com> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> Cc: Christian Koenig <christian.koenig@amd.com> >> Cc: Huang Rui <ray.huang@amd.com> >> Cc: Alex Deucher <alexander.deucher@amd.com> >> Cc: Felix Kuehling <Felix.Kuehling@amd.com> >> Cc: Philip Yang <Philip.Yang@amd.com> >> Cc: Qiang Yu <qiang.yu@amd.com> >> Cc: Matthew Auld <matthew.auld@intel.com> >> Cc: Nirmoy Das <nirmoy.das@intel.com> >> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com> >> Cc: Anshuman Gupta <anshuman.gupta@intel.com> >> Cc: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com> >> Cc: dri-devel@lists.freedesktop.org >> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> >> Reviewed-by: Christian König <christian.koenig@amd.com> >> --- >> drivers/gpu/drm/ttm/ttm_device.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/ttm/ttm_device.c >> b/drivers/gpu/drm/ttm/ttm_device.c >> index c7a1862f322a..ae2f19dc9f81 100644 >> --- a/drivers/gpu/drm/ttm/ttm_device.c >> +++ b/drivers/gpu/drm/ttm/ttm_device.c >> @@ -158,7 +158,7 @@ int ttm_device_swapout(struct ttm_device *bdev, >> struct ttm_operation_ctx *ctx, >> struct ttm_buffer_object *bo = res->bo; >> uint32_t num_pages; >> - if (!bo) >> + if (!bo || bo->resource != res) >> continue; >> num_pages = PFN_UP(bo->base.size); >
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index c7a1862f322a..ae2f19dc9f81 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -158,7 +158,7 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx, struct ttm_buffer_object *bo = res->bo; uint32_t num_pages; - if (!bo) + if (!bo || bo->resource != res) continue; num_pages = PFN_UP(bo->base.size);
The LRU mechanism may look up a resource in the process of being removed from an object. The locking rules here are a bit unclear but it looks currently like res->bo assignment is protected by the LRU lock, whereas bo->resource is protected by the object lock, while *clearing* of bo->resource is also protected by the LRU lock. This means that if we check that bo->resource points to the LRU resource under the LRU lock we should be safe. So perform that check before deciding to swap out a bo. That avoids dereferencing a NULL bo->resource in ttm_bo_swapout(). Fixes: 6a9b02899402 ("drm/ttm: move the LRU into resource handling v4") Cc: Christian König <christian.koenig@amd.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Christian Koenig <christian.koenig@amd.com> Cc: Huang Rui <ray.huang@amd.com> Cc: Alex Deucher <alexander.deucher@amd.com> Cc: Felix Kuehling <Felix.Kuehling@amd.com> Cc: Philip Yang <Philip.Yang@amd.com> Cc: Qiang Yu <qiang.yu@amd.com> Cc: Matthew Auld <matthew.auld@intel.com> Cc: Nirmoy Das <nirmoy.das@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com> Cc: Anshuman Gupta <anshuman.gupta@intel.com> Cc: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com> Cc: dri-devel@lists.freedesktop.org Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> Reviewed-by: Christian König <christian.koenig@amd.com> --- drivers/gpu/drm/ttm/ttm_device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)