On Sun, 7 Feb 2010 02:11:20 +0100 Maarten Maathuis <[email protected]> wrote:
> - 'joi' on irc pointed out that this triggers a BUG_ON, because > kzalloc could sleep. > - The irq handler should restore the value NV03_PFIFO_CACHES, but > still it's better if this stuff doesn't happen in the middle of > fifo create context. I see no reason in spin locking pgraph > create context, it isn't activated at that stage. > - Move and rename the lock after some discussion with 'joi', even > better naming suggestions are appreciated. > > Signed-off-by: Maarten Maathuis <[email protected]> > --- > drivers/gpu/drm/nouveau/nouveau_channel.c | 9 ++------- > drivers/gpu/drm/nouveau/nouveau_drv.h | 4 +++- > drivers/gpu/drm/nouveau/nouveau_irq.c | 4 ++-- > drivers/gpu/drm/nouveau/nouveau_state.c | 2 +- > drivers/gpu/drm/nouveau/nv04_fifo.c | 5 +++++ > drivers/gpu/drm/nouveau/nv40_fifo.c | 5 +++++ > drivers/gpu/drm/nouveau/nv50_fifo.c | 4 ++++ > 7 files changed, 22 insertions(+), 11 deletions(-) <...> > diff --git a/drivers/gpu/drm/nouveau/nv50_fifo.c > b/drivers/gpu/drm/nouveau/nv50_fifo.c index 204a79f..983e43b > 100644 --- a/drivers/gpu/drm/nouveau/nv50_fifo.c > +++ b/drivers/gpu/drm/nouveau/nv50_fifo.c > @@ -243,6 +243,7 @@ nv50_fifo_create_context(struct > nouveau_channel *chan) struct drm_device *dev = chan->dev; > struct drm_nouveau_private *dev_priv = dev->dev_private; > struct nouveau_gpuobj *ramfc = NULL; > + unsigned long flags; > int ret; > > NV_DEBUG(dev, "ch%d\n", chan->id); > @@ -278,6 +279,8 @@ nv50_fifo_create_context(struct > nouveau_channel *chan) return ret; > } > > + spin_lock_irqsave(&dev_priv->context_switch_lock, flags); > + > dev_priv->engine.instmem.prepare_access(dev, true); > > nv_wo32(dev, ramfc, 0x08/4, chan->pushbuf_base); > @@ -310,6 +313,7 @@ nv50_fifo_create_context(struct > nouveau_channel *chan) return ret; > } > > + spin_unlock_irqrestore(&dev_priv->context_switch_lock, > flags); return 0; > } After this patch, sparse complains: drivers/gpu/drm/nouveau/nv50_fifo.c:241:1: warning: context imbalance in 'nv50_fifo_create_context' - different lock contexts for basic block Near the end of the function, the failure exit path does not unlock the spinlock. Btw. before this patch nouveau_channel_alloc() contains two cases of failure path not releasing the spinlock, plus, under the spinlock, a call to a function that re-locks the spinlock, hence hangs. Sparse does warn about exiting a function without releasing a spinlock in every path, i.e., inconsistent lock behaviour. Thanks. -- Pekka Paalanen http://www.iki.fi/pq/ _______________________________________________ Nouveau mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/nouveau
