* Re: Warnings from PM QoS plist usage
[not found] <20100714093510.GC5933@sirena.org.uk>
@ 2010-07-14 9:35 ` Mark Brown
2010-07-15 6:12 ` James Bottomley
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2010-07-14 9:35 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-pm, mark gross
On Wed, Jul 14, 2010 at 10:35:10AM +0100, Mark Brown wrote:
CCing in the mailing list like I meant to on the original post, sorry:
> When running with 'pm_qos: Reimplement using plists' and
> CONFIG_DEBUG_PI_LIST enabled ALSA (and presumably other PM QoS users)
> generate tracebacks like this whenever they use PM QoS:
>
> [ 118.250000] ------------[ cut here ]------------
> [ 118.250000] WARNING: at lib/plist.c:57 plist_check_head+0x2c/0x44()
> [ 118.250000] Modules linked in: snd_soc_fnord snd_soc_s3c64xx_i2s snd_soc_s3c_6
> [ 118.250000] [<c0034aac>] (unwind_backtrace+0x0/0xec) from [<c004a8f0>] (warn_)
> [ 118.250000] [<c004a8f0>] (warn_slowpath_common+0x4c/0x7c) from [<c004a93c>] ()
> [ 118.250000] [<c004a93c>] (warn_slowpath_null+0x1c/0x24) from [<c0187c2c>] (pl)
> [ 118.250000] [<c0187c2c>] (plist_check_head+0x2c/0x44) from [<c0187cf0>] (plis)
> [ 118.250000] [<c0187cf0>] (plist_add+0x18/0xbc) from [<c006755c>] (update_targ)
> [ 118.250000] [<c006755c>] (update_target+0xb8/0x124) from [<c025336c>] (snd_pc)
> [ 118.250000] [<c025336c>] (snd_pcm_hw_params+0x2cc/0x310) from [<c0253780>] (s)
> [ 118.250000] [<c0253780>] (snd_pcm_common_ioctl1+0x1f8/0x10ac) from [<c0254e20)
> [ 118.250000] [<c0254e20>] (snd_pcm_playback_ioctl1+0x3d8/0x3fc) from [<c00c7e5)
> [ 118.250000] [<c00c7e50>] (vfs_ioctl+0x2c/0x70) from [<c00c8534>] (do_vfs_ioct)
> [ 118.250000] [<c00c8534>] (do_vfs_ioctl+0x4d0/0x524) from [<c00c85c0>] (sys_io)
> [ 118.250000] [<c00c85c0>] (sys_ioctl+0x38/0x5c) from [<c002eec0>] (ret_fast_sy)
> [ 118.250000] ---[ end trace 810ec758dcd5bc5a ]---
> This is because the plists introduced by the above commit are being
> initialised with the _PLIST_HEAD_INIT() rather than PLIST_HEAD_INIT()
> and don't have the associated locks which the debug code relies on.
>
> I've not looked at the PM QoS code at all so I don't know what the
> appropriate place to fix this is - do we perhaps want to fix the debug
> code to cope with no locks? Looking at the plist.h code it seems fairly
> clear that there's currently no expectation that users should use
> _PLIST_INIT_HEAD() directly.
--
"You grabbed my hand and we fell into it, like a daydream - or a fever."
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Warnings from PM QoS plist usage
2010-07-14 9:35 ` Warnings from PM QoS plist usage Mark Brown
@ 2010-07-15 6:12 ` James Bottomley
2010-07-15 16:28 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: James Bottomley @ 2010-07-15 6:12 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-pm, mark gross
On Wed, 2010-07-14 at 10:35 +0100, Mark Brown wrote:
> On Wed, Jul 14, 2010 at 10:35:10AM +0100, Mark Brown wrote:
>
> CCing in the mailing list like I meant to on the original post, sorry:
>
> > When running with 'pm_qos: Reimplement using plists' and
> > CONFIG_DEBUG_PI_LIST enabled ALSA (and presumably other PM QoS users)
> > generate tracebacks like this whenever they use PM QoS:
> >
> > [ 118.250000] ------------[ cut here ]------------
> > [ 118.250000] WARNING: at lib/plist.c:57 plist_check_head+0x2c/0x44()
> > [ 118.250000] Modules linked in: snd_soc_fnord snd_soc_s3c64xx_i2s snd_soc_s3c_6
> > [ 118.250000] [<c0034aac>] (unwind_backtrace+0x0/0xec) from [<c004a8f0>] (warn_)
> > [ 118.250000] [<c004a8f0>] (warn_slowpath_common+0x4c/0x7c) from [<c004a93c>] ()
> > [ 118.250000] [<c004a93c>] (warn_slowpath_null+0x1c/0x24) from [<c0187c2c>] (pl)
> > [ 118.250000] [<c0187c2c>] (plist_check_head+0x2c/0x44) from [<c0187cf0>] (plis)
> > [ 118.250000] [<c0187cf0>] (plist_add+0x18/0xbc) from [<c006755c>] (update_targ)
> > [ 118.250000] [<c006755c>] (update_target+0xb8/0x124) from [<c025336c>] (snd_pc)
> > [ 118.250000] [<c025336c>] (snd_pcm_hw_params+0x2cc/0x310) from [<c0253780>] (s)
> > [ 118.250000] [<c0253780>] (snd_pcm_common_ioctl1+0x1f8/0x10ac) from [<c0254e20)
> > [ 118.250000] [<c0254e20>] (snd_pcm_playback_ioctl1+0x3d8/0x3fc) from [<c00c7e5)
> > [ 118.250000] [<c00c7e50>] (vfs_ioctl+0x2c/0x70) from [<c00c8534>] (do_vfs_ioct)
> > [ 118.250000] [<c00c8534>] (do_vfs_ioctl+0x4d0/0x524) from [<c00c85c0>] (sys_io)
> > [ 118.250000] [<c00c85c0>] (sys_ioctl+0x38/0x5c) from [<c002eec0>] (ret_fast_sy)
> > [ 118.250000] ---[ end trace 810ec758dcd5bc5a ]---
> > This is because the plists introduced by the above commit are being
> > initialised with the _PLIST_HEAD_INIT() rather than PLIST_HEAD_INIT()
> > and don't have the associated locks which the debug code relies on.
> >
> > I've not looked at the PM QoS code at all so I don't know what the
> > appropriate place to fix this is - do we perhaps want to fix the debug
> > code to cope with no locks? Looking at the plist.h code it seems fairly
> > clear that there's currently no expectation that users should use
> > _PLIST_INIT_HEAD() directly.
Sorry ... I just wasn't sure what it wanted without reading through the
plist code more ... and also that debug item is hidden by
CONFIG_DEBUG_RT_MUTEXES, which is why I couldn't enable it.
This should fix the warning.
James
---
diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
index bff4053..7971678 100644
--- a/kernel/pm_qos_params.c
+++ b/kernel/pm_qos_params.c
@@ -62,10 +62,12 @@ struct pm_qos_object {
enum pm_qos_type type;
};
+static DEFINE_SPINLOCK(pm_qos_lock);
+
static struct pm_qos_object null_pm_qos;
static BLOCKING_NOTIFIER_HEAD(cpu_dma_lat_notifier);
static struct pm_qos_object cpu_dma_pm_qos = {
- .requests = {_PLIST_HEAD_INIT(cpu_dma_pm_qos.requests)},
+ .requests = PLIST_HEAD_INIT(cpu_dma_pm_qos.requests, pm_qos_lock),
.notifiers = &cpu_dma_lat_notifier,
.name = "cpu_dma_latency",
.default_value = 2000 * USEC_PER_SEC,
@@ -74,7 +76,7 @@ static struct pm_qos_object cpu_dma_pm_qos = {
static BLOCKING_NOTIFIER_HEAD(network_lat_notifier);
static struct pm_qos_object network_lat_pm_qos = {
- .requests = {_PLIST_HEAD_INIT(network_lat_pm_qos.requests)},
+ .requests = PLIST_HEAD_INIT(network_lat_pm_qos.requests, pm_qos_lock),
.notifiers = &network_lat_notifier,
.name = "network_latency",
.default_value = 2000 * USEC_PER_SEC,
@@ -84,7 +86,7 @@ static struct pm_qos_object network_lat_pm_qos = {
static BLOCKING_NOTIFIER_HEAD(network_throughput_notifier);
static struct pm_qos_object network_throughput_pm_qos = {
- .requests = {_PLIST_HEAD_INIT(network_throughput_pm_qos.requests)},
+ .requests = PLIST_HEAD_INIT(network_throughput_pm_qos.requests, pm_qos_lock),
.notifiers = &network_throughput_notifier,
.name = "network_throughput",
.default_value = 0,
@@ -99,8 +101,6 @@ static struct pm_qos_object *pm_qos_array[] = {
&network_throughput_pm_qos
};
-static DEFINE_SPINLOCK(pm_qos_lock);
-
static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
size_t count, loff_t *f_pos);
static int pm_qos_power_open(struct inode *inode, struct file *filp);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Warnings from PM QoS plist usage
2010-07-15 6:12 ` James Bottomley
@ 2010-07-15 16:28 ` Mark Brown
2010-07-15 16:58 ` Rafael J. Wysocki
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2010-07-15 16:28 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-pm, mark gross
On Thu, Jul 15, 2010 at 08:12:58AM +0200, James Bottomley wrote:
> On Wed, 2010-07-14 at 10:35 +0100, Mark Brown wrote:
> > > I've not looked at the PM QoS code at all so I don't know what the
> > > appropriate place to fix this is - do we perhaps want to fix the debug
> > > code to cope with no locks? Looking at the plist.h code it seems fairly
> > > clear that there's currently no expectation that users should use
> > > _PLIST_INIT_HEAD() directly.
> Sorry ... I just wasn't sure what it wanted without reading through the
> plist code more ... and also that debug item is hidden by
> CONFIG_DEBUG_RT_MUTEXES, which is why I couldn't enable it.
Ah, yes - I missed the existing spinlock.
> This should fix the warning.
It does indeed - thanks!
Tested-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Warnings from PM QoS plist usage
2010-07-15 16:28 ` Mark Brown
@ 2010-07-15 16:58 ` Rafael J. Wysocki
2010-07-15 17:02 ` James Bottomley
0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2010-07-15 16:58 UTC (permalink / raw)
To: Mark Brown, James Bottomley; +Cc: linux-pm, mark gross
On Thursday, July 15, 2010, Mark Brown wrote:
> On Thu, Jul 15, 2010 at 08:12:58AM +0200, James Bottomley wrote:
> > On Wed, 2010-07-14 at 10:35 +0100, Mark Brown wrote:
>
> > > > I've not looked at the PM QoS code at all so I don't know what the
> > > > appropriate place to fix this is - do we perhaps want to fix the debug
> > > > code to cope with no locks? Looking at the plist.h code it seems fairly
> > > > clear that there's currently no expectation that users should use
> > > > _PLIST_INIT_HEAD() directly.
>
> > Sorry ... I just wasn't sure what it wanted without reading through the
> > plist code more ... and also that debug item is hidden by
> > CONFIG_DEBUG_RT_MUTEXES, which is why I couldn't enable it.
>
> Ah, yes - I missed the existing spinlock.
>
> > This should fix the warning.
>
> It does indeed - thanks!
>
> Tested-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Thanks for testing.
James, I hope it's appropriate to add your sign-off to the patch?
Rafael
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Warnings from PM QoS plist usage
2010-07-15 16:58 ` Rafael J. Wysocki
@ 2010-07-15 17:02 ` James Bottomley
2010-07-19 1:43 ` Rafael J. Wysocki
0 siblings, 1 reply; 6+ messages in thread
From: James Bottomley @ 2010-07-15 17:02 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: linux-pm, Mark Brown, mark gross
On Thu, 2010-07-15 at 18:58 +0200, Rafael J. Wysocki wrote:
> On Thursday, July 15, 2010, Mark Brown wrote:
> > On Thu, Jul 15, 2010 at 08:12:58AM +0200, James Bottomley wrote:
> > > On Wed, 2010-07-14 at 10:35 +0100, Mark Brown wrote:
> >
> > > > > I've not looked at the PM QoS code at all so I don't know what the
> > > > > appropriate place to fix this is - do we perhaps want to fix the debug
> > > > > code to cope with no locks? Looking at the plist.h code it seems fairly
> > > > > clear that there's currently no expectation that users should use
> > > > > _PLIST_INIT_HEAD() directly.
> >
> > > Sorry ... I just wasn't sure what it wanted without reading through the
> > > plist code more ... and also that debug item is hidden by
> > > CONFIG_DEBUG_RT_MUTEXES, which is why I couldn't enable it.
> >
> > Ah, yes - I missed the existing spinlock.
> >
> > > This should fix the warning.
> >
> > It does indeed - thanks!
> >
> > Tested-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
>
> Thanks for testing.
>
> James, I hope it's appropriate to add your sign-off to the patch?
Certainly
Signed-off-by: James Bottomley <James.Bottomley@suse.de>
Although I'd advise just folding it into the plist patch since it's not
gone upstream yet.
James
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Warnings from PM QoS plist usage
2010-07-15 17:02 ` James Bottomley
@ 2010-07-19 1:43 ` Rafael J. Wysocki
0 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2010-07-19 1:43 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-pm, Mark Brown, mark gross
On Thursday, July 15, 2010, James Bottomley wrote:
> On Thu, 2010-07-15 at 18:58 +0200, Rafael J. Wysocki wrote:
> > On Thursday, July 15, 2010, Mark Brown wrote:
> > > On Thu, Jul 15, 2010 at 08:12:58AM +0200, James Bottomley wrote:
> > > > On Wed, 2010-07-14 at 10:35 +0100, Mark Brown wrote:
> > >
> > > > > > I've not looked at the PM QoS code at all so I don't know what the
> > > > > > appropriate place to fix this is - do we perhaps want to fix the debug
> > > > > > code to cope with no locks? Looking at the plist.h code it seems fairly
> > > > > > clear that there's currently no expectation that users should use
> > > > > > _PLIST_INIT_HEAD() directly.
> > >
> > > > Sorry ... I just wasn't sure what it wanted without reading through the
> > > > plist code more ... and also that debug item is hidden by
> > > > CONFIG_DEBUG_RT_MUTEXES, which is why I couldn't enable it.
> > >
> > > Ah, yes - I missed the existing spinlock.
> > >
> > > > This should fix the warning.
> > >
> > > It does indeed - thanks!
> > >
> > > Tested-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> >
> > Thanks for testing.
> >
> > James, I hope it's appropriate to add your sign-off to the patch?
>
> Certainly
>
> Signed-off-by: James Bottomley <James.Bottomley@suse.de>
>
> Although I'd advise just folding it into the plist patch since it's not
> gone upstream yet.
Done.
Rafael
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-07-19 1:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20100714093510.GC5933@sirena.org.uk>
2010-07-14 9:35 ` Warnings from PM QoS plist usage Mark Brown
2010-07-15 6:12 ` James Bottomley
2010-07-15 16:28 ` Mark Brown
2010-07-15 16:58 ` Rafael J. Wysocki
2010-07-15 17:02 ` James Bottomley
2010-07-19 1:43 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox