Op 25-10-18 om 10:53 schreef Chunming Zhou: > drivers/gpu/drm/drm_syncobj.c:202:4-14: ERROR: function drm_syncobj_find_signal_pt_for_point called on line 390 inside lock on line 389 but uses GFP_KERNEL > > Find functions that refer to GFP_KERNEL but are called with locks held. > > Semantic patch information: > The proposed change of converting the GFP_KERNEL is not necessarily the > correct one. It may be desired to unlock the lock, or to not call the > function under the lock in the first place. > > Generated by: scripts/coccinelle/locks/call_kern.cocci > > v2: > syncobj->timeline still needs protect. > > Signed-off-by: Chunming Zhou <david1.zhou@xxxxxxx> > Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Christian König <easy2remember.chk@xxxxxxxxxxxxxx> > --- > drivers/gpu/drm/drm_syncobj.c | 25 +++++++++++++------------ > 1 file changed, 13 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c > index b7eaa603f368..b843d4c2778c 100644 > --- a/drivers/gpu/drm/drm_syncobj.c > +++ b/drivers/gpu/drm/drm_syncobj.c > @@ -111,15 +111,16 @@ static struct dma_fence > uint64_t point) > { > struct drm_syncobj_signal_pt *signal_pt; > + struct dma_fence *f = NULL; > + struct drm_syncobj_stub_fence *fence = > + kzalloc(sizeof(struct drm_syncobj_stub_fence), > + GFP_KERNEL); > > + if (!fence) > + return NULL; > + spin_lock(&syncobj->pt_lock); > if ((syncobj->type == DRM_SYNCOBJ_TYPE_TIMELINE) && > (point <= syncobj->timeline)) { > - struct drm_syncobj_stub_fence *fence = > - kzalloc(sizeof(struct drm_syncobj_stub_fence), > - GFP_KERNEL); > - > - if (!fence) > - return NULL; > spin_lock_init(&fence->lock); > dma_fence_init(&fence->base, > &drm_syncobj_stub_fence_ops, > @@ -128,6 +129,7 @@ static struct dma_fence > point); > > dma_fence_signal(&fence->base); > + spin_unlock(&syncobj->pt_lock); > return &fence->base; > } > > @@ -137,9 +139,12 @@ static struct dma_fence > if ((syncobj->type == DRM_SYNCOBJ_TYPE_BINARY) && > (point != signal_pt->value)) > continue; > - return dma_fence_get(&signal_pt->fence_array->base); > + f = dma_fence_get(&signal_pt->fence_array->base); > + break; > } > - return NULL; > + spin_unlock(&syncobj->pt_lock); > + kfree(fence); > + return f; > } > > static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj, > @@ -166,9 +171,7 @@ static void drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj, > } > > mutex_lock(&syncobj->cb_mutex); > - spin_lock(&syncobj->pt_lock); > *fence = drm_syncobj_find_signal_pt_for_point(syncobj, pt_value); > - spin_unlock(&syncobj->pt_lock); > if (!*fence) > drm_syncobj_add_callback_locked(syncobj, cb, func); > mutex_unlock(&syncobj->cb_mutex); Maybe slightly offtopic, but seems that this function should return an error if fence == NULL, at least when it's a result of -ENOMEM. It would be nice if this was sent as a separate patch, instead of waiting indefinitely.. > @@ -379,11 +382,9 @@ drm_syncobj_point_get(struct drm_syncobj *syncobj, u64 point, u64 flags, > if (ret < 0) > return ret; > } > - spin_lock(&syncobj->pt_lock); > *fence = drm_syncobj_find_signal_pt_for_point(syncobj, point); > if (!*fence) > ret = -EINVAL; There's no way to make a destinction between -ENOMEM and -EINVAL, I think it would be good to use ERR_PTR, and then return PTR_ERR_OR_ZERO().. I would also change the subject line to something more descriptive, rather than the generic one it's now. But otherwise looks good to me. :) _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx