On Wed, Oct 19, 2016 at 05:00:53PM +0000, Sell, Timothy C wrote:
> On Wednesday, October 19, 2016 7:31 AM, Cathal Mullaney wrote:
> > This patch makes locking in visorchannel_signalempty statically deterministic.
> > As a result this patch fixes the sparse warning:
> > Context imbalance in 'visorchannel_signalempty' - different lock contexts for
> > basic block.
> >
> > The logic of the locking code doesn't change but the layout of the original
> > code is "frowned upon"
> > according to mails on sparse context checking.
> > Refactoring removes the warning and makes the code more readable.
> >
> > Signed-off-by: Cathal Mullaney <chuckleberryfinn@xxxxxxxxx>
> > ---
> > drivers/staging/unisys/visorbus/visorchannel.c | 26 +++++++++++++++++---
> > ------
> > 1 file changed, 17 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/staging/unisys/visorbus/visorchannel.c
> > b/drivers/staging/unisys/visorbus/visorchannel.c
> > index a1381eb..1eea5d8 100644
> > --- a/drivers/staging/unisys/visorbus/visorchannel.c
> > +++ b/drivers/staging/unisys/visorbus/visorchannel.c
> > @@ -300,22 +300,30 @@
> > ---
> > bool
> > visorchannel_signalempty(struct visorchannel *channel, u32 queue)
> > {
> > unsigned long flags = 0;
> > - struct signal_queue_header sig_hdr;
> > bool rc = false;
>
> It appears as if you no longer need to initialize 'rc' above.
>
> Although this is NOT caused by your patch, it also looks like 'flags'
> is being unnecessarily initialized. You may want to fix that too
> while you're in the neighborhood.
>
> (Kernel folks seem to frown on unnecessary variable initializations.)
>
> >
> > - if (channel->needs_lock)
> > - spin_lock_irqsave(&channel->remove_lock, flags);
> > + if (!channel->needs_lock)
> > + return queue_empty(channel, queue);
> >
> > - if (sig_read_header(channel, queue, &sig_hdr))
> > - rc = true;
> > - if (sig_hdr.head == sig_hdr.tail)
> > - rc = true;
> > - if (channel->needs_lock)
> > - spin_unlock_irqrestore(&channel->remove_lock, flags);
> > + spin_lock_irqsave(&channel->remove_lock, flags);
> > + rc = queue_empty(channel, queue);
> > + spin_unlock_irqrestore(&channel->remove_lock, flags);
> >
> > return rc;
> > }
> > --
> > 2.7.4
>
> Besides that, your patch looks good to me. Thanks.
>
> - Tim Sell
>
Thanks for your feedback Tim.
Submitted v2 with the suggested updates.
Thanks again.
Kind regards,
Cathal
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
