From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maarten Lankhorst Subject: [PATCH] drm/nouveau: fix lockdep splat in display Date: Tue, 05 Feb 2013 11:29:59 +0100 Message-ID: <5110DF27.2020200@canonical.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: nouveau-bounces+gcfxn-nouveau=m.gmane.org-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Errors-To: nouveau-bounces+gcfxn-nouveau=m.gmane.org-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org To: Ben Skeggs Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, "dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org" List-Id: nouveau.vger.kernel.org Add a flag NVOBJ_FLAG_CREAT_EXCL, which will make sure that only 1 engctx will be created. This removes the need of having multiple Fixes the following lockdep splat: ============================================= [ INFO: possible recursive locking detected ] 3.8.0-rc6-ninja+ #1 Not tainted --------------------------------------------- modprobe/585 is trying to acquire lock: (&subdev->mutex){+.+.+.}, at: [] nouveau_instobj_create_+0x43/0x90 [nouveau] but task is already holding lock: (&subdev->mutex){+.+.+.}, at: [] nv50_disp_data_ctor+0x5d/0xd0 [nouveau] other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(&subdev->mutex); lock(&subdev->mutex); *** DEADLOCK *** May be due to missing lock nesting notation 4 locks held by modprobe/585: #0: (&__lockdep_no_validate__){......}, at: [] __driver_attach+0x53/0xb0 #1: (&__lockdep_no_validate__){......}, at: [] __driver_attach+0x61/0xb0 #2: (drm_global_mutex){+.+.+.}, at: [] drm_get_pci_dev+0xbc/0x2b0 #3: (&subdev->mutex){+.+.+.}, at: [] nv50_disp_data_ctor+0x5d/0xd0 [nouveau] stack backtrace: Pid: 585, comm: modprobe Not tainted 3.8.0-rc6-expert+ #1 Call Trace: [] validate_chain.isra.33+0xd72/0x10d0 [] ? __kernel_text_address+0x58/0x80 [] ? print_context_stack+0x5d/0xd0 [] __lock_acquire+0x3a1/0xb60 [] ? __lock_is_held+0x54/0x80 [] lock_acquire+0x5a/0x70 [] ? nouveau_instobj_create_+0x43/0x90 [nouveau] [] mutex_lock_nested+0x69/0x340 [] ? nouveau_instobj_create_+0x43/0x90 [nouveau] [] ? nouveau_object_create_+0x60/0xa0 [nouveau] [] nouveau_instobj_create_+0x43/0x90 [nouveau] [] nv50_instobj_ctor+0x4c/0xf0 [nouveau] [] nouveau_object_ctor+0x33/0xc0 [nouveau] [] nv50_instmem_alloc+0x21/0x30 [nouveau] [] nouveau_gpuobj_create_+0x247/0x2f0 [nouveau] [] ? _raw_spin_unlock_irqrestore+0x3a/0x70 [] ? trace_hardirqs_on_caller+0x10d/0x1a0 [] nouveau_engctx_create_+0x25c/0x2a0 [nouveau] [] nv50_disp_data_ctor+0xc1/0xd0 [nouveau] [] ? nouveau_subdev_reset+0x52/0x60 [nouveau] [] nouveau_object_ctor+0x33/0xc0 [nouveau] [] nouveau_object_new+0x112/0x240 [nouveau] [] nv50_display_create+0x18d/0x860 [nouveau] [] ? __cancel_work_timer+0x6d/0xc0 [] nouveau_display_create+0x3cb/0x670 [nouveau] [] nouveau_drm_load+0x26f/0x590 [nouveau] [] ? device_register+0x19/0x20 [] ? drm_sysfs_device_add+0x81/0xb0 [] drm_get_pci_dev+0x17e/0x2b0 [] ? __pci_set_master+0x26/0x80 [] nouveau_drm_probe+0x25a/0x2a0 [nouveau] [] local_pci_probe+0x46/0x80 [] pci_device_probe+0x101/0x110 [] driver_probe_device+0x76/0x240 [] __driver_attach+0xa3/0xb0 [] ? driver_probe_device+0x240/0x240 [] bus_for_each_dev+0x4d/0x90 [] driver_attach+0x19/0x20 [] bus_add_driver+0x1a0/0x270 [] ? 0xffffffffa023cfff [] driver_register+0x72/0x170 [] ? 0xffffffffa023cfff [] __pci_register_driver+0x5f/0x70 [] drm_pci_init+0x115/0x130 [] ? 0xffffffffa023cfff [] ? 0xffffffffa023cfff [] nouveau_drm_init+0x4d/0x1000 [nouveau] [] do_one_initcall+0x11a/0x170 [] load_module+0xe84/0x1470 [] ? in_lock_functions+0x20/0x20 [] ? trace_hardirqs_on_thunk+0x3a/0x3f [] sys_init_module+0xb7/0xe0 [] system_call_fastpath+0x1a/0x1f Reported-by: Arend van Spriel Reported-by: Peter Hurley Reported-by: Daniel J Blueman Signed-off-by: Maarten Lankhorst --- XXX: Add Cc: stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org if it applies to 3.7 ? diff --git a/drivers/gpu/drm/nouveau/core/core/engctx.c b/drivers/gpu/drm/nouveau/core/core/engctx.c index 84c71fa..4a0ab2b 100644 --- a/drivers/gpu/drm/nouveau/core/core/engctx.c +++ b/drivers/gpu/drm/nouveau/core/core/engctx.c @@ -31,12 +31,15 @@ #include static inline int -nouveau_engctx_exists(struct nouveau_object *parent, +nouveau_engctx_exists(struct nouveau_object *parent, u32 flags, struct nouveau_engine *engine, void **pobject) { struct nouveau_engctx *engctx; struct nouveau_object *parctx; + if ((flags & NVOBJ_FLAG_CREAT_EXCL) && !list_empty(&engine->contexts)) + return -EBUSY; + list_for_each_entry(engctx, &engine->contexts, head) { parctx = nv_pclass(nv_object(engctx), NV_PARENT_CLASS); if (parctx == parent) { @@ -67,7 +70,7 @@ nouveau_engctx_create_(struct nouveau_object *parent, * and reference it instead of creating a new one */ spin_lock_irqsave(&engine->lock, save); - ret = nouveau_engctx_exists(parent, engine, pobject); + ret = nouveau_engctx_exists(parent, flags, engine, pobject); spin_unlock_irqrestore(&engine->lock, save); if (ret) return ret; @@ -94,7 +97,7 @@ nouveau_engctx_create_(struct nouveau_object *parent, * it's not possible to allocate the object with it held. */ spin_lock_irqsave(&engine->lock, save); - ret = nouveau_engctx_exists(parent, engine, pobject); + ret = nouveau_engctx_exists(parent, flags, engine, pobject); if (ret) { spin_unlock_irqrestore(&engine->lock, save); nouveau_object_ref(NULL, &engctx); diff --git a/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c b/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c index ca1a7d7..1d3dcd0 100644 --- a/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c +++ b/drivers/gpu/drm/nouveau/core/engine/disp/nv50.c @@ -695,9 +695,8 @@ nv50_disp_data_ctor(struct nouveau_object *parent, struct nouveau_oclass *oclass, void *data, u32 size, struct nouveau_object **pobject) { - struct nv50_disp_priv *priv = (void *)engine; struct nouveau_engctx *ectx; - int ret = -EBUSY; + int ret; /* no context needed for channel objects... */ if (nv_mclass(parent) != NV_DEVICE_CLASS) { @@ -707,14 +706,10 @@ nv50_disp_data_ctor(struct nouveau_object *parent, } /* allocate display hardware to client */ - mutex_lock(&nv_subdev(priv)->mutex); - if (list_empty(&nv_engine(priv)->contexts)) { - ret = nouveau_engctx_create(parent, engine, oclass, NULL, - 0x10000, 0x10000, - NVOBJ_FLAG_HEAP, &ectx); - *pobject = nv_object(ectx); - } - mutex_unlock(&nv_subdev(priv)->mutex); + ret = nouveau_engctx_create(parent, engine, oclass, NULL, 0x10000, 0x10000, + NVOBJ_FLAG_HEAP | NVOBJ_FLAG_CREAT_EXCL, &ectx); + + *pobject = nv_object(ectx); return ret; } diff --git a/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h b/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h index b3b9ce4..9d47d51 100644 --- a/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h +++ b/drivers/gpu/drm/nouveau/core/include/core/gpuobj.h @@ -12,6 +12,7 @@ struct nouveau_vm; #define NVOBJ_FLAG_ZERO_ALLOC 0x00000001 #define NVOBJ_FLAG_ZERO_FREE 0x00000002 #define NVOBJ_FLAG_HEAP 0x00000004 +#define NVOBJ_FLAG_CREAT_EXCL 0x00000008 struct nouveau_gpuobj { struct nouveau_object base;