diff mbox

[2/3] omap: Do not BUG_ON(!spin_is_locked()) on UP

Message ID 20141221174355.4bdc8b49@neptune.home (mailing list archive)
State New, archived
Headers show

Commit Message

Bruno Prémont Dec. 21, 2014, 4:43 p.m. UTC
On !SMP systems spinlocks do not exist. Thus checking of they
are active will always fail.

Use
  assert_spin_locked(lock);
instead of
  BUG_ON(!spin_is_locked(lock));
to not BUG() on all UP systems.

Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
---
 drivers/gpu/drm/omapdrm/omap_irq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Rob Clark Dec. 22, 2014, 1:46 p.m. UTC | #1
On Sun, Dec 21, 2014 at 11:43 AM, Bruno Prémont
<bonbons@linux-vserver.org> wrote:
> On !SMP systems spinlocks do not exist. Thus checking of they
> are active will always fail.
>
> Use
>   assert_spin_locked(lock);
> instead of
>   BUG_ON(!spin_is_locked(lock));
> to not BUG() on all UP systems.
>
> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
> ---
>  drivers/gpu/drm/omapdrm/omap_irq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c
> index f035d2b..6ca9253 100644
> --- a/drivers/gpu/drm/omapdrm/omap_irq.c
> +++ b/drivers/gpu/drm/omapdrm/omap_irq.c
> @@ -34,7 +34,7 @@ static void omap_irq_update(struct drm_device *dev)
>         struct omap_drm_irq *irq;
>         uint32_t irqmask = priv->vblank_mask;
>
> -       BUG_ON(!spin_is_locked(&list_lock));
> +       assert_spin_locked(&list_lock));

btw, one too many ')' there...  I've fixed up the same issue w/ msm
patch as I applied it

BR,
-R


>
>         list_for_each_entry(irq, &priv->irq_list, node)
>                 irqmask |= irq->irqmask;
> --
> 1.8.1.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Bruno Prémont Dec. 22, 2014, 2:20 p.m. UTC | #2
On Mon, 22 Dec 2014 08:46:48 -0500 Rob Clark wrote:
> On Sun, Dec 21, 2014 at 11:43 AM, Bruno Prémont wrote:
> > On !SMP systems spinlocks do not exist. Thus checking of they
> > are active will always fail.
> >
> > Use
> >   assert_spin_locked(lock);
> > instead of
> >   BUG_ON(!spin_is_locked(lock));
> > to not BUG() on all UP systems.
> >
> > Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
> > ---
> >  drivers/gpu/drm/omapdrm/omap_irq.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c
> > index f035d2b..6ca9253 100644
> > --- a/drivers/gpu/drm/omapdrm/omap_irq.c
> > +++ b/drivers/gpu/drm/omapdrm/omap_irq.c
> > @@ -34,7 +34,7 @@ static void omap_irq_update(struct drm_device *dev)
> >         struct omap_drm_irq *irq;
> >         uint32_t irqmask = priv->vblank_mask;
> >
> > -       BUG_ON(!spin_is_locked(&list_lock));
> > +       assert_spin_locked(&list_lock));
> 
> btw, one too many ')' there...  I've fixed up the same issue w/ msm
> patch as I applied it

Oops.

I got it right for the nouveau patch, so I was probably too quick
doing the manual replace for the other occurrences in drivers/gpu/.


I didn't check how far back these BUG_ON() go though they might be worth
applying to stable trees as well (could be the assert_spin_locked()
macro appeared only after the introduction of some of these BUG_ON()s though).

I've hit them on nouveau on 3.18 and 3.19-rc1.

Bruno

> BR,
> -R
> 
> 
> >
> >         list_for_each_entry(irq, &priv->irq_list, node)
> >                 irqmask |= irq->irqmask;
> > --
> > 1.8.1.5
Ben Skeggs Dec. 22, 2014, 2:49 p.m. UTC | #3
On 23 Dec 2014 00:27, "Bruno Prémont" <bonbons@linux-vserver.org> wrote:
>
> On Mon, 22 Dec 2014 08:46:48 -0500 Rob Clark wrote:
> > On Sun, Dec 21, 2014 at 11:43 AM, Bruno Prémont wrote:
> > > On !SMP systems spinlocks do not exist. Thus checking of they
> > > are active will always fail.
> > >
> > > Use
> > >   assert_spin_locked(lock);
> > > instead of
> > >   BUG_ON(!spin_is_locked(lock));
> > > to not BUG() on all UP systems.
> > >
> > > Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
> > > ---
> > >  drivers/gpu/drm/omapdrm/omap_irq.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c
b/drivers/gpu/drm/omapdrm/omap_irq.c
> > > index f035d2b..6ca9253 100644
> > > --- a/drivers/gpu/drm/omapdrm/omap_irq.c
> > > +++ b/drivers/gpu/drm/omapdrm/omap_irq.c
> > > @@ -34,7 +34,7 @@ static void omap_irq_update(struct drm_device *dev)
> > >         struct omap_drm_irq *irq;
> > >         uint32_t irqmask = priv->vblank_mask;
> > >
> > > -       BUG_ON(!spin_is_locked(&list_lock));
> > > +       assert_spin_locked(&list_lock));
> >
> > btw, one too many ')' there...  I've fixed up the same issue w/ msm
> > patch as I applied it
>
> Oops.
>
> I got it right for the nouveau patch, so I was probably too quick
> doing the manual replace for the other occurrences in drivers/gpu/.
>
>
> I didn't check how far back these BUG_ON() go though they might be worth
> applying to stable trees as well (could be the assert_spin_locked()
> macro appeared only after the introduction of some of these BUG_ON()s
though).
>
> I've hit them on nouveau on 3.18 and 3.19-rc1.
I've taken the nouveau patch locally fwiw, and forwarded it on to Dave for
- fixes.

Ben.
>
> Bruno
>
> > BR,
> > -R
> >
> >
> > >
> > >         list_for_each_entry(irq, &priv->irq_list, node)
> > >                 irqmask |= irq->irqmask;
> > > --
> > > 1.8.1.5
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c
index f035d2b..6ca9253 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.c
+++ b/drivers/gpu/drm/omapdrm/omap_irq.c
@@ -34,7 +34,7 @@  static void omap_irq_update(struct drm_device *dev)
 	struct omap_drm_irq *irq;
 	uint32_t irqmask = priv->vblank_mask;
 
-	BUG_ON(!spin_is_locked(&list_lock));
+	assert_spin_locked(&list_lock));
 
 	list_for_each_entry(irq, &priv->irq_list, node)
 		irqmask |= irq->irqmask;