* Re: [RFC PATCH 4/4] sdio: pm: set device's power state after driver runtime suspended it
From: Rafael J. Wysocki @ 2012-10-21 19:57 UTC (permalink / raw)
To: Aaron Lu; +Cc: Chris Ball, linux-mmc, linux-pm, linux-acpi, Aaron Lu
In-Reply-To: <20121020071539.GB4798@localhost.localdomain>
On Saturday 20 of October 2012 15:15:41 Aaron Lu wrote:
> On Fri, Oct 19, 2012 at 08:08:38PM +0200, Rafael J. Wysocki wrote:
> > On Friday 19 of October 2012 01:39:25 Rafael J. Wysocki wrote:
> > > On Friday 12 of October 2012 11:12:41 Aaron Lu wrote:
> > > > In sdio bus level runtime callback function, after call the driver's
> > > > runtime suspend callback, we will check if the device supports a
> > > > platform level power management, and if so, a proper power state is
> > > > chosen by the corresponding platform callback and then set.
> > > >
> > > > Platform level runtime wakeup is also set, if device is enabled for
> > > > runtime wakeup by its driver, it will be armed the ability to generate
> > > > a wakeup event by the platform.
> > > >
> > > > Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> > > > ---
> > > > drivers/mmc/core/sdio_bus.c | 49 +++++++++++++++++++++++++++++++++++++++++++--
> > > > 1 file changed, 47 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
> > > > index aaec9e2..d83dea8 100644
> > > > --- a/drivers/mmc/core/sdio_bus.c
> > > > +++ b/drivers/mmc/core/sdio_bus.c
> > > > @@ -23,6 +23,7 @@
> > > >
> > > > #include "sdio_cis.h"
> > > > #include "sdio_bus.h"
> > > > +#include "sdio.h"
> > > > #include "sdio_acpi.h"
> > > >
> > > > /* show configuration fields */
> > > > @@ -194,10 +195,54 @@ static int sdio_bus_remove(struct device *dev)
> > > > }
> > > >
> > > > #ifdef CONFIG_PM
> > > > +
> > > > +static int sdio_bus_runtime_suspend(struct device *dev)
> > > > +{
> > > > + int ret;
> > > > + sdio_power_t state;
> > > > +
> > > > + ret = pm_generic_runtime_suspend(dev);
> > > > + if (ret)
> > > > + goto out;
> > > > +
> > > > + if (!platform_sdio_power_manageable(dev))
> > > > + goto out;
> > > > +
> > > > + platform_sdio_run_wake(dev, true);
> > > > +
> > > > + state = platform_sdio_choose_power_state(dev);
> > > > + if (state == SDIO_POWER_ERROR) {
> > > > + ret = -EIO;
> > > > + goto out;
> > > > + }
> > > > +
> > > > + ret = platform_sdio_set_power_state(dev, state);
> > > > +
> > > > +out:
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static int sdio_bus_runtime_resume(struct device *dev)
> > > > +{
> > > > + int ret;
> > > > +
> > > > + if (platform_sdio_power_manageable(dev)) {
> > > > + platform_sdio_run_wake(dev, false);
> > > > + ret = platform_sdio_set_power_state(dev, SDIO_D0);
> > > > + if (ret)
> > > > + goto out;
> > > > + }
> > > > +
> > > > + ret = pm_generic_runtime_resume(dev);
> > > > +
> > > > +out:
> > > > + return ret;
> > > > +}
> > > > +
> > >
> > > Most likely we will need to make analogous changes for other bus types that
> > > don't support power management natively, like platform, SPI, I2C etc. In all
> > > of them the _runtime_suspend() and _runtime_resume() routine will look
> > > almost exactly the same except for the platform_sdio_ prefix.
> > >
> > > For this reason, I think it would be better to simply define two functions
> > > acpi_pm_runtime_suspend() and acpi_pm_runtime_resume() that will do all of
> > > the ACPI-specific operations related to runtime suspend/resume. Then, we
> > > will be able to use these functions for all of the bus types in question
> > > in the same way (we may also need to add analogous functions for system
> > > suspend/resume handling).
> >
> > Something like in the (totally untested) patch below.
>
> Looks good to me.
> I'll test the code and put it into v2 of the patchset with your
> sign-off, is it OK?
I'd rather do it a bit differently in the signed-off version (I'm working
on these patches, they should be ready around Tuesday), but if you can test
it in its current form, that'd be useful too.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* Re: [RFC PATCH 4/4] sdio: pm: set device's power state after driver runtime suspended it
From: Aaron Lu @ 2012-10-22 0:49 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Chris Ball, linux-mmc, linux-pm, linux-acpi, Aaron Lu
In-Reply-To: <1729978.qEmlCqSTY2@vostro.rjw.lan>
On 10/22/2012 03:57 AM, Rafael J. Wysocki wrote:
> On Saturday 20 of October 2012 15:15:41 Aaron Lu wrote:
>> On Fri, Oct 19, 2012 at 08:08:38PM +0200, Rafael J. Wysocki wrote:
>>> On Friday 19 of October 2012 01:39:25 Rafael J. Wysocki wrote:
>>>> On Friday 12 of October 2012 11:12:41 Aaron Lu wrote:
>>>>> In sdio bus level runtime callback function, after call the driver's
>>>>> runtime suspend callback, we will check if the device supports a
>>>>> platform level power management, and if so, a proper power state is
>>>>> chosen by the corresponding platform callback and then set.
>>>>>
>>>>> Platform level runtime wakeup is also set, if device is enabled for
>>>>> runtime wakeup by its driver, it will be armed the ability to generate
>>>>> a wakeup event by the platform.
>>>>>
>>>>> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
>>>>> ---
>>>>> drivers/mmc/core/sdio_bus.c | 49 +++++++++++++++++++++++++++++++++++++++++++--
>>>>> 1 file changed, 47 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
>>>>> index aaec9e2..d83dea8 100644
>>>>> --- a/drivers/mmc/core/sdio_bus.c
>>>>> +++ b/drivers/mmc/core/sdio_bus.c
>>>>> @@ -23,6 +23,7 @@
>>>>>
>>>>> #include "sdio_cis.h"
>>>>> #include "sdio_bus.h"
>>>>> +#include "sdio.h"
>>>>> #include "sdio_acpi.h"
>>>>>
>>>>> /* show configuration fields */
>>>>> @@ -194,10 +195,54 @@ static int sdio_bus_remove(struct device *dev)
>>>>> }
>>>>>
>>>>> #ifdef CONFIG_PM
>>>>> +
>>>>> +static int sdio_bus_runtime_suspend(struct device *dev)
>>>>> +{
>>>>> + int ret;
>>>>> + sdio_power_t state;
>>>>> +
>>>>> + ret = pm_generic_runtime_suspend(dev);
>>>>> + if (ret)
>>>>> + goto out;
>>>>> +
>>>>> + if (!platform_sdio_power_manageable(dev))
>>>>> + goto out;
>>>>> +
>>>>> + platform_sdio_run_wake(dev, true);
>>>>> +
>>>>> + state = platform_sdio_choose_power_state(dev);
>>>>> + if (state == SDIO_POWER_ERROR) {
>>>>> + ret = -EIO;
>>>>> + goto out;
>>>>> + }
>>>>> +
>>>>> + ret = platform_sdio_set_power_state(dev, state);
>>>>> +
>>>>> +out:
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>> +static int sdio_bus_runtime_resume(struct device *dev)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + if (platform_sdio_power_manageable(dev)) {
>>>>> + platform_sdio_run_wake(dev, false);
>>>>> + ret = platform_sdio_set_power_state(dev, SDIO_D0);
>>>>> + if (ret)
>>>>> + goto out;
>>>>> + }
>>>>> +
>>>>> + ret = pm_generic_runtime_resume(dev);
>>>>> +
>>>>> +out:
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>
>>>> Most likely we will need to make analogous changes for other bus types that
>>>> don't support power management natively, like platform, SPI, I2C etc. In all
>>>> of them the _runtime_suspend() and _runtime_resume() routine will look
>>>> almost exactly the same except for the platform_sdio_ prefix.
>>>>
>>>> For this reason, I think it would be better to simply define two functions
>>>> acpi_pm_runtime_suspend() and acpi_pm_runtime_resume() that will do all of
>>>> the ACPI-specific operations related to runtime suspend/resume. Then, we
>>>> will be able to use these functions for all of the bus types in question
>>>> in the same way (we may also need to add analogous functions for system
>>>> suspend/resume handling).
>>>
>>> Something like in the (totally untested) patch below.
>>
>> Looks good to me.
>> I'll test the code and put it into v2 of the patchset with your
>> sign-off, is it OK?
>
> I'd rather do it a bit differently in the signed-off version (I'm working
> on these patches, they should be ready around Tuesday), but if you can test
OK, thanks.
> it in its current form, that'd be useful too.
I was planning to test it some time later, so looks like I can directly
test your signed-off version :-)
Thanks,
Aaron
^ permalink raw reply
* [RFC PATCH v2 1/6] mm: teach mm by current context info to not do I/O during memory allocation
From: Ming Lei @ 2012-10-22 8:33 UTC (permalink / raw)
To: linux-kernel
Cc: Alan Stern, Oliver Neukum, Minchan Kim, Greg Kroah-Hartman,
Rafael J. Wysocki, Jens Axboe, David S. Miller, Andrew Morton,
netdev, linux-usb, linux-pm, linux-mm, Ming Lei, Jiri Kosina,
Mel Gorman, KAMEZAWA Hiroyuki, Michal Hocko, Ingo Molnar,
Peter Zijlstra
In-Reply-To: <1350894794-1494-1-git-send-email-ming.lei@canonical.com>
This patch introduces PF_MEMALLOC_NOIO on process flag('flags' field of
'struct task_struct'), so that the flag can be set by one task
to avoid doing I/O inside memory allocation in the task's context.
The patch trys to solve one deadlock problem caused by block device,
and the problem may happen at least in the below situations:
- during block device runtime resume, if memory allocation with
GFP_KERNEL is called inside runtime resume callback of any one
of its ancestors(or the block device itself), the deadlock may be
triggered inside the memory allocation since it might not complete
until the block device becomes active and the involed page I/O finishes.
The situation is pointed out first by Alan Stern. It is not a good
approach to convert all GFP_KERNEL[1] in the path into GFP_NOIO because
several subsystems may be involved(for example, PCI, USB and SCSI may
be involved for usb mass stoarage device, network devices involved too
in the iSCSI case)
- during error handling of usb mass storage deivce, USB bus reset
will be put on the device, so there shouldn't have any
memory allocation with GFP_KERNEL during USB bus reset, otherwise
the deadlock similar with above may be triggered. Unfortunately, any
usb device may include one mass storage interface in theory, so it
requires all usb interface drivers to handle the situation. In fact,
most usb drivers don't know how to handle bus reset on the device
and don't provide .pre_set() and .post_reset() callback at all, so
USB core has to unbind and bind driver for these devices. So it
is still not practical to resort to GFP_NOIO for solving the problem.
Also the introduced solution can be used by block subsystem or block
drivers too, for example, set the PF_MEMALLOC_NOIO flag before doing
actual I/O transfer.
It is not a good idea to convert all these GFP_KERNEL in the
affected path into GFP_NOIO because these functions doing that may be
implemented as library and will be called in many other contexts.
In fact, memalloc_noio() can convert some of current static GFP_NOIO
allocation into GFP_KERNEL back in other non-affected contexts, at least
almost all GFP_NOIO in USB subsystem can be converted into GFP_KERNEL
after applying the approach and make allocation with GFP_IO
only happen in runtime resume/bus reset/block I/O transfer contexts
generally.
[1], several GFP_KERNEL allocation examples in runtime resume path
- pci subsystem
acpi_os_allocate
<-acpi_ut_allocate
<-ACPI_ALLOCATE_ZEROED
<-acpi_evaluate_object
<-__acpi_bus_set_power
<-acpi_bus_set_power
<-acpi_pci_set_power_state
<-platform_pci_set_power_state
<-pci_platform_power_transition
<-__pci_complete_power_transition
<-pci_set_power_state
<-pci_restore_standard_config
<-pci_pm_runtime_resume
- usb subsystem
usb_get_status
<-finish_port_resume
<-usb_port_resume
<-generic_resume
<-usb_resume_device
<-usb_resume_both
<-usb_runtime_resume
- some individual usb drivers
usblp, uvc, gspca, most of dvb-usb-v2 media drivers, cpia2, az6007, ....
That is just what I have found. Unfortunately, this allocation can
only be found by human being now, and there should be many not found
since any function in the resume path(call tree) may allocate memory
with GFP_KERNEL.
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Oliver Neukum <oneukum@suse.de>
Cc: Jiri Kosina <jiri.kosina@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Mel Gorman <mel@csn.ul.ie>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Signed-off-by: Minchan Kim <minchan@kernel.org>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
v2:
- remove changes on 'may_writepage' and 'may_swap' because that
isn't related with the patchset, and can't introduce I/O in
allocation path if GFP_IOFS is unset, so handing 'may_swap'
and may_writepage on GFP_NOIO or GFP_NOFS should be a
mm internal thing, and let mm guys deal with that, :-).
Looks clearing the two may_XXX flag only excludes dirty pages and
anon pages for relaiming, and the behaviour should be decided
by GFP FLAG, IMO.
- unset GFP_IOFS in try_to_free_pages() path since alloc_page_buffers()
and dma_alloc_from_contiguous may drop into the path, as
pointed by KAMEZAWA Hiroyuki
v1:
- take Minchan's change to avoid the check in alloc_page hot path
- change the helpers' style into save/restore as suggested by Alan
Stern
---
include/linux/sched.h | 10 ++++++++++
mm/page_alloc.c | 10 +++++++++-
mm/vmscan.c | 12 ++++++++++++
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index f7a76fa..ac5234a 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1793,6 +1793,7 @@ extern void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *
#define PF_FROZEN 0x00010000 /* frozen for system suspend */
#define PF_FSTRANS 0x00020000 /* inside a filesystem transaction */
#define PF_KSWAPD 0x00040000 /* I am kswapd */
+#define PF_MEMALLOC_NOIO 0x00080000 /* Allocating memory without IO involved */
#define PF_LESS_THROTTLE 0x00100000 /* Throttle me less: I clean memory */
#define PF_KTHREAD 0x00200000 /* I am a kernel thread */
#define PF_RANDOMIZE 0x00400000 /* randomize virtual address space */
@@ -1830,6 +1831,15 @@ extern void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *
#define tsk_used_math(p) ((p)->flags & PF_USED_MATH)
#define used_math() tsk_used_math(current)
+#define memalloc_noio() (current->flags & PF_MEMALLOC_NOIO)
+#define memalloc_noio_save(flag) do { \
+ (flag) = current->flags & PF_MEMALLOC_NOIO; \
+ current->flags |= PF_MEMALLOC_NOIO; \
+} while (0)
+#define memalloc_noio_restore(flag) do { \
+ current->flags = (current->flags & ~PF_MEMALLOC_NOIO) | flag; \
+} while (0)
+
/*
* task->jobctl flags
*/
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 0c871fc..a7b76ae 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2630,10 +2630,18 @@ retry_cpuset:
page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, nodemask, order,
zonelist, high_zoneidx, alloc_flags,
preferred_zone, migratetype);
- if (unlikely(!page))
+ if (unlikely(!page)) {
+ /*
+ * Resume, block IO and its error handling path
+ * can deadlock because I/O on the device might not
+ * complete.
+ */
+ if (unlikely(memalloc_noio()))
+ gfp_mask &= ~GFP_IOFS;
page = __alloc_pages_slowpath(gfp_mask, order,
zonelist, high_zoneidx, nodemask,
preferred_zone, migratetype);
+ }
trace_mm_page_alloc(page, order, gfp_mask, migratetype);
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 2624edc..5bf8290 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2298,6 +2298,12 @@ unsigned long try_to_free_pages(struct zonelist *zonelist, int order,
.gfp_mask = sc.gfp_mask,
};
+ if (unlikely(memalloc_noio())) {
+ gfp_mask &= ~GFP_IOFS;
+ sc.gfp_mask = gfp_mask;
+ shrink.gfp_mask = sc.gfp_mask;
+ }
+
throttle_direct_reclaim(gfp_mask, zonelist, nodemask);
/*
@@ -3298,6 +3304,12 @@ static int __zone_reclaim(struct zone *zone, gfp_t gfp_mask, unsigned int order)
};
unsigned long nr_slab_pages0, nr_slab_pages1;
+ if (unlikely(memalloc_noio())) {
+ gfp_mask &= ~GFP_IOFS;
+ sc.gfp_mask = gfp_mask;
+ shrink.gfp_mask = sc.gfp_mask;
+ }
+
cond_resched();
/*
* We need to be able to allocate from the reserves for RECLAIM_SWAP
--
1.7.9.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH v2 2/6] PM / Runtime: introduce pm_runtime_set_memalloc_noio()
From: Ming Lei @ 2012-10-22 8:33 UTC (permalink / raw)
To: linux-kernel
Cc: Alan Stern, Oliver Neukum, Minchan Kim, Greg Kroah-Hartman,
Rafael J. Wysocki, Jens Axboe, David S. Miller, Andrew Morton,
netdev, linux-usb, linux-pm, linux-mm, Ming Lei
In-Reply-To: <1350894794-1494-1-git-send-email-ming.lei@canonical.com>
The patch introduces the flag of memalloc_noio_resume in
'struct dev_pm_info' to help PM core to teach mm not allocating
memory with GFP_KERNEL flag for avoiding probable deadlock
problem.
As explained in the comment, any GFP_KERNEL allocation inside
runtime_resume on any one of device in the path from one block
or network device to the root device in the device tree may cause
deadlock, the introduced pm_runtime_set_memalloc_noio() sets or
clears the flag on device of the path recursively.
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/base/power/runtime.c | 53 ++++++++++++++++++++++++++++++++++++++++++
include/linux/pm.h | 1 +
include/linux/pm_runtime.h | 3 +++
3 files changed, 57 insertions(+)
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 3148b10..a75eeca 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -124,6 +124,59 @@ unsigned long pm_runtime_autosuspend_expiration(struct device *dev)
}
EXPORT_SYMBOL_GPL(pm_runtime_autosuspend_expiration);
+static int dev_memalloc_noio(struct device *dev, void *data)
+{
+ if (dev->power.memalloc_noio_resume)
+ return 1;
+ return 0;
+}
+
+/*
+ * pm_runtime_set_memalloc_noio - Set a device's memalloc_noio flag.
+ * @dev: Device to handle.
+ * @enable: True for setting the flag and False for clearing the flag.
+ *
+ * Set the flag for all devices in the path from the device to the
+ * root device in the device tree if @enable is true, otherwise clear
+ * the flag for devices in the path which sibliings don't set the flag.
+ *
+ * The function should only be called by block device, or network
+ * device driver for solving the deadlock problem during runtime
+ * resume:
+ * if memory allocation with GFP_KERNEL is called inside runtime
+ * resume callback of any one of its ancestors(or the block device
+ * itself), the deadlock may be triggered inside the memory
+ * allocation since it might not complete until the block device
+ * becomes active and the involed page I/O finishes. The situation
+ * is pointed out first by Alan Stern. Network device are involved
+ * in iSCSI kind of situation.
+ *
+ * No lock is provovided in the function for handling hotplog race
+ * because pm_runtime_set_memalloc_noio(false) is called in parent's
+ * remove path.
+ */
+void pm_runtime_set_memalloc_noio(struct device *dev, bool enable)
+{
+ dev->power.memalloc_noio_resume = enable;
+
+ if (!dev->parent)
+ return;
+
+ if (enable) {
+ pm_runtime_set_memalloc_noio(dev->parent, 1);
+ } else {
+ /* only clear the flag for one device if all
+ * children of the device don't set the flag.
+ */
+ if (device_for_each_child(dev->parent, NULL,
+ dev_memalloc_noio))
+ return;
+
+ pm_runtime_set_memalloc_noio(dev->parent, 0);
+ }
+}
+EXPORT_SYMBOL_GPL(pm_runtime_set_memalloc_noio);
+
/**
* rpm_check_suspend_allowed - Test whether a device may be suspended.
* @dev: Device to test.
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 007e687..5b0ee4d 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -538,6 +538,7 @@ struct dev_pm_info {
unsigned int irq_safe:1;
unsigned int use_autosuspend:1;
unsigned int timer_autosuspends:1;
+ unsigned int memalloc_noio_resume:1;
enum rpm_request request;
enum rpm_status runtime_status;
int runtime_error;
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index f271860..775e063 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -47,6 +47,7 @@ extern void pm_runtime_set_autosuspend_delay(struct device *dev, int delay);
extern unsigned long pm_runtime_autosuspend_expiration(struct device *dev);
extern void pm_runtime_update_max_time_suspended(struct device *dev,
s64 delta_ns);
+extern void pm_runtime_set_memalloc_noio(struct device *dev, bool enable);
static inline bool pm_children_suspended(struct device *dev)
{
@@ -149,6 +150,8 @@ static inline void pm_runtime_set_autosuspend_delay(struct device *dev,
int delay) {}
static inline unsigned long pm_runtime_autosuspend_expiration(
struct device *dev) { return 0; }
+static inline void pm_runtime_set_memalloc_noio(struct device *dev,
+ bool enable){}
#endif /* !CONFIG_PM_RUNTIME */
--
1.7.9.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH v2 3/6] block/genhd.c: apply pm_runtime_set_memalloc_noio on block devices
From: Ming Lei @ 2012-10-22 8:33 UTC (permalink / raw)
To: linux-kernel
Cc: Alan Stern, Oliver Neukum, Minchan Kim, Greg Kroah-Hartman,
Rafael J. Wysocki, Jens Axboe, David S. Miller, Andrew Morton,
netdev, linux-usb, linux-pm, linux-mm, Ming Lei
In-Reply-To: <1350894794-1494-1-git-send-email-ming.lei@canonical.com>
This patch applyes the introduced pm_runtime_set_memalloc_noio on
block device so that PM core will teach mm to not allocate memory with
GFP_IOFS when calling the runtime_resume callback for block devices.
Cc: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
block/genhd.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/block/genhd.c b/block/genhd.c
index 9e02cd6..c5f10ea 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -18,6 +18,7 @@
#include <linux/mutex.h>
#include <linux/idr.h>
#include <linux/log2.h>
+#include <linux/pm_runtime.h>
#include "blk.h"
@@ -519,6 +520,12 @@ static void register_disk(struct gendisk *disk)
dev_set_name(ddev, disk->disk_name);
+ /* avoid probable deadlock caused by allocate memory with
+ * GFP_KERNEL in runtime_resume callback of its all ancestor
+ * deivces
+ */
+ pm_runtime_set_memalloc_noio(ddev, true);
+
/* delay uevents, until we scanned partition table */
dev_set_uevent_suppress(ddev, 1);
@@ -661,6 +668,7 @@ void del_gendisk(struct gendisk *disk)
disk->driverfs_dev = NULL;
if (!sysfs_deprecated)
sysfs_remove_link(block_depr, dev_name(disk_to_dev(disk)));
+ pm_runtime_set_memalloc_noio(disk_to_dev(disk), false);
device_del(disk_to_dev(disk));
}
EXPORT_SYMBOL(del_gendisk);
--
1.7.9.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH v2 4/6] net/core: apply pm_runtime_set_memalloc_noio on network devices
From: Ming Lei @ 2012-10-22 8:33 UTC (permalink / raw)
To: linux-kernel
Cc: Alan Stern, Oliver Neukum, Minchan Kim, Greg Kroah-Hartman,
Rafael J. Wysocki, Jens Axboe, David S. Miller, Andrew Morton,
netdev, linux-usb, linux-pm, linux-mm, Ming Lei, Eric Dumazet,
David Decotigny, Tom Herbert, Ingo Molnar
In-Reply-To: <1350894794-1494-1-git-send-email-ming.lei@canonical.com>
Deadlock might be caused by allocating memory with GFP_KERNEL in
runtime_resume callback of network devices in iSCSI situation, so
mark network devices and its ancestor as 'memalloc_noio_resume'
with the introduced pm_runtime_set_memalloc_noio().
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David Decotigny <david.decotigny@google.com>
Cc: Tom Herbert <therbert@google.com>
Cc: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
net/core/net-sysfs.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index bcf02f6..9aba5be 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -22,6 +22,7 @@
#include <linux/vmalloc.h>
#include <linux/export.h>
#include <linux/jiffies.h>
+#include <linux/pm_runtime.h>
#include <net/wext.h>
#include "net-sysfs.h"
@@ -1386,6 +1387,8 @@ void netdev_unregister_kobject(struct net_device * net)
remove_queue_kobjects(net);
+ pm_runtime_set_memalloc_noio(dev, false);
+
device_del(dev);
}
@@ -1411,6 +1414,8 @@ int netdev_register_kobject(struct net_device *net)
*groups++ = &netstat_group;
#endif /* CONFIG_SYSFS */
+ pm_runtime_set_memalloc_noio(dev, true);
+
error = device_add(dev);
if (error)
return error;
--
1.7.9.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH v2 5/6] PM / Runtime: force memory allocation with no I/O during runtime_resume callbcack
From: Ming Lei @ 2012-10-22 8:33 UTC (permalink / raw)
To: linux-kernel
Cc: Alan Stern, Oliver Neukum, Minchan Kim, Greg Kroah-Hartman,
Rafael J. Wysocki, Jens Axboe, David S. Miller, Andrew Morton,
netdev, linux-usb, linux-pm, linux-mm, Ming Lei
In-Reply-To: <1350894794-1494-1-git-send-email-ming.lei@canonical.com>
This patch applies the introduced memalloc_noio_save() and
memalloc_noio_restore() to force memory allocation with no I/O
during runtime_resume callback on device which is marked as
memalloc_noio_resume.
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Oliver Neukum <oneukum@suse.de>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/base/power/runtime.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index a75eeca..c61b7b0 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -556,6 +556,7 @@ static int rpm_resume(struct device *dev, int rpmflags)
int (*callback)(struct device *);
struct device *parent = NULL;
int retval = 0;
+ unsigned int noio_flag;
trace_rpm_resume(dev, rpmflags);
@@ -705,7 +706,20 @@ static int rpm_resume(struct device *dev, int rpmflags)
if (!callback && dev->driver && dev->driver->pm)
callback = dev->driver->pm->runtime_resume;
- retval = rpm_callback(callback, dev);
+ /*
+ * Deadlock might be caused if memory allocation with GFP_KERNEL
+ * happens inside runtime_resume callback of one block device's
+ * ancestor or the block device itself. Network device might be
+ * thought as part of iSCSI block device, so network device and
+ * its ancestor should be marked as memalloc_noio_resume.
+ */
+ if (dev->power.memalloc_noio_resume) {
+ memalloc_noio_save(noio_flag);
+ retval = rpm_callback(callback, dev);
+ memalloc_noio_restore(noio_flag);
+ } else {
+ retval = rpm_callback(callback, dev);
+ }
if (retval) {
__update_runtime_status(dev, RPM_SUSPENDED);
pm_runtime_cancel_pending(dev);
--
1.7.9.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH v2 6/6] USB: forbid memory allocation with I/O during bus reset
From: Ming Lei @ 2012-10-22 8:33 UTC (permalink / raw)
To: linux-kernel
Cc: Alan Stern, Oliver Neukum, Minchan Kim, Greg Kroah-Hartman,
Rafael J. Wysocki, Jens Axboe, David S. Miller, Andrew Morton,
netdev, linux-usb, linux-pm, linux-mm, Ming Lei
In-Reply-To: <1350894794-1494-1-git-send-email-ming.lei@canonical.com>
If one storage interface or usb network interface(iSCSI case)
exists in current configuration, memory allocation with
GFP_KERNEL during usb_device_reset() might trigger I/O transfer
on the storage interface itself and cause deadlock because
the 'us->dev_mutex' is held in .pre_reset() and the storage
interface can't do I/O transfer when the reset is triggered
by other interface, or the error handling can't be completed
if the reset is triggered by the storage itself(error handling path).
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Oliver Neukum <oneukum@suse.de>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/usb/core/hub.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 522ad57..106a80a 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -5038,6 +5038,7 @@ int usb_reset_device(struct usb_device *udev)
{
int ret;
int i;
+ unsigned int noio_flag;
struct usb_host_config *config = udev->actconfig;
if (udev->state == USB_STATE_NOTATTACHED ||
@@ -5047,6 +5048,15 @@ int usb_reset_device(struct usb_device *udev)
return -EINVAL;
}
+ /*
+ * Don't allocate memory with GFP_KERNEL in current
+ * context to avoid possible deadlock if usb mass
+ * storage interface or usbnet interface(iSCSI case)
+ * is included in current configuration. The easiest
+ * approach is to do it for all devices.
+ */
+ memalloc_noio_save(noio_flag);
+
/* Prevent autosuspend during the reset */
usb_autoresume_device(udev);
@@ -5091,6 +5101,7 @@ int usb_reset_device(struct usb_device *udev)
}
usb_autosuspend_device(udev);
+ memalloc_noio_restore(noio_flag);
return ret;
}
EXPORT_SYMBOL_GPL(usb_reset_device);
--
1.7.9.5
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH v2 0/6] solve deadlock caused by memory allocation with I/O
From: Ming Lei @ 2012-10-22 8:33 UTC (permalink / raw)
To: linux-kernel
Cc: Alan Stern, Oliver Neukum, Minchan Kim, Greg Kroah-Hartman,
Rafael J. Wysocki, Jens Axboe, David S. Miller, Andrew Morton,
netdev, linux-usb, linux-pm, linux-mm
Hi,
This patchset try to solve one deadlock problem which might be caused
by memory allocation with block I/O during runtime resume and block device
error handling path. Traditionly, the problem is addressed by passing
GFP_NOIO statically to mm, but that is not a effective solution, see
detailed description in patch 1's commit log.
This patch set introduces one process flag and trys to fix one deadlock
problem on block device/network device during runtime resume or usb bus reset.
The 1st one is the change on include/sched.h and mm.
The 2nd patch introduces the flag of memalloc_noio_resume on 'dev_pm_info',
and pm_runtime_set_memalloc_noio(), so that PM Core can teach mm to not
allocate mm with GFP_IOFS during the runtime_resume callback only on
device with the flag set.
The following 2 patches apply the introduced pm_runtime_set_memalloc_noio()
to mark all devices as memalloc_noio_resume in the path from the block or
network device to the root device in device tree.
The last 2 patches are applied again PM and USB subsystem to demonstrate
how to use the introduced mechanism to fix the deadlock problem.
V2:
- remove changes on 'may_writepage' and 'may_swap'(1/6)
- unset GFP_IOFS in try_to_free_pages() path(1/6)
- introduce pm_runtime_set_memalloc_noio()
- only apply the meachnism on block/network device and its ancestors
for runtime resume context
V1:
- take Minchan's change to avoid the check in alloc_page hot path
- change the helpers' style into save/restore as suggested by Alan
- memory allocation with no io in usb bus reset path for all devices
as suggested by Greg and Oliver
block/genhd.c | 8 +++++
drivers/base/power/runtime.c | 69 +++++++++++++++++++++++++++++++++++++++++-
drivers/usb/core/hub.c | 11 +++++++
include/linux/pm.h | 1 +
include/linux/pm_runtime.h | 3 ++
include/linux/sched.h | 10 ++++++
mm/page_alloc.c | 10 +++++-
mm/vmscan.c | 12 ++++++++
net/core/net-sysfs.c | 5 +++
9 files changed, 127 insertions(+), 2 deletions(-)
Thanks,
--
Ming Lei
^ permalink raw reply
* Re: [PATCH 2/2] cpufreq: governors: remove redundant code
From: Viresh Kumar @ 2012-10-22 8:46 UTC (permalink / raw)
To: rjw
Cc: cpufreq, linux-pm, linux-kernel, linux-arm-kernel, linaro-dev,
patches, pdsw-power-team, arvind.chauhan, Viresh Kumar
In-Reply-To: <98478a02ae69afc01b3c7860e42a4d763b2045e8.1350677395.git.viresh.kumar@linaro.org>
On 20 October 2012 01:42, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Initially ondemand governor was written and then using its code conservative
> governor is written. It used a lot of code from ondemand governor, but copy of
> code was created instead of using the same routines from both governors. Which
> increased code redundancy, which is difficult to manage.
>
> This patch is an attempt to move common part of both the governors to
> cpufreq_governor.c file to come over above mentioned issues.
>
> This shouldn't change anything from functionality point of view.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>
> Hi Rafael,
>
> This patch is *NOT TESTED*... (only compiled)
> Out of office now, so can't test it on board.
>
> Floated it, so that i can get some early comments if possible.
Didn't crashed :)
Tested both ondemand and conservative governors and they are working
fine.
> drivers/cpufreq/Makefile | 4 +-
> drivers/cpufreq/cpufreq_conservative.c | 558 +++++++------------------
> drivers/cpufreq/cpufreq_governor.c | 314 ++++++++++++++
> drivers/cpufreq/cpufreq_governor.h | 177 ++++++++
> drivers/cpufreq/cpufreq_ondemand.c | 728 ++++++++++-----------------------
> include/linux/cpufreq.h | 2 +-
> 6 files changed, 864 insertions(+), 919 deletions(-)
diffstat might not look awesome, but considering two new files with
file headers, #includes, etc added.. this looks reasonable :)
--
viresh
^ permalink raw reply
* Re: [PATCH 5/5] Thermal: Add ST-Ericsson db8500 thermal dirver.
From: Hongbo Zhang @ 2012-10-22 12:02 UTC (permalink / raw)
To: Francesco Lavra
Cc: linaro-dev, linux-kernel, linux-pm, STEricsson_nomadik_linux,
kernel, linaro-kernel, hongbo.zhang, patches
In-Reply-To: <50840E3D.1060204@gmail.com>
On 21 October 2012 23:01, Francesco Lavra <francescolavra.fl@gmail.com> wrote:
> Hi Hongbo,
Hi Francesco,
Thanks for your review, I will accept all the comments except the ones
I have some comments under them.
>
> On 10/16/2012 01:44 PM, hongbo.zhang wrote:
>> From: "hongbo.zhang" <hongbo.zhang@linaro.com>
>>
>> This diver is based on the thermal management framework in thermal_sys.c.
>> A thermal zone device is created with the trip points to which cooling
>> devices can be bound, the current cooling device is cpufreq, e.g. CPU
>> frequency is clipped down to cool the CPU, and other cooling devices can
>> be added and bound to the trip points dynamically.
>> The platform specific PRCMU interrupts are used to active thermal update
>> when trip points are reached.
> [...]
>> diff --git a/drivers/thermal/db8500_cpufreq_cooling.c b/drivers/thermal/db8500_cpufreq_cooling.c
>> new file mode 100644
>> index 0000000..bb065d4
>> --- /dev/null
>> +++ b/drivers/thermal/db8500_cpufreq_cooling.c
>> @@ -0,0 +1,118 @@
>> +/*
>> + * db8500_cpufreq_cooling.c - db8500 cpufreq works as cooling device.
>> + *
>> + * Copyright (C) 2012 ST-Ericsson
>> + * Copyright (C) 2012 Linaro Ltd.
>> + *
>> + * Author: Hongbo Zhang <hognbo.zhang@stericsson.com>
>
> Your e-mail address is misspelled :)
>
> [...]
>> diff --git a/drivers/thermal/db8500_thermal.c b/drivers/thermal/db8500_thermal.c
>> new file mode 100644
>> index 0000000..34dcc52
>> --- /dev/null
>> +++ b/drivers/thermal/db8500_thermal.c
>> @@ -0,0 +1,507 @@
>> +/*
>> + * db8500_thermal.c - db8500 Thermal Management Implementation
>> + *
>> + * Copyright (C) 2012 ST-Ericsson
>> + * Copyright (C) 2012 Linaro Ltd.
>> + *
>> + * Author: Hongbo Zhang <hognbo.zhang@stericsson.com>
>
> Misspelled address
>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/slab.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/thermal.h>
>> +#include <linux/cpu_cooling.h>
>> +#include <linux/mfd/dbx500-prcmu.h>
>> +#include <linux/platform_data/db8500_thermal.h>
>> +
>> +#define PRCMU_DEFAULT_MEASURE_TIME 0xFFF
>> +#define PRCMU_DEFAULT_LOW_TEMP 0
>> +
>> +struct db8500_thermal_zone {
>> + struct thermal_zone_device *therm_dev;
>> + struct mutex th_lock;
>> + struct platform_device *thsens_pdev;
>
> This member is set in db8500_thermal_probe(), but is never used. I would
> suggest removing it.
>
>> + struct work_struct therm_work;
>> + struct db8500_thsens_platform_data *trip_tab;
>> + enum thermal_device_mode mode;
>> + enum thermal_trend trend;
>> + unsigned long cur_temp_pseudo;
>> + unsigned int cur_index;
>> + int low_irq;
>> + int high_irq;
>
> Same story as thsens_pdev, low_irq and high_irq are set in
> db8500_thermal_probe(), but are never used. Should be removed.
>
>> +};
>> +
>> +/* Callback to bind cooling device to thermal zone */
>> +static int db8500_cdev_bind(struct thermal_zone_device *thermal,
>> + struct thermal_cooling_device *cdev)
>> +{
>> + struct db8500_thermal_zone *pzone;
>> + struct db8500_thsens_platform_data *ptrips;
>> + char *cdev_name;
>> + unsigned long max_state, upper, lower;
>> + int i, j, ret;
>> +
>> + pzone = (struct db8500_thermal_zone *)thermal->devdata;
>> + ptrips = pzone->trip_tab;
>> +
>> + if (!cdev->type)
>> + return -EINVAL;
>
> cdev->type is an array, not a simple pointer, so it cannot be NULL.
>
>> +
>> + ret = -ENODEV;
>> + for (i = 0; i < ptrips->num_trips; i++)
>> + for (j = 0; j < COOLING_DEV_MAX; j++) {
>> + cdev_name = ptrips->trip_points[i].cooling_dev_name[j];
>> + if (!cdev_name)
>> + continue;
>> +
>> + if (strcmp(cdev_name, cdev->type))
>> + continue;
>> +
>> + cdev->ops->get_max_state(cdev, &max_state);
>> + upper = (i > max_state) ? max_state : i;
>> + lower = (i > max_state) ? max_state : i;
>
> You may want to merge these two lines: upper = lower = ...
>
>> +
>> + ret = thermal_zone_bind_cooling_device(thermal, i,
>> + cdev, upper, lower);
>> + if (ret)
>> + pr_err("Error binding cooling device.\n");
>> + else
>> + pr_info("Cdev %s bound.\n", cdev->type);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +/* Callback to unbind cooling device from thermal zone */
>> +static int db8500_cdev_unbind(struct thermal_zone_device *thermal,
>> + struct thermal_cooling_device *cdev)
>> +{
>> + struct db8500_thermal_zone *pzone;
>> + struct db8500_thsens_platform_data *ptrips;
>> + char *cdev_name;
>> + int i, j, ret;
>> +
>> + pzone = (struct db8500_thermal_zone *)thermal->devdata;
>> + ptrips = pzone->trip_tab;
>> +
>> + if (!cdev->type)
>> + return -EINVAL;
>
> cdev->type cannot be NULL.
>
>> +
>> + ret = -ENODEV;
>> + for (i = 0; i < ptrips->num_trips; i++)
>> + for (j = 0; j < COOLING_DEV_MAX; j++) {
>> + cdev_name = ptrips->trip_points[i].cooling_dev_name[j];
>> + if (!cdev_name)
>> + continue;
>> +
>> + if (strcmp(cdev_name, cdev->type))
>> + continue;
>> +
>> + ret = thermal_zone_unbind_cooling_device(
>> + thermal, i, cdev);
>> + if (ret)
>> + pr_err("Error unbinding cooling device.\n");
>> + else
>> + pr_info("Cdev %s unbound.\n", cdev->type);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +/* Callback to get current temperature */
>> +static int db8500_sys_get_temp(struct thermal_zone_device *thermal,
>> + unsigned long *temp)
>> +{
>> + struct db8500_thermal_zone *pzone;
>> + pzone = (struct db8500_thermal_zone *)thermal->devdata;
>> +
>> + /* TODO: There is no PRCMU interface to get temperature data currently,
>> + so a pseudo temperature is returned , it works for the thermal framework
>> + and this will be fixed when the PRCMU interface is available */
>> + *temp = pzone->cur_temp_pseudo;
>> +
>> + return 0;
>> +}
>> +
>> +/* Callback to get temperature changing trend */
>> +static int db8500_sys_get_trend(struct thermal_zone_device *thermal,
>> + int trip, enum thermal_trend *trend)
>> +{
>> + struct db8500_thermal_zone *pzone;
>> + pzone = (struct db8500_thermal_zone *)thermal->devdata;
>> +
>> + *trend = pzone->trend;
>> +
>> + return 0;
>> +}
>> +
>> +/* Callback to get thermal zone mode */
>> +static int db8500_sys_get_mode(struct thermal_zone_device *thermal,
>> + enum thermal_device_mode *mode)
>> +{
>> + struct db8500_thermal_zone *pzone;
>> + pzone = (struct db8500_thermal_zone *)thermal->devdata;
>> +
>> + mutex_lock(&pzone->th_lock);
>> + *mode = pzone->mode;
>> + mutex_unlock(&pzone->th_lock);
>> +
>> + return 0;
>> +}
>> +
>> +/* Callback to set thermal zone mode */
>> +static int db8500_sys_set_mode(struct thermal_zone_device *thermal,
>> + enum thermal_device_mode mode)
>> +{
>> + struct db8500_thermal_zone *pzone;
>> + struct thermal_zone_device *pthdev;
>> +
>> + pzone = (struct db8500_thermal_zone *)thermal->devdata;
>> + pthdev = pzone->therm_dev;
>> +
>> + if (!pthdev) {
>> + pr_err("Thermal zone not registered.\n");
>> + return 0;
>> + }
>
> If this function is called, you are sure the thermal zone has been
> registered.
>
>> +
>> + mutex_lock(&pzone->th_lock);
>> +
>> + pzone->mode = mode;
>> +
>> + if (mode == THERMAL_DEVICE_ENABLED)
>> + schedule_work(&pzone->therm_work);
>> +
>> + mutex_unlock(&pzone->th_lock);
>> +
>> + return 0;
>> +}
>> +
>> +/* Callback to get trip point type */
>> +static int db8500_sys_get_trip_type(struct thermal_zone_device *thermal,
>> + int trip, enum thermal_trip_type *type)
>> +{
>> + struct db8500_thermal_zone *pzone;
>> + struct db8500_thsens_platform_data *ptrips;
>> +
>> + pzone = (struct db8500_thermal_zone *)thermal->devdata;
>> + ptrips = pzone->trip_tab;
>> +
>> + if (trip >= ptrips->num_trips)
>> + return -EINVAL;
>> +
>> + *type = ptrips->trip_points[trip].type;
>> +
>> + return 0;
>> +}
>> +
>> +/* Callback to get trip point temperature */
>> +static int db8500_sys_get_trip_temp(struct thermal_zone_device *thermal,
>> + int trip, unsigned long *temp)
>> +{
>> + struct db8500_thermal_zone *pzone;
>> + struct db8500_thsens_platform_data *ptrips;
>> +
>> + pzone = (struct db8500_thermal_zone *)thermal->devdata;
>> + ptrips = pzone->trip_tab;
>> +
>> + if (trip >= ptrips->num_trips)
>> + return -EINVAL;
>> +
>> + *temp = ptrips->trip_points[trip].temp;
>> +
>> + return 0;
>> +}
>> +
>> +/* Callback to get critical trip point temperature */
>> +static int db8500_sys_get_crit_temp(struct thermal_zone_device *thermal,
>> + unsigned long *temp)
>> +{
>> + struct db8500_thermal_zone *pzone;
>> + struct db8500_thsens_platform_data *ptrips;
>> + int i;
>> +
>> + pzone = (struct db8500_thermal_zone *)thermal->devdata;
>> + ptrips = pzone->trip_tab;
>> +
>> + for (i = (ptrips->num_trips - 1); i > 0; i--) {
>> + if (ptrips->trip_points[i].type == THERMAL_TRIP_CRITICAL) {
>> + *temp = ptrips->trip_points[i].temp;
>> + return 0;
>> + }
>> + }
>> +
>> + return -EINVAL;
>> +}
>> +
>> +static struct thermal_zone_device_ops thdev_ops = {
>> + .bind = db8500_cdev_bind,
>> + .unbind = db8500_cdev_unbind,
>> + .get_temp = db8500_sys_get_temp,
>> + .get_trend = db8500_sys_get_trend,
>> + .get_mode = db8500_sys_get_mode,
>> + .set_mode = db8500_sys_set_mode,
>> + .get_trip_type = db8500_sys_get_trip_type,
>> + .get_trip_temp = db8500_sys_get_trip_temp,
>> + .get_crit_temp = db8500_sys_get_crit_temp,
>> +};
>> +
>> +static irqreturn_t prcmu_low_irq_handler(int irq, void *irq_data)
>> +{
>> + struct db8500_thermal_zone *pzone = irq_data;
>> + struct db8500_thsens_platform_data *ptrips;
>> + unsigned long next_low, next_high;
>> + unsigned int idx;
>> +
>> + ptrips = pzone->trip_tab;
>> + idx = pzone->cur_index;
>> + if (unlikely(idx == 0))
>> + /* Meaningless for thermal management, ignoring it */
>> + return IRQ_HANDLED;
>> +
>> + if (idx == 1) {
>> + next_high = ptrips->trip_points[0].temp;
>> + next_low = PRCMU_DEFAULT_LOW_TEMP;
>> + } else {
>> + next_high = ptrips->trip_points[idx-1].temp;
>> + next_low = ptrips->trip_points[idx-2].temp;
>> + }
>> +
>> + pzone->cur_index -= 1;
>> + pzone->cur_temp_pseudo = (next_high + next_low)/2;
>> +
>> + prcmu_stop_temp_sense();
>> + prcmu_config_hotmon((u8)(next_low/1000), (u8)(next_high/1000));
>> + prcmu_start_temp_sense(PRCMU_DEFAULT_MEASURE_TIME);
>> +
>> + pr_debug("PRCMU set max %ld, set min %ld\n", next_high, next_low);
>> +
>> + pzone->trend = THERMAL_TREND_DROPPING;
>> + schedule_work(&pzone->therm_work);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static irqreturn_t prcmu_high_irq_handler(int irq, void *irq_data)
>> +{
>> + struct db8500_thermal_zone *pzone = irq_data;
>> + struct db8500_thsens_platform_data *ptrips;
>> + unsigned long next_low, next_high;
>> + unsigned int idx;
>> +
>> + ptrips = pzone->trip_tab;
>> + idx = pzone->cur_index;
>> +
>> + if (idx < ptrips->num_trips - 1) {
>> + next_high = ptrips->trip_points[idx+1].temp;
>> + next_low = ptrips->trip_points[idx].temp;
>> +
>> + pzone->cur_index += 1;
>> + pzone->cur_temp_pseudo = (next_high + next_low)/2;
>> +
>> + prcmu_stop_temp_sense();
>> + prcmu_config_hotmon((u8)(next_low/1000), (u8)(next_high/1000));
>> + prcmu_start_temp_sense(PRCMU_DEFAULT_MEASURE_TIME);
>> +
>> + pr_debug("PRCMU set max %ld, min %ld\n", next_high, next_low);
>> + }
>> +
>> + if (idx == ptrips->num_trips - 1)
>
> } else if ()
There is no else condition here, because it it the highest critical
trip point, system will be shut down in thermal_work.
But I'd like to add some comments here to state this situation.
>
>> + pzone->cur_temp_pseudo = ptrips->trip_points[idx].temp + 1;
>> +
>> + pzone->trend = THERMAL_TREND_RAISING;
>> + schedule_work(&pzone->therm_work);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static void db8500_thermal_work(struct work_struct *work)
>> +{
>> + enum thermal_device_mode cur_mode;
>> + struct db8500_thermal_zone *pzone;
>> +
>> + pzone = container_of(work, struct db8500_thermal_zone, therm_work);
>> +
>> + mutex_lock(&pzone->th_lock);
>> + cur_mode = pzone->mode;
>> + mutex_unlock(&pzone->th_lock);
>> +
>> + if (cur_mode == THERMAL_DEVICE_DISABLED) {
>> + pr_warn("Warning: thermal function disabled.\n");
>> + return;
>> + }
>> +
>> + thermal_zone_device_update(pzone->therm_dev);
>> + pr_debug("db8500_thermal_work finished.\n");
>> +}
>> +
>> +static int __devinit db8500_thermal_probe(struct platform_device *pdev)
>> +{
>> + struct db8500_thermal_zone *pzone = NULL;
>> + struct db8500_thsens_platform_data *ptrips;
>> + int low_irq, high_irq, ret = 0;
>> + unsigned long dft_low, dft_high;
>> +
>> + pzone = devm_kzalloc(&pdev->dev,
>> + sizeof(struct db8500_thermal_zone), GFP_KERNEL);
>> + if (!pzone)
>> + return -ENOMEM;
>> +
>> + pzone->thsens_pdev = pdev;
>> +
>> + low_irq = platform_get_irq_byname(pdev, "IRQ_HOTMON_LOW");
>> + if (low_irq < 0) {
>> + pr_err("Get IRQ_HOTMON_LOW failed.\n");
>> + return low_irq;
>> + }
>> +
>> + ret = devm_request_threaded_irq(&pdev->dev, low_irq, NULL,
>> + prcmu_low_irq_handler,
>> + IRQF_NO_SUSPEND | IRQF_ONESHOT, "dbx500_temp_low", pzone);
>> + if (ret < 0) {
>> + pr_err("Failed to allocate temp low irq.\n");
>> + return ret;
>> + }
>> +
>> + high_irq = platform_get_irq_byname(pdev, "IRQ_HOTMON_HIGH");
>> + if (high_irq < 0) {
>> + pr_err("Get IRQ_HOTMON_HIGH failed.\n");
>> + return high_irq;
>> + }
>> +
>> + ret = devm_request_threaded_irq(&pdev->dev, high_irq, NULL,
>> + prcmu_high_irq_handler,
>> + IRQF_NO_SUSPEND | IRQF_ONESHOT, "dbx500_temp_high", pzone);
>> + if (ret < 0) {
>> + pr_err("Failed to allocate temp high irq.\n");
>> + return ret;
>> + }
>> +
>> + pzone->low_irq = low_irq;
>> + pzone->high_irq = high_irq;
>> +
>> + pzone->mode = THERMAL_DEVICE_DISABLED;
>> +
>> + mutex_init(&pzone->th_lock);
>> +
>> + INIT_WORK(&pzone->therm_work, db8500_thermal_work);
>> +
>> + ptrips = pdev->dev.platform_data;
>> + pzone->trip_tab = ptrips;
>> +
>> + pzone->therm_dev = thermal_zone_device_register("db8500_thermal_zone",
>> + ptrips->num_trips, 0, pzone, &thdev_ops, 0, 0);
>> +
>> + if (IS_ERR(pzone->therm_dev)) {
>> + pr_err("Failed to register thermal zone device\n");
>> + return PTR_ERR(pzone->therm_dev);
>> + }
>> +
>> + dft_low = PRCMU_DEFAULT_LOW_TEMP;
>> + dft_high = ptrips->trip_points[0].temp;
>> +
>> + prcmu_stop_temp_sense();
>> + prcmu_config_hotmon((u8)(dft_low/1000), (u8)(dft_high/1000));
>> + prcmu_start_temp_sense(PRCMU_DEFAULT_MEASURE_TIME);
>> +
>> + pzone->cur_index = 0;
>> + pzone->cur_temp_pseudo = (dft_low + dft_high)/2;
>> + pzone->trend = THERMAL_TREND_STABLE;
>
> All the stuff from prcmu_stop_temp_sense() up to this line can race with
> the irq handlers, I would suggest doing it before requesting the irqs.
>
>> + pzone->mode = THERMAL_DEVICE_ENABLED;
>
> Shouldn't this be protected by pzone->th_lock? Otherwise it should be
> set before the thermal zone is registered.
>
>> +
>> + platform_set_drvdata(pdev, pzone);
>> +
>> + return 0;
>> +}
>> +
>> +static int __devexit db8500_thermal_remove(struct platform_device *pdev)
>> +{
>> + struct db8500_thermal_zone *pzone;
>> + pzone = platform_get_drvdata(pdev);
>> +
>> + cancel_work_sync(&pzone->therm_work);
>> +
>> + if (pzone->therm_dev)
>> + thermal_zone_device_unregister(pzone->therm_dev);
>> +
>> + return 0;
>> +}
>
> mutex_destroy() should be called on pzone->th_lock
>
>> +
>> +static int db8500_thermal_suspend(struct platform_device *pdev,
>> + pm_message_t state)
>> +{
>> + struct db8500_thermal_zone *pzone;
>> + pzone = platform_get_drvdata(pdev);
>> +
>> + flush_work_sync(&pzone->therm_work);
>> + prcmu_stop_temp_sense();
>> +
>> + return 0;
>> +}
>> +
>> +static int db8500_thermal_resume(struct platform_device *pdev)
>> +{
>> + struct db8500_thermal_zone *pzone;
>> + struct db8500_thsens_platform_data *ptrips;
>> + unsigned long dft_low, dft_high;
>> +
>> + pzone = platform_get_drvdata(pdev);
>> + ptrips = pzone->trip_tab;
>> + dft_low = PRCMU_DEFAULT_LOW_TEMP;
>> + dft_high = ptrips->trip_points[0].temp;
>> +
>> + prcmu_config_hotmon((u8)(dft_low/1000), (u8)(dft_high/1000));
>> + prcmu_start_temp_sense(PRCMU_DEFAULT_MEASURE_TIME);
>
> Shouldn't cur_index and cur_temp_pseudo be updated as well? You may want
> to define a helper function with all the code shared by irq handlers
> (both high and low), probe and resume.
No, they cannot be update because we don't know the actual current
temp[1] after short or long time suspend, everything goes as
beginning.
If a helper function is introduced, it can be only used in probe and
resume I think, different and a bit complicated algorithm in irq
handlers.
[1] due to lack of corresponding interface, search "TODO" in this file
to get more explanation.
>
>> +
>> + return 0;
>> +}
> [...]
>> diff --git a/include/linux/platform_data/db8500_thermal.h b/include/linux/platform_data/db8500_thermal.h
>> new file mode 100644
>> index 0000000..0b6d164
>> --- /dev/null
>> +++ b/include/linux/platform_data/db8500_thermal.h
>> @@ -0,0 +1,39 @@
>> +/*
>> + * db8500_thermal.h - db8500 Thermal Management Implementation
>> + *
>> + * Copyright (C) 2012 ST-Ericsson
>> + * Copyright (C) 2012 Linaro Ltd.
>> + *
>> + * Author: Hongbo Zhang <hognbo.zhang@stericsson.com>
>
> Misspelled address
>
> --
> Francesco
^ permalink raw reply
* [PATCH V2] PM / OPP: predictable fail results for opp_find* functions
From: Nishanth Menon @ 2012-10-22 13:33 UTC (permalink / raw)
To: linux-pm
Cc: Nishanth Menon, MyungJoo Ham, Kyungmin Park, Rafael J. Wysocki,
Kevin Hilman, linux-kernel
Currently the opp_find* functions return -ENODEV when:
a) it cant find a device (e.g. request for an OPP search on device
which was not registered)
b) When it cant find a match for the search strategy used
This makes life a little in-efficient for users such as devfreq
to make reasonable judgement before switching search strategies.
So, standardize the return results as following:
-EINVAL for bad pointer parameters
-ENODEV when device cannot be found
-ERANGE when search fails
This has the following benefit for devfreq implementation:
The search fails when an unregistered device pointer is provided.
This is a trigger to change the search direction and search for
a better fit, however, if we cannot differentiate between a valid
search range failure Vs an unregistered device, second search goes
through the same fail return condition. This can be avoided by
appropriate handling of error return code.
With this change, we also fix devfreq for the improved search
strategy with updated error code.
Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Kevin Hilman <khilman@ti.com>
Cc: linux-pm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Reviewed-by: Kevin Hilman <khilman@ti.com>
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
V2:
* commit message change - updated frome example code, to verbose description
* picked up Acked-by and Reviewed-by tags from V1.
* rebased to v3.7-rc2
V1: https://patchwork.kernel.org/patch/1582111/
drivers/base/power/opp.c | 27 +++++++++++++++++++--------
drivers/devfreq/devfreq.c | 4 ++--
2 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index 4730a58..99d0573 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -234,7 +234,10 @@ EXPORT_SYMBOL(opp_get_opp_count);
*
* Searches for exact match in the opp list and returns pointer to the matching
* opp if found, else returns ERR_PTR in case of error and should be handled
- * using IS_ERR.
+ * using IS_ERR. Error return values can be:
+ * EINVAL: for bad pointer
+ * ERANGE: no match found for search
+ * ENODEV: if device not found in list of registered devices
*
* Note: available is a modifier for the search. if available=true, then the
* match is for exact matching frequency and is available in the stored OPP
@@ -253,7 +256,7 @@ struct opp *opp_find_freq_exact(struct device *dev, unsigned long freq,
bool available)
{
struct device_opp *dev_opp;
- struct opp *temp_opp, *opp = ERR_PTR(-ENODEV);
+ struct opp *temp_opp, *opp = ERR_PTR(-ERANGE);
dev_opp = find_device_opp(dev);
if (IS_ERR(dev_opp)) {
@@ -283,7 +286,11 @@ EXPORT_SYMBOL(opp_find_freq_exact);
* for a device.
*
* Returns matching *opp and refreshes *freq accordingly, else returns
- * ERR_PTR in case of error and should be handled using IS_ERR.
+ * ERR_PTR in case of error and should be handled using IS_ERR. Error return
+ * values can be:
+ * EINVAL: for bad pointer
+ * ERANGE: no match found for search
+ * ENODEV: if device not found in list of registered devices
*
* Locking: This function must be called under rcu_read_lock(). opp is a rcu
* protected pointer. The reason for the same is that the opp pointer which is
@@ -294,7 +301,7 @@ EXPORT_SYMBOL(opp_find_freq_exact);
struct opp *opp_find_freq_ceil(struct device *dev, unsigned long *freq)
{
struct device_opp *dev_opp;
- struct opp *temp_opp, *opp = ERR_PTR(-ENODEV);
+ struct opp *temp_opp, *opp = ERR_PTR(-ERANGE);
if (!dev || !freq) {
dev_err(dev, "%s: Invalid argument freq=%p\n", __func__, freq);
@@ -303,7 +310,7 @@ struct opp *opp_find_freq_ceil(struct device *dev, unsigned long *freq)
dev_opp = find_device_opp(dev);
if (IS_ERR(dev_opp))
- return opp;
+ return ERR_CAST(dev_opp);
list_for_each_entry_rcu(temp_opp, &dev_opp->opp_list, node) {
if (temp_opp->available && temp_opp->rate >= *freq) {
@@ -326,7 +333,11 @@ EXPORT_SYMBOL(opp_find_freq_ceil);
* for a device.
*
* Returns matching *opp and refreshes *freq accordingly, else returns
- * ERR_PTR in case of error and should be handled using IS_ERR.
+ * ERR_PTR in case of error and should be handled using IS_ERR. Error return
+ * values can be:
+ * EINVAL: for bad pointer
+ * ERANGE: no match found for search
+ * ENODEV: if device not found in list of registered devices
*
* Locking: This function must be called under rcu_read_lock(). opp is a rcu
* protected pointer. The reason for the same is that the opp pointer which is
@@ -337,7 +348,7 @@ EXPORT_SYMBOL(opp_find_freq_ceil);
struct opp *opp_find_freq_floor(struct device *dev, unsigned long *freq)
{
struct device_opp *dev_opp;
- struct opp *temp_opp, *opp = ERR_PTR(-ENODEV);
+ struct opp *temp_opp, *opp = ERR_PTR(-ERANGE);
if (!dev || !freq) {
dev_err(dev, "%s: Invalid argument freq=%p\n", __func__, freq);
@@ -346,7 +357,7 @@ struct opp *opp_find_freq_floor(struct device *dev, unsigned long *freq)
dev_opp = find_device_opp(dev);
if (IS_ERR(dev_opp))
- return opp;
+ return ERR_CAST(dev_opp);
list_for_each_entry_rcu(temp_opp, &dev_opp->opp_list, node) {
if (temp_opp->available) {
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 0920d75..bee1c29 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -656,14 +656,14 @@ struct opp *devfreq_recommended_opp(struct device *dev, unsigned long *freq,
opp = opp_find_freq_floor(dev, freq);
/* If not available, use the closest opp */
- if (opp == ERR_PTR(-ENODEV))
+ if (opp == ERR_PTR(-ERANGE))
opp = opp_find_freq_ceil(dev, freq);
} else {
/* The freq is an lower bound. opp should be higher */
opp = opp_find_freq_ceil(dev, freq);
/* If not available, use the closest opp */
- if (opp == ERR_PTR(-ENODEV))
+ if (opp == ERR_PTR(-ERANGE))
opp = opp_find_freq_floor(dev, freq);
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH] cpufreq: fix jiffies/cputime mixup in conservative/ondemand governors
From: Andreas Schwab @ 2012-10-22 13:35 UTC (permalink / raw)
To: cpufreq; +Cc: linux-pm
The function get_cpu_idle_time_jiffy in both the conservative and
ondemand governors use jiffies_to_usecs to convert a cputime value to
usecs which gives the wrong value on architectures where cputime and
jiffies use different units. Only matters if NO_HZ is disabled, since
otherwise get_cpu_idle_time_us should already return a valid value, and
get_cpu_idle_time_jiffy isn't actually called.
Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
---
drivers/cpufreq/cpufreq_conservative.c | 4 ++--
drivers/cpufreq/cpufreq_ondemand.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
index a152af7..96af7d5 100644
--- a/drivers/cpufreq/cpufreq_conservative.c
+++ b/drivers/cpufreq/cpufreq_conservative.c
@@ -112,9 +112,9 @@ static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall)
idle_time = cur_wall_time - busy_time;
if (wall)
- *wall = jiffies_to_usecs(cur_wall_time);
+ *wall = cputime_to_usecs(cur_wall_time);
- return jiffies_to_usecs(idle_time);
+ return cputime_to_usecs(idle_time);
}
static inline cputime64_t get_cpu_idle_time(unsigned int cpu, cputime64_t *wall)
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 396322f..6c19a66 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -136,9 +136,9 @@ static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall)
idle_time = cur_wall_time - busy_time;
if (wall)
- *wall = jiffies_to_usecs(cur_wall_time);
+ *wall = cputime_to_usecs(cur_wall_time);
- return jiffies_to_usecs(idle_time);
+ return cputime_to_usecs(idle_time);
}
static inline cputime64_t get_cpu_idle_time(unsigned int cpu, cputime64_t *wall)
--
1.8.0
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply related
* Re: [RFC PATCH v2 2/6] PM / Runtime: introduce pm_runtime_set_memalloc_noio()
From: Alan Stern @ 2012-10-22 14:33 UTC (permalink / raw)
To: Ming Lei
Cc: linux-kernel, Oliver Neukum, Minchan Kim, Greg Kroah-Hartman,
Rafael J. Wysocki, Jens Axboe, David S. Miller, Andrew Morton,
netdev, linux-usb, linux-pm, linux-mm
In-Reply-To: <1350894794-1494-3-git-send-email-ming.lei@canonical.com>
On Mon, 22 Oct 2012, Ming Lei wrote:
> +void pm_runtime_set_memalloc_noio(struct device *dev, bool enable)
> +{
> + dev->power.memalloc_noio_resume = enable;
> +
> + if (!dev->parent)
> + return;
> +
> + if (enable) {
> + pm_runtime_set_memalloc_noio(dev->parent, 1);
> + } else {
> + /* only clear the flag for one device if all
> + * children of the device don't set the flag.
> + */
> + if (device_for_each_child(dev->parent, NULL,
> + dev_memalloc_noio))
> + return;
> +
> + pm_runtime_set_memalloc_noio(dev->parent, 0);
> + }
> +}
> +EXPORT_SYMBOL_GPL(pm_runtime_set_memalloc_noio);
Tail recursion should be implemented as a loop, not as an explicit
recursion. That is, the function should be:
void pm_runtime_set_memalloc_noio(struct device *dev, bool enable)
{
do {
dev->power.memalloc_noio_resume = enable;
if (!enable) {
/*
* Don't clear the parent's flag if any of the
* parent's children have their flag set.
*/
if (device_for_each_child(dev->parent, NULL,
dev_memalloc_noio))
return;
}
dev = dev->parent;
} while (dev);
}
except that you need to add locking, for two reasons:
There's a race. What happens if another child sets the flag
between the time device_for_each_child() runs and the next loop
iteration?
Even without a race, access to bitfields is not SMP-safe
without locking.
Alan Stern
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC PATCH v2 6/6] USB: forbid memory allocation with I/O during bus reset
From: Alan Stern @ 2012-10-22 14:37 UTC (permalink / raw)
To: Ming Lei
Cc: linux-kernel, Oliver Neukum, Minchan Kim, Greg Kroah-Hartman,
Rafael J. Wysocki, Jens Axboe, David S. Miller, Andrew Morton,
netdev, linux-usb, linux-pm, linux-mm
In-Reply-To: <1350894794-1494-7-git-send-email-ming.lei@canonical.com>
On Mon, 22 Oct 2012, Ming Lei wrote:
> If one storage interface or usb network interface(iSCSI case)
> exists in current configuration, memory allocation with
> GFP_KERNEL during usb_device_reset() might trigger I/O transfer
> on the storage interface itself and cause deadlock because
> the 'us->dev_mutex' is held in .pre_reset() and the storage
> interface can't do I/O transfer when the reset is triggered
> by other interface, or the error handling can't be completed
> if the reset is triggered by the storage itself(error handling path).
>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Cc: Oliver Neukum <oneukum@suse.de>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
> drivers/usb/core/hub.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 522ad57..106a80a 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -5038,6 +5038,7 @@ int usb_reset_device(struct usb_device *udev)
> {
> int ret;
> int i;
> + unsigned int noio_flag;
> struct usb_host_config *config = udev->actconfig;
>
> if (udev->state == USB_STATE_NOTATTACHED ||
> @@ -5047,6 +5048,15 @@ int usb_reset_device(struct usb_device *udev)
> return -EINVAL;
> }
>
> + /*
> + * Don't allocate memory with GFP_KERNEL in current
> + * context to avoid possible deadlock if usb mass
> + * storage interface or usbnet interface(iSCSI case)
> + * is included in current configuration. The easiest
> + * approach is to do it for all devices.
> + */
> + memalloc_noio_save(noio_flag);
Why not check dev->power.memalloc_noio_resume here too?
Alan Stern
^ permalink raw reply
* Re: USB port power off discussion
From: Sarah Sharp @ 2012-10-22 18:08 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Alan Stern, Lan Tianyu, Greg KH, Len Brown, Arjan Van De Ven,
Oliver Neukum, linux-usb, linux-pm
In-Reply-To: <84398681.LDNNF19qAk@vostro.rjw.lan>
On Mon, Oct 15, 2012 at 08:42:47AM +0200, Rafael J. Wysocki wrote:
> Sorry for the delay, I have been distracted by a number of things.
Me too. :)
> On Monday 24 of September 2012 15:10:36 Sarah Sharp wrote:
> > On Tue, Sep 25, 2012 at 12:09:06AM +0200, Rafael J. Wysocki wrote:
> > > What about hubs connected to such ports? I don't think they're going to
> > > work when power is removed from it, are they?
> >
> > USB hubs would have remote wakeup enabled, so we would never power off
> > their port with the "auto" policy.
>
> Here's my idea how to arrange things.
>
> Why don't make runtime PM manage the USB port power on/off such that the
> port's .runtime_suspend() routine will remove power from it, if
> PM_QOS_FLAG_NO_POWER_OFF is not (effectively) set for it, and its
> .runtime_resume() will restore power to it (if removed previously).
Ok, so you're basically proposing we power off suspended devices, except
when userspace (or perhaps the kernel) sets PM_QOS_FLAG_NO_POWER_OFF?
What happens if userspace clears the PM_QOS_FLAG_NO_POWER_OFF flag while
the port is suspended? Does the USB core then re-power it on? Should
it also resume the device?
> The USB core will then do pm_runtime_get_sync() on the port every time
> a device depending on it is added and pm_runtime_put() every time such
> a device is removed.
Are you saying that every time a device is connected to an _external_
hub, the USB core would call pm_runtime_get_sync()? If you apply that
policy to roothubs, then you're basically disabling port power off when
a device is connected, which doesn't make sense in conjunction to your
last sentence, so now I'm just confused, sorry.
Wouldn't it make more sense to call pm_runtime_get_sync() when remote
wakeup is enabled for a device, and pm_runtime_put() when remote wakeup
is disabled? That way, when an external hub is attached to the port, it
always has remote wakeup enabled, so we will always increment the PM
counter. [1]
> The initial setting of PM_QOS_FLAG_NO_POWER_OFF in the PM QoS request
> structure used by user space will depend on the type of the port (e.g.
> it will be unset for ports that aren't visible to the user and connectable).
>
> That should allow things to work automatically and it should allow user space
> to override the defaults in any case, either by disabling runtime PM for the
> ports altogether (by writing "on" to their device objects' "control" sysfs
> files), or by setting/unsetting PM_QOS_FLAG_NO_POWER_OFF in the PM QoS
> request controlled by it as desired.
Are you then pushing the policy decision for whether ports should be
powered off completely into userspace then? I.e. you want userspace to
read the ACPI port connect type info and clear PM_QOS_FLAG_NO_POWER_OFF
in the userspace structure if the port is not user visible and not
connectable?
Or are you thinking the kernel will set PM_QOS_FLAG_NO_POWER_OFF based
on the ACPI tables and userspace will have the option to override it?
We also probably need to allow USB interface drivers to set
PM_QOS_FLAG_NO_POWER_OFF as well, for cases like devices that have
firmware or other internal state. We probably need a USB core API to
add a request for PM_QOS_FLAG_NO_POWER_OFF to be set, and a way to clear
that flag if the driver is unbound or the driver decides it's safe to
power off the device.
> > There are things like USB lights or fans that pull port power, but don't
> > actually enumerate as a USB device. So to the OS, those would be
> > "empty" ports. But they would be external ports, so the default "auto"
> > policy would never power off external ports. The user could write a
> > small script to PWM the fan on and off to control the speed. It would
> > just be a novelty though, as Alan said.
>
> For those things we can provide a "raw port driver" with ioctls allowing
> user space to get the port's physical location information from the kernel,
> if available, and manipulate the port's power on the low level.
Sure, I think it's fine to push that corner case off to usbdevfs.
Sarah Sharp
[1] The above sentence doesn't apply to VIA USB 3.0 hubs, because they
don't actually advertise remote wakeup in their device descriptors.
However, the USB core will never suspend those hubs, so we'll never
power off the USB 3.0 port the device is connected to. Not sure how
they'll react to having the USB 2.0 portion powered off while the USB
3.0 potion is powered. We'll see. I really hate these uncertified
hubs.
^ permalink raw reply
* Re: [RFC 3/4] usb: Send Set SEL before enabling parent U1/U2 timeout.
From: Sarah Sharp @ 2012-10-22 18:37 UTC (permalink / raw)
To: Felipe Balbi; +Cc: Alan Stern, linux-usb, linux-pm, Rafael J. Wysocki
In-Reply-To: <20121019161800.GA1493@arwen.pp.htv.fi>
On Fri, Oct 19, 2012 at 07:18:00PM +0300, Felipe Balbi wrote:
> Hi,
>
> On Mon, Oct 08, 2012 at 03:47:35PM -0700, Sarah Sharp wrote:
> > On Mon, Oct 08, 2012 at 04:05:00PM -0400, Alan Stern wrote:
> > > On Mon, 8 Oct 2012, Sarah Sharp wrote:
> > >
> > > > The Set SEL control transfer tells a device the exit latencies
> > > > associated with a device-initated U1 or U2 exit. Since a parent hub may
> > > > initiate a transition to U1 soon after a downstream port's U1 timeout is
> > > > set, we need to make sure the device receives the Set SEL transfer
> > > > before the parent hub timeout is set.
> > >
> > > Just a question, not a comment on the patch. Why is "Set SEL" needed?
> > > Doesn't the device already know its own exit latencies? After all, the
> > > values come from descriptors that were originally provided by the
> > > device.
> >
> > The device knows its own exit latency, but it doesn't know the exit
> > latencies for the path above it. It could be directly attached to the
> > roothub (which may have a bigger exit latency than the device) or a
> > couple tiers deep in the tree. If the device wants to be smart about
> > when it requests to come out of U1 or U2, then it needs to know the
> > whole path exit latency.
> >
> > For example, maybe there's an interrupt endpoint on a self-powered radio
> > device. As soon as the device doesn't have any data to send, it
> > transmits an NRDY (Not Ready) response to the request for data. Then it
> > puts its link into U1 or U2 (or maybe the host controller does this when
> > the U1/U2 timeouts expire).
> >
> > Later, the device starts receiving a packet. Let's assume it takes some
> > time to process that packet. If the device knows how long it takes to
> > wake up the link and send an ERDY to the host, it can parallelize the
> > link wakeup and the packet processing. It doesn't want to send an ERDY
> > too soon, because the host could ask for the data before it's ready. So
> > the device needs to know the time it takes for the link to come out of
> > U1 or U2, the time it takes for the host to process the ERDY, and the
> > time it takes for the request for data to reach the device. That's what
> > the Set SEL request does.
> >
> > Of course, I'm not a USB device designer, so I don't know how the Set
> > SEL command is really used. Felipe, do we use it at all in any of the
> > Linux USB 3.0 gadget drivers?
>
> sorry for the delay. Currently we're not using Set SEL at all, but we
> could use, eventually, to pass it to pm_qos ?!? Just a thought.
Yes, I think it could be useful to pass that information on to pm_qos.
I don't think the subsystem currently supports multiple power states
though, but Rafael would know. On the host side, it's all custom code
to deal with the USB device U1/U2 low power states. Only the U3 (device
suspend) is handled by the runtime PM subsystem.
Rafael is adding support for a power-off pm_qos flag, and I'm not sure
if it makes sense to add support for multiple power state flags as well.
Probably we should see how the new pm_qos flag architecture shakes out,
and then decide whether we should tackle that.
Sarah Sharp
^ permalink raw reply
* Re: USB port power off discussion
From: Alan Stern @ 2012-10-22 18:37 UTC (permalink / raw)
To: Sarah Sharp
Cc: Rafael J. Wysocki, Lan Tianyu, Greg KH, Len Brown,
Arjan Van De Ven, Oliver Neukum, USB list, Linux-pm mailing list
In-Reply-To: <20121022180844.GC5378@xanatos>
On Mon, 22 Oct 2012, Sarah Sharp wrote:
> On Mon, Oct 15, 2012 at 08:42:47AM +0200, Rafael J. Wysocki wrote:
> > Sorry for the delay, I have been distracted by a number of things.
>
> Me too. :)
>
> > On Monday 24 of September 2012 15:10:36 Sarah Sharp wrote:
> > > On Tue, Sep 25, 2012 at 12:09:06AM +0200, Rafael J. Wysocki wrote:
> > > > What about hubs connected to such ports? I don't think they're going to
> > > > work when power is removed from it, are they?
> > >
> > > USB hubs would have remote wakeup enabled, so we would never power off
> > > their port with the "auto" policy.
> >
> > Here's my idea how to arrange things.
> >
> > Why don't make runtime PM manage the USB port power on/off such that the
> > port's .runtime_suspend() routine will remove power from it, if
> > PM_QOS_FLAG_NO_POWER_OFF is not (effectively) set for it, and its
> > .runtime_resume() will restore power to it (if removed previously).
>
> Ok, so you're basically proposing we power off suspended devices, except
> when userspace (or perhaps the kernel) sets PM_QOS_FLAG_NO_POWER_OFF?
Rafael is suggesting that the interface to control when ports get
powered off should be associated with the usb_port device, rather than
with the usb_device attached to the port.
> What happens if userspace clears the PM_QOS_FLAG_NO_POWER_OFF flag while
> the port is suspended? Does the USB core then re-power it on? Should
> it also resume the device?
I don't know how the PM QOS flags interact with runtime PM. Rafael may
still be working on that.
At any rate, runtime suspend for the usb_port will remove power.
Runtime resume will turn power back on. But if there's a usb_device
attached to the port, resuming the port will not necessarily resume the
device. (However, we will be careful to make sure that the port cannot
be runtime suspended unless the device is suspended first.)
> > The USB core will then do pm_runtime_get_sync() on the port every time
> > a device depending on it is added and pm_runtime_put() every time such
> > a device is removed.
>
> Are you saying that every time a device is connected to an _external_
> hub, the USB core would call pm_runtime_get_sync()? If you apply that
> policy to roothubs, then you're basically disabling port power off when
> a device is connected, which doesn't make sense in conjunction to your
> last sentence, so now I'm just confused, sorry.
He is saying that the core would call pm_runtime_get_sync(&port->dev),
not pm_runtime_get_sync(&udev->dev). And this would only be for while
the device was active, not for when the device is suspended (unless the
device can't handle loss of power).
> Wouldn't it make more sense to call pm_runtime_get_sync() when remote
> wakeup is enabled for a device, and pm_runtime_put() when remote wakeup
> is disabled? That way, when an external hub is attached to the port, it
> always has remote wakeup enabled, so we will always increment the PM
> counter. [1]
>
> > The initial setting of PM_QOS_FLAG_NO_POWER_OFF in the PM QoS request
> > structure used by user space will depend on the type of the port (e.g.
> > it will be unset for ports that aren't visible to the user and connectable).
> >
> > That should allow things to work automatically and it should allow user space
> > to override the defaults in any case, either by disabling runtime PM for the
> > ports altogether (by writing "on" to their device objects' "control" sysfs
> > files), or by setting/unsetting PM_QOS_FLAG_NO_POWER_OFF in the PM QoS
> > request controlled by it as desired.
>
> Are you then pushing the policy decision for whether ports should be
> powered off completely into userspace then? I.e. you want userspace to
> read the ACPI port connect type info and clear PM_QOS_FLAG_NO_POWER_OFF
> in the userspace structure if the port is not user visible and not
> connectable?
>
> Or are you thinking the kernel will set PM_QOS_FLAG_NO_POWER_OFF based
> on the ACPI tables and userspace will have the option to override it?
The latter.
> We also probably need to allow USB interface drivers to set
> PM_QOS_FLAG_NO_POWER_OFF as well, for cases like devices that have
> firmware or other internal state. We probably need a USB core API to
> add a request for PM_QOS_FLAG_NO_POWER_OFF to be set, and a way to clear
> that flag if the driver is unbound or the driver decides it's safe to
> power off the device.
How about adding a no_power_off flag to the usb_interface structure,
analogous to the needs_remote_wakeup flag?
Alan Stern
^ permalink raw reply
* Re: [PATCH 5/5] Thermal: Add ST-Ericsson db8500 thermal dirver.
From: Francesco Lavra @ 2012-10-22 18:51 UTC (permalink / raw)
To: Hongbo Zhang
Cc: linaro-dev, linux-kernel, linux-pm, STEricsson_nomadik_linux,
kernel, linaro-kernel, hongbo.zhang, patches
In-Reply-To: <CAJLyvQxC8eNRmB40o18JR0k7f38+=fpt4UimSYxDCNzNH03Cfw@mail.gmail.com>
On 10/22/2012 02:02 PM, Hongbo Zhang wrote:
[...]
>>> +static irqreturn_t prcmu_low_irq_handler(int irq, void *irq_data)
>>> +{
>>> + struct db8500_thermal_zone *pzone = irq_data;
>>> + struct db8500_thsens_platform_data *ptrips;
>>> + unsigned long next_low, next_high;
>>> + unsigned int idx;
>>> +
>>> + ptrips = pzone->trip_tab;
>>> + idx = pzone->cur_index;
>>> + if (unlikely(idx == 0))
>>> + /* Meaningless for thermal management, ignoring it */
>>> + return IRQ_HANDLED;
>>> +
>>> + if (idx == 1) {
>>> + next_high = ptrips->trip_points[0].temp;
>>> + next_low = PRCMU_DEFAULT_LOW_TEMP;
>>> + } else {
>>> + next_high = ptrips->trip_points[idx-1].temp;
>>> + next_low = ptrips->trip_points[idx-2].temp;
>>> + }
>>> +
>>> + pzone->cur_index -= 1;
>>> + pzone->cur_temp_pseudo = (next_high + next_low)/2;
>>> +
>>> + prcmu_stop_temp_sense();
>>> + prcmu_config_hotmon((u8)(next_low/1000), (u8)(next_high/1000));
>>> + prcmu_start_temp_sense(PRCMU_DEFAULT_MEASURE_TIME);
>>> +
>>> + pr_debug("PRCMU set max %ld, set min %ld\n", next_high, next_low);
>>> +
>>> + pzone->trend = THERMAL_TREND_DROPPING;
>>> + schedule_work(&pzone->therm_work);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static irqreturn_t prcmu_high_irq_handler(int irq, void *irq_data)
>>> +{
>>> + struct db8500_thermal_zone *pzone = irq_data;
>>> + struct db8500_thsens_platform_data *ptrips;
>>> + unsigned long next_low, next_high;
>>> + unsigned int idx;
>>> +
>>> + ptrips = pzone->trip_tab;
>>> + idx = pzone->cur_index;
>>> +
>>> + if (idx < ptrips->num_trips - 1) {
>>> + next_high = ptrips->trip_points[idx+1].temp;
>>> + next_low = ptrips->trip_points[idx].temp;
>>> +
>>> + pzone->cur_index += 1;
>>> + pzone->cur_temp_pseudo = (next_high + next_low)/2;
>>> +
>>> + prcmu_stop_temp_sense();
>>> + prcmu_config_hotmon((u8)(next_low/1000), (u8)(next_high/1000));
>>> + prcmu_start_temp_sense(PRCMU_DEFAULT_MEASURE_TIME);
>>> +
>>> + pr_debug("PRCMU set max %ld, min %ld\n", next_high, next_low);
>>> + }
>>> +
>>> + if (idx == ptrips->num_trips - 1)
>>
>> } else if ()
> There is no else condition here, because it it the highest critical
> trip point, system will be shut down in thermal_work.
> But I'd like to add some comments here to state this situation.
I didn't mean adding a new else condition, what I meant is that if the
first condition (idx < ptrips->num_trips - 1) is verified, then there is
no need to check for the second condition (idx == ptrips->num_trips -
1). So I was thinking of changing the code to:
if (idx < ptrips->num_trips - 1)
...
else if (idx == ptrips->num_trips - 1)
...
Sorry if I wasn't clear.
>>
>>> + pzone->cur_temp_pseudo = ptrips->trip_points[idx].temp + 1;
>>> +
>>> + pzone->trend = THERMAL_TREND_RAISING;
>>> + schedule_work(&pzone->therm_work);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static void db8500_thermal_work(struct work_struct *work)
>>> +{
>>> + enum thermal_device_mode cur_mode;
>>> + struct db8500_thermal_zone *pzone;
>>> +
>>> + pzone = container_of(work, struct db8500_thermal_zone, therm_work);
>>> +
>>> + mutex_lock(&pzone->th_lock);
>>> + cur_mode = pzone->mode;
>>> + mutex_unlock(&pzone->th_lock);
>>> +
>>> + if (cur_mode == THERMAL_DEVICE_DISABLED) {
>>> + pr_warn("Warning: thermal function disabled.\n");
>>> + return;
>>> + }
>>> +
>>> + thermal_zone_device_update(pzone->therm_dev);
>>> + pr_debug("db8500_thermal_work finished.\n");
>>> +}
>>> +
>>> +static int __devinit db8500_thermal_probe(struct platform_device *pdev)
>>> +{
>>> + struct db8500_thermal_zone *pzone = NULL;
>>> + struct db8500_thsens_platform_data *ptrips;
>>> + int low_irq, high_irq, ret = 0;
>>> + unsigned long dft_low, dft_high;
>>> +
>>> + pzone = devm_kzalloc(&pdev->dev,
>>> + sizeof(struct db8500_thermal_zone), GFP_KERNEL);
>>> + if (!pzone)
>>> + return -ENOMEM;
>>> +
>>> + pzone->thsens_pdev = pdev;
>>> +
>>> + low_irq = platform_get_irq_byname(pdev, "IRQ_HOTMON_LOW");
>>> + if (low_irq < 0) {
>>> + pr_err("Get IRQ_HOTMON_LOW failed.\n");
>>> + return low_irq;
>>> + }
>>> +
>>> + ret = devm_request_threaded_irq(&pdev->dev, low_irq, NULL,
>>> + prcmu_low_irq_handler,
>>> + IRQF_NO_SUSPEND | IRQF_ONESHOT, "dbx500_temp_low", pzone);
>>> + if (ret < 0) {
>>> + pr_err("Failed to allocate temp low irq.\n");
>>> + return ret;
>>> + }
>>> +
>>> + high_irq = platform_get_irq_byname(pdev, "IRQ_HOTMON_HIGH");
>>> + if (high_irq < 0) {
>>> + pr_err("Get IRQ_HOTMON_HIGH failed.\n");
>>> + return high_irq;
>>> + }
>>> +
>>> + ret = devm_request_threaded_irq(&pdev->dev, high_irq, NULL,
>>> + prcmu_high_irq_handler,
>>> + IRQF_NO_SUSPEND | IRQF_ONESHOT, "dbx500_temp_high", pzone);
>>> + if (ret < 0) {
>>> + pr_err("Failed to allocate temp high irq.\n");
>>> + return ret;
>>> + }
>>> +
>>> + pzone->low_irq = low_irq;
>>> + pzone->high_irq = high_irq;
>>> +
>>> + pzone->mode = THERMAL_DEVICE_DISABLED;
>>> +
>>> + mutex_init(&pzone->th_lock);
>>> +
>>> + INIT_WORK(&pzone->therm_work, db8500_thermal_work);
>>> +
>>> + ptrips = pdev->dev.platform_data;
>>> + pzone->trip_tab = ptrips;
>>> +
>>> + pzone->therm_dev = thermal_zone_device_register("db8500_thermal_zone",
>>> + ptrips->num_trips, 0, pzone, &thdev_ops, 0, 0);
>>> +
>>> + if (IS_ERR(pzone->therm_dev)) {
>>> + pr_err("Failed to register thermal zone device\n");
>>> + return PTR_ERR(pzone->therm_dev);
>>> + }
>>> +
>>> + dft_low = PRCMU_DEFAULT_LOW_TEMP;
>>> + dft_high = ptrips->trip_points[0].temp;
>>> +
>>> + prcmu_stop_temp_sense();
>>> + prcmu_config_hotmon((u8)(dft_low/1000), (u8)(dft_high/1000));
>>> + prcmu_start_temp_sense(PRCMU_DEFAULT_MEASURE_TIME);
>>> +
>>> + pzone->cur_index = 0;
>>> + pzone->cur_temp_pseudo = (dft_low + dft_high)/2;
>>> + pzone->trend = THERMAL_TREND_STABLE;
>>
>> All the stuff from prcmu_stop_temp_sense() up to this line can race with
>> the irq handlers, I would suggest doing it before requesting the irqs.
>>
>>> + pzone->mode = THERMAL_DEVICE_ENABLED;
>>
>> Shouldn't this be protected by pzone->th_lock? Otherwise it should be
>> set before the thermal zone is registered.
>>
>>> +
>>> + platform_set_drvdata(pdev, pzone);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __devexit db8500_thermal_remove(struct platform_device *pdev)
>>> +{
>>> + struct db8500_thermal_zone *pzone;
>>> + pzone = platform_get_drvdata(pdev);
>>> +
>>> + cancel_work_sync(&pzone->therm_work);
>>> +
>>> + if (pzone->therm_dev)
>>> + thermal_zone_device_unregister(pzone->therm_dev);
>>> +
>>> + return 0;
>>> +}
>>
>> mutex_destroy() should be called on pzone->th_lock
>>
>>> +
>>> +static int db8500_thermal_suspend(struct platform_device *pdev,
>>> + pm_message_t state)
>>> +{
>>> + struct db8500_thermal_zone *pzone;
>>> + pzone = platform_get_drvdata(pdev);
>>> +
>>> + flush_work_sync(&pzone->therm_work);
>>> + prcmu_stop_temp_sense();
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int db8500_thermal_resume(struct platform_device *pdev)
>>> +{
>>> + struct db8500_thermal_zone *pzone;
>>> + struct db8500_thsens_platform_data *ptrips;
>>> + unsigned long dft_low, dft_high;
>>> +
>>> + pzone = platform_get_drvdata(pdev);
>>> + ptrips = pzone->trip_tab;
>>> + dft_low = PRCMU_DEFAULT_LOW_TEMP;
>>> + dft_high = ptrips->trip_points[0].temp;
>>> +
>>> + prcmu_config_hotmon((u8)(dft_low/1000), (u8)(dft_high/1000));
>>> + prcmu_start_temp_sense(PRCMU_DEFAULT_MEASURE_TIME);
>>
>> Shouldn't cur_index and cur_temp_pseudo be updated as well? You may want
>> to define a helper function with all the code shared by irq handlers
>> (both high and low), probe and resume.
> No, they cannot be update because we don't know the actual current
> temp[1] after short or long time suspend, everything goes as
> beginning.
That's what I wanted to say, if everything is reset to a default value,
then cur_index and cur_temp should be reset too, as it's done in the
probe function. Otherwise you may have a current pseudo-temp and a
current index unrelated to how the hotmon is configured.
> If a helper function is introduced, it can be only used in probe and
> resume I think, different and a bit complicated algorithm in irq
> handlers.
I was thinking about a function which takes the new index and the new
low and high parameters, and updates cur_index and cur_pseudo_temp and
does the prcmu stuff. It seems to me this is (or should be) common to
all the 4 functions.
> [1] due to lack of corresponding interface, search "TODO" in this file
> to get more explanation.
--
Francesco
^ permalink raw reply
* Re: [RFC PATCH v2 4/6] net/core: apply pm_runtime_set_memalloc_noio on network devices
From: Alan Stern @ 2012-10-22 19:18 UTC (permalink / raw)
To: Ming Lei
Cc: linux-kernel, Oliver Neukum, Minchan Kim, Greg Kroah-Hartman,
Rafael J. Wysocki, Jens Axboe, David S. Miller, Andrew Morton,
netdev, linux-usb, linux-pm, linux-mm, Eric Dumazet,
David Decotigny, Tom Herbert, Ingo Molnar
In-Reply-To: <1350894794-1494-5-git-send-email-ming.lei@canonical.com>
On Mon, 22 Oct 2012, Ming Lei wrote:
> Deadlock might be caused by allocating memory with GFP_KERNEL in
> runtime_resume callback of network devices in iSCSI situation, so
> mark network devices and its ancestor as 'memalloc_noio_resume'
> with the introduced pm_runtime_set_memalloc_noio().
Is this really needed? Even with iSCSI, doesn't register_disk() have
to be called for the underlying block device? And given your 3/6
patch, wouldn't that mark the network device?
Alan Stern
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* drivers/cpuidle/governors/menu.c:268:2: warning: comparison of distinct pointer types lacks a cast
From: Fengguang Wu @ 2012-10-23 3:37 UTC (permalink / raw)
To: Youquan Song; +Cc: linux-pm, Rik van Riel, Rafael J. Wysocki
Hi Youquan,
FYI, there are new compile warnings show up in
tree: git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
head: 13f5e2d9a915373dd1573d8fe0214738bc69004f
commit: 0c963c99eb65aa9991cec6b3ea3188862167c427 cpuidle: Get typical recent sleep interval
date: 4 hours ago
config: sh-sh7785lcr_32bit_defconfig # make ARCH=sh sh7785lcr_32bit_defconfig
All warnings:
drivers/cpuidle/governors/menu.c: In function 'get_typical_interval':
drivers/cpuidle/governors/menu.c:268:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
drivers/cpuidle/governors/menu.c:277:2: warning: comparison of distinct pointer types lacks a cast [enabled by default]
vim +268 drivers/cpuidle/governors/menu.c
0c963c99 Youquan Song 2012-10-23 252 int64_t thresh = LLONG_MAX; /* Discard outliers above this value. */
0c963c99 Youquan Song 2012-10-23 253 unsigned int ret = 0;
1f85f87d Arjan van de Ven 2010-05-24 254
0c963c99 Youquan Song 2012-10-23 255 again:
1f85f87d Arjan van de Ven 2010-05-24 256
0c963c99 Youquan Song 2012-10-23 257 /* first calculate average and standard deviation of the past */
0c963c99 Youquan Song 2012-10-23 258 max = avg = divisor = stddev = 0;
0c963c99 Youquan Song 2012-10-23 259 for (i = 0; i < INTERVALS; i++) {
0c963c99 Youquan Song 2012-10-23 260 int64_t value = data->intervals[i];
0c963c99 Youquan Song 2012-10-23 261 if (value <= thresh) {
0c963c99 Youquan Song 2012-10-23 262 avg += value;
0c963c99 Youquan Song 2012-10-23 263 divisor++;
0c963c99 Youquan Song 2012-10-23 264 if (value > max)
0c963c99 Youquan Song 2012-10-23 265 max = value;
0c963c99 Youquan Song 2012-10-23 266 }
0c963c99 Youquan Song 2012-10-23 267 }
0c963c99 Youquan Song 2012-10-23 @268 do_div(avg, divisor);
0c963c99 Youquan Song 2012-10-23 269
0c963c99 Youquan Song 2012-10-23 270 for (i = 0; i < INTERVALS; i++) {
0c963c99 Youquan Song 2012-10-23 271 int64_t value = data->intervals[i];
0c963c99 Youquan Song 2012-10-23 272 if (value <= thresh) {
0c963c99 Youquan Song 2012-10-23 273 int64_t diff = value - avg;
0c963c99 Youquan Song 2012-10-23 274 stddev += diff * diff;
0c963c99 Youquan Song 2012-10-23 275 }
0c963c99 Youquan Song 2012-10-23 276 }
---
0-DAY kernel build testing backend Open Source Technology Center
Fengguang Wu, Yuanhan Liu Intel Corporation
^ permalink raw reply
* drivers/base/power/qos.c:330:5: sparse: symbol '__dev_pm_qos_update_request' was not declared. Should it be static?
From: Fengguang Wu @ 2012-10-23 4:22 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: linux-pm
[-- Attachment #1: Type: text/plain, Size: 2298 bytes --]
Hi Rafael,
FYI, there are new sparse warnings show up in
tree: git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
head: 13f5e2d9a915373dd1573d8fe0214738bc69004f
commit: a7a02d1a295d3ab9cd894af4f138e30a3000df3d PM / QoS: Make it possible to expose PM QoS device flags to user space
date: 5 hours ago
+ drivers/base/power/qos.c:330:5: sparse: symbol '__dev_pm_qos_update_request' was not declared. Should it be static?
vim +330 drivers/base/power/qos.c
ae0fb4b7 Rafael J. Wysocki 2012-10-23 314 req->type = type;
b66213cd Jean Pihet 2011-08-25 315 ret = apply_constraint(req, PM_QOS_ADD_REQ, value);
ae0fb4b7 Rafael J. Wysocki 2012-10-23 316 }
91ff4cb8 Jean Pihet 2011-08-25 317
1a9a9152 Rafael J. Wysocki 2011-09-29 318 out:
91ff4cb8 Jean Pihet 2011-08-25 319 mutex_unlock(&dev_pm_qos_mtx);
1a9a9152 Rafael J. Wysocki 2011-09-29 320
91ff4cb8 Jean Pihet 2011-08-25 321 return ret;
91ff4cb8 Jean Pihet 2011-08-25 322 }
91ff4cb8 Jean Pihet 2011-08-25 323 EXPORT_SYMBOL_GPL(dev_pm_qos_add_request);
91ff4cb8 Jean Pihet 2011-08-25 324
91ff4cb8 Jean Pihet 2011-08-25 325 /**
a7a02d1a Rafael J. Wysocki 2012-10-23 326 * __dev_pm_qos_update_request - Modify an existing device PM QoS request.
a7a02d1a Rafael J. Wysocki 2012-10-23 327 * @req : PM QoS request to modify.
a7a02d1a Rafael J. Wysocki 2012-10-23 328 * @new_value: New value to request.
a7a02d1a Rafael J. Wysocki 2012-10-23 329 */
a7a02d1a Rafael J. Wysocki 2012-10-23 @330 int __dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value)
a7a02d1a Rafael J. Wysocki 2012-10-23 331 {
a7a02d1a Rafael J. Wysocki 2012-10-23 332 s32 curr_value;
a7a02d1a Rafael J. Wysocki 2012-10-23 333 int ret = 0;
a7a02d1a Rafael J. Wysocki 2012-10-23 334
a7a02d1a Rafael J. Wysocki 2012-10-23 335 if (!req->dev->power.qos)
a7a02d1a Rafael J. Wysocki 2012-10-23 336 return -ENODEV;
a7a02d1a Rafael J. Wysocki 2012-10-23 337
a7a02d1a Rafael J. Wysocki 2012-10-23 338 switch(req->type) {
Please consider folding the attached diff :-)
---
0-DAY kernel build testing backend Open Source Technology Center
Fengguang Wu, Yuanhan Liu Intel Corporation
[-- Attachment #2: make-it-static-a7a02d1.diff --]
[-- Type: text/x-diff, Size: 503 bytes --]
diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
index 3fcbf36..aba1218 100644
--- a/drivers/base/power/qos.c
+++ b/drivers/base/power/qos.c
@@ -327,7 +327,7 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_add_request);
* @req : PM QoS request to modify.
* @new_value: New value to request.
*/
-int __dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value)
+static int __dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value)
{
s32 curr_value;
int ret = 0;
^ permalink raw reply related
* drivers/cpufreq/cpufreq_governor.c:46:53: sparse: incorrect type in argument 2 (different modifiers)
From: Fengguang Wu @ 2012-10-23 4:51 UTC (permalink / raw)
To: viresh kumar; +Cc: linux-pm, Rafael J. Wysocki
[-- Attachment #1: Type: text/plain, Size: 2754 bytes --]
Hi viresh,
FYI, there are new sparse warnings show up in
tree: git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
head: 13f5e2d9a915373dd1573d8fe0214738bc69004f
commit: 83a73f712f2275033b2dc7f5c664988a1823ebc7 cpufreq: Move common part from governors to separate file, v2
date: 5 hours ago
+ drivers/cpufreq/cpufreq_governor.c:46:53: sparse: incorrect type in argument 2 (different modifiers)
drivers/cpufreq/cpufreq_governor.c:46:53: expected unsigned long long [usertype] *wall
drivers/cpufreq/cpufreq_governor.c:46:53: got unsigned long long [nocast] [usertype] *wall
+ drivers/cpufreq/cpufreq_governor.c:46:53: sparse: implicit cast from nocast type
drivers/cpufreq/cpufreq_governor.c:48:58: sparse: incorrect type in argument 2 (different modifiers)
drivers/cpufreq/cpufreq_governor.c:48:58: expected unsigned long long [usertype] *last_update_time
drivers/cpufreq/cpufreq_governor.c:48:58: got unsigned long long [nocast] [usertype] *wall
drivers/cpufreq/cpufreq_governor.c:48:58: sparse: implicit cast from nocast type
vim +46 drivers/cpufreq/cpufreq_governor.c
83a73f71 viresh kumar 2012-10-23 30 busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_SOFTIRQ];
83a73f71 viresh kumar 2012-10-23 31 busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_STEAL];
83a73f71 viresh kumar 2012-10-23 32 busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_NICE];
83a73f71 viresh kumar 2012-10-23 33
83a73f71 viresh kumar 2012-10-23 34 idle_time = cur_wall_time - busy_time;
83a73f71 viresh kumar 2012-10-23 35 if (wall)
83a73f71 viresh kumar 2012-10-23 36 *wall = jiffies_to_usecs(cur_wall_time);
83a73f71 viresh kumar 2012-10-23 37
83a73f71 viresh kumar 2012-10-23 38 return jiffies_to_usecs(idle_time);
83a73f71 viresh kumar 2012-10-23 39 }
83a73f71 viresh kumar 2012-10-23 40
83a73f71 viresh kumar 2012-10-23 41 cputime64_t get_cpu_idle_time(unsigned int cpu, cputime64_t *wall)
83a73f71 viresh kumar 2012-10-23 42 {
83a73f71 viresh kumar 2012-10-23 43 u64 idle_time = get_cpu_idle_time_us(cpu, NULL);
83a73f71 viresh kumar 2012-10-23 44
83a73f71 viresh kumar 2012-10-23 45 if (idle_time == -1ULL)
83a73f71 viresh kumar 2012-10-23 @46 return get_cpu_idle_time_jiffy(cpu, wall);
83a73f71 viresh kumar 2012-10-23 47 else
83a73f71 viresh kumar 2012-10-23 48 idle_time += get_cpu_iowait_time_us(cpu, wall);
83a73f71 viresh kumar 2012-10-23 49
83a73f71 viresh kumar 2012-10-23 50 return idle_time;
83a73f71 viresh kumar 2012-10-23 51 }
83a73f71 viresh kumar 2012-10-23 52 EXPORT_SYMBOL_GPL(get_cpu_idle_time);
---
0-DAY kernel build testing backend Open Source Technology Center
Fengguang Wu, Yuanhan Liu Intel Corporation
[-- Attachment #2: cpufreq_governor.c --]
[-- Type: text/x-csrc, Size: 1425 bytes --]
/*
* drivers/cpufreq/cpufreq_governor.c
*
* CPUFREQ governors common code
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*/
#include <asm/cputime.h>
#include <linux/export.h>
#include <linux/kernel_stat.h>
#include <linux/tick.h>
#include <linux/types.h>
/*
* Code picked from earlier governer implementations
*/
static inline u64 get_cpu_idle_time_jiffy(unsigned int cpu, u64 *wall)
{
u64 idle_time;
u64 cur_wall_time;
u64 busy_time;
cur_wall_time = jiffies64_to_cputime64(get_jiffies_64());
busy_time = kcpustat_cpu(cpu).cpustat[CPUTIME_USER];
busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_SYSTEM];
busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_IRQ];
busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_SOFTIRQ];
busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_STEAL];
busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_NICE];
idle_time = cur_wall_time - busy_time;
if (wall)
*wall = jiffies_to_usecs(cur_wall_time);
return jiffies_to_usecs(idle_time);
}
cputime64_t get_cpu_idle_time(unsigned int cpu, cputime64_t *wall)
{
u64 idle_time = get_cpu_idle_time_us(cpu, NULL);
if (idle_time == -1ULL)
return get_cpu_idle_time_jiffy(cpu, wall);
else
idle_time += get_cpu_iowait_time_us(cpu, wall);
return idle_time;
}
EXPORT_SYMBOL_GPL(get_cpu_idle_time);
^ permalink raw reply
* drivers/devfreq/governor_simpleondemand.c:92:5: sparse: symbol 'devfreq_simple_ondemand_handler' was not declared. Should it be static?
From: Fengguang Wu @ 2012-10-23 5:13 UTC (permalink / raw)
To: Rajagopal Venkat; +Cc: linux-pm, Rafael J. Wysocki
[-- Attachment #1: Type: text/plain, Size: 2521 bytes --]
Hi Rajagopal,
FYI, there are new sparse warnings show up in
tree: git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
head: 13f5e2d9a915373dd1573d8fe0214738bc69004f
commit: 5001044272888cc4c1aa7e29b0e601b7719742f2 PM / devfreq: Core updates to support devices which can idle
date: 6 hours ago
+ drivers/devfreq/governor_simpleondemand.c:92:5: sparse: symbol 'devfreq_simple_ondemand_handler' was not declared. Should it be static?
--
+ drivers/devfreq/governor_userspace.c:119:5: sparse: symbol 'devfreq_userspace_handler' was not declared. Should it be static?
vim +92 drivers/devfreq/governor_simpleondemand.c
ce26c5bb MyungJoo Ham 2011-10-02 76 /* Set the desired frequency based on the load */
ce26c5bb MyungJoo Ham 2011-10-02 77 a = stat.busy_time;
ce26c5bb MyungJoo Ham 2011-10-02 78 a *= stat.current_frequency;
ce26c5bb MyungJoo Ham 2011-10-02 79 b = div_u64(a, stat.total_time);
ce26c5bb MyungJoo Ham 2011-10-02 80 b *= 100;
ce26c5bb MyungJoo Ham 2011-10-02 81 b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
ce26c5bb MyungJoo Ham 2011-10-02 82 *freq = (unsigned long) b;
ce26c5bb MyungJoo Ham 2011-10-02 83
6530b9de MyungJoo Ham 2011-12-09 84 if (df->min_freq && *freq < df->min_freq)
6530b9de MyungJoo Ham 2011-12-09 85 *freq = df->min_freq;
6530b9de MyungJoo Ham 2011-12-09 86 if (df->max_freq && *freq > df->max_freq)
6530b9de MyungJoo Ham 2011-12-09 87 *freq = df->max_freq;
6530b9de MyungJoo Ham 2011-12-09 88
ce26c5bb MyungJoo Ham 2011-10-02 89 return 0;
ce26c5bb MyungJoo Ham 2011-10-02 90 }
ce26c5bb MyungJoo Ham 2011-10-02 91
50010442 Rajagopal Venkat 2012-10-23 @92 int devfreq_simple_ondemand_handler(struct devfreq *devfreq,
50010442 Rajagopal Venkat 2012-10-23 93 unsigned int event, void *data)
50010442 Rajagopal Venkat 2012-10-23 94 {
50010442 Rajagopal Venkat 2012-10-23 95 switch (event) {
50010442 Rajagopal Venkat 2012-10-23 96 case DEVFREQ_GOV_START:
50010442 Rajagopal Venkat 2012-10-23 97 devfreq_monitor_start(devfreq);
50010442 Rajagopal Venkat 2012-10-23 98 break;
50010442 Rajagopal Venkat 2012-10-23 99
50010442 Rajagopal Venkat 2012-10-23 100 case DEVFREQ_GOV_STOP:
Please consider folding the attached diff :-)
---
0-DAY kernel build testing backend Open Source Technology Center
Fengguang Wu, Yuanhan Liu Intel Corporation
[-- Attachment #2: make-it-static-5001044.diff --]
[-- Type: text/x-diff, Size: 989 bytes --]
diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
index cf94218..3716a65 100644
--- a/drivers/devfreq/governor_simpleondemand.c
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -89,7 +89,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
return 0;
}
-int devfreq_simple_ondemand_handler(struct devfreq *devfreq,
+static int devfreq_simple_ondemand_handler(struct devfreq *devfreq,
unsigned int event, void *data)
{
switch (event) {
diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
index 1fddee5..7067555 100644
--- a/drivers/devfreq/governor_userspace.c
+++ b/drivers/devfreq/governor_userspace.c
@@ -116,7 +116,7 @@ static void userspace_exit(struct devfreq *devfreq)
devfreq->data = NULL;
}
-int devfreq_userspace_handler(struct devfreq *devfreq,
+static int devfreq_userspace_handler(struct devfreq *devfreq,
unsigned int event, void *data)
{
int ret = 0;
^ permalink raw reply related
* Re: drivers/cpufreq/cpufreq_governor.c:46:53: sparse: incorrect type in argument 2 (different modifiers)
From: Viresh Kumar @ 2012-10-23 5:25 UTC (permalink / raw)
To: Rafael J. Wysocki, Fengguang Wu; +Cc: linux-pm, Lists linaro-dev
In-Reply-To: <50862255.Foit2oXcLVE39lAk%fengguang.wu@intel.com>
Hi Fengguang,
Thanks for your mail. Few things i learnt from it:
- Rafael has already applied my patches :)
- I haven't used sparse in my life till this point. Now i know how to use it.
On 23 October 2012 10:21, Fengguang Wu <fengguang.wu@intel.com> wrote:
> Hi viresh,
>
> FYI, there are new sparse warnings show up in
Actually these aren't new warnings, but old. Because some part is moved from
one file to another, that's why your script shows them as new warnings.
Anyway, i don't hesitate in fixing them.
@Rafael: Most of these are due to mixed use of u64 and cputime64_t. Both of
which are u64 if i am not wrong. Any specific reason that we used cputime64_t
instead of u64? Or can i make everything u64 instead in governors?
--
viresh
> tree: git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
> head: 13f5e2d9a915373dd1573d8fe0214738bc69004f
> commit: 83a73f712f2275033b2dc7f5c664988a1823ebc7 cpufreq: Move common part from governors to separate file, v2
> date: 5 hours ago
>
> + drivers/cpufreq/cpufreq_governor.c:46:53: sparse: incorrect type in argument 2 (different modifiers)
> drivers/cpufreq/cpufreq_governor.c:46:53: expected unsigned long long [usertype] *wall
> drivers/cpufreq/cpufreq_governor.c:46:53: got unsigned long long [nocast] [usertype] *wall
> + drivers/cpufreq/cpufreq_governor.c:46:53: sparse: implicit cast from nocast type
> drivers/cpufreq/cpufreq_governor.c:48:58: sparse: incorrect type in argument 2 (different modifiers)
> drivers/cpufreq/cpufreq_governor.c:48:58: expected unsigned long long [usertype] *last_update_time
> drivers/cpufreq/cpufreq_governor.c:48:58: got unsigned long long [nocast] [usertype] *wall
> drivers/cpufreq/cpufreq_governor.c:48:58: sparse: implicit cast from nocast type
>
> vim +46 drivers/cpufreq/cpufreq_governor.c
>
> 83a73f71 viresh kumar 2012-10-23 30 busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_SOFTIRQ];
> 83a73f71 viresh kumar 2012-10-23 31 busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_STEAL];
> 83a73f71 viresh kumar 2012-10-23 32 busy_time += kcpustat_cpu(cpu).cpustat[CPUTIME_NICE];
> 83a73f71 viresh kumar 2012-10-23 33
> 83a73f71 viresh kumar 2012-10-23 34 idle_time = cur_wall_time - busy_time;
> 83a73f71 viresh kumar 2012-10-23 35 if (wall)
> 83a73f71 viresh kumar 2012-10-23 36 *wall = jiffies_to_usecs(cur_wall_time);
> 83a73f71 viresh kumar 2012-10-23 37
> 83a73f71 viresh kumar 2012-10-23 38 return jiffies_to_usecs(idle_time);
> 83a73f71 viresh kumar 2012-10-23 39 }
> 83a73f71 viresh kumar 2012-10-23 40
> 83a73f71 viresh kumar 2012-10-23 41 cputime64_t get_cpu_idle_time(unsigned int cpu, cputime64_t *wall)
> 83a73f71 viresh kumar 2012-10-23 42 {
> 83a73f71 viresh kumar 2012-10-23 43 u64 idle_time = get_cpu_idle_time_us(cpu, NULL);
> 83a73f71 viresh kumar 2012-10-23 44
> 83a73f71 viresh kumar 2012-10-23 45 if (idle_time == -1ULL)
> 83a73f71 viresh kumar 2012-10-23 @46 return get_cpu_idle_time_jiffy(cpu, wall);
> 83a73f71 viresh kumar 2012-10-23 47 else
> 83a73f71 viresh kumar 2012-10-23 48 idle_time += get_cpu_iowait_time_us(cpu, wall);
> 83a73f71 viresh kumar 2012-10-23 49
> 83a73f71 viresh kumar 2012-10-23 50 return idle_time;
> 83a73f71 viresh kumar 2012-10-23 51 }
> 83a73f71 viresh kumar 2012-10-23 52 EXPORT_SYMBOL_GPL(get_cpu_idle_time);
>
> ---
> 0-DAY kernel build testing backend Open Source Technology Center
> Fengguang Wu, Yuanhan Liu Intel Corporation
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox