* Re: linux-next : cpuidle - could you add my tree please
From: Linus Walleij @ 2012-07-03 19:20 UTC (permalink / raw)
To: Daniel Lezcano, Len.Brown
Cc: Kevin Hilman, Stephen Rothwell, Rob Lee, Lists Linaro-dev,
Peter De Schrijver, Linux Kernel Mailing List, Amit Kucheria,
linux-acpi, linux-next, Colin Cross, linux-pm, Linus Torvalds,
Andrew Morton
In-Reply-To: <4FF21D48.50301@linaro.org>
On Tue, Jul 3, 2012 at 12:14 AM, Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
> If that makes sense to add myself to the MAINTAINER file as a
> co-maintainer (understand: send to me also the patches, so I can take
> care of them if Len does not respond), I am ok with that.
What about a patch adding both you and Len as MAINTAINERs,
right now there is noone just some informal consensus and noone
really knows who to send patches to. Let's formalize it.
Yours,
Linus Walleij
^ permalink raw reply
* Re: linux-next : cpuidle - could you add my tree please
From: Rafael J. Wysocki @ 2012-07-03 16:54 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Stephen Rothwell, Linus Walleij, linux-acpi, linux-pm,
Lists Linaro-dev, Linux Kernel Mailing List, Kevin Hilman,
Peter De Schrijver, Amit Kucheria, linux-next, Colin Cross,
Andrew Morton, Linus Torvalds, Rob Lee, lenb@kernel.org
In-Reply-To: <4FF2F2D3.4090200@linaro.org>
On Tuesday, July 03, 2012, Daniel Lezcano wrote:
> On 07/03/2012 03:19 PM, Stephen Rothwell wrote:
> > Hi Daniel,
> >
> > On Tue, 03 Jul 2012 14:56:58 +0200 Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> >>
> >>> So do you have a branch in the cpuidle-next.git tree that isn't going to
> >>> be rebased?
> >>
> >> No. I am following Linus tree and adding the patches on top of it.
> >
> > Please don't rebase your tree more than necessary - it just makes thing
> > hard for anyone using your tree as a base for further development and
> > throws away any testing you may have done.
>
> Ok, let me sync with Len and Rafael about the best way to do that.
Please create a branch in your tree for me to pull from and let me know
which one it is. Please note that this branch must not be rebased after I've
pulled from it and it's going to be included into my linux-next branch
automatically.
I'll include it into my v3.6 push, because I have a couple of cpuidle patches
queued up already. We'll need to discuss the future of it after 3.6, though.
Thanks,
Rafael
^ permalink raw reply
* Re: linux-next : cpuidle - could you add my tree please
From: Daniel Lezcano @ 2012-07-03 13:25 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Kevin Hilman, Rob Lee, Lists Linaro-dev, Andrew Morton,
Linux Kernel Mailing List, Amit Kucheria, linux-acpi, linux-next,
Colin Cross, linux-pm, Linus Torvalds, Peter De Schrijver
In-Reply-To: <20120703231948.33cca18e3cc3d6e497ab022d@canb.auug.org.au>
On 07/03/2012 03:19 PM, Stephen Rothwell wrote:
> Hi Daniel,
>
> On Tue, 03 Jul 2012 14:56:58 +0200 Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>
>>> So do you have a branch in the cpuidle-next.git tree that isn't going to
>>> be rebased?
>>
>> No. I am following Linus tree and adding the patches on top of it.
>
> Please don't rebase your tree more than necessary - it just makes thing
> hard for anyone using your tree as a base for further development and
> throws away any testing you may have done.
Ok, let me sync with Len and Rafael about the best way to do that.
Thanks
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: linux-next : cpuidle - could you add my tree please
From: Stephen Rothwell @ 2012-07-03 13:19 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Rafael J. Wysocki, Linus Walleij, linux-acpi, linux-pm,
Lists Linaro-dev, Linux Kernel Mailing List, Kevin Hilman,
Peter De Schrijver, Amit Kucheria, linux-next, Colin Cross,
Andrew Morton, Linus Torvalds, Rob Lee, lenb@kernel.org
In-Reply-To: <4FF2EC1A.7020702@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 536 bytes --]
Hi Daniel,
On Tue, 03 Jul 2012 14:56:58 +0200 Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> > So do you have a branch in the cpuidle-next.git tree that isn't going to
> > be rebased?
>
> No. I am following Linus tree and adding the patches on top of it.
Please don't rebase your tree more than necessary - it just makes thing
hard for anyone using your tree as a base for further development and
throws away any testing you may have done.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: linux-next : cpuidle - could you add my tree please
From: Daniel Lezcano @ 2012-07-03 12:56 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linus Walleij, Stephen Rothwell, linux-acpi, linux-pm,
Lists Linaro-dev, Linux Kernel Mailing List, Kevin Hilman,
Peter De Schrijver, Amit Kucheria, linux-next, Colin Cross,
Andrew Morton, Linus Torvalds, Rob Lee, lenb@kernel.org
In-Reply-To: <201207031059.13758.rjw@sisk.pl>
On 07/03/2012 10:59 AM, Rafael J. Wysocki wrote:
> On Tuesday, July 03, 2012, Daniel Lezcano wrote:
>> On 07/02/2012 09:49 PM, Rafael J. Wysocki wrote:
>>> On Monday, July 02, 2012, Daniel Lezcano wrote:
>>>>
>>>> On 07/02/2012 11:09 AM, Linus Walleij wrote:
>>>>> On Mon, Jun 25, 2012 at 3:27 PM, Daniel Lezcano
>>>>> <daniel.lezcano@linaro.org> wrote:
>>>>>
>>>>>> we discussed last week to put in place a tree grouping the cpuidle
>>>>>> modifications [1]. Is it possible to add the tree ?
>>>>>>
>>>>>> git://git.linaro.org/people/dlezcano/cpuidle-next.git #cpuidle-next
>>>>>
>>>>> Thanks for doing this.
>>>>>
>>>>> Since MAINTAINERS is lacking a listed maintainer for cpuidle,
>>>>> are you also going to add yourself as maintainer and list this
>>>>> tree in that file, or is this a one-time exercise?
>>>>
>>>> I will be glad to do that if Len and Rafael agree on that.
>>>
>>> Len Brown has been a cpuidle maintainer for some time now. Moreover,
>>> he's been taking patches, but Linus refused to pull his entire tree during the
>>> last merge window (as you probably know). I honestly don't think this is
>>> a good enough reason for replacing him as a cpuidle maintainer by force.
>>>
>>> So, you should ask Len whether or not he's willing to pass the cpuidle
>>> maintenance to someone else.
>>
>> No, no. You are misunderstanding what I am proposing. I don't want to
>> replace Len I just want to act as a "proxy". I understand a maintainer
>> can be busy and could not have enough time to take care of the subsystem
>> is maintaining during a period because he's too busy for that.
>> Trust me, I fully understand that :)
>>
>> As there are a lot of modifications of cpuidle, I am proposing to take
>> the patches when they are acked-by, to create a consolidated tree,
>> providing a better integration for cpuidle, a wider testing, preventing
>> conflicts and facilitating Len's work if he agrees to pull from this tree.
>>
>> If that makes sense to add myself to the MAINTAINER file as a
>> co-maintainer (understand: send to me also the patches, so I can take
>> care of them if Len does not respond), I am ok with that.
>>
>> It is just about helping :)
>
> Cool. :-)
>
> So do you have a branch in the cpuidle-next.git tree that isn't going to
> be rebased?
No. I am following Linus tree and adding the patches on top of it.
>>> I know that Len hasn't been responsive recently, but I also know that he
>>> _does_ respond to inquiries sent directly to him.
>>
>> Do you mean to its intel address ?
>
> Yes, CCing the Len's Intel address won't hurt I think.
Ok, I will ping him and give him the pointers to the discussion we had.
Thanks
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH v3 2/7] mfd: omap: control: core system control driver
From: Valentin, Eduardo @ 2012-07-03 11:00 UTC (permalink / raw)
To: Konstantin Baydarov
Cc: balbi, kishon, amit.kucheria, linux-pm, linux-omap,
linux-arm-kernel
In-Reply-To: <CAGF5oy-50evWVX8+U6qe0wwA0UXJUAXxXXbwrEkebSVtT8vxRA@mail.gmail.com>
Hello,
On Thu, Jun 28, 2012 at 1:55 PM, Valentin, Eduardo
<eduardo.valentin@ti.com> wrote:
> Hello again,
>
> On Thu, Jun 28, 2012 at 1:51 PM, Valentin, Eduardo
> <eduardo.valentin@ti.com> wrote:
>> Hello,
>>
>> On Thu, Jun 28, 2012 at 1:26 PM, Konstantin Baydarov
>> <kbaidarov@dev.rtsoft.ru> wrote:
>>> On 06/28/2012 02:12 PM, Konstantin Baydarov wrote:
>>>> The interface(design) of omap-control-core.c has already been discussed many times :(
>>>> Eduardo, in his patch set, suggested following design:
>>>> - omap-control-core.c ioremaps SCM window and provide functions to read/write SCP register for bandgap and usb.
>>>>
>>>> IIRC, this approach didn't satisfy and it was suggested to have private read/write in bandgap and usb.
>>>>
>>>> So, my patch set introduces following design:
>>>> - omap-control-core.c don't provide read/write functions for bandgap and usb.
>>>> - bandgap and usb use their own private read/write functions
>>>> - Each omap-control-core.c, bandgap and usb drivers remap SCM window. It's OK because SCM window is statically mapped to the same virtual address. But the problem is that SMP memory window isn't protected. I'm not sure whether it's possible to protect SCM window using this approach.
>>> I mean:
>>>
>>> - Each omap-control-core.c, bandgap and usb drivers remap SCM window. It's OK because SCM window is statically mapped. So each call of ioremap in omap-control-core.c, bandgap and usb drivers returns the same virtual address. But the problem is that SCM memory window isn't protected. I'm not sure whether it's possible to protect SCM window using this approach(when each driver remaps the same IOMEM).
>>>
>>>>
>>>> Another possible design is:
>>>> - omap-control-core.c ioremaps and reserves SCM IOMEM window
>>>> - omap-control-core.c exports omap_control_get_base(virtual base address is returned) to use in bandgap and usb_phy driver.
>>>> - Bandgap and usb phy uses their own private read/write function.
>>>> IIUC, this way was suggested by Tony.
>>
>> Well I understood slightly different :-)
>>
>> I think the point is not really where to put the access functions, but
>> to have each driver handling a separate part of the the io window. As
>> I said before, so that they don't access each other io area.
>>
>> If you have 1 io window, for the above mentioned constraint, you won't
>> protect anything. So, in that sense, it doesn't make much difference
>> if you have access functions in core, or in the children, as they are
>> all sharing the same io window. Of course, in case we put only 1 io
>> window, for me it is safer if that window is managed in only one
>> place, instead of several places.
>>
>> The question is then, can we split the io area into smaller windows
>> for each children? Considering the children registers are not
>> contiguous :-(. In theory we can put several entries in the 'reg' DT
>> property, but that becomes a bit messy as it will change depending on
>> OMAP version. Anyways, if we split the scm io window into several io
>> smaller areas/chunks, then it makes sense to have access functions in
>> each children.
>>
>>>>
>>>> I guess It's better to settle the design(interface) of omap-control-core.c, bandgap and usb phy and then submit the next version of patch set.
>>>
>>
>> Agreed. Here. We need to decide how to have this design and stick to it.
>
> Once the design is agreed, the series can probably be split into
> parts, so we can have the scm core and its children worked separately
Just to be clear, what I was proposing is to have each driver taking
care of its own io window.
For instance, for BG, assuming we have a DT like this:
ctrl_module_core: ctrl_module_core@4a002000 {
compatible = "ti,omap4-control";
ti,hwmods = "ctrl_module_core";
#address-cells = <1>;
#size-cells = <1>;
ranges;
bandgap: bandgap@4a002000 {
reg = <0x4a00232C 0x4 0x4a002378 0x18>;
compatible = "ti,omap4460-bandgap";
interrupts = <0 126 4>; /* talert */
ti,tshut-gpio = <86>; /* tshut */
};
usb {
compatible = "ti,omap4-usb-phy";
};
};
then, while probing, it can request those by simply:
i = 0;
do {
void __iomem *chunk;
res = platform_get_resource(pdev, IORESOURCE_MEM, i);
if (!res)
break;
chunk = devm_request_and_ioremap(&pdev->dev, res);
if (i == 0)
bg_ptr->base = chunk;
if (!chunk) {
dev_err(&pdev->dev,
"failed to request the IO (%d:%pR).\n",
i, res);
return ERR_PTR(-EADDRNOTAVAIL);
}
i++;
} while (res);
The driver needs to adapt its reg offsets, of course, it is doable and
with the current design it is just a matter of redefining the register
offsets.
But for the above to work, the core driver must not request the
complete IO area.
>
>>
>>
>> --
>>
>> Eduardo Valentin
>
>
>
> --
>
> Eduardo Valentin
--
Eduardo Valentin
^ permalink raw reply
* Re: linux-next : cpuidle - could you add my tree please
From: Rafael J. Wysocki @ 2012-07-03 8:59 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Linus Walleij, Stephen Rothwell, linux-acpi, linux-pm,
Lists Linaro-dev, Linux Kernel Mailing List, Kevin Hilman,
Peter De Schrijver, Amit Kucheria, linux-next, Colin Cross,
Andrew Morton, Linus Torvalds, Rob Lee, lenb@kernel.org
In-Reply-To: <4FF21D48.50301@linaro.org>
On Tuesday, July 03, 2012, Daniel Lezcano wrote:
> On 07/02/2012 09:49 PM, Rafael J. Wysocki wrote:
> > On Monday, July 02, 2012, Daniel Lezcano wrote:
> >>
> >> On 07/02/2012 11:09 AM, Linus Walleij wrote:
> >>> On Mon, Jun 25, 2012 at 3:27 PM, Daniel Lezcano
> >>> <daniel.lezcano@linaro.org> wrote:
> >>>
> >>>> we discussed last week to put in place a tree grouping the cpuidle
> >>>> modifications [1]. Is it possible to add the tree ?
> >>>>
> >>>> git://git.linaro.org/people/dlezcano/cpuidle-next.git #cpuidle-next
> >>>
> >>> Thanks for doing this.
> >>>
> >>> Since MAINTAINERS is lacking a listed maintainer for cpuidle,
> >>> are you also going to add yourself as maintainer and list this
> >>> tree in that file, or is this a one-time exercise?
> >>
> >> I will be glad to do that if Len and Rafael agree on that.
> >
> > Len Brown has been a cpuidle maintainer for some time now. Moreover,
> > he's been taking patches, but Linus refused to pull his entire tree during the
> > last merge window (as you probably know). I honestly don't think this is
> > a good enough reason for replacing him as a cpuidle maintainer by force.
> >
> > So, you should ask Len whether or not he's willing to pass the cpuidle
> > maintenance to someone else.
>
> No, no. You are misunderstanding what I am proposing. I don't want to
> replace Len I just want to act as a "proxy". I understand a maintainer
> can be busy and could not have enough time to take care of the subsystem
> is maintaining during a period because he's too busy for that.
> Trust me, I fully understand that :)
>
> As there are a lot of modifications of cpuidle, I am proposing to take
> the patches when they are acked-by, to create a consolidated tree,
> providing a better integration for cpuidle, a wider testing, preventing
> conflicts and facilitating Len's work if he agrees to pull from this tree.
>
> If that makes sense to add myself to the MAINTAINER file as a
> co-maintainer (understand: send to me also the patches, so I can take
> care of them if Len does not respond), I am ok with that.
>
> It is just about helping :)
Cool. :-)
So do you have a branch in the cpuidle-next.git tree that isn't going to
be rebased?
> > I know that Len hasn't been responsive recently, but I also know that he
> > _does_ respond to inquiries sent directly to him.
>
> Do you mean to its intel address ?
Yes, CCing the Len's Intel address won't hurt I think.
Thanks,
Rafael
^ permalink raw reply
* Re: linux-next : cpuidle - could you add my tree please
From: Daniel Lezcano @ 2012-07-02 22:14 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linus Walleij, Stephen Rothwell, linux-acpi, linux-pm,
Lists Linaro-dev, Linux Kernel Mailing List, Kevin Hilman,
Peter De Schrijver, Amit Kucheria, linux-next, Colin Cross,
Andrew Morton, Linus Torvalds, Rob Lee, lenb@kernel.org
In-Reply-To: <201207022149.36629.rjw@sisk.pl>
On 07/02/2012 09:49 PM, Rafael J. Wysocki wrote:
> On Monday, July 02, 2012, Daniel Lezcano wrote:
>>
>> On 07/02/2012 11:09 AM, Linus Walleij wrote:
>>> On Mon, Jun 25, 2012 at 3:27 PM, Daniel Lezcano
>>> <daniel.lezcano@linaro.org> wrote:
>>>
>>>> we discussed last week to put in place a tree grouping the cpuidle
>>>> modifications [1]. Is it possible to add the tree ?
>>>>
>>>> git://git.linaro.org/people/dlezcano/cpuidle-next.git #cpuidle-next
>>>
>>> Thanks for doing this.
>>>
>>> Since MAINTAINERS is lacking a listed maintainer for cpuidle,
>>> are you also going to add yourself as maintainer and list this
>>> tree in that file, or is this a one-time exercise?
>>
>> I will be glad to do that if Len and Rafael agree on that.
>
> Len Brown has been a cpuidle maintainer for some time now. Moreover,
> he's been taking patches, but Linus refused to pull his entire tree during the
> last merge window (as you probably know). I honestly don't think this is
> a good enough reason for replacing him as a cpuidle maintainer by force.
>
> So, you should ask Len whether or not he's willing to pass the cpuidle
> maintenance to someone else.
No, no. You are misunderstanding what I am proposing. I don't want to
replace Len I just want to act as a "proxy". I understand a maintainer
can be busy and could not have enough time to take care of the subsystem
is maintaining during a period because he's too busy for that.
Trust me, I fully understand that :)
As there are a lot of modifications of cpuidle, I am proposing to take
the patches when they are acked-by, to create a consolidated tree,
providing a better integration for cpuidle, a wider testing, preventing
conflicts and facilitating Len's work if he agrees to pull from this tree.
If that makes sense to add myself to the MAINTAINER file as a
co-maintainer (understand: send to me also the patches, so I can take
care of them if Len does not respond), I am ok with that.
It is just about helping :)
> I know that Len hasn't been responsive recently, but I also know that he
> _does_ respond to inquiries sent directly to him.
Do you mean to its intel address ?
Thanks
-- Daniel
> --
> To unsubscribe from this list: send the line "unsubscribe linux-next" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: linux-next : cpuidle - could you add my tree please
From: Rafael J. Wysocki @ 2012-07-02 20:04 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Kevin Hilman, Stephen Rothwell, Rob Lee, Lists Linaro-dev,
Peter De Schrijver, Linux Kernel Mailing List, Amit Kucheria,
linux-acpi, linux-next, Colin Cross, linux-pm, Linus Torvalds,
Andrew Morton
In-Reply-To: <4FE86727.5010400@linaro.org>
Hi,
On Monday, June 25, 2012, Daniel Lezcano wrote:
>
> Hi Stephen,
>
> we discussed last week to put in place a tree grouping the cpuidle
> modifications [1]. Is it possible to add the tree ?
>
> git://git.linaro.org/people/dlezcano/cpuidle-next.git #cpuidle-next
>
> It contains for the moment Colin Cross's cpuidle coupled states.
Do you have a stable branch in that tree, i.e. such that it is guaranteed not
to be rebased?
Rafael
^ permalink raw reply
* Re: linux-next : cpuidle - could you add my tree please
From: Rafael J. Wysocki @ 2012-07-02 19:49 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Linus Walleij, Stephen Rothwell, linux-acpi, linux-pm,
Lists Linaro-dev, Linux Kernel Mailing List, Kevin Hilman,
Peter De Schrijver, Amit Kucheria, linux-next, Colin Cross,
Andrew Morton, Linus Torvalds, Rob Lee, lenb@kernel.org
In-Reply-To: <4FF19947.3040608@linaro.org>
On Monday, July 02, 2012, Daniel Lezcano wrote:
>
> On 07/02/2012 11:09 AM, Linus Walleij wrote:
> > On Mon, Jun 25, 2012 at 3:27 PM, Daniel Lezcano
> > <daniel.lezcano@linaro.org> wrote:
> >
> >> we discussed last week to put in place a tree grouping the cpuidle
> >> modifications [1]. Is it possible to add the tree ?
> >>
> >> git://git.linaro.org/people/dlezcano/cpuidle-next.git #cpuidle-next
> >
> > Thanks for doing this.
> >
> > Since MAINTAINERS is lacking a listed maintainer for cpuidle,
> > are you also going to add yourself as maintainer and list this
> > tree in that file, or is this a one-time exercise?
>
> I will be glad to do that if Len and Rafael agree on that.
Len Brown has been a cpuidle maintainer for some time now. Moreover,
he's been taking patches, but Linus refused to pull his entire tree during the
last merge window (as you probably know). I honestly don't think this is
a good enough reason for replacing him as a cpuidle maintainer by force.
So, you should ask Len whether or not he's willing to pass the cpuidle
maintenance to someone else.
I know that Len hasn't been responsive recently, but I also know that he
_does_ respond to inquiries sent directly to him.
Thanks,
Rafael
^ permalink raw reply
* Re: linux-next : cpuidle - could you add my tree please
From: Daniel Lezcano @ 2012-07-02 12:51 UTC (permalink / raw)
To: Linus Walleij
Cc: Stephen Rothwell, linux-acpi, linux-pm, Lists Linaro-dev,
Linux Kernel Mailing List, Kevin Hilman, Peter De Schrijver,
Amit Kucheria, linux-next, Colin Cross, Andrew Morton,
Linus Torvalds, Rob Lee, lenb@kernel.org, rjw@sisk.pl
In-Reply-To: <CACRpkdYaLweX-4YkWotuFFWoqEjOLKoNCsurCxxWNfq7QznPnw@mail.gmail.com>
On 07/02/2012 11:09 AM, Linus Walleij wrote:
> On Mon, Jun 25, 2012 at 3:27 PM, Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
>
>> we discussed last week to put in place a tree grouping the cpuidle
>> modifications [1]. Is it possible to add the tree ?
>>
>> git://git.linaro.org/people/dlezcano/cpuidle-next.git #cpuidle-next
>
> Thanks for doing this.
>
> Since MAINTAINERS is lacking a listed maintainer for cpuidle,
> are you also going to add yourself as maintainer and list this
> tree in that file, or is this a one-time exercise?
I will be glad to do that if Len and Rafael agree on that.
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: linux-next : cpuidle - could you add my tree please
From: Linus Walleij @ 2012-07-02 9:09 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Kevin Hilman, Stephen Rothwell, Rob Lee, Lists Linaro-dev,
Peter De Schrijver, Linux Kernel Mailing List, Amit Kucheria,
linux-acpi, linux-next, Colin Cross, linux-pm, Linus Torvalds,
Andrew Morton
In-Reply-To: <4FE86727.5010400@linaro.org>
On Mon, Jun 25, 2012 at 3:27 PM, Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
> we discussed last week to put in place a tree grouping the cpuidle
> modifications [1]. Is it possible to add the tree ?
>
> git://git.linaro.org/people/dlezcano/cpuidle-next.git #cpuidle-next
Thanks for doing this.
Since MAINTAINERS is lacking a listed maintainer for cpuidle,
are you also going to add yourself as maintainer and list this
tree in that file, or is this a one-time exercise?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [RFC PATCH 11/12] thermal: introduce cooling state arbitrator
From: Valentin, Eduardo @ 2012-07-02 7:22 UTC (permalink / raw)
To: Zhang Rui
Cc: linux-pm, linux-acpi@vger.kernel.org, Amit Kachhap,
Rafael J. Wysocki, Len, Brown, R, Durgadoss, Matthew Garrett
In-Reply-To: <1341208343.1682.74.camel@rui.sh.intel.com>
Hello Rui,
On Mon, Jul 2, 2012 at 8:52 AM, Zhang Rui <rui.zhang@intel.com> wrote:
> Hi, Eduardo,
>
> On 日, 2012-06-24 at 18:19 +0300, Valentin, Eduardo wrote:
>> Hello Rui,
>>
>> On Wed, Jun 20, 2012 at 9:47 AM, Zhang Rui <rui.zhang@intel.com> wrote:
>> > On 三, 2012-06-13 at 17:23 +0300, Eduardo Valentin wrote:
>> >> Hello Rui,
>> >>
>> >> On Mon, Jun 11, 2012 at 11:20:39AM +0800, Zhang Rui wrote:
>> >> >
>> >> > Introduce simple arbitrator for setting device cooling state,
>> >> > to fix the problem that a cooling device may be referenced by
>> >> > by multiple trip points in multiple thermal zones.
>> >> >
>> >> >
>> >> > With this patch, we have two stages for updating a thermal zone,
>> >> > 1. check if a thermal_instance needs to be updated or not
>> >> > 2. update the cooling device, based on the target cooling state
>> >> > of all its instances.
>> >> >
>> >> > Note that, currently, the cooling device is set to the deepest
>> >> > cooling state required.
>> >> >
>> >> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
>> >> > ---
>> >> > drivers/thermal/thermal_sys.c | 41 ++++++++++++++++++++++++++++++++++++++---
>> >> > include/linux/thermal.h | 1 +
>> >> > 2 files changed, 39 insertions(+), 3 deletions(-)
>> >> >
>> >> > Index: rtd3/drivers/thermal/thermal_sys.c
>> >> > ===================================================================
>> >> > --- rtd3.orig/drivers/thermal/thermal_sys.c
>> >> > +++ rtd3/drivers/thermal/thermal_sys.c
>> >> > @@ -54,6 +54,7 @@ struct thermal_instance {
>> >> > int trip;
>> >> > unsigned long upper; /* Highest cooling state for this trip point */
>> >> > unsigned long lower; /* Lowest cooling state for this trip point */
>> >> > + unsigned long target; /* expected cooling state */
>> >> > char attr_name[THERMAL_NAME_LENGTH];
>> >> > struct device_attribute attr;
>> >> > struct list_head tz_node; /* node in tz->instances */
>> >> > @@ -812,6 +813,7 @@ int thermal_zone_bind_cooling_device(str
>> >> > dev->trip = trip;
>> >> > dev->upper = upper;
>> >> > dev->lower = lower;
>> >> > + dev->target = -1;
>> >> >
>> >> > result = get_idr(&tz->idr, &tz->lock, &dev->id);
>> >> > if (result)
>> >> > @@ -949,6 +951,7 @@ thermal_cooling_device_register(char *ty
>> >> > strcpy(cdev->type, type);
>> >> > INIT_LIST_HEAD(&cdev->instances);
>> >> > cdev->ops = ops;
>> >> > + cdev->updated = 1;
>> >> > cdev->device.class = &thermal_class;
>> >> > cdev->devdata = devdata;
>> >> > dev_set_name(&cdev->device, "cooling_device%d", cdev->id);
>> >> > @@ -1040,6 +1043,32 @@ void thermal_cooling_device_unregister(s
>> >> > }
>> >> > EXPORT_SYMBOL(thermal_cooling_device_unregister);
>> >> >
>> >> > +static void thermal_zone_do_update(struct thermal_zone_device *tz)
>> >> > +{
>> >> > + struct thermal_instance *instance1, *instance2;
>> >> > + struct thermal_cooling_device *cdev;
>> >> > + int target;
>> >> > +
>> >> > + list_for_each_entry(instance1, &tz->instances, tz_node) {
>> >> > + cdev = instance1->cdev;
>> >> > +
>> >> > + /* cooling device has already been updated*/
>> >> > + if (cdev->updated)
>> >> > + continue;
>> >> > +
>> >> > + target = 0;
>> >> > + /* Make sure cdev enters the deepest cooling state */
>> >> > + list_for_each_entry(instance2, &cdev->instances, cdev_node) {
>> >> > + if (instance2->target == -1)
>> >> > + continue;
>> >> > + if (instance2->target > target)
>> >> > + target = instance2->target;
>> >> > + }
>> >> > + cdev->ops->set_cur_state(cdev, target);
>> >> > + cdev->updated = 1;
>> >> > + }
>> >> > +}
>> >>
>> >> I think the above arbitrator solution does not escalate. As I can see,
>> >> the arbitrator takes care of cooling device instances in the same thermal zone.
>> >>
>> > why?
>> > It parses the thermal instances in one thermal zone to find the cooling
>> > devices, and then parse all the thermal instances for the cooling
>> > devices, no matter which thermal zone it belongs to.
>>
>> I think I got confused with the nomenclature change. So, yes, it does
>> go through all zones and cooling devices. But still, ideally it should
>> be a plist per device, don't you think? This way your search space is
>> reduced.
>>
> I'm not aware of this before, but here is a problem when I tried to use
> plist.
OK.
> First, we should use plist_node->prio as the thermal_instance->target
> instead, right?
Right.
> But the problem is that thermal_instance->target needs to be updated
> when temperature is changed. But plist does not support dynamic updating
> the priority, right?
No AFAIK,
> So when changing the cooling state target of a thermal instance, I need
> to delete and re-insert the plist_node? This does not seems efficient to
> me.
Right. That would mean you want to do remove the current zone
constraint and add the next entry zone constraint. Another way would
be to add current target if you are increasing target and remove if
you are reducing target.
Then if there is a device shared among existing zones, the highest
target set would be prevalent.
In fact it is not that efficient, but what do you think is less
efficient? Going through all zones or removing and re-adding 1
constraint ?
If you look kernel/power/qos.c you will see that it is a very similar situation.
case PM_QOS_UPDATE_REQ:
/*
* to change the list, we atomically remove, reinit
* with new value and add, then see if the extremal
* changed
*/
plist_del(node, &c->list);
case PM_QOS_ADD_REQ:
plist_node_init(node, new_value);
plist_add(node, &c->list);
break;
In fact, if you review that file you will understand why I was talking
about improving/reusing pm-qos to address this specific thermal
constraint management topic.
>
> thanks,
> rui
>
>> >
>> >> What if you have a device which expose cooling device instances in different
>> >> thermal zones?
>> >>
>> >> Besides, the thermal constraint might collide with pm QoS or with settings
>> >> coming from user space interfaces.
>> >>
>> >
>> >> That's actually why I was suggesting to have this 'arbitrator' or constraint
>> >> management outside the thermal framework. And managed per device at some
>> >> other abstraction layer.
>> >>
>> > Yeah, but I think we can have an incremental patch to handle the pm Qos
>> > constrains. Especially Durga already has the governor patches to move
>> > this out of the thermal framework.
>> > what do you think?
>>
>> I think it sounds like a plan. I also like incremental approach.
>>
>> >
>> > thanks,
>> > rui
>> >
>> >> > +
>> >> > /*
>> >> > * Cooling algorithm for active trip points
>> >> > *
>> >> > @@ -1086,19 +1115,24 @@ static void thermal_zone_trip_update(str
>> >> > cur_state = cur_state > instance->lower ?
>> >> > (cur_state - 1) : instance->lower;
>> >> > }
>> >> > - cdev->ops->set_cur_state(cdev, cur_state);
>> >> > + instance->target = cur_state;
>> >> > + cdev->updated = 0; /* cooling device needs update */
>> >> > }
>> >> > } else { /* below trip */
>> >> > list_for_each_entry(instance, &tz->instances, tz_node) {
>> >> > if (instance->trip != trip)
>> >> > continue;
>> >> >
>> >> > + /* Do not use the deacitve thermal instance */
>> >> > + if (instance->target == -1)
>> >> > + continue;
>> >> > cdev = instance->cdev;
>> >> > cdev->ops->get_cur_state(cdev, &cur_state);
>> >> >
>> >> > cur_state = cur_state > instance->lower ?
>> >> > - (cur_state - 1) : instance->lower;
>> >> > - cdev->ops->set_cur_state(cdev, cur_state);
>> >> > + (cur_state - 1) : -1;
>> >> > + instance->target = cur_state;
>> >> > + cdev->updated = 0; /* cooling device needs update */
>> >> > }
>> >> > }
>> >> >
>> >> > @@ -1159,6 +1193,7 @@ void thermal_zone_device_update(struct t
>> >> > }
>> >> > }
>> >> >
>> >> > + thermal_zone_do_update(tz);
>> >> > if (tz->forced_passive)
>> >> > thermal_zone_device_passive(tz, temp, tz->forced_passive,
>> >> > THERMAL_TRIPS_NONE);
>> >> > Index: rtd3/include/linux/thermal.h
>> >> > ===================================================================
>> >> > --- rtd3.orig/include/linux/thermal.h
>> >> > +++ rtd3/include/linux/thermal.h
>> >> > @@ -86,6 +86,7 @@ struct thermal_cooling_device {
>> >> > struct device device;
>> >> > void *devdata;
>> >> > const struct thermal_cooling_device_ops *ops;
>> >> > + int updated; /* 1 if the cooling device does not need update */
>> >> > struct list_head instances;
>> >> > struct list_head node;
>> >> > };
>> >> >
>> >> >
>> >
>> >
>>
>>
>>
>
>
--
Eduardo Valentin
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH 11/12] thermal: introduce cooling state arbitrator
From: Zhang Rui @ 2012-07-02 5:52 UTC (permalink / raw)
To: Valentin, Eduardo; +Cc: linux-acpi@vger.kernel.org, linux-pm
In-Reply-To: <CAGF5oy-ncSVLj55xCHV_WO3R8HkOZ6OPfaNkL09XKGq8z0Wv7A@mail.gmail.com>
Hi, Eduardo,
On 日, 2012-06-24 at 18:19 +0300, Valentin, Eduardo wrote:
> Hello Rui,
>
> On Wed, Jun 20, 2012 at 9:47 AM, Zhang Rui <rui.zhang@intel.com> wrote:
> > On 三, 2012-06-13 at 17:23 +0300, Eduardo Valentin wrote:
> >> Hello Rui,
> >>
> >> On Mon, Jun 11, 2012 at 11:20:39AM +0800, Zhang Rui wrote:
> >> >
> >> > Introduce simple arbitrator for setting device cooling state,
> >> > to fix the problem that a cooling device may be referenced by
> >> > by multiple trip points in multiple thermal zones.
> >> >
> >> >
> >> > With this patch, we have two stages for updating a thermal zone,
> >> > 1. check if a thermal_instance needs to be updated or not
> >> > 2. update the cooling device, based on the target cooling state
> >> > of all its instances.
> >> >
> >> > Note that, currently, the cooling device is set to the deepest
> >> > cooling state required.
> >> >
> >> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> >> > ---
> >> > drivers/thermal/thermal_sys.c | 41 ++++++++++++++++++++++++++++++++++++++---
> >> > include/linux/thermal.h | 1 +
> >> > 2 files changed, 39 insertions(+), 3 deletions(-)
> >> >
> >> > Index: rtd3/drivers/thermal/thermal_sys.c
> >> > ===================================================================
> >> > --- rtd3.orig/drivers/thermal/thermal_sys.c
> >> > +++ rtd3/drivers/thermal/thermal_sys.c
> >> > @@ -54,6 +54,7 @@ struct thermal_instance {
> >> > int trip;
> >> > unsigned long upper; /* Highest cooling state for this trip point */
> >> > unsigned long lower; /* Lowest cooling state for this trip point */
> >> > + unsigned long target; /* expected cooling state */
> >> > char attr_name[THERMAL_NAME_LENGTH];
> >> > struct device_attribute attr;
> >> > struct list_head tz_node; /* node in tz->instances */
> >> > @@ -812,6 +813,7 @@ int thermal_zone_bind_cooling_device(str
> >> > dev->trip = trip;
> >> > dev->upper = upper;
> >> > dev->lower = lower;
> >> > + dev->target = -1;
> >> >
> >> > result = get_idr(&tz->idr, &tz->lock, &dev->id);
> >> > if (result)
> >> > @@ -949,6 +951,7 @@ thermal_cooling_device_register(char *ty
> >> > strcpy(cdev->type, type);
> >> > INIT_LIST_HEAD(&cdev->instances);
> >> > cdev->ops = ops;
> >> > + cdev->updated = 1;
> >> > cdev->device.class = &thermal_class;
> >> > cdev->devdata = devdata;
> >> > dev_set_name(&cdev->device, "cooling_device%d", cdev->id);
> >> > @@ -1040,6 +1043,32 @@ void thermal_cooling_device_unregister(s
> >> > }
> >> > EXPORT_SYMBOL(thermal_cooling_device_unregister);
> >> >
> >> > +static void thermal_zone_do_update(struct thermal_zone_device *tz)
> >> > +{
> >> > + struct thermal_instance *instance1, *instance2;
> >> > + struct thermal_cooling_device *cdev;
> >> > + int target;
> >> > +
> >> > + list_for_each_entry(instance1, &tz->instances, tz_node) {
> >> > + cdev = instance1->cdev;
> >> > +
> >> > + /* cooling device has already been updated*/
> >> > + if (cdev->updated)
> >> > + continue;
> >> > +
> >> > + target = 0;
> >> > + /* Make sure cdev enters the deepest cooling state */
> >> > + list_for_each_entry(instance2, &cdev->instances, cdev_node) {
> >> > + if (instance2->target == -1)
> >> > + continue;
> >> > + if (instance2->target > target)
> >> > + target = instance2->target;
> >> > + }
> >> > + cdev->ops->set_cur_state(cdev, target);
> >> > + cdev->updated = 1;
> >> > + }
> >> > +}
> >>
> >> I think the above arbitrator solution does not escalate. As I can see,
> >> the arbitrator takes care of cooling device instances in the same thermal zone.
> >>
> > why?
> > It parses the thermal instances in one thermal zone to find the cooling
> > devices, and then parse all the thermal instances for the cooling
> > devices, no matter which thermal zone it belongs to.
>
> I think I got confused with the nomenclature change. So, yes, it does
> go through all zones and cooling devices. But still, ideally it should
> be a plist per device, don't you think? This way your search space is
> reduced.
>
I'm not aware of this before, but here is a problem when I tried to use
plist.
First, we should use plist_node->prio as the thermal_instance->target
instead, right?
But the problem is that thermal_instance->target needs to be updated
when temperature is changed. But plist does not support dynamic updating
the priority, right?
So when changing the cooling state target of a thermal instance, I need
to delete and re-insert the plist_node? This does not seems efficient to
me.
thanks,
rui
> >
> >> What if you have a device which expose cooling device instances in different
> >> thermal zones?
> >>
> >> Besides, the thermal constraint might collide with pm QoS or with settings
> >> coming from user space interfaces.
> >>
> >
> >> That's actually why I was suggesting to have this 'arbitrator' or constraint
> >> management outside the thermal framework. And managed per device at some
> >> other abstraction layer.
> >>
> > Yeah, but I think we can have an incremental patch to handle the pm Qos
> > constrains. Especially Durga already has the governor patches to move
> > this out of the thermal framework.
> > what do you think?
>
> I think it sounds like a plan. I also like incremental approach.
>
> >
> > thanks,
> > rui
> >
> >> > +
> >> > /*
> >> > * Cooling algorithm for active trip points
> >> > *
> >> > @@ -1086,19 +1115,24 @@ static void thermal_zone_trip_update(str
> >> > cur_state = cur_state > instance->lower ?
> >> > (cur_state - 1) : instance->lower;
> >> > }
> >> > - cdev->ops->set_cur_state(cdev, cur_state);
> >> > + instance->target = cur_state;
> >> > + cdev->updated = 0; /* cooling device needs update */
> >> > }
> >> > } else { /* below trip */
> >> > list_for_each_entry(instance, &tz->instances, tz_node) {
> >> > if (instance->trip != trip)
> >> > continue;
> >> >
> >> > + /* Do not use the deacitve thermal instance */
> >> > + if (instance->target == -1)
> >> > + continue;
> >> > cdev = instance->cdev;
> >> > cdev->ops->get_cur_state(cdev, &cur_state);
> >> >
> >> > cur_state = cur_state > instance->lower ?
> >> > - (cur_state - 1) : instance->lower;
> >> > - cdev->ops->set_cur_state(cdev, cur_state);
> >> > + (cur_state - 1) : -1;
> >> > + instance->target = cur_state;
> >> > + cdev->updated = 0; /* cooling device needs update */
> >> > }
> >> > }
> >> >
> >> > @@ -1159,6 +1193,7 @@ void thermal_zone_device_update(struct t
> >> > }
> >> > }
> >> >
> >> > + thermal_zone_do_update(tz);
> >> > if (tz->forced_passive)
> >> > thermal_zone_device_passive(tz, temp, tz->forced_passive,
> >> > THERMAL_TRIPS_NONE);
> >> > Index: rtd3/include/linux/thermal.h
> >> > ===================================================================
> >> > --- rtd3.orig/include/linux/thermal.h
> >> > +++ rtd3/include/linux/thermal.h
> >> > @@ -86,6 +86,7 @@ struct thermal_cooling_device {
> >> > struct device device;
> >> > void *devdata;
> >> > const struct thermal_cooling_device_ops *ops;
> >> > + int updated; /* 1 if the cooling device does not need update */
> >> > struct list_head instances;
> >> > struct list_head node;
> >> > };
> >> >
> >> >
> >
> >
>
>
>
^ permalink raw reply
* Re: Cpuidle drivers,Suspend : Fix suspend/resume hang with intel_idle driver
From: Srivatsa S. Bhat @ 2012-07-02 5:28 UTC (permalink / raw)
To: preeti
Cc: Rafael J. Wysocki, Dave Hansen, Linux PM mailing list, linux-pm,
linux-acpi, Deepthi Dharwar
In-Reply-To: <4FF13133.2090700@linux.vnet.ibm.com>
On 07/02/2012 10:57 AM, preeti wrote:
> On 07/02/2012 10:34 AM, Srivatsa S. Bhat wrote:
>> On 07/02/2012 10:07 AM, preeti wrote:
>>> On 06/30/2012 03:41 AM, Rafael J. Wysocki wrote:
>>>> On Friday, June 29, 2012, preeti wrote:
>>>>> On 06/29/2012 09:41 AM, preeti wrote:
>>>>>> On 06/29/2012 12:41 AM, Rafael J. Wysocki wrote:
>>>>>>> On Thursday, June 28, 2012, preeti wrote:
>>>>>>>> On 06/28/2012 03:23 PM, Rafael J. Wysocki wrote:
>>>>>>>>> On Thursday, June 28, 2012, preeti wrote:
>>>>>>>>>>
>>>>>>>>>> From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
>>>>>>> [...]
>>>>>>>> cpuidle is an architecture independent part of the kernel code.Since
>>>>>>>> this patch aims at x86 architecture in specific,I considered it
>>>>>>>> inappropriate.
>>>>>>>>
>>>>>>>> In addition to this,suspend happens on x86 only if ACPI is configured.
>>>>>>>
>>>>>>> But that is not required for intel_idle, so if it hangs with intel_idle,
>>>>>>> then it is not dependent on ACPI after all.
>>>>>> True intel_idle does not need ACPI to be configured,but that also means
>>>>>> that the kernel will not provide you the means to suspend.There is no
>>>>>> question of resume hang here at all as you cannot suspend in the first
>>>>>> place.
>>>>>>
>>>>>> The issue is when ACPI is configured,and intel_idle is chosen to be the
>>>>>> cpuidle driver.In this situation when the user suspends,cpus can enter
>>>>>> deep sleep states as intel_idle driver does not prevent then from doing so.
>>>>>> This is when resume hangs.
>>>>>>>
>>>>>>>> Therefore it seemed right to put the callback in ACPI specific code
>>>>>>>> which deals with ACPI sleep support.
>>>>>>>
>>>>>>> I wonder if we can address this issue correctly. That is, in a non-racy
>>>>>>> way and in a better place.
>>>>>>>
>>>>>>> First, I really don't think it is necessary to "suspend" cpuidle (be it
>>>>>>> ACPI or any other) when device drivers' suspend routines are being
>>>>>>> executed (which also is racy, because the cpuidle "suspend" may be running
>>>>>>> concurrently with cpuidle on another CPU) or earlier. We really may want
>>>>>>> to disable the deeper C-states when we're about to execute
>>>>>>> suspend_ops->prepare_late(), or hibernation_ops->prepare(), i.e. after
>>>>>>> we've run dpm_suspend_end() successfully.
>>>>>>
>>>>>> The commit "ACPI:disable lower idle C-states across suspend/resume"
>>>>>> states that device_suspend() calls ACPI suspend functions which cause
>>>>>> side effects on the lower idle C-states.This means we need to disable
>>>>>> entry into deeper C-states even before dpm_suspend_start(),but how much
>>>>>> before?
>>>>>>
>>>>>> The commit answers this too.It says removing the functionality of
>>>>>> entering deep C-states before suspend removed the side effects.Besides,
>>>>>> the commit title says'across suspend/resume'.So I think to address the
>>>>>> resume hang effectively,it is desirable to disable entry into deeper
>>>>>> C-states during suspend_prepare operations.
>>>>>
>>>>> To clarify this further,since we take action upon PM_SUSPEND_PREPARE
>>>>> notification,which is called before suspend begins,we avoid race
>>>>> condition between suspend operations and disabling entry into deeper
>>>>> c-states altogether.
>>>>
>>>> Well, what about races between disabling deeper C-states and cpuidle?
>>>
>>> Yes.The question still remains about the cpus that have already entered
>>> deep C-states even before suspend routines have begun.We are not taking
>>> precautions to prevent them from going into idle.
>>>
>>
>> Actually we need *not* take such precautions. See below.
>>
>>> If the resume hang does depend on the cpus being in deep C-state,even
>>> after the fix with acpi_idle_suspend, there should have been a hang
>>> in scenarios where the cpus have already entered deep C-states before
>>> suspend has begun.
>>>
>>
>> Nope, that won't happen because we have CPU hotplug in between. The suspend
>> path goes through CPU hotplug (cpu offline), and one of the phases of the
>> cpu offline operation requires that the cpu that is going down runs the
>> CPU_DYING_FROZEN callbacks. No other cpu can execute that. So even if a cpu
>> was in a deep C-state, it would be kicked out of cpu idle and will run
>> these callbacks during cpu hotplug. Its enough if we ensure that it doesn't
>> enter deep C-states again, *after* the cpu hotplug operation. And the flag you
>> are using or the callback method that Rafael suggested looks sufficient for
>> ensuring that.
>>
>> So, we need not break our heads on too many race conditions here :-)
>
> But let us note that without the acpi_idle_suspend check,it was at the
> device layer,that the hang was happening.Before cpu hotplug even begins.
>
acpi_idle_suspend check has nothing to do with hang at the device layer,
IIRC. The hang at device layer was because we were coming out of cpu idle
without enabling interrupts. And Deepthi already fixed that issue
(commit 75cc523 upstream).
So the problem that still remains is the _resume_ hang when using intel idle
as the idle driver. And not allowing cpus to be in deep C-states while
doing suspend should fix that.
Regards,
Srivatsa S. Bhat
^ permalink raw reply
* Re: Cpuidle drivers,Suspend : Fix suspend/resume hang with intel_idle driver
From: preeti @ 2012-07-02 5:27 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: Rafael J. Wysocki, Dave Hansen, Linux PM mailing list, linux-pm,
linux-acpi, Deepthi Dharwar
In-Reply-To: <4FF12BD4.5080803@linux.vnet.ibm.com>
On 07/02/2012 10:34 AM, Srivatsa S. Bhat wrote:
> On 07/02/2012 10:07 AM, preeti wrote:
>> On 06/30/2012 03:41 AM, Rafael J. Wysocki wrote:
>>> On Friday, June 29, 2012, preeti wrote:
>>>> On 06/29/2012 09:41 AM, preeti wrote:
>>>>> On 06/29/2012 12:41 AM, Rafael J. Wysocki wrote:
>>>>>> On Thursday, June 28, 2012, preeti wrote:
>>>>>>> On 06/28/2012 03:23 PM, Rafael J. Wysocki wrote:
>>>>>>>> On Thursday, June 28, 2012, preeti wrote:
>>>>>>>>>
>>>>>>>>> From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
>>>>>> [...]
>>>>>>> cpuidle is an architecture independent part of the kernel code.Since
>>>>>>> this patch aims at x86 architecture in specific,I considered it
>>>>>>> inappropriate.
>>>>>>>
>>>>>>> In addition to this,suspend happens on x86 only if ACPI is configured.
>>>>>>
>>>>>> But that is not required for intel_idle, so if it hangs with intel_idle,
>>>>>> then it is not dependent on ACPI after all.
>>>>> True intel_idle does not need ACPI to be configured,but that also means
>>>>> that the kernel will not provide you the means to suspend.There is no
>>>>> question of resume hang here at all as you cannot suspend in the first
>>>>> place.
>>>>>
>>>>> The issue is when ACPI is configured,and intel_idle is chosen to be the
>>>>> cpuidle driver.In this situation when the user suspends,cpus can enter
>>>>> deep sleep states as intel_idle driver does not prevent then from doing so.
>>>>> This is when resume hangs.
>>>>>>
>>>>>>> Therefore it seemed right to put the callback in ACPI specific code
>>>>>>> which deals with ACPI sleep support.
>>>>>>
>>>>>> I wonder if we can address this issue correctly. That is, in a non-racy
>>>>>> way and in a better place.
>>>>>>
>>>>>> First, I really don't think it is necessary to "suspend" cpuidle (be it
>>>>>> ACPI or any other) when device drivers' suspend routines are being
>>>>>> executed (which also is racy, because the cpuidle "suspend" may be running
>>>>>> concurrently with cpuidle on another CPU) or earlier. We really may want
>>>>>> to disable the deeper C-states when we're about to execute
>>>>>> suspend_ops->prepare_late(), or hibernation_ops->prepare(), i.e. after
>>>>>> we've run dpm_suspend_end() successfully.
>>>>>
>>>>> The commit "ACPI:disable lower idle C-states across suspend/resume"
>>>>> states that device_suspend() calls ACPI suspend functions which cause
>>>>> side effects on the lower idle C-states.This means we need to disable
>>>>> entry into deeper C-states even before dpm_suspend_start(),but how much
>>>>> before?
>>>>>
>>>>> The commit answers this too.It says removing the functionality of
>>>>> entering deep C-states before suspend removed the side effects.Besides,
>>>>> the commit title says'across suspend/resume'.So I think to address the
>>>>> resume hang effectively,it is desirable to disable entry into deeper
>>>>> C-states during suspend_prepare operations.
>>>>
>>>> To clarify this further,since we take action upon PM_SUSPEND_PREPARE
>>>> notification,which is called before suspend begins,we avoid race
>>>> condition between suspend operations and disabling entry into deeper
>>>> c-states altogether.
>>>
>>> Well, what about races between disabling deeper C-states and cpuidle?
>>
>> Yes.The question still remains about the cpus that have already entered
>> deep C-states even before suspend routines have begun.We are not taking
>> precautions to prevent them from going into idle.
>>
>
> Actually we need *not* take such precautions. See below.
>
>> If the resume hang does depend on the cpus being in deep C-state,even
>> after the fix with acpi_idle_suspend, there should have been a hang
>> in scenarios where the cpus have already entered deep C-states before
>> suspend has begun.
>>
>
> Nope, that won't happen because we have CPU hotplug in between. The suspend
> path goes through CPU hotplug (cpu offline), and one of the phases of the
> cpu offline operation requires that the cpu that is going down runs the
> CPU_DYING_FROZEN callbacks. No other cpu can execute that. So even if a cpu
> was in a deep C-state, it would be kicked out of cpu idle and will run
> these callbacks during cpu hotplug. Its enough if we ensure that it doesn't
> enter deep C-states again, *after* the cpu hotplug operation. And the flag you
> are using or the callback method that Rafael suggested looks sufficient for
> ensuring that.
>
> So, we need not break our heads on too many race conditions here :-)
But let us note that without the acpi_idle_suspend check,it was at the
device layer,that the hang was happening.Before cpu hotplug even begins.
This means that with the acpi_idle_suspend fix, we ensure cpus do not
enter deeper C-states but we do not check if there are already idle
cpus.But with the fix,device layer suspend happens fine.Cpu hotplug does
not come to the rescue of the cpus already in deep C-states.
This is where the question about 'are the idle cpus actually causing
problems' arises.
>
> Regards,
> Srivatsa S. Bhat
>
Regards
Preeti
^ permalink raw reply
* Re: Cpuidle drivers, Suspend : Fix suspend/resume hang with intel_idle driver
From: Srivatsa S. Bhat @ 2012-07-02 5:04 UTC (permalink / raw)
To: preeti; +Cc: Linux PM mailing list, Dave Hansen, linux-acpi, linux-pm
In-Reply-To: <4FF1258F.7020805@linux.vnet.ibm.com>
On 07/02/2012 10:07 AM, preeti wrote:
> On 06/30/2012 03:41 AM, Rafael J. Wysocki wrote:
>> On Friday, June 29, 2012, preeti wrote:
>>> On 06/29/2012 09:41 AM, preeti wrote:
>>>> On 06/29/2012 12:41 AM, Rafael J. Wysocki wrote:
>>>>> On Thursday, June 28, 2012, preeti wrote:
>>>>>> On 06/28/2012 03:23 PM, Rafael J. Wysocki wrote:
>>>>>>> On Thursday, June 28, 2012, preeti wrote:
>>>>>>>>
>>>>>>>> From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
>>>>> [...]
>>>>>> cpuidle is an architecture independent part of the kernel code.Since
>>>>>> this patch aims at x86 architecture in specific,I considered it
>>>>>> inappropriate.
>>>>>>
>>>>>> In addition to this,suspend happens on x86 only if ACPI is configured.
>>>>>
>>>>> But that is not required for intel_idle, so if it hangs with intel_idle,
>>>>> then it is not dependent on ACPI after all.
>>>> True intel_idle does not need ACPI to be configured,but that also means
>>>> that the kernel will not provide you the means to suspend.There is no
>>>> question of resume hang here at all as you cannot suspend in the first
>>>> place.
>>>>
>>>> The issue is when ACPI is configured,and intel_idle is chosen to be the
>>>> cpuidle driver.In this situation when the user suspends,cpus can enter
>>>> deep sleep states as intel_idle driver does not prevent then from doing so.
>>>> This is when resume hangs.
>>>>>
>>>>>> Therefore it seemed right to put the callback in ACPI specific code
>>>>>> which deals with ACPI sleep support.
>>>>>
>>>>> I wonder if we can address this issue correctly. That is, in a non-racy
>>>>> way and in a better place.
>>>>>
>>>>> First, I really don't think it is necessary to "suspend" cpuidle (be it
>>>>> ACPI or any other) when device drivers' suspend routines are being
>>>>> executed (which also is racy, because the cpuidle "suspend" may be running
>>>>> concurrently with cpuidle on another CPU) or earlier. We really may want
>>>>> to disable the deeper C-states when we're about to execute
>>>>> suspend_ops->prepare_late(), or hibernation_ops->prepare(), i.e. after
>>>>> we've run dpm_suspend_end() successfully.
>>>>
>>>> The commit "ACPI:disable lower idle C-states across suspend/resume"
>>>> states that device_suspend() calls ACPI suspend functions which cause
>>>> side effects on the lower idle C-states.This means we need to disable
>>>> entry into deeper C-states even before dpm_suspend_start(),but how much
>>>> before?
>>>>
>>>> The commit answers this too.It says removing the functionality of
>>>> entering deep C-states before suspend removed the side effects.Besides,
>>>> the commit title says'across suspend/resume'.So I think to address the
>>>> resume hang effectively,it is desirable to disable entry into deeper
>>>> C-states during suspend_prepare operations.
>>>
>>> To clarify this further,since we take action upon PM_SUSPEND_PREPARE
>>> notification,which is called before suspend begins,we avoid race
>>> condition between suspend operations and disabling entry into deeper
>>> c-states altogether.
>>
>> Well, what about races between disabling deeper C-states and cpuidle?
>
> Yes.The question still remains about the cpus that have already entered
> deep C-states even before suspend routines have begun.We are not taking
> precautions to prevent them from going into idle.
>
Actually we need *not* take such precautions. See below.
> If the resume hang does depend on the cpus being in deep C-state,even
> after the fix with acpi_idle_suspend, there should have been a hang
> in scenarios where the cpus have already entered deep C-states before
> suspend has begun.
>
Nope, that won't happen because we have CPU hotplug in between. The suspend
path goes through CPU hotplug (cpu offline), and one of the phases of the
cpu offline operation requires that the cpu that is going down runs the
CPU_DYING_FROZEN callbacks. No other cpu can execute that. So even if a cpu
was in a deep C-state, it would be kicked out of cpu idle and will run
these callbacks during cpu hotplug. Its enough if we ensure that it doesn't
enter deep C-states again, *after* the cpu hotplug operation. And the flag you
are using or the callback method that Rafael suggested looks sufficient for
ensuring that.
So, we need not break our heads on too many race conditions here :-)
Regards,
Srivatsa S. Bhat
^ permalink raw reply
* Re: Cpuidle drivers,Suspend : Fix suspend/resume hang with intel_idle driver
From: preeti @ 2012-07-02 4:37 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Dave Hansen, Linux PM mailing list, linux-pm, linux-acpi,
Srivatsa.S.Bhat, Deepthi Dharwar
In-Reply-To: <201206300011.51402.rjw@sisk.pl>
On 06/30/2012 03:41 AM, Rafael J. Wysocki wrote:
> On Friday, June 29, 2012, preeti wrote:
>> On 06/29/2012 09:41 AM, preeti wrote:
>>> On 06/29/2012 12:41 AM, Rafael J. Wysocki wrote:
>>>> On Thursday, June 28, 2012, preeti wrote:
>>>>> On 06/28/2012 03:23 PM, Rafael J. Wysocki wrote:
>>>>>> On Thursday, June 28, 2012, preeti wrote:
>>>>>>>
>>>>>>> From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
>>>> [...]
>>>>> cpuidle is an architecture independent part of the kernel code.Since
>>>>> this patch aims at x86 architecture in specific,I considered it
>>>>> inappropriate.
>>>>>
>>>>> In addition to this,suspend happens on x86 only if ACPI is configured.
>>>>
>>>> But that is not required for intel_idle, so if it hangs with intel_idle,
>>>> then it is not dependent on ACPI after all.
>>> True intel_idle does not need ACPI to be configured,but that also means
>>> that the kernel will not provide you the means to suspend.There is no
>>> question of resume hang here at all as you cannot suspend in the first
>>> place.
>>>
>>> The issue is when ACPI is configured,and intel_idle is chosen to be the
>>> cpuidle driver.In this situation when the user suspends,cpus can enter
>>> deep sleep states as intel_idle driver does not prevent then from doing so.
>>> This is when resume hangs.
>>>>
>>>>> Therefore it seemed right to put the callback in ACPI specific code
>>>>> which deals with ACPI sleep support.
>>>>
>>>> I wonder if we can address this issue correctly. That is, in a non-racy
>>>> way and in a better place.
>>>>
>>>> First, I really don't think it is necessary to "suspend" cpuidle (be it
>>>> ACPI or any other) when device drivers' suspend routines are being
>>>> executed (which also is racy, because the cpuidle "suspend" may be running
>>>> concurrently with cpuidle on another CPU) or earlier. We really may want
>>>> to disable the deeper C-states when we're about to execute
>>>> suspend_ops->prepare_late(), or hibernation_ops->prepare(), i.e. after
>>>> we've run dpm_suspend_end() successfully.
>>>
>>> The commit "ACPI:disable lower idle C-states across suspend/resume"
>>> states that device_suspend() calls ACPI suspend functions which cause
>>> side effects on the lower idle C-states.This means we need to disable
>>> entry into deeper C-states even before dpm_suspend_start(),but how much
>>> before?
>>>
>>> The commit answers this too.It says removing the functionality of
>>> entering deep C-states before suspend removed the side effects.Besides,
>>> the commit title says'across suspend/resume'.So I think to address the
>>> resume hang effectively,it is desirable to disable entry into deeper
>>> C-states during suspend_prepare operations.
>>
>> To clarify this further,since we take action upon PM_SUSPEND_PREPARE
>> notification,which is called before suspend begins,we avoid race
>> condition between suspend operations and disabling entry into deeper
>> c-states altogether.
>
> Well, what about races between disabling deeper C-states and cpuidle?
Yes.The question still remains about the cpus that have already entered
deep C-states even before suspend routines have begun.We are not taking
precautions to prevent them from going into idle.
If the resume hang does depend on the cpus being in deep C-state,even
after the fix with acpi_idle_suspend, there should have been a hang
in scenarios where the cpus have already entered deep C-states before
suspend has begun.
>
> Rafael
>
Regards
Preeti
^ permalink raw reply
* Re: [PATCH v2] acpi: intel_idle : break dependency between modules
From: Daniel Lezcano @ 2012-07-01 19:36 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linaro-dev, Andrew Morton, linux-pm, x86, linux-kernel,
linux-acpi, srivatsa.bhat, Colin Cross, linux-pm
In-Reply-To: <201206300027.05175.rjw@sisk.pl>
On 06/30/2012 12:27 AM, Rafael J. Wysocki wrote:
> On Friday, June 29, 2012, Daniel Lezcano wrote:
>> On 06/28/2012 09:24 PM, Rafael J. Wysocki wrote:
>>> On Thursday, June 28, 2012, Daniel Lezcano wrote:
>>>> When the system is booted with some cpus offline, the idle
>>>> driver is not initialized. When a cpu is set online, the
>>>> acpi code call the intel idle init function. Unfortunately
>>>> this code introduce a dependency between intel_idle and acpi.
>>>>
>>>> This patch is intended to remove this dependency by using the
>>>> notifier of intel_idle. This patch has the benefit of
>>>> encapsulating the intel_idle driver and remove some exported
>>>> functions.
>>>>
>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>
>>> This one looks good to me too.
>>>
>>> Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
>>
>> Thanks for the review Rafael.
>>
>>> Len, are you going to take it?
>>
>> Rafael, Len,
>>
>> After the discussion [1], I put in place a tree at:
>>
>> ssh://git.linaro.org/srv/git.linaro.org/git/people/dlezcano/cpuidle-next.git
>> #cpuidle-next
>>
>> I proposed this tree to group the cpuidle modifications and prevent the
>> last minutes conflict when Len will apply them.
>>
>> I also included the tree into linux-next for wider testing.
>
> I can't speak for Len, but I'm not sure he'll like this.
I sent the proposition a week ago and Len was Cc'ed. I guess he is very
busy but the problem we are facing is there are a lot of pending
modifications for cpuidle because of some new architecture (like the
big.LITTLE and tegra3). Colin's patchset is one of them.
> Anyway, you seem to have the same material as Len in that tree, won't there
> be any conflicts in linux-next?
At the first glance no, until he merge the patches we sent. As I already
said the purpose is to help to consolidate the patches sent for cpuidle
by acting as a proxy.
I hope that helps.
Thanks
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* Re: [PATCH 1/8] ACPI sysfs.c strlen fix
From: Alan Stern @ 2012-06-30 14:25 UTC (permalink / raw)
To: Len Brown; +Cc: linux-acpi, linux-pm, linux-kernel, Len Brown, Pavel Vasilyev
In-Reply-To: <9f132652d94c96476b0b0a8caf0c10e96ab10fa8.1341032550.git.len.brown@intel.com>
On Sat, 30 Jun 2012, Len Brown wrote:
> From: Pavel Vasilyev <pavel@pavlinux.ru>
>
> Current code is ignoring the last character of "enable" and "disable"
> in comparisons.
>
> https://bugzilla.kernel.org/show_bug.cgi?id=33732
>
> Signed-off-by: Len Brown <len.brown@intel.com>
> ---
> drivers/acpi/sysfs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
> index 9f66181..240a244 100644
> --- a/drivers/acpi/sysfs.c
> +++ b/drivers/acpi/sysfs.c
> @@ -173,7 +173,7 @@ static int param_set_trace_state(const char *val, struct kernel_param *kp)
> {
> int result = 0;
>
> - if (!strncmp(val, "enable", strlen("enable") - 1)) {
> + if (!strncmp(val, "enable", strlen("enable"))) {
Ironically, this is the less efficient of two possible fixes.
It would have been better to leave the "- 1" in place and change strlen
to sizeof. That's probably what the original author had in mind.
Alan Stern
^ permalink raw reply
* [PATCH -v5 6/6] fault-injection: add selftests for cpu and memory hotplug
From: Akinobu Mita @ 2012-06-30 5:59 UTC (permalink / raw)
To: linux-kernel, akpm
Cc: Akinobu Mita, Pavel Machek, Rafael J. Wysocki, linux-pm, Greg KH,
linux-mm, Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev,
Américo Wang, Dave Jones
In-Reply-To: <1341035970-20490-1-git-send-email-akinobu.mita@gmail.com>
This adds two selftests
* tools/testing/selftests/cpu-hotplug/on-off-test.sh is testing script
for CPU hotplug
1. Online all hot-pluggable CPUs
2. Offline all hot-pluggable CPUs
3. Online all hot-pluggable CPUs again
4. Exit if cpu-notifier-error-inject.ko is not available
5. Offline all hot-pluggable CPUs in preparation for testing
6. Test CPU hot-add error handling by injecting notifier errors
7. Online all hot-pluggable CPUs in preparation for testing
8. Test CPU hot-remove error handling by injecting notifier errors
* tools/testing/selftests/memory-hotplug/on-off-test.sh is doing the
similar thing for memory hotplug.
1. Online all hot-pluggable memory
2. Offline 10% of hot-pluggable memory
3. Online all hot-pluggable memory again
4. Exit if memory-notifier-error-inject.ko is not available
5. Offline 10% of hot-pluggable memory in preparation for testing
6. Test memory hot-add error handling by injecting notifier errors
7. Online all hot-pluggable memory in preparation for testing
8. Test memory hot-remove error handling by injecting notifier errors
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: linux-pm@lists.linux-foundation.org
Cc: Greg KH <greg@kroah.com>
Cc: linux-mm@kvack.org
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Américo Wang <xiyou.wangcong@gmail.com>
Cc: Dave Jones <davej@redhat.com>
---
* v5
- make testing scripts a part of tools/testing/selftests
- do simple on/offline tests even if no notifier error injection support
tools/testing/selftests/Makefile | 2 +-
tools/testing/selftests/cpu-hotplug/Makefile | 6 +
tools/testing/selftests/cpu-hotplug/on-off-test.sh | 221 +++++++++++++++++++
tools/testing/selftests/memory-hotplug/Makefile | 6 +
.../selftests/memory-hotplug/on-off-test.sh | 230 ++++++++++++++++++++
5 files changed, 464 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/cpu-hotplug/Makefile
create mode 100755 tools/testing/selftests/cpu-hotplug/on-off-test.sh
create mode 100644 tools/testing/selftests/memory-hotplug/Makefile
create mode 100755 tools/testing/selftests/memory-hotplug/on-off-test.sh
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index a4162e1..85baf11 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -1,4 +1,4 @@
-TARGETS = breakpoints kcmp mqueue vm
+TARGETS = breakpoints kcmp mqueue vm cpu-hotplug memory-hotplug
all:
for TARGET in $(TARGETS); do \
diff --git a/tools/testing/selftests/cpu-hotplug/Makefile b/tools/testing/selftests/cpu-hotplug/Makefile
new file mode 100644
index 0000000..7c9c20f
--- /dev/null
+++ b/tools/testing/selftests/cpu-hotplug/Makefile
@@ -0,0 +1,6 @@
+all:
+
+run_tests:
+ ./on-off-test.sh
+
+clean:
diff --git a/tools/testing/selftests/cpu-hotplug/on-off-test.sh b/tools/testing/selftests/cpu-hotplug/on-off-test.sh
new file mode 100755
index 0000000..bdde7cf
--- /dev/null
+++ b/tools/testing/selftests/cpu-hotplug/on-off-test.sh
@@ -0,0 +1,221 @@
+#!/bin/bash
+
+SYSFS=
+
+prerequisite()
+{
+ msg="skip all tests:"
+
+ if [ $UID != 0 ]; then
+ echo $msg must be run as root >&2
+ exit 0
+ fi
+
+ SYSFS=`mount -t sysfs | head -1 | awk '{ print $3 }'`
+
+ if [ ! -d "$SYSFS" ]; then
+ echo $msg sysfs is not mounted >&2
+ exit 0
+ fi
+
+ if ! ls $SYSFS/devices/system/cpu/cpu* > /dev/null 2>&1; then
+ echo $msg cpu hotplug is not supported >&2
+ exit 0
+ fi
+}
+
+#
+# list all hot-pluggable CPUs
+#
+hotpluggable_cpus()
+{
+ local state=${1:-.\*}
+
+ for cpu in $SYSFS/devices/system/cpu/cpu*; do
+ if [ -f $cpu/online ] && grep -q $state $cpu/online; then
+ echo ${cpu##/*/cpu}
+ fi
+ done
+}
+
+hotplaggable_offline_cpus()
+{
+ hotpluggable_cpus 0
+}
+
+hotpluggable_online_cpus()
+{
+ hotpluggable_cpus 1
+}
+
+cpu_is_online()
+{
+ grep -q 1 $SYSFS/devices/system/cpu/cpu$1/online
+}
+
+cpu_is_offline()
+{
+ grep -q 0 $SYSFS/devices/system/cpu/cpu$1/online
+}
+
+online_cpu()
+{
+ echo 1 > $SYSFS/devices/system/cpu/cpu$1/online
+}
+
+offline_cpu()
+{
+ echo 0 > $SYSFS/devices/system/cpu/cpu$1/online
+}
+
+online_cpu_expect_success()
+{
+ local cpu=$1
+
+ if ! online_cpu $cpu; then
+ echo $FUNCNAME $cpu: unexpected fail >&2
+ elif ! cpu_is_online $cpu; then
+ echo $FUNCNAME $cpu: unexpected offline >&2
+ fi
+}
+
+online_cpu_expect_fail()
+{
+ local cpu=$1
+
+ if online_cpu $cpu 2> /dev/null; then
+ echo $FUNCNAME $cpu: unexpected success >&2
+ elif ! cpu_is_offline $cpu; then
+ echo $FUNCNAME $cpu: unexpected online >&2
+ fi
+}
+
+offline_cpu_expect_success()
+{
+ local cpu=$1
+
+ if ! offline_cpu $cpu; then
+ echo $FUNCNAME $cpu: unexpected fail >&2
+ elif ! cpu_is_offline $cpu; then
+ echo $FUNCNAME $cpu: unexpected offline >&2
+ fi
+}
+
+offline_cpu_expect_fail()
+{
+ local cpu=$1
+
+ if offline_cpu $cpu 2> /dev/null; then
+ echo $FUNCNAME $cpu: unexpected success >&2
+ elif ! cpu_is_online $cpu; then
+ echo $FUNCNAME $cpu: unexpected offline >&2
+ fi
+}
+
+error=-12
+priority=0
+
+while getopts e:hp: opt; do
+ case $opt in
+ e)
+ error=$OPTARG
+ ;;
+ h)
+ echo "Usage $0 [ -e errno ] [ -p notifier-priority ]"
+ exit
+ ;;
+ p)
+ priority=$OPTARG
+ ;;
+ esac
+done
+
+if ! [ "$error" -ge -4095 -a "$error" -lt 0 ]; then
+ echo "error code must be -4095 <= errno < 0" >&2
+ exit 1
+fi
+
+prerequisite
+
+#
+# Online all hot-pluggable CPUs
+#
+for cpu in `hotplaggable_offline_cpus`; do
+ online_cpu_expect_success $cpu
+done
+
+#
+# Offline all hot-pluggable CPUs
+#
+for cpu in `hotpluggable_online_cpus`; do
+ offline_cpu_expect_success $cpu
+done
+
+#
+# Online all hot-pluggable CPUs again
+#
+for cpu in `hotplaggable_offline_cpus`; do
+ online_cpu_expect_success $cpu
+done
+
+#
+# Test with cpu notifier error injection
+#
+
+DEBUGFS=`mount -t debugfs | head -1 | awk '{ print $3 }'`
+NOTIFIER_ERR_INJECT_DIR=$DEBUGFS/notifier-error-inject/cpu
+
+prerequisite_extra()
+{
+ msg="skip extra tests:"
+
+ /sbin/modprobe -q -r cpu-notifier-error-inject
+ /sbin/modprobe -q cpu-notifier-error-inject priority=$priority
+
+ if [ ! -d "$DEBUGFS" ]; then
+ echo $msg debugfs is not mounted >&2
+ exit 0
+ fi
+
+ if [ ! -d $NOTIFIER_ERR_INJECT_DIR ]; then
+ echo $msg cpu-notifier-error-inject module is not available >&2
+ exit 0
+ fi
+}
+
+prerequisite_extra
+
+#
+# Offline all hot-pluggable CPUs
+#
+echo 0 > $NOTIFIER_ERR_INJECT_DIR/actions/CPU_DOWN_PREPARE/error
+for cpu in `hotpluggable_online_cpus`; do
+ offline_cpu_expect_success $cpu
+done
+
+#
+# Test CPU hot-add error handling (offline => online)
+#
+echo $error > $NOTIFIER_ERR_INJECT_DIR/actions/CPU_UP_PREPARE/error
+for cpu in `hotplaggable_offline_cpus`; do
+ online_cpu_expect_fail $cpu
+done
+
+#
+# Online all hot-pluggable CPUs
+#
+echo 0 > $NOTIFIER_ERR_INJECT_DIR/actions/CPU_UP_PREPARE/error
+for cpu in `hotplaggable_offline_cpus`; do
+ online_cpu_expect_success $cpu
+done
+
+#
+# Test CPU hot-remove error handling (online => offline)
+#
+echo $error > $NOTIFIER_ERR_INJECT_DIR/actions/CPU_DOWN_PREPARE/error
+for cpu in `hotpluggable_online_cpus`; do
+ offline_cpu_expect_fail $cpu
+done
+
+echo 0 > $NOTIFIER_ERR_INJECT_DIR/actions/CPU_DOWN_PREPARE/error
+/sbin/modprobe -q -r cpu-notifier-error-inject
diff --git a/tools/testing/selftests/memory-hotplug/Makefile b/tools/testing/selftests/memory-hotplug/Makefile
new file mode 100644
index 0000000..7c9c20f
--- /dev/null
+++ b/tools/testing/selftests/memory-hotplug/Makefile
@@ -0,0 +1,6 @@
+all:
+
+run_tests:
+ ./on-off-test.sh
+
+clean:
diff --git a/tools/testing/selftests/memory-hotplug/on-off-test.sh b/tools/testing/selftests/memory-hotplug/on-off-test.sh
new file mode 100755
index 0000000..a2816f6
--- /dev/null
+++ b/tools/testing/selftests/memory-hotplug/on-off-test.sh
@@ -0,0 +1,230 @@
+#!/bin/bash
+
+SYSFS=
+
+prerequisite()
+{
+ msg="skip all tests:"
+
+ if [ $UID != 0 ]; then
+ echo $msg must be run as root >&2
+ exit 0
+ fi
+
+ SYSFS=`mount -t sysfs | head -1 | awk '{ print $3 }'`
+
+ if [ ! -d "$SYSFS" ]; then
+ echo $msg sysfs is not mounted >&2
+ exit 0
+ fi
+
+ if ! ls $SYSFS/devices/system/memory/memory* > /dev/null 2>&1; then
+ echo $msg memory hotplug is not supported >&2
+ exit 0
+ fi
+}
+
+#
+# list all hot-pluggable memory
+#
+hotpluggable_memory()
+{
+ local state=${1:-.\*}
+
+ for memory in $SYSFS/devices/system/memory/memory*; do
+ if grep -q 1 $memory/removable &&
+ grep -q $state $memory/state; then
+ echo ${memory##/*/memory}
+ fi
+ done
+}
+
+hotplaggable_offline_memory()
+{
+ hotpluggable_memory offline
+}
+
+hotpluggable_online_memory()
+{
+ hotpluggable_memory online
+}
+
+memory_is_online()
+{
+ grep -q online $SYSFS/devices/system/memory/memory$1/state
+}
+
+memory_is_offline()
+{
+ grep -q offline $SYSFS/devices/system/memory/memory$1/state
+}
+
+online_memory()
+{
+ echo online > $SYSFS/devices/system/memory/memory$1/state
+}
+
+offline_memory()
+{
+ echo offline > $SYSFS/devices/system/memory/memory$1/state
+}
+
+online_memory_expect_success()
+{
+ local memory=$1
+
+ if ! online_memory $memory; then
+ echo $FUNCNAME $memory: unexpected fail >&2
+ elif ! memory_is_online $memory; then
+ echo $FUNCNAME $memory: unexpected offline >&2
+ fi
+}
+
+online_memory_expect_fail()
+{
+ local memory=$1
+
+ if online_memory $memory 2> /dev/null; then
+ echo $FUNCNAME $memory: unexpected success >&2
+ elif ! memory_is_offline $memory; then
+ echo $FUNCNAME $memory: unexpected online >&2
+ fi
+}
+
+offline_memory_expect_success()
+{
+ local memory=$1
+
+ if ! offline_memory $memory; then
+ echo $FUNCNAME $memory: unexpected fail >&2
+ elif ! memory_is_offline $memory; then
+ echo $FUNCNAME $memory: unexpected offline >&2
+ fi
+}
+
+offline_memory_expect_fail()
+{
+ local memory=$1
+
+ if offline_memory $memory 2> /dev/null; then
+ echo $FUNCNAME $memory: unexpected success >&2
+ elif ! memory_is_online $memory; then
+ echo $FUNCNAME $memory: unexpected offline >&2
+ fi
+}
+
+error=-12
+priority=0
+ratio=10
+
+while getopts e:hp:r: opt; do
+ case $opt in
+ e)
+ error=$OPTARG
+ ;;
+ h)
+ echo "Usage $0 [ -e errno ] [ -p notifier-priority ] [ -r percent-of-memory-to-offline ]"
+ exit
+ ;;
+ p)
+ priority=$OPTARG
+ ;;
+ r)
+ ratio=$OPTARG
+ ;;
+ esac
+done
+
+if ! [ "$error" -ge -4095 -a "$error" -lt 0 ]; then
+ echo "error code must be -4095 <= errno < 0" >&2
+ exit 1
+fi
+
+prerequisite
+
+#
+# Online all hot-pluggable memory
+#
+for memory in `hotplaggable_offline_memory`; do
+ online_memory_expect_success $memory
+done
+
+#
+# Offline $ratio percent of hot-pluggable memory
+#
+for memory in `hotpluggable_online_memory`; do
+ if [ $((RANDOM % 100)) -lt $ratio ]; then
+ offline_memory_expect_success $memory
+ fi
+done
+
+#
+# Online all hot-pluggable memory again
+#
+for memory in `hotplaggable_offline_memory`; do
+ online_memory_expect_success $memory
+done
+
+#
+# Test with memory notifier error injection
+#
+
+DEBUGFS=`mount -t debugfs | head -1 | awk '{ print $3 }'`
+NOTIFIER_ERR_INJECT_DIR=$DEBUGFS/notifier-error-inject/memory
+
+prerequisite_extra()
+{
+ msg="skip extra tests:"
+
+ /sbin/modprobe -q -r memory-notifier-error-inject
+ /sbin/modprobe -q memory-notifier-error-inject priority=$priority
+
+ if [ ! -d "$DEBUGFS" ]; then
+ echo $msg debugfs is not mounted >&2
+ exit 0
+ fi
+
+ if [ ! -d $NOTIFIER_ERR_INJECT_DIR ]; then
+ echo $msg memory-notifier-error-inject module is not available >&2
+ exit 0
+ fi
+}
+
+prerequisite_extra
+
+#
+# Offline $ratio percent of hot-pluggable memory
+#
+echo 0 > $NOTIFIER_ERR_INJECT_DIR/actions/MEM_GOING_OFFLINE/error
+for memory in `hotpluggable_online_memory`; do
+ if [ $((RANDOM % 100)) -lt $ratio ]; then
+ offline_memory_expect_success $memory
+ fi
+done
+
+#
+# Test memory hot-add error handling (offline => online)
+#
+echo $error > $NOTIFIER_ERR_INJECT_DIR/actions/MEM_GOING_ONLINE/error
+for memory in `hotplaggable_offline_memory`; do
+ online_memory_expect_fail $memory
+done
+
+#
+# Online all hot-pluggable memory
+#
+echo 0 > $NOTIFIER_ERR_INJECT_DIR/actions/MEM_GOING_ONLINE/error
+for memory in `hotplaggable_offline_memory`; do
+ online_memory_expect_success $memory
+done
+
+#
+# Test memory hot-remove error handling (online => offline)
+#
+echo $error > $NOTIFIER_ERR_INJECT_DIR/actions/MEM_GOING_OFFLINE/error
+for memory in `hotpluggable_online_memory`; do
+ offline_memory_expect_fail $memory
+done
+
+echo 0 > $NOTIFIER_ERR_INJECT_DIR/actions/MEM_GOING_OFFLINE/error
+/sbin/modprobe -q -r memory-notifier-error-inject
--
1.7.10.4
--
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
* [PATCH -v5 3/6] PM: PM notifier error injection module
From: Akinobu Mita @ 2012-06-30 5:59 UTC (permalink / raw)
To: linux-kernel, akpm; +Cc: linux-pm, Akinobu Mita
In-Reply-To: <1341035970-20490-1-git-send-email-akinobu.mita@gmail.com>
This provides the ability to inject artifical errors to PM notifier chain
callbacks. It is controlled through debugfs interface under
/sys/kernel/debug/notifier-error-inject/pm
Each of the files in "error" directory represents an event which can be
failed and contains the error code. If the notifier call chain should
be failed with some events notified, write the error code to the files.
If the notifier call chain should be failed with some events notified,
write the error code to "actions/<notifier event>/error".
Example: Inject PM suspend error (-12 = -ENOMEM)
# cd /sys/kernel/debug/notifier-error-inject/pm
# echo -12 > actions/PM_SUSPEND_PREPARE/error
# echo mem > /sys/power/state
bash: echo: write error: Cannot allocate memory
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Acked-by: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: linux-pm@lists.linux-foundation.org
---
No changes since v4
lib/Kconfig.debug | 24 ++++++++++++++++++++
lib/Makefile | 1 +
lib/pm-notifier-error-inject.c | 49 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 74 insertions(+)
create mode 100644 lib/pm-notifier-error-inject.c
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index be0c197..246cea6 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1119,6 +1119,30 @@ config CPU_NOTIFIER_ERROR_INJECT
If unsure, say N.
+config PM_NOTIFIER_ERROR_INJECT
+ tristate "PM notifier error injection module"
+ depends on PM && NOTIFIER_ERROR_INJECTION
+ default m if PM_DEBUG
+ help
+ This option provides the ability to inject artifical errors to
+ PM notifier chain callbacks. It is controlled through debugfs
+ interface /sys/kernel/debug/notifier-error-inject/pm
+
+ If the notifier call chain should be failed with some events
+ notified, write the error code to "actions/<notifier event>/error".
+
+ Example: Inject PM suspend error (-12 = -ENOMEM)
+
+ # cd /sys/kernel/debug/notifier-error-inject/pm/
+ # echo -12 > actions/PM_SUSPEND_PREPARE/error
+ # echo mem > /sys/power/state
+ bash: echo: write error: Cannot allocate memory
+
+ To compile this code as a module, choose M here: the module will
+ be called pm-notifier-error-inject.
+
+ If unsure, say N.
+
config FAULT_INJECTION
bool "Fault-injection framework"
depends on DEBUG_KERNEL
diff --git a/lib/Makefile b/lib/Makefile
index 23fba9e..230a949 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -92,6 +92,7 @@ obj-$(CONFIG_IOMMU_HELPER) += iommu-helper.o
obj-$(CONFIG_FAULT_INJECTION) += fault-inject.o
obj-$(CONFIG_NOTIFIER_ERROR_INJECTION) += notifier-error-inject.o
obj-$(CONFIG_CPU_NOTIFIER_ERROR_INJECT) += cpu-notifier-error-inject.o
+obj-$(CONFIG_PM_NOTIFIER_ERROR_INJECT) += pm-notifier-error-inject.o
lib-$(CONFIG_GENERIC_BUG) += bug.o
diff --git a/lib/pm-notifier-error-inject.c b/lib/pm-notifier-error-inject.c
new file mode 100644
index 0000000..c094b2d
--- /dev/null
+++ b/lib/pm-notifier-error-inject.c
@@ -0,0 +1,49 @@
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/suspend.h>
+
+#include "notifier-error-inject.h"
+
+static int priority;
+module_param(priority, int, 0);
+MODULE_PARM_DESC(priority, "specify PM notifier priority");
+
+static struct notifier_err_inject pm_notifier_err_inject = {
+ .actions = {
+ { NOTIFIER_ERR_INJECT_ACTION(PM_HIBERNATION_PREPARE) },
+ { NOTIFIER_ERR_INJECT_ACTION(PM_SUSPEND_PREPARE) },
+ { NOTIFIER_ERR_INJECT_ACTION(PM_RESTORE_PREPARE) },
+ {}
+ }
+};
+
+static struct dentry *dir;
+
+static int err_inject_init(void)
+{
+ int err;
+
+ dir = notifier_err_inject_init("pm", notifier_err_inject_dir,
+ &pm_notifier_err_inject, priority);
+ if (IS_ERR(dir))
+ return PTR_ERR(dir);
+
+ err = register_pm_notifier(&pm_notifier_err_inject.nb);
+ if (err)
+ debugfs_remove_recursive(dir);
+
+ return err;
+}
+
+static void err_inject_exit(void)
+{
+ unregister_pm_notifier(&pm_notifier_err_inject.nb);
+ debugfs_remove_recursive(dir);
+}
+
+module_init(err_inject_init);
+module_exit(err_inject_exit);
+
+MODULE_DESCRIPTION("PM notifier error injection module");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Akinobu Mita <akinobu.mita@gmail.com>");
--
1.7.10.4
^ permalink raw reply related
* [PATCH -v5 1/6] fault-injection: notifier error injection
From: Akinobu Mita @ 2012-06-30 5:59 UTC (permalink / raw)
To: linux-kernel, akpm
Cc: Akinobu Mita, Pavel Machek, Rafael J. Wysocki, linux-pm, Greg KH,
linux-mm, Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev,
Michael Ellerman
In-Reply-To: <1341035970-20490-1-git-send-email-akinobu.mita@gmail.com>
The notifier error injection provides the ability to inject artifical
errors to specified notifier chain callbacks. It is useful to test the
error handling of notifier call chain failures.
This adds common basic functions to define which type of events can be
fail and to initialize the debugfs interface to control what error code
should be returned and which event should be failed.
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: linux-pm@lists.linux-foundation.org
Cc: Greg KH <greg@kroah.com>
Cc: linux-mm@kvack.org
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Michael Ellerman <michael@ellerman.id.au>
---
No changes since v4
lib/Kconfig.debug | 11 +++++
lib/Makefile | 1 +
lib/notifier-error-inject.c | 112 +++++++++++++++++++++++++++++++++++++++++++
lib/notifier-error-inject.h | 24 ++++++++++
4 files changed, 148 insertions(+)
create mode 100644 lib/notifier-error-inject.c
create mode 100644 lib/notifier-error-inject.h
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index ff5bdee..c848758 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1084,6 +1084,17 @@ config LKDTM
Documentation on how to use the module can be found in
Documentation/fault-injection/provoke-crashes.txt
+config NOTIFIER_ERROR_INJECTION
+ tristate "Notifier error injection"
+ depends on DEBUG_KERNEL
+ select DEBUG_FS
+ help
+ This option provides the ability to inject artifical errors to
+ specified notifier chain callbacks. It is useful to test the error
+ handling of notifier call chain failures.
+
+ Say N if unsure.
+
config CPU_NOTIFIER_ERROR_INJECT
tristate "CPU notifier error injection module"
depends on HOTPLUG_CPU && DEBUG_KERNEL
diff --git a/lib/Makefile b/lib/Makefile
index 8c31a0c..23fba9e 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -90,6 +90,7 @@ obj-$(CONFIG_AUDIT_GENERIC) += audit.o
obj-$(CONFIG_SWIOTLB) += swiotlb.o
obj-$(CONFIG_IOMMU_HELPER) += iommu-helper.o
obj-$(CONFIG_FAULT_INJECTION) += fault-inject.o
+obj-$(CONFIG_NOTIFIER_ERROR_INJECTION) += notifier-error-inject.o
obj-$(CONFIG_CPU_NOTIFIER_ERROR_INJECT) += cpu-notifier-error-inject.o
lib-$(CONFIG_GENERIC_BUG) += bug.o
diff --git a/lib/notifier-error-inject.c b/lib/notifier-error-inject.c
new file mode 100644
index 0000000..44b92cb
--- /dev/null
+++ b/lib/notifier-error-inject.c
@@ -0,0 +1,112 @@
+#include <linux/module.h>
+
+#include "notifier-error-inject.h"
+
+static int debugfs_errno_set(void *data, u64 val)
+{
+ *(int *)data = clamp_t(int, val, -MAX_ERRNO, 0);
+ return 0;
+}
+
+static int debugfs_errno_get(void *data, u64 *val)
+{
+ *val = *(int *)data;
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(fops_errno, debugfs_errno_get, debugfs_errno_set,
+ "%lld\n");
+
+static struct dentry *debugfs_create_errno(const char *name, mode_t mode,
+ struct dentry *parent, int *value)
+{
+ return debugfs_create_file(name, mode, parent, value, &fops_errno);
+}
+
+static int notifier_err_inject_callback(struct notifier_block *nb,
+ unsigned long val, void *p)
+{
+ int err = 0;
+ struct notifier_err_inject *err_inject =
+ container_of(nb, struct notifier_err_inject, nb);
+ struct notifier_err_inject_action *action;
+
+ for (action = err_inject->actions; action->name; action++) {
+ if (action->val == val) {
+ err = action->error;
+ break;
+ }
+ }
+ if (err)
+ pr_info("Injecting error (%d) to %s\n", err, action->name);
+
+ return notifier_from_errno(err);
+}
+
+struct dentry *notifier_err_inject_dir;
+EXPORT_SYMBOL_GPL(notifier_err_inject_dir);
+
+struct dentry *notifier_err_inject_init(const char *name, struct dentry *parent,
+ struct notifier_err_inject *err_inject, int priority)
+{
+ struct notifier_err_inject_action *action;
+ mode_t mode = S_IFREG | S_IRUSR | S_IWUSR;
+ struct dentry *dir;
+ struct dentry *actions_dir;
+
+ err_inject->nb.notifier_call = notifier_err_inject_callback;
+ err_inject->nb.priority = priority;
+
+ dir = debugfs_create_dir(name, parent);
+ if (!dir)
+ return ERR_PTR(-ENOMEM);
+
+ actions_dir = debugfs_create_dir("actions", dir);
+ if (!actions_dir)
+ goto fail;
+
+ for (action = err_inject->actions; action->name; action++) {
+ struct dentry *action_dir;
+
+ action_dir = debugfs_create_dir(action->name, actions_dir);
+ if (!action_dir)
+ goto fail;
+
+ /*
+ * Create debugfs r/w file containing action->error. If
+ * notifier call chain is called with action->val, it will
+ * fail with the error code
+ */
+ if (!debugfs_create_errno("error", mode, action_dir,
+ &action->error))
+ goto fail;
+ }
+ return dir;
+fail:
+ debugfs_remove_recursive(dir);
+ return ERR_PTR(-ENOMEM);
+}
+EXPORT_SYMBOL_GPL(notifier_err_inject_init);
+
+static int __init err_inject_init(void)
+{
+ notifier_err_inject_dir =
+ debugfs_create_dir("notifier-error-inject", NULL);
+
+ if (!notifier_err_inject_dir)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static void __exit err_inject_exit(void)
+{
+ debugfs_remove_recursive(notifier_err_inject_dir);
+}
+
+module_init(err_inject_init);
+module_exit(err_inject_exit);
+
+MODULE_DESCRIPTION("Notifier error injection module");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Akinobu Mita <akinobu.mita@gmail.com>");
diff --git a/lib/notifier-error-inject.h b/lib/notifier-error-inject.h
new file mode 100644
index 0000000..99b3b6f
--- /dev/null
+++ b/lib/notifier-error-inject.h
@@ -0,0 +1,24 @@
+#include <linux/atomic.h>
+#include <linux/debugfs.h>
+#include <linux/notifier.h>
+
+struct notifier_err_inject_action {
+ unsigned long val;
+ int error;
+ const char *name;
+};
+
+#define NOTIFIER_ERR_INJECT_ACTION(action) \
+ .name = #action, .val = (action),
+
+struct notifier_err_inject {
+ struct notifier_block nb;
+ struct notifier_err_inject_action actions[];
+ /* The last slot must be terminated with zero sentinel */
+};
+
+extern struct dentry *notifier_err_inject_dir;
+
+extern struct dentry *notifier_err_inject_init(const char *name,
+ struct dentry *parent, struct notifier_err_inject *err_inject,
+ int priority);
--
1.7.10.4
--
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
* [PATCH -v5 0/6] notifier error injection
From: Akinobu Mita @ 2012-06-30 5:59 UTC (permalink / raw)
To: linux-kernel, akpm
Cc: Michael Ellerman, Greg KH, Akinobu Mita, linux-mm, Paul Mackerras,
Américo Wang, linux-pm, linuxppc-dev
This provides kernel modules that can be used to test the error handling
of notifier call chain failures by injecting artifical errors to the
following notifier chain callbacks.
* CPU notifier
* PM notifier
* memory hotplug notifier
* powerpc pSeries reconfig notifier
Example: Inject CPU offline error (-1 == -EPERM)
# cd /sys/kernel/debug/notifier-error-inject/cpu
# echo -1 > actions/CPU_DOWN_PREPARE/error
# echo 0 > /sys/devices/system/cpu/cpu1/online
bash: echo: write error: Operation not permitted
This also adds cpu and memory hotplug tests to tools/testing/selftests
These tests first do simple online and offline test and then do fault
injection tests if notifier error injection module is available.
Changelog:
* v5 (change only testing scripts)
- make testing scripts a part of tools/testing/selftests
- do simple on/offline tests even if no notifier error injection support
* v4 (It is about 11 months since v3)
- prefix all APIs with notifier_err_inject_*
- rearrange debugfs interface
(e.g. $DEBUGFS/cpu-notifier-error-inject/CPU_DOWN_PREPARE -->
$DEBUGFS/notifier-error-inject/cpu/actions/CPU_DOWN_PREPARE/error)
- update modules to follow new interface
- add -r option for memory-notifier.sh to specify percent of offlining
memory blocks
* v3
- rewrite to be kernel modules instead of initializing at late_initcall()s
(it makes the diffstat look different but most code remains unchanged)
- export err_inject_notifier_block_{init,cleanup} for modules
- export pSeries_reconfig_notifier_{,un}register symbols for a module
- notifier priority can be specified as a module parameter
- add testing scripts in tools/testing/fault-injection
* v2
- "PM: Improve error code of pm_notifier_call_chain()" is now in -next
- "debugfs: add debugfs_create_int" is dropped
- put a comment in err_inject_notifier_block_init()
- only allow valid errno to be injected (-MAX_ERRNO <= errno <= 0)
- improve Kconfig help text
- make CONFIG_PM_NOTIFIER_ERROR_INJECTION visible even if PM_DEBUG is disabled
- make CONFIG_PM_NOTIFIER_ERROR_INJECTION default if PM_DEBUG is enabled
Akinobu Mita (6):
fault-injection: notifier error injection
cpu: rewrite cpu-notifier-error-inject module
PM: PM notifier error injection module
memory: memory notifier error injection module
powerpc: pSeries reconfig notifier error injection module
fault-injection: add selftests for cpu and memory hotplug
lib/Kconfig.debug | 91 +++++++-
lib/Makefile | 5 +
lib/cpu-notifier-error-inject.c | 63 +++---
lib/memory-notifier-error-inject.c | 48 ++++
lib/notifier-error-inject.c | 112 ++++++++++
lib/notifier-error-inject.h | 24 ++
lib/pSeries-reconfig-notifier-error-inject.c | 51 +++++
lib/pm-notifier-error-inject.c | 49 +++++
tools/testing/selftests/Makefile | 2 +-
tools/testing/selftests/cpu-hotplug/Makefile | 6 +
tools/testing/selftests/cpu-hotplug/on-off-test.sh | 221 +++++++++++++++++++
tools/testing/selftests/memory-hotplug/Makefile | 6 +
.../selftests/memory-hotplug/on-off-test.sh | 230 ++++++++++++++++++++
13 files changed, 867 insertions(+), 41 deletions(-)
create mode 100644 lib/memory-notifier-error-inject.c
create mode 100644 lib/notifier-error-inject.c
create mode 100644 lib/notifier-error-inject.h
create mode 100644 lib/pSeries-reconfig-notifier-error-inject.c
create mode 100644 lib/pm-notifier-error-inject.c
create mode 100644 tools/testing/selftests/cpu-hotplug/Makefile
create mode 100755 tools/testing/selftests/cpu-hotplug/on-off-test.sh
create mode 100644 tools/testing/selftests/memory-hotplug/Makefile
create mode 100755 tools/testing/selftests/memory-hotplug/on-off-test.sh
Cc: Pavel Machek <pavel@ucw.cz>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: linux-pm@lists.linux-foundation.org
Cc: Greg KH <greg@kroah.com>
Cc: linux-mm@kvack.org
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Américo Wang <xiyou.wangcong@gmail.com>
Cc: Michael Ellerman <michael@ellerman.id.au>
Cc: Dave Jones <davej@redhat.com>
--
1.7.10.4
^ permalink raw reply
* [PATCH 8/8] acpi_pad: fix power_saving thread deadlock
From: Len Brown @ 2012-06-30 5:07 UTC (permalink / raw)
To: linux-acpi, linux-pm; +Cc: Len Brown, linux-kernel, stable, Stuart Hayes
In-Reply-To: <9f132652d94c96476b0b0a8caf0c10e96ab10fa8.1341032550.git.len.brown@intel.com>
From: Stuart Hayes <Stuart_Hayes@Dell.com>
The acpi_pad driver can get stuck in destroy_power_saving_task()
waiting for kthread_stop() to stop a power_saving thread. The problem
is that the isolated_cpus_lock mutex is owned when
destroy_power_saving_task() calls kthread_stop(), which waits for a
power_saving thread to end, and the power_saving thread tries to
acquire the isolated_cpus_lock when it calls round_robin_cpu(). This
patch fixes the issue by making round_robin_cpu() use its own mutex.
https://bugzilla.kernel.org/show_bug.cgi?id=42981
Cc: stable@vger.kernel.org
Signed-off-by: Stuart Hayes <Stuart_Hayes@Dell.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
drivers/acpi/acpi_pad.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c
index a43fa1a..1502c502 100644
--- a/drivers/acpi/acpi_pad.c
+++ b/drivers/acpi/acpi_pad.c
@@ -36,6 +36,7 @@
#define ACPI_PROCESSOR_AGGREGATOR_DEVICE_NAME "Processor Aggregator"
#define ACPI_PROCESSOR_AGGREGATOR_NOTIFY 0x80
static DEFINE_MUTEX(isolated_cpus_lock);
+static DEFINE_MUTEX(round_robin_lock);
static unsigned long power_saving_mwait_eax;
@@ -107,7 +108,7 @@ static void round_robin_cpu(unsigned int tsk_index)
if (!alloc_cpumask_var(&tmp, GFP_KERNEL))
return;
- mutex_lock(&isolated_cpus_lock);
+ mutex_lock(&round_robin_lock);
cpumask_clear(tmp);
for_each_cpu(cpu, pad_busy_cpus)
cpumask_or(tmp, tmp, topology_thread_cpumask(cpu));
@@ -116,7 +117,7 @@ static void round_robin_cpu(unsigned int tsk_index)
if (cpumask_empty(tmp))
cpumask_andnot(tmp, cpu_online_mask, pad_busy_cpus);
if (cpumask_empty(tmp)) {
- mutex_unlock(&isolated_cpus_lock);
+ mutex_unlock(&round_robin_lock);
return;
}
for_each_cpu(cpu, tmp) {
@@ -131,7 +132,7 @@ static void round_robin_cpu(unsigned int tsk_index)
tsk_in_cpu[tsk_index] = preferred_cpu;
cpumask_set_cpu(preferred_cpu, pad_busy_cpus);
cpu_weight[preferred_cpu]++;
- mutex_unlock(&isolated_cpus_lock);
+ mutex_unlock(&round_robin_lock);
set_cpus_allowed_ptr(current, cpumask_of(preferred_cpu));
}
--
1.7.11.1.104.ge7b44f1
^ permalink raw reply related
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