Re: [Freedreno] [PATCH 00/14] drm/msm: de-struct_mutex-ification
On Mon, Oct 5, 2020 at 11:20 AM Daniel Vetter wrote: > > On Mon, Oct 5, 2020 at 6:24 PM Kristian Høgsberg wrote: > > > > On Sun, Oct 4, 2020 at 9:21 PM Rob Clark wrote: > > > > > > From: Rob Clark > > > > > > This doesn't remove *all* the struct_mutex, but it covers the worst > > > of it, ie. shrinker/madvise/free/retire. The submit path still uses > > > struct_mutex, but it still needs *something* serialize a portion of > > > the submit path, and lock_stat mostly just shows the lock contention > > > there being with other submits. And there are a few other bits of > > > struct_mutex usage in less critical paths (debugfs, etc). But this > > > seems like a reasonable step in the right direction. > > > > What a great patch set. Daniel has some good points and nothing that > > requires big changes, but on the other hand, I'm not sure it's > > something that needs to block this set either. > > Personally I'd throw the lockdep priming on top to make sure this > stays correct (it's 3 lines), but yes imo this is all good to go. Just > figured I'll sprinkle the latest in terms of gem locking over the > series while it's here :-) Yeah, I'll defn throw the lockdep priming into v2.. and I've got using obj->resv for locking on the todo list but looks like enough churn that it will probably be it's own series (but seems like there is room to introduce some lock/unlock helpers that don't really change anything but make an obj->lock transition easier) BR, -R > -Daniel > > > Either way, for the series > > > > Reviewed-by: Kristian H. Kristensen > > > > > Rob Clark (14): > > > drm/msm: Use correct drm_gem_object_put() in fail case > > > drm/msm: Drop chatty trace > > > drm/msm: Move update_fences() > > > drm/msm: Add priv->mm_lock to protect active/inactive lists > > > drm/msm: Document and rename preempt_lock > > > drm/msm: Protect ring->submits with it's own lock > > > drm/msm: Refcount submits > > > drm/msm: Remove obj->gpu > > > drm/msm: Drop struct_mutex from the retire path > > > drm/msm: Drop struct_mutex in free_object() path > > > drm/msm: remove msm_gem_free_work > > > drm/msm: drop struct_mutex in madvise path > > > drm/msm: Drop struct_mutex in shrinker path > > > drm/msm: Don't implicit-sync if only a single ring > > > > > > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 +- > > > drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 12 +-- > > > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 4 +- > > > drivers/gpu/drm/msm/msm_debugfs.c | 7 ++ > > > drivers/gpu/drm/msm/msm_drv.c | 15 +--- > > > drivers/gpu/drm/msm/msm_drv.h | 19 +++-- > > > drivers/gpu/drm/msm/msm_gem.c | 76 ++ > > > drivers/gpu/drm/msm/msm_gem.h | 53 + > > > drivers/gpu/drm/msm/msm_gem_shrinker.c| 58 ++ > > > drivers/gpu/drm/msm/msm_gem_submit.c | 17 ++-- > > > drivers/gpu/drm/msm/msm_gpu.c | 96 ++- > > > drivers/gpu/drm/msm/msm_gpu.h | 5 +- > > > drivers/gpu/drm/msm/msm_ringbuffer.c | 3 +- > > > drivers/gpu/drm/msm/msm_ringbuffer.h | 13 ++- > > > 14 files changed, 188 insertions(+), 194 deletions(-) > > > > > > -- > > > 2.26.2 > > > > > > ___ > > > Freedreno mailing list > > > freedr...@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/freedreno > > ___ > > dri-devel mailing list > > dri-de...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
Re: [Freedreno] [PATCH 00/14] drm/msm: de-struct_mutex-ification
On Mon, Oct 5, 2020 at 6:24 PM Kristian Høgsberg wrote: > > On Sun, Oct 4, 2020 at 9:21 PM Rob Clark wrote: > > > > From: Rob Clark > > > > This doesn't remove *all* the struct_mutex, but it covers the worst > > of it, ie. shrinker/madvise/free/retire. The submit path still uses > > struct_mutex, but it still needs *something* serialize a portion of > > the submit path, and lock_stat mostly just shows the lock contention > > there being with other submits. And there are a few other bits of > > struct_mutex usage in less critical paths (debugfs, etc). But this > > seems like a reasonable step in the right direction. > > What a great patch set. Daniel has some good points and nothing that > requires big changes, but on the other hand, I'm not sure it's > something that needs to block this set either. Personally I'd throw the lockdep priming on top to make sure this stays correct (it's 3 lines), but yes imo this is all good to go. Just figured I'll sprinkle the latest in terms of gem locking over the series while it's here :-) -Daniel > Either way, for the series > > Reviewed-by: Kristian H. Kristensen > > > Rob Clark (14): > > drm/msm: Use correct drm_gem_object_put() in fail case > > drm/msm: Drop chatty trace > > drm/msm: Move update_fences() > > drm/msm: Add priv->mm_lock to protect active/inactive lists > > drm/msm: Document and rename preempt_lock > > drm/msm: Protect ring->submits with it's own lock > > drm/msm: Refcount submits > > drm/msm: Remove obj->gpu > > drm/msm: Drop struct_mutex from the retire path > > drm/msm: Drop struct_mutex in free_object() path > > drm/msm: remove msm_gem_free_work > > drm/msm: drop struct_mutex in madvise path > > drm/msm: Drop struct_mutex in shrinker path > > drm/msm: Don't implicit-sync if only a single ring > > > > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 +- > > drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 12 +-- > > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 4 +- > > drivers/gpu/drm/msm/msm_debugfs.c | 7 ++ > > drivers/gpu/drm/msm/msm_drv.c | 15 +--- > > drivers/gpu/drm/msm/msm_drv.h | 19 +++-- > > drivers/gpu/drm/msm/msm_gem.c | 76 ++ > > drivers/gpu/drm/msm/msm_gem.h | 53 + > > drivers/gpu/drm/msm/msm_gem_shrinker.c| 58 ++ > > drivers/gpu/drm/msm/msm_gem_submit.c | 17 ++-- > > drivers/gpu/drm/msm/msm_gpu.c | 96 ++- > > drivers/gpu/drm/msm/msm_gpu.h | 5 +- > > drivers/gpu/drm/msm/msm_ringbuffer.c | 3 +- > > drivers/gpu/drm/msm/msm_ringbuffer.h | 13 ++- > > 14 files changed, 188 insertions(+), 194 deletions(-) > > > > -- > > 2.26.2 > > > > ___ > > Freedreno mailing list > > freedr...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/freedreno > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [Freedreno] [PATCH 00/14] drm/msm: de-struct_mutex-ification
On Sun, Oct 4, 2020 at 9:21 PM Rob Clark wrote: > > From: Rob Clark > > This doesn't remove *all* the struct_mutex, but it covers the worst > of it, ie. shrinker/madvise/free/retire. The submit path still uses > struct_mutex, but it still needs *something* serialize a portion of > the submit path, and lock_stat mostly just shows the lock contention > there being with other submits. And there are a few other bits of > struct_mutex usage in less critical paths (debugfs, etc). But this > seems like a reasonable step in the right direction. What a great patch set. Daniel has some good points and nothing that requires big changes, but on the other hand, I'm not sure it's something that needs to block this set either. Either way, for the series Reviewed-by: Kristian H. Kristensen > Rob Clark (14): > drm/msm: Use correct drm_gem_object_put() in fail case > drm/msm: Drop chatty trace > drm/msm: Move update_fences() > drm/msm: Add priv->mm_lock to protect active/inactive lists > drm/msm: Document and rename preempt_lock > drm/msm: Protect ring->submits with it's own lock > drm/msm: Refcount submits > drm/msm: Remove obj->gpu > drm/msm: Drop struct_mutex from the retire path > drm/msm: Drop struct_mutex in free_object() path > drm/msm: remove msm_gem_free_work > drm/msm: drop struct_mutex in madvise path > drm/msm: Drop struct_mutex in shrinker path > drm/msm: Don't implicit-sync if only a single ring > > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 +- > drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 12 +-- > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 4 +- > drivers/gpu/drm/msm/msm_debugfs.c | 7 ++ > drivers/gpu/drm/msm/msm_drv.c | 15 +--- > drivers/gpu/drm/msm/msm_drv.h | 19 +++-- > drivers/gpu/drm/msm/msm_gem.c | 76 ++ > drivers/gpu/drm/msm/msm_gem.h | 53 + > drivers/gpu/drm/msm/msm_gem_shrinker.c| 58 ++ > drivers/gpu/drm/msm/msm_gem_submit.c | 17 ++-- > drivers/gpu/drm/msm/msm_gpu.c | 96 ++- > drivers/gpu/drm/msm/msm_gpu.h | 5 +- > drivers/gpu/drm/msm/msm_ringbuffer.c | 3 +- > drivers/gpu/drm/msm/msm_ringbuffer.h | 13 ++- > 14 files changed, 188 insertions(+), 194 deletions(-) > > -- > 2.26.2 > > ___ > Freedreno mailing list > freedr...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno
