* [PATCH] staging: unisys: uislib: uisqueue.c: fixed sparse warning of context imbalance @ 2014-09-02 11:09 Sudip Mukherjee 2014-09-02 14:05 ` Dan Carpenter 0 siblings, 1 reply; 5+ messages in thread From: Sudip Mukherjee @ 2014-09-02 11:09 UTC (permalink / raw) To: Benjamin Romer, David Kershner, Greg Kroah-Hartman Cc: Sudip Mukherjee, sparmaintainer, devel, linux-kernel fixed sparse warning : context imbalance in 'do_locked_client_insert' different lock contexts for basic block spin_unlock_irqrestore is called at a later stage before returning from the function if locked is 1. Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> --- drivers/staging/unisys/uislib/uisqueue.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/unisys/uislib/uisqueue.c b/drivers/staging/unisys/uislib/uisqueue.c index f52bca1..63dbe7c 100644 --- a/drivers/staging/unisys/uislib/uisqueue.c +++ b/drivers/staging/unisys/uislib/uisqueue.c @@ -96,8 +96,6 @@ do_locked_client_insert(struct uisqueue_info *queueinfo, goto Away; ULTRA_CHANNEL_CLIENT_RELEASE_OS(queueinfo->chan, channelId, NULL); acquired = 0; - spin_unlock_irqrestore(lock, flags); - locked = 0; queueinfo->packets_sent++; -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: unisys: uislib: uisqueue.c: fixed sparse warning of context imbalance 2014-09-02 11:09 [PATCH] staging: unisys: uislib: uisqueue.c: fixed sparse warning of context imbalance Sudip Mukherjee @ 2014-09-02 14:05 ` Dan Carpenter 2014-09-02 15:14 ` Sudip Mukherjee 0 siblings, 1 reply; 5+ messages in thread From: Dan Carpenter @ 2014-09-02 14:05 UTC (permalink / raw) To: Sudip Mukherjee Cc: Benjamin Romer, David Kershner, Greg Kroah-Hartman, devel, sparmaintainer, linux-kernel On Tue, Sep 02, 2014 at 04:39:47PM +0530, Sudip Mukherjee wrote: > fixed sparse warning : context imbalance in 'do_locked_client_insert' > different lock contexts for basic block > > spin_unlock_irqrestore is called at a later stage before returning > from the function if locked is 1. > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> This doesn't match the email address you are using. Really, your patch isn't bad but I would prefer if you re-wrote this entire function because currently it is garbage. static u8 do_locked_client_insert(struct uisqueue_info *queueinfo, unsigned int whichqueue, void *pSignal, spinlock_t *lock, unsigned char issueInterruptIfEmpty, u64 interruptHandle, u8 *channelId) { unsigned long flags; unsigned char queueWasEmpty; spin_lock_irqsave(lock, flags); if (!ULTRA_CHANNEL_CLIENT_ACQUIRE_OS(queueinfo->chan, channelId, NULL)) goto unlock; queueWasEmpty = visor_signalqueue_empty(queueinfo->chan, whichqueue); if (!visor_signal_insert(queueinfo->chan, whichqueue, pSignal)) goto release; ULTRA_CHANNEL_CLIENT_RELEASE_OS(queueinfo->chan, channelId, NULL); spin_unlock_irqrestore(lock, flags); queueinfo->packets_sent++; return 1; release: ULTRA_CHANNEL_CLIENT_RELEASE_OS(queueinfo->chan, channelId, NULL); unlock: spin_unlock_irqrestore(lock, flags); return 0; } The queueWasEmpty variable is kind of silly. It should just be an int or maybe a bool if you are being pedantic but instead we very specifically set it to be an unsigned variable of the incorrect type. Also we don't use queueWasEmpty at all. I think we could delete it... The problem with the original code was that the error paths and the success paths were mixed together like spaghetti. If you separate them out and unwind in the proper order with normal label names then the code is easy to understand. regards, dan carpenter ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: unisys: uislib: uisqueue.c: fixed sparse warning of context imbalance 2014-09-02 14:05 ` Dan Carpenter @ 2014-09-02 15:14 ` Sudip Mukherjee 2014-09-02 15:23 ` Dan Carpenter 0 siblings, 1 reply; 5+ messages in thread From: Sudip Mukherjee @ 2014-09-02 15:14 UTC (permalink / raw) To: Dan Carpenter Cc: Benjamin Romer, David Kershner, Greg Kroah-Hartman, devel, sparmaintainer, linux-kernel On Tue, Sep 02, 2014 at 05:05:32PM +0300, Dan Carpenter wrote: > On Tue, Sep 02, 2014 at 04:39:47PM +0530, Sudip Mukherjee wrote: > > fixed sparse warning : context imbalance in 'do_locked_client_insert' > > different lock contexts for basic block > > > > spin_unlock_irqrestore is called at a later stage before returning > > from the function if locked is 1. > > > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> > > This doesn't match the email address you are using. > for all my patch the Signed-off email address is different from the From email address. I am not sure if I should do that, but since no one pointed it out so I guessed it is ok. > Really, your patch isn't bad but I would prefer if you re-wrote this > entire function because currently it is garbage. > > static u8 > do_locked_client_insert(struct uisqueue_info *queueinfo, > unsigned int whichqueue, > void *pSignal, > spinlock_t *lock, > unsigned char issueInterruptIfEmpty, > u64 interruptHandle, u8 *channelId) > { > unsigned long flags; > unsigned char queueWasEmpty; > > spin_lock_irqsave(lock, flags); > > if (!ULTRA_CHANNEL_CLIENT_ACQUIRE_OS(queueinfo->chan, channelId, NULL)) > goto unlock; > > queueWasEmpty = visor_signalqueue_empty(queueinfo->chan, whichqueue); > if (!visor_signal_insert(queueinfo->chan, whichqueue, pSignal)) > goto release; > ULTRA_CHANNEL_CLIENT_RELEASE_OS(queueinfo->chan, channelId, NULL); > spin_unlock_irqrestore(lock, flags); > > queueinfo->packets_sent++; > > return 1; > > release: > ULTRA_CHANNEL_CLIENT_RELEASE_OS(queueinfo->chan, channelId, NULL); > unlock: > spin_unlock_irqrestore(lock, flags); > > return 0; > } > > The queueWasEmpty variable is kind of silly. It should just be an int > or maybe a bool if you are being pedantic but instead we very > specifically set it to be an unsigned variable of the incorrect type. > Also we don't use queueWasEmpty at all. I think we could delete it... > > The problem with the original code was that the error paths and the > success paths were mixed together like spaghetti. If you separate them > out and unwind in the proper order with normal label names then the > code is easy to understand. > > regards, > dan carpenter > I will send a modified patch with the required changes. thanks sudip ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: unisys: uislib: uisqueue.c: fixed sparse warning of context imbalance 2014-09-02 15:14 ` Sudip Mukherjee @ 2014-09-02 15:23 ` Dan Carpenter 2014-09-02 16:58 ` Sudip Mukherjee 0 siblings, 1 reply; 5+ messages in thread From: Dan Carpenter @ 2014-09-02 15:23 UTC (permalink / raw) To: Sudip Mukherjee Cc: Benjamin Romer, David Kershner, Greg Kroah-Hartman, devel, sparmaintainer, linux-kernel On Tue, Sep 02, 2014 at 08:44:27PM +0530, Sudip Mukherjee wrote: > On Tue, Sep 02, 2014 at 05:05:32PM +0300, Dan Carpenter wrote: > > On Tue, Sep 02, 2014 at 04:39:47PM +0530, Sudip Mukherjee wrote: > > > fixed sparse warning : context imbalance in 'do_locked_client_insert' > > > different lock contexts for basic block > > > > > > spin_unlock_irqrestore is called at a later stage before returning > > > from the function if locked is 1. > > > > > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> > > > > This doesn't match the email address you are using. > > > > for all my patch the Signed-off email address is different from the From email address. > I am not sure if I should do that, but since no one pointed it out so I guessed it is ok. Sign-off-by are like signing a legal document that you didn't steal copyright code. We like to check that they match up with the email sending the patch. regards, dan carpenter ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: unisys: uislib: uisqueue.c: fixed sparse warning of context imbalance 2014-09-02 15:23 ` Dan Carpenter @ 2014-09-02 16:58 ` Sudip Mukherjee 0 siblings, 0 replies; 5+ messages in thread From: Sudip Mukherjee @ 2014-09-02 16:58 UTC (permalink / raw) To: Dan Carpenter Cc: Benjamin Romer, David Kershner, Greg Kroah-Hartman, devel, sparmaintainer, linux-kernel On Tue, Sep 02, 2014 at 06:23:27PM +0300, Dan Carpenter wrote: > On Tue, Sep 02, 2014 at 08:44:27PM +0530, Sudip Mukherjee wrote: > > On Tue, Sep 02, 2014 at 05:05:32PM +0300, Dan Carpenter wrote: > > > On Tue, Sep 02, 2014 at 04:39:47PM +0530, Sudip Mukherjee wrote: > > > > fixed sparse warning : context imbalance in 'do_locked_client_insert' > > > > different lock contexts for basic block > > > > > > > > spin_unlock_irqrestore is called at a later stage before returning > > > > from the function if locked is 1. > > > > > > > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> > > > > > > This doesn't match the email address you are using. > > > > > > > for all my patch the Signed-off email address is different from the From email address. > > I am not sure if I should do that, but since no one pointed it out so I guessed it is ok. > > Sign-off-by are like signing a legal document that you didn't steal > copyright code. We like to check that they match up with the email > sending the patch. > > regards, > dan carpenter > one last question regarding it . can i add both the email addresses as two Signed-off ? thanks sudip ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-09-02 16:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-09-02 11:09 [PATCH] staging: unisys: uislib: uisqueue.c: fixed sparse warning of context imbalance Sudip Mukherjee 2014-09-02 14:05 ` Dan Carpenter 2014-09-02 15:14 ` Sudip Mukherjee 2014-09-02 15:23 ` Dan Carpenter 2014-09-02 16:58 ` Sudip Mukherjee
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox