Re: [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
Am 26.10.18 um 10:28 schrieb zhoucm1: > Thanks, Could you help to submit to drm-misc again? Done. Christian. > > -David > > > On 2018年10月26日 15:43, Christian König wrote: >> Am 26.10.18 um 08:20 schrieb 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. >>> >>> Generated by: scripts/coccinelle/locks/call_kern.cocci >>> >>> v2: >>> syncobj->timeline still needs protect. >>> >>> v3: >>> use a global signaled fence instead of re-allocation. >>> >>> v4: >>> Don't need moving lock. >>> Don't expose func. >>> >>> v5: >>> rename func and directly return. >>> >>> Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with >>> lock debug kernel options enabled. >>> >>> Signed-off-by: Chunming Zhou >>> Cc: Maarten Lankhorst >>> Cc: intel-...@lists.freedesktop.org >>> Cc: Christian König >>> Cc: Chris Wilson >>> CC: Julia Lawall >>> Reviewed-by: Chris Wilson >> >> Reviewed-by: Christian König >> >>> --- >>> drivers/gpu/drm/drm_syncobj.c | 36 >>> ++- >>> 1 file changed, 19 insertions(+), 17 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_syncobj.c >>> b/drivers/gpu/drm/drm_syncobj.c >>> index b7eaa603f368..d1c6f21c72b5 100644 >>> --- a/drivers/gpu/drm/drm_syncobj.c >>> +++ b/drivers/gpu/drm/drm_syncobj.c >>> @@ -80,6 +80,23 @@ struct drm_syncobj_signal_pt { >>> struct list_head list; >>> }; >>> +static DEFINE_SPINLOCK(signaled_fence_lock); >>> +static struct dma_fence signaled_fence; >>> + >>> +static struct dma_fence *drm_syncobj_get_stub_fence(void) >>> +{ >>> + spin_lock(&signaled_fence_lock); >>> + if (!signaled_fence.ops) { >>> + dma_fence_init(&signaled_fence, >>> + &drm_syncobj_stub_fence_ops, >>> + &signaled_fence_lock, >>> + 0, 0); >>> + dma_fence_signal_locked(&signaled_fence); >>> + } >>> + spin_unlock(&signaled_fence_lock); >>> + >>> + return dma_fence_get(&signaled_fence); >>> +} >>> /** >>> * drm_syncobj_find - lookup and reference a sync object. >>> * @file_private: drm file private pointer >>> @@ -113,23 +130,8 @@ static struct dma_fence >>> struct drm_syncobj_signal_pt *signal_pt; >>> 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, >>> - &fence->lock, >>> - syncobj->timeline_context, >>> - point); >>> - >>> - dma_fence_signal(&fence->base); >>> - return &fence->base; >>> - } >>> + (point <= syncobj->timeline)) >>> + return drm_syncobj_get_stub_fence(); >>> list_for_each_entry(signal_pt, &syncobj->signal_pt_list, >>> list) { >>> if (point > signal_pt->value) >> > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
Op 26-10-18 om 08:20 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. > > Generated by: scripts/coccinelle/locks/call_kern.cocci > > v2: > syncobj->timeline still needs protect. > > v3: > use a global signaled fence instead of re-allocation. > > v4: > Don't need moving lock. > Don't expose func. > > v5: > rename func and directly return. > > Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with > lock debug kernel options enabled. > > Signed-off-by: Chunming Zhou > Cc: Maarten Lankhorst > Cc: intel-...@lists.freedesktop.org > Cc: Christian König > Cc: Chris Wilson > CC: Julia Lawall > Reviewed-by: Chris Wilson > --- > drivers/gpu/drm/drm_syncobj.c | 36 ++- > 1 file changed, 19 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c > index b7eaa603f368..d1c6f21c72b5 100644 > --- a/drivers/gpu/drm/drm_syncobj.c > +++ b/drivers/gpu/drm/drm_syncobj.c > @@ -80,6 +80,23 @@ struct drm_syncobj_signal_pt { > struct list_head list; > }; > > +static DEFINE_SPINLOCK(signaled_fence_lock); > +static struct dma_fence signaled_fence; > + > +static struct dma_fence *drm_syncobj_get_stub_fence(void) > +{ > + spin_lock(&signaled_fence_lock); > + if (!signaled_fence.ops) { > + dma_fence_init(&signaled_fence, > +&drm_syncobj_stub_fence_ops, > +&signaled_fence_lock, > +0, 0); > + dma_fence_signal_locked(&signaled_fence); > + } > + spin_unlock(&signaled_fence_lock); Could this be used by drm_syncobj_assign_null_handle too? Maybe as a separate patch? Reviewed-by: Maarten Lankhorst ~Maarten ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
Thanks, Could you help to submit to drm-misc again? -David On 2018年10月26日 15:43, Christian König wrote: Am 26.10.18 um 08:20 schrieb 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. Generated by: scripts/coccinelle/locks/call_kern.cocci v2: syncobj->timeline still needs protect. v3: use a global signaled fence instead of re-allocation. v4: Don't need moving lock. Don't expose func. v5: rename func and directly return. Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with lock debug kernel options enabled. Signed-off-by: Chunming Zhou Cc: Maarten Lankhorst Cc: intel-...@lists.freedesktop.org Cc: Christian König Cc: Chris Wilson CC: Julia Lawall Reviewed-by: Chris Wilson Reviewed-by: Christian König --- drivers/gpu/drm/drm_syncobj.c | 36 ++- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index b7eaa603f368..d1c6f21c72b5 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -80,6 +80,23 @@ struct drm_syncobj_signal_pt { struct list_head list; }; +static DEFINE_SPINLOCK(signaled_fence_lock); +static struct dma_fence signaled_fence; + +static struct dma_fence *drm_syncobj_get_stub_fence(void) +{ + spin_lock(&signaled_fence_lock); + if (!signaled_fence.ops) { + dma_fence_init(&signaled_fence, + &drm_syncobj_stub_fence_ops, + &signaled_fence_lock, + 0, 0); + dma_fence_signal_locked(&signaled_fence); + } + spin_unlock(&signaled_fence_lock); + + return dma_fence_get(&signaled_fence); +} /** * drm_syncobj_find - lookup and reference a sync object. * @file_private: drm file private pointer @@ -113,23 +130,8 @@ static struct dma_fence struct drm_syncobj_signal_pt *signal_pt; 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, - &fence->lock, - syncobj->timeline_context, - point); - - dma_fence_signal(&fence->base); - return &fence->base; - } + (point <= syncobj->timeline)) + return drm_syncobj_get_stub_fence(); list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) { if (point > signal_pt->value) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
Am 25.10.18 um 17:08 schrieb 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. Generated by: scripts/coccinelle/locks/call_kern.cocci v2: syncobj->timeline still needs protect. v3: use a global signaled fence instead of re-allocation. v4: Don't need moving lock. Don't expose func. Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with lock debug kernel options enabled. Signed-off-by: Chunming Zhou Cc: Maarten Lankhorst Cc: intel-...@lists.freedesktop.org Cc: Christian König Cc: Chris Wilson CC: Julia Lawall --- drivers/gpu/drm/drm_syncobj.c | 41 --- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index b7eaa603f368..fab0a2cf672e 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -80,6 +80,23 @@ struct drm_syncobj_signal_pt { struct list_head list; }; +static DEFINE_SPINLOCK(signaled_fence_lock); +static struct dma_fence signaled_fence; + +static struct dma_fence *drm_syncobj_signaled_fence_get(void) Maybe name that drm_syncobj_get_stub_fence(). +{ + spin_lock(&signaled_fence_lock); + if (!signaled_fence.ops) { + dma_fence_init(&signaled_fence, + &drm_syncobj_stub_fence_ops, + &signaled_fence_lock, + 0, 0); + dma_fence_signal_locked(&signaled_fence); + } + spin_unlock(&signaled_fence_lock); + + return dma_fence_get(&signaled_fence); +} /** * drm_syncobj_find - lookup and reference a sync object. * @file_private: drm file private pointer @@ -111,24 +128,12 @@ static struct dma_fence uint64_t point) { struct drm_syncobj_signal_pt *signal_pt; + struct dma_fence *f = NULL; 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, - &fence->lock, - syncobj->timeline_context, - point); - - dma_fence_signal(&fence->base); - return &fence->base; + f = drm_syncobj_signaled_fence_get(); + goto out; } list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) { @@ -137,9 +142,11 @@ 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); + goto out; } - return NULL; +out: + return f; I think we can drop this change now and just return directly. Christian. } static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj, ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
Am 26.10.18 um 08:20 schrieb 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. Generated by: scripts/coccinelle/locks/call_kern.cocci v2: syncobj->timeline still needs protect. v3: use a global signaled fence instead of re-allocation. v4: Don't need moving lock. Don't expose func. v5: rename func and directly return. Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with lock debug kernel options enabled. Signed-off-by: Chunming Zhou Cc: Maarten Lankhorst Cc: intel-...@lists.freedesktop.org Cc: Christian König Cc: Chris Wilson CC: Julia Lawall Reviewed-by: Chris Wilson Reviewed-by: Christian König --- drivers/gpu/drm/drm_syncobj.c | 36 ++- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index b7eaa603f368..d1c6f21c72b5 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -80,6 +80,23 @@ struct drm_syncobj_signal_pt { struct list_head list; }; +static DEFINE_SPINLOCK(signaled_fence_lock); +static struct dma_fence signaled_fence; + +static struct dma_fence *drm_syncobj_get_stub_fence(void) +{ + spin_lock(&signaled_fence_lock); + if (!signaled_fence.ops) { + dma_fence_init(&signaled_fence, + &drm_syncobj_stub_fence_ops, + &signaled_fence_lock, + 0, 0); + dma_fence_signal_locked(&signaled_fence); + } + spin_unlock(&signaled_fence_lock); + + return dma_fence_get(&signaled_fence); +} /** * drm_syncobj_find - lookup and reference a sync object. * @file_private: drm file private pointer @@ -113,23 +130,8 @@ static struct dma_fence struct drm_syncobj_signal_pt *signal_pt; 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, - &fence->lock, - syncobj->timeline_context, - point); - - dma_fence_signal(&fence->base); - return &fence->base; - } + (point <= syncobj->timeline)) + return drm_syncobj_get_stub_fence(); list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) { if (point > signal_pt->value) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
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. Generated by: scripts/coccinelle/locks/call_kern.cocci v2: syncobj->timeline still needs protect. v3: use a global signaled fence instead of re-allocation. v4: Don't need moving lock. Don't expose func. v5: rename func and directly return. Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with lock debug kernel options enabled. Signed-off-by: Chunming Zhou Cc: Maarten Lankhorst Cc: intel-...@lists.freedesktop.org Cc: Christian König Cc: Chris Wilson CC: Julia Lawall Reviewed-by: Chris Wilson --- drivers/gpu/drm/drm_syncobj.c | 36 ++- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index b7eaa603f368..d1c6f21c72b5 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -80,6 +80,23 @@ struct drm_syncobj_signal_pt { struct list_head list; }; +static DEFINE_SPINLOCK(signaled_fence_lock); +static struct dma_fence signaled_fence; + +static struct dma_fence *drm_syncobj_get_stub_fence(void) +{ + spin_lock(&signaled_fence_lock); + if (!signaled_fence.ops) { + dma_fence_init(&signaled_fence, + &drm_syncobj_stub_fence_ops, + &signaled_fence_lock, + 0, 0); + dma_fence_signal_locked(&signaled_fence); + } + spin_unlock(&signaled_fence_lock); + + return dma_fence_get(&signaled_fence); +} /** * drm_syncobj_find - lookup and reference a sync object. * @file_private: drm file private pointer @@ -113,23 +130,8 @@ static struct dma_fence struct drm_syncobj_signal_pt *signal_pt; 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, - &fence->lock, - syncobj->timeline_context, - point); - - dma_fence_signal(&fence->base); - return &fence->base; - } + (point <= syncobj->timeline)) + return drm_syncobj_get_stub_fence(); list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) { if (point > signal_pt->value) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
Quoting Chunming Zhou (2018-10-25 16:08:31) > 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. > > Generated by: scripts/coccinelle/locks/call_kern.cocci > > v2: > syncobj->timeline still needs protect. > > v3: > use a global signaled fence instead of re-allocation. > > v4: > Don't need moving lock. > Don't expose func. > > Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with > lock debug kernel options enabled. > > Signed-off-by: Chunming Zhou > Cc: Maarten Lankhorst > Cc: intel-...@lists.freedesktop.org > Cc: Christian König > Cc: Chris Wilson > CC: Julia Lawall > --- > - return NULL; > +out: > + return f; As it reduced to just a return, I'd probably have gone with multiple returns in this instance. Still the compiler should have done the equivalent and jumped to a single ret. Reviewed-by: Chris Wilson -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock
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. Generated by: scripts/coccinelle/locks/call_kern.cocci v2: syncobj->timeline still needs protect. v3: use a global signaled fence instead of re-allocation. v4: Don't need moving lock. Don't expose func. Tested by: syncobj_wait and ./deqp-vk -n dEQP-VK.*semaphore* with lock debug kernel options enabled. Signed-off-by: Chunming Zhou Cc: Maarten Lankhorst Cc: intel-...@lists.freedesktop.org Cc: Christian König Cc: Chris Wilson CC: Julia Lawall --- drivers/gpu/drm/drm_syncobj.c | 41 --- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index b7eaa603f368..fab0a2cf672e 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -80,6 +80,23 @@ struct drm_syncobj_signal_pt { struct list_head list; }; +static DEFINE_SPINLOCK(signaled_fence_lock); +static struct dma_fence signaled_fence; + +static struct dma_fence *drm_syncobj_signaled_fence_get(void) +{ + spin_lock(&signaled_fence_lock); + if (!signaled_fence.ops) { + dma_fence_init(&signaled_fence, + &drm_syncobj_stub_fence_ops, + &signaled_fence_lock, + 0, 0); + dma_fence_signal_locked(&signaled_fence); + } + spin_unlock(&signaled_fence_lock); + + return dma_fence_get(&signaled_fence); +} /** * drm_syncobj_find - lookup and reference a sync object. * @file_private: drm file private pointer @@ -111,24 +128,12 @@ static struct dma_fence uint64_t point) { struct drm_syncobj_signal_pt *signal_pt; + struct dma_fence *f = NULL; 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, - &fence->lock, - syncobj->timeline_context, - point); - - dma_fence_signal(&fence->base); - return &fence->base; + f = drm_syncobj_signaled_fence_get(); + goto out; } list_for_each_entry(signal_pt, &syncobj->signal_pt_list, list) { @@ -137,9 +142,11 @@ 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); + goto out; } - return NULL; +out: + return f; } static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj, -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel