* drivers/spi/spi.c:1160:3-9: preceding lock on line 1153
@ 2016-08-09 16:19 Julia Lawall
2016-08-09 16:31 ` Mark Brown
0 siblings, 1 reply; 4+ messages in thread
From: Julia Lawall @ 2016-08-09 16:19 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, kbuild-all
Looks worth checking.
julia
---------- Forwarded message ----------
Date: Wed, 10 Aug 2016 00:17:07 +0800
From: kbuild test robot <fengguang.wu@intel.com>
To: kbuild@01.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Subject: drivers/spi/spi.c:1160:3-9: preceding lock on line 1153
CC: kbuild-all@01.org
CC: linux-kernel@vger.kernel.org
TO: Mark Brown <broonie@kernel.org>
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 81abf2525827b29839a78fd55ab0699f033c41a5
commit: ef4d96ec4ad947360f48677b6007a4c77953b090 spi: Split bus and I/O locking
date: 2 weeks ago
:::::: branch date: 18 hours ago
:::::: commit date: 2 weeks ago
>> drivers/spi/spi.c:1160:3-9: preceding lock on line 1153
git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git remote update linus
git checkout ef4d96ec4ad947360f48677b6007a4c77953b090
vim +1160 drivers/spi/spi.c
ffbbdd213 Linus Walleij 2012-02-22 1147 if (master->busy)
ffbbdd213 Linus Walleij 2012-02-22 1148 was_busy = true;
ffbbdd213 Linus Walleij 2012-02-22 1149 else
ffbbdd213 Linus Walleij 2012-02-22 1150 master->busy = true;
ffbbdd213 Linus Walleij 2012-02-22 1151 spin_unlock_irqrestore(&master->queue_lock, flags);
ffbbdd213 Linus Walleij 2012-02-22 1152
ef4d96ec4 Mark Brown 2016-07-21 @1153 mutex_lock(&master->io_mutex);
ef4d96ec4 Mark Brown 2016-07-21 1154
49834de23 Mark Brown 2013-07-28 1155 if (!was_busy && master->auto_runtime_pm) {
49834de23 Mark Brown 2013-07-28 1156 ret = pm_runtime_get_sync(master->dev.parent);
49834de23 Mark Brown 2013-07-28 1157 if (ret < 0) {
49834de23 Mark Brown 2013-07-28 1158 dev_err(&master->dev, "Failed to power device: %d\n",
49834de23 Mark Brown 2013-07-28 1159 ret);
49834de23 Mark Brown 2013-07-28 @1160 return;
49834de23 Mark Brown 2013-07-28 1161 }
49834de23 Mark Brown 2013-07-28 1162 }
49834de23 Mark Brown 2013-07-28 1163
:::::: The code at line 1160 was first introduced by commit
:::::: 49834de234f3cf592c3d242c889ca603db8e7050 spi: Provide core support for runtime PM during transfers
:::::: TO: Mark Brown <broonie@linaro.org>
:::::: CC: Mark Brown <broonie@linaro.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: drivers/spi/spi.c:1160:3-9: preceding lock on line 1153
2016-08-09 16:19 drivers/spi/spi.c:1160:3-9: preceding lock on line 1153 Julia Lawall
@ 2016-08-09 16:31 ` Mark Brown
2016-08-09 16:35 ` Julia Lawall
0 siblings, 1 reply; 4+ messages in thread
From: Mark Brown @ 2016-08-09 16:31 UTC (permalink / raw)
To: Julia Lawall; +Cc: linux-kernel, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2705 bytes --]
On Tue, Aug 09, 2016 at 06:19:13PM +0200, Julia Lawall wrote:
> Looks worth checking.
I don't really have any idea what this is trying to tell me, sorry.
>
> julia
>
> ---------- Forwarded message ----------
> Date: Wed, 10 Aug 2016 00:17:07 +0800
> From: kbuild test robot <fengguang.wu@intel.com>
> To: kbuild@01.org
> Cc: Julia Lawall <julia.lawall@lip6.fr>
> Subject: drivers/spi/spi.c:1160:3-9: preceding lock on line 1153
>
> CC: kbuild-all@01.org
> CC: linux-kernel@vger.kernel.org
> TO: Mark Brown <broonie@kernel.org>
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 81abf2525827b29839a78fd55ab0699f033c41a5
> commit: ef4d96ec4ad947360f48677b6007a4c77953b090 spi: Split bus and I/O locking
> date: 2 weeks ago
> :::::: branch date: 18 hours ago
> :::::: commit date: 2 weeks ago
>
> >> drivers/spi/spi.c:1160:3-9: preceding lock on line 1153
>
> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git remote update linus
> git checkout ef4d96ec4ad947360f48677b6007a4c77953b090
> vim +1160 drivers/spi/spi.c
>
> ffbbdd213 Linus Walleij 2012-02-22 1147 if (master->busy)
> ffbbdd213 Linus Walleij 2012-02-22 1148 was_busy = true;
> ffbbdd213 Linus Walleij 2012-02-22 1149 else
> ffbbdd213 Linus Walleij 2012-02-22 1150 master->busy = true;
> ffbbdd213 Linus Walleij 2012-02-22 1151 spin_unlock_irqrestore(&master->queue_lock, flags);
> ffbbdd213 Linus Walleij 2012-02-22 1152
> ef4d96ec4 Mark Brown 2016-07-21 @1153 mutex_lock(&master->io_mutex);
> ef4d96ec4 Mark Brown 2016-07-21 1154
> 49834de23 Mark Brown 2013-07-28 1155 if (!was_busy && master->auto_runtime_pm) {
> 49834de23 Mark Brown 2013-07-28 1156 ret = pm_runtime_get_sync(master->dev.parent);
> 49834de23 Mark Brown 2013-07-28 1157 if (ret < 0) {
> 49834de23 Mark Brown 2013-07-28 1158 dev_err(&master->dev, "Failed to power device: %d\n",
> 49834de23 Mark Brown 2013-07-28 1159 ret);
> 49834de23 Mark Brown 2013-07-28 @1160 return;
> 49834de23 Mark Brown 2013-07-28 1161 }
> 49834de23 Mark Brown 2013-07-28 1162 }
> 49834de23 Mark Brown 2013-07-28 1163
>
> :::::: The code at line 1160 was first introduced by commit
> :::::: 49834de234f3cf592c3d242c889ca603db8e7050 spi: Provide core support for runtime PM during transfers
>
> :::::: TO: Mark Brown <broonie@linaro.org>
> :::::: CC: Mark Brown <broonie@linaro.org>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: drivers/spi/spi.c:1160:3-9: preceding lock on line 1153
2016-08-09 16:31 ` Mark Brown
@ 2016-08-09 16:35 ` Julia Lawall
2016-08-09 16:43 ` Mark Brown
0 siblings, 1 reply; 4+ messages in thread
From: Julia Lawall @ 2016-08-09 16:35 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-kernel, kbuild-all
On Tue, 9 Aug 2016, Mark Brown wrote:
> On Tue, Aug 09, 2016 at 06:19:13PM +0200, Julia Lawall wrote:
>
> > Looks worth checking.
>
> I don't really have any idea what this is trying to tell me, sorry.
The important lines are the ones with the @. Line 1153 takes a lock and
in line 1160 there is a return in error handling code with the lock still
held. The semantic patch that reported this should also only give a
report if there is some patch out of the function that releases the lock.
Overall, it seems unusual to keep the lock in an error case.
julia
>
> >
> > julia
> >
> > ---------- Forwarded message ----------
> > Date: Wed, 10 Aug 2016 00:17:07 +0800
> > From: kbuild test robot <fengguang.wu@intel.com>
> > To: kbuild@01.org
> > Cc: Julia Lawall <julia.lawall@lip6.fr>
> > Subject: drivers/spi/spi.c:1160:3-9: preceding lock on line 1153
> >
> > CC: kbuild-all@01.org
> > CC: linux-kernel@vger.kernel.org
> > TO: Mark Brown <broonie@kernel.org>
> >
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head: 81abf2525827b29839a78fd55ab0699f033c41a5
> > commit: ef4d96ec4ad947360f48677b6007a4c77953b090 spi: Split bus and I/O locking
> > date: 2 weeks ago
> > :::::: branch date: 18 hours ago
> > :::::: commit date: 2 weeks ago
> >
> > >> drivers/spi/spi.c:1160:3-9: preceding lock on line 1153
> >
> > git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > git remote update linus
> > git checkout ef4d96ec4ad947360f48677b6007a4c77953b090
> > vim +1160 drivers/spi/spi.c
> >
> > ffbbdd213 Linus Walleij 2012-02-22 1147 if (master->busy)
> > ffbbdd213 Linus Walleij 2012-02-22 1148 was_busy = true;
> > ffbbdd213 Linus Walleij 2012-02-22 1149 else
> > ffbbdd213 Linus Walleij 2012-02-22 1150 master->busy = true;
> > ffbbdd213 Linus Walleij 2012-02-22 1151 spin_unlock_irqrestore(&master->queue_lock, flags);
> > ffbbdd213 Linus Walleij 2012-02-22 1152
> > ef4d96ec4 Mark Brown 2016-07-21 @1153 mutex_lock(&master->io_mutex);
> > ef4d96ec4 Mark Brown 2016-07-21 1154
> > 49834de23 Mark Brown 2013-07-28 1155 if (!was_busy && master->auto_runtime_pm) {
> > 49834de23 Mark Brown 2013-07-28 1156 ret = pm_runtime_get_sync(master->dev.parent);
> > 49834de23 Mark Brown 2013-07-28 1157 if (ret < 0) {
> > 49834de23 Mark Brown 2013-07-28 1158 dev_err(&master->dev, "Failed to power device: %d\n",
> > 49834de23 Mark Brown 2013-07-28 1159 ret);
> > 49834de23 Mark Brown 2013-07-28 @1160 return;
> > 49834de23 Mark Brown 2013-07-28 1161 }
> > 49834de23 Mark Brown 2013-07-28 1162 }
> > 49834de23 Mark Brown 2013-07-28 1163
> >
> > :::::: The code at line 1160 was first introduced by commit
> > :::::: 49834de234f3cf592c3d242c889ca603db8e7050 spi: Provide core support for runtime PM during transfers
> >
> > :::::: TO: Mark Brown <broonie@linaro.org>
> > :::::: CC: Mark Brown <broonie@linaro.org>
> >
> > ---
> > 0-DAY kernel test infrastructure Open Source Technology Center
> > https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: drivers/spi/spi.c:1160:3-9: preceding lock on line 1153
2016-08-09 16:35 ` Julia Lawall
@ 2016-08-09 16:43 ` Mark Brown
0 siblings, 0 replies; 4+ messages in thread
From: Mark Brown @ 2016-08-09 16:43 UTC (permalink / raw)
To: Julia Lawall; +Cc: linux-kernel, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 879 bytes --]
On Tue, Aug 09, 2016 at 06:35:06PM +0200, Julia Lawall wrote:
> On Tue, 9 Aug 2016, Mark Brown wrote:
> > On Tue, Aug 09, 2016 at 06:19:13PM +0200, Julia Lawall wrote:
> > > Looks worth checking.
> > I don't really have any idea what this is trying to tell me, sorry.
> The important lines are the ones with the @. Line 1153 takes a lock and
> in line 1160 there is a return in error handling code with the lock still
> held. The semantic patch that reported this should also only give a
> report if there is some patch out of the function that releases the lock.
> Overall, it seems unusual to keep the lock in an error case.
I'm not sure how I'm supposed to identify that from the message to be
honest - I'd expect something that more directly referenced both lines
and ideally said something like "lock held when returning". But yes, it
does look like an issue thanks.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-08-09 16:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-09 16:19 drivers/spi/spi.c:1160:3-9: preceding lock on line 1153 Julia Lawall
2016-08-09 16:31 ` Mark Brown
2016-08-09 16:35 ` Julia Lawall
2016-08-09 16:43 ` Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox