2014-10-10 13:09:53

by Sudip Mukherjee

Subject: [PATCH] usb: dwc2: gadget: sparse warning of context imbalance

sparse was giving the following warning:
warning: context imbalance in 's3c_hsotg_ep_enable'
- different lock contexts for basic block

we were returning ENOMEM while still holding the spinlock.
The sparse warning was fixed by releasing the spinlock before return.

Signed-off-by: Sudip Mukherjee <[email protected]>
---
drivers/usb/dwc2/gadget.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 7b5856f..046e90d 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2561,8 +2561,10 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
hs_ep->fifo_size = val;
break;
}
- if (i == 8)
+ if (i == 8) {
+ spin_unlock_irqrestore(&hsotg->lock, flags);
return -ENOMEM;
+ }
}

/* for non control endpoints, set PID to D0 */
--
1.8.1.2

2014-10-14 20:58:23

by Paul Zimmerman

Subject: RE: [PATCH] usb: dwc2: gadget: sparse warning of context imbalance

> From: Sudip Mukherjee [mailto:[email protected]]
> Sent: Friday, October 10, 2014 6:10 AM
>
> sparse was giving the following warning:
> warning: context imbalance in 's3c_hsotg_ep_enable'
> - different lock contexts for basic block
>
> we were returning ENOMEM while still holding the spinlock.
> The sparse warning was fixed by releasing the spinlock before return.
>
> Signed-off-by: Sudip Mukherjee <[email protected]>
> ---
> drivers/usb/dwc2/gadget.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 7b5856f..046e90d 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -2561,8 +2561,10 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
> hs_ep->fifo_size = val;
> break;
> }
> - if (i == 8)
> + if (i == 8) {
> + spin_unlock_irqrestore(&hsotg->lock, flags);
> return -ENOMEM;
> + }
> }
>
> /* for non control endpoints, set PID to D0 */

For a long function like this, I'd rather keep a single return point at
the end. Something like this:

}
- if (i == 8)
- return -ENOMEM;
+ if (i == 8) {
+ ret = -ENOMEM;
+ goto out;
+ }
}
...

+out:
spin_unlock_irqrestore(&hsotg->lock, flags);
return ret;
}

Care to respin the patch like that?

--
Paul

2014-10-15 09:18:11

by Sudip Mukherjee

Subject: Re: [PATCH] usb: dwc2: gadget: sparse warning of context imbalance

On Tue, Oct 14, 2014 at 08:55:55PM +0000, Paul Zimmerman wrote:
> > From: Sudip Mukherjee [mailto:[email protected]]
> > Sent: Friday, October 10, 2014 6:10 AM
> >
> > sparse was giving the following warning:
> > warning: context imbalance in 's3c_hsotg_ep_enable'
> > - different lock contexts for basic block
> >
> > we were returning ENOMEM while still holding the spinlock.
> > The sparse warning was fixed by releasing the spinlock before return.
> >
> > Signed-off-by: Sudip Mukherjee <[email protected]>
> > ---
> > drivers/usb/dwc2/gadget.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> > index 7b5856f..046e90d 100644
> > --- a/drivers/usb/dwc2/gadget.c
> > +++ b/drivers/usb/dwc2/gadget.c
> > @@ -2561,8 +2561,10 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
> > hs_ep->fifo_size = val;
> > break;
> > }
> > - if (i == 8)
> > + if (i == 8) {
> > + spin_unlock_irqrestore(&hsotg->lock, flags);
> > return -ENOMEM;
> > + }
> > }
> >
> > /* for non control endpoints, set PID to D0 */
>
> For a long function like this, I'd rather keep a single return point at
> the end. Something like this:
>
> }
> - if (i == 8)
> - return -ENOMEM;
> + if (i == 8) {
> + ret = -ENOMEM;
> + goto out;
> + }
> }
> ...
>
> +out:
> spin_unlock_irqrestore(&hsotg->lock, flags);
> return ret;
> }
>
> Care to respin the patch like that?
>
sure , modified patch will be in your inbox in a short time.

thanks
sudip
> --
> Paul
>