From: Ville Syrjälä ville.syrjala@linux.intel.com
When building drm+i915 I get around 150 lines of sparse noise from dma_fence __rcu warnings. This series eliminates all of that.
The first two patches were already posted by Chris, but there wasn't any real reaction, so I figured I'd repost with a wider Cc list.
As for the other two patches, I'm no expert on dma_fence and I didn't spend a lot of time looking at it so I can't be sure I annotated all the accesses correctly. But I figured someone will scream at me if I got it wrong ;)
Cc: Dave Airlie airlied@redhat.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: linaro-mm-sig@lists.linaro.org Cc: linux-media@vger.kernel.org Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Chris Wilson chris@chris-wilson.co.uk
Chris Wilson (2): drm/syncobj: Mark up the fence as an RCU protected pointer dma-buf/fence: Sparse wants __rcu on the object itself
Ville Syrjälä (2): drm/syncobj: Use proper methods for accessing rcu protected pointers dma-buf: Use rcu_assign_pointer() to set rcu protected pointers
drivers/dma-buf/reservation.c | 2 +- drivers/gpu/drm/drm_syncobj.c | 11 +++++++---- include/drm/drm_syncobj.h | 2 +- include/linux/dma-fence.h | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-)
From: Chris Wilson chris@chris-wilson.co.uk
We take advantage of that syncobj->fence is an RCU-protected pointer, and so sparse complains that it is lacking annotation.
Cc: Dave Airlie airlied@redhat.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: linaro-mm-sig@lists.linaro.org Cc: linux-media@vger.kernel.org Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Sumit Semwal sumit.semwal@linaro.org Signed-off-by: Chris Wilson chris@chris-wilson.co.uk --- include/drm/drm_syncobj.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index 43e2f382d2f0..9e8ba90c6784 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -49,7 +49,7 @@ struct drm_syncobj { * This field should not be used directly. Use drm_syncobj_fence_get * and drm_syncobj_replace_fence instead. */ - struct dma_fence *fence; + struct dma_fence __rcu *fence; /** * @cb_list: * List of callbacks to call when the fence gets replaced
From: Chris Wilson chris@chris-wilson.co.uk
In order to silent sparse in dma_fence_get_rcu_safe(), we need to mark the incoming fence object as being RCU protected and not the pointer to the object.
Cc: Dave Airlie airlied@redhat.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: linaro-mm-sig@lists.linaro.org Cc: linux-media@vger.kernel.org Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Sumit Semwal sumit.semwal@linaro.org Signed-off-by: Chris Wilson chris@chris-wilson.co.uk --- include/linux/dma-fence.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index efdabbb64e3c..4c008170fe65 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -242,7 +242,7 @@ static inline struct dma_fence *dma_fence_get_rcu(struct dma_fence *fence) * The caller is required to hold the RCU read lock. */ static inline struct dma_fence * -dma_fence_get_rcu_safe(struct dma_fence * __rcu *fencep) +dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep) { do { struct dma_fence *fence;
On Thu, Nov 02, 2017 at 10:03:34PM +0200, Ville Syrjala wrote:
From: Chris Wilson chris@chris-wilson.co.uk
In order to silent sparse in dma_fence_get_rcu_safe(), we need to mark
s/silent/silence/
On the series (assuming sparse is indeed happy now, I didn't check that):
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
the incoming fence object as being RCU protected and not the pointer to the object.
Cc: Dave Airlie airlied@redhat.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: linaro-mm-sig@lists.linaro.org Cc: linux-media@vger.kernel.org Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Sumit Semwal sumit.semwal@linaro.org Signed-off-by: Chris Wilson chris@chris-wilson.co.uk
include/linux/dma-fence.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index efdabbb64e3c..4c008170fe65 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -242,7 +242,7 @@ static inline struct dma_fence *dma_fence_get_rcu(struct dma_fence *fence)
- The caller is required to hold the RCU read lock.
*/ static inline struct dma_fence * -dma_fence_get_rcu_safe(struct dma_fence * __rcu *fencep) +dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep) { do { struct dma_fence *fence; -- 2.13.6
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
From: Ville Syrjälä ville.syrjala@linux.intel.com
Use rcu_dereference_protected() and rcu_assign_pointer() for accessing the rcu protected syncobj->fence pointer. This eliminates several sparse warnings.
Cc: Dave Airlie airlied@redhat.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: linaro-mm-sig@lists.linaro.org Cc: linux-media@vger.kernel.org Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Chris Wilson chris@chris-wilson.co.uk Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_syncobj.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index f776fc1cc543..9b733c510cbf 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -106,7 +106,8 @@ static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj, * callback when a fence has already been set. */ if (syncobj->fence) { - *fence = dma_fence_get(syncobj->fence); + *fence = dma_fence_get(rcu_dereference_protected(syncobj->fence, + lockdep_is_held(&syncobj->lock))); ret = 1; } else { *fence = NULL; @@ -168,8 +169,9 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj,
spin_lock(&syncobj->lock);
- old_fence = syncobj->fence; - syncobj->fence = fence; + old_fence = rcu_dereference_protected(syncobj->fence, + lockdep_is_held(&syncobj->lock)); + rcu_assign_pointer(syncobj->fence, fence);
if (fence != old_fence) { list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) { @@ -659,7 +661,8 @@ static void syncobj_wait_syncobj_func(struct drm_syncobj *syncobj, container_of(cb, struct syncobj_wait_entry, syncobj_cb);
/* This happens inside the syncobj lock */ - wait->fence = dma_fence_get(syncobj->fence); + wait->fence = dma_fence_get(rcu_dereference_protected(syncobj->fence, + lockdep_is_held(&syncobj->lock))); wake_up_process(wait->task); }
From: Ville Syrjälä ville.syrjala@linux.intel.com
Use rcu_assign_pointer() when setting an rcu protected pointer. This gets rid of another sparse warning.
Cc: Dave Airlie airlied@redhat.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: linaro-mm-sig@lists.linaro.org Cc: linux-media@vger.kernel.org Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Chris Wilson chris@chris-wilson.co.uk Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/dma-buf/reservation.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index b44d9d7db347..d90333e0b6d5 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -318,7 +318,7 @@ int reservation_object_copy_fences(struct reservation_object *dst, continue; }
- dst_list->shared[dst_list->shared_count++] = fence; + rcu_assign_pointer(dst_list->shared[dst_list->shared_count++], fence); } } else { dst_list = NULL;
Patch #4 is Reviewed-by: Christian König christian.koenig@amd.com.
The rest is Acked-by: Christian König christian.koenig@amd.com.
Regards, Christian.
Am 02.11.2017 um 21:03 schrieb Ville Syrjala:
From: Ville Syrjälä ville.syrjala@linux.intel.com
When building drm+i915 I get around 150 lines of sparse noise from dma_fence __rcu warnings. This series eliminates all of that.
The first two patches were already posted by Chris, but there wasn't any real reaction, so I figured I'd repost with a wider Cc list.
As for the other two patches, I'm no expert on dma_fence and I didn't spend a lot of time looking at it so I can't be sure I annotated all the accesses correctly. But I figured someone will scream at me if I got it wrong ;)
Cc: Dave Airlie airlied@redhat.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: linaro-mm-sig@lists.linaro.org Cc: linux-media@vger.kernel.org Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Chris Wilson chris@chris-wilson.co.uk
Chris Wilson (2): drm/syncobj: Mark up the fence as an RCU protected pointer dma-buf/fence: Sparse wants __rcu on the object itself
Ville Syrjälä (2): drm/syncobj: Use proper methods for accessing rcu protected pointers dma-buf: Use rcu_assign_pointer() to set rcu protected pointers
drivers/dma-buf/reservation.c | 2 +- drivers/gpu/drm/drm_syncobj.c | 11 +++++++---- include/drm/drm_syncobj.h | 2 +- include/linux/dma-fence.h | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-)
Hi Ville,
On 3 November 2017 at 13:18, Christian König christian.koenig@amd.com wrote:
Patch #4 is Reviewed-by: Christian König christian.koenig@amd.com.
The rest is Acked-by: Christian König christian.koenig@amd.com.
Regards, Christian.
Am 02.11.2017 um 21:03 schrieb Ville Syrjala:
From: Ville Syrjälä ville.syrjala@linux.intel.com
When building drm+i915 I get around 150 lines of sparse noise from dma_fence __rcu warnings. This series eliminates all of that.
The first two patches were already posted by Chris, but there wasn't any real reaction, so I figured I'd repost with a wider Cc list.
As for the other two patches, I'm no expert on dma_fence and I didn't spend a lot of time looking at it so I can't be sure I annotated all the accesses correctly. But I figured someone will scream at me if I got it wrong ;)
Cc: Dave Airlie airlied@redhat.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: linaro-mm-sig@lists.linaro.org Cc: linux-media@vger.kernel.org Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Chris Wilson chris@chris-wilson.co.uk
Chris Wilson (2): drm/syncobj: Mark up the fence as an RCU protected pointer dma-buf/fence: Sparse wants __rcu on the object itself
Ville Syrjälä (2): drm/syncobj: Use proper methods for accessing rcu protected pointers dma-buf: Use rcu_assign_pointer() to set rcu protected pointers
For patches 2 (with Daniel's minor comment) and 4, please feel free to add my Acked-by: Sumit Semwal <sumit.semwal@linaro.org.
drivers/dma-buf/reservation.c | 2 +- drivers/gpu/drm/drm_syncobj.c | 11 +++++++---- include/drm/drm_syncobj.h | 2 +- include/linux/dma-fence.h | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-)
Best, Sumit.
On Tue, Nov 07, 2017 at 01:37:10PM +0530, Sumit Semwal wrote:
Hi Ville,
On 3 November 2017 at 13:18, Christian König christian.koenig@amd.com wrote:
Patch #4 is Reviewed-by: Christian König christian.koenig@amd.com.
The rest is Acked-by: Christian König christian.koenig@amd.com.
Regards, Christian.
Am 02.11.2017 um 21:03 schrieb Ville Syrjala:
From: Ville Syrjälä ville.syrjala@linux.intel.com
When building drm+i915 I get around 150 lines of sparse noise from dma_fence __rcu warnings. This series eliminates all of that.
The first two patches were already posted by Chris, but there wasn't any real reaction, so I figured I'd repost with a wider Cc list.
As for the other two patches, I'm no expert on dma_fence and I didn't spend a lot of time looking at it so I can't be sure I annotated all the accesses correctly. But I figured someone will scream at me if I got it wrong ;)
Cc: Dave Airlie airlied@redhat.com Cc: Jason Ekstrand jason@jlekstrand.net Cc: linaro-mm-sig@lists.linaro.org Cc: linux-media@vger.kernel.org Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Chris Wilson chris@chris-wilson.co.uk
Chris Wilson (2): drm/syncobj: Mark up the fence as an RCU protected pointer dma-buf/fence: Sparse wants __rcu on the object itself
Ville Syrjälä (2): drm/syncobj: Use proper methods for accessing rcu protected pointers dma-buf: Use rcu_assign_pointer() to set rcu protected pointers
For patches 2 (with Daniel's minor comment) and 4, please feel free to add my Acked-by: Sumit Semwal <sumit.semwal@linaro.org.
Thanks everyone. Series pushed to drm-misc-next.
drivers/dma-buf/reservation.c | 2 +- drivers/gpu/drm/drm_syncobj.c | 11 +++++++---- include/drm/drm_syncobj.h | 2 +- include/linux/dma-fence.h | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-)
Best, Sumit.
linaro-mm-sig@lists.linaro.org
-
Christian König -
Daniel Vetter -
Sumit Semwal -
Ville Syrjala -
Ville Syrjälä