| Message ID | 20181026062027.12179-1-david1.zhou@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/syncobj: Avoid kmalloc(GFP_KERNEL) under spinlock | expand |
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 <david1.zhou@amd.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: intel-gfx@lists.freedesktop.org > Cc: Christian König <easy2remember.chk@googlemail.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > CC: Julia Lawall <julia.lawall@lip6.fr> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Christian König <christian.koenig@amd.com> > --- > 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)
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 <david1.zhou@amd.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: intel-gfx@lists.freedesktop.org > Cc: Christian König <easy2remember.chk@googlemail.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > CC: Julia Lawall <julia.lawall@lip6.fr> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > 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.lankhorst@linux.intel.com> ~Maarten
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 <david1.zhou@amd.com> >>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >>> Cc: intel-gfx@lists.freedesktop.org >>> Cc: Christian König <easy2remember.chk@googlemail.com> >>> Cc: Chris Wilson <chris@chris-wilson.co.uk> >>> CC: Julia Lawall <julia.lawall@lip6.fr> >>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> >> >> Reviewed-by: Christian König <christian.koenig@amd.com> >> >>> --- >>> 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) >> >
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)