Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-03-16 Thread Pavel Machek
Hi!

> > Defaults are not right solution; system should still work if I select
> > non-default settings. Which you claim is not a case, but you don't see
> > why you should fix it.
> 
> If something is implemented using IPMI, then IPMI has to be there to
> use it.  Matthew's statement was:
> 
> For example, if you load the ACPI power meter driver before you've
> installed the ACPI IPMI driver you'll typically get failures (most
> vendors implement it via IPMI).

Yes, and this failure is not fixed with different default.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-03-13 Thread Corey Minyard
On 03/13/2014 03:38 AM, Pavel Machek wrote:
> On Thu 2014-03-13 07:24:36, Matthew Garrett wrote:
>> On Thu, 2014-03-13 at 08:22 +0100, Pavel Machek wrote:
>>> On Wed 2014-03-12 23:22:49, Matthew Garrett wrote:
 No. The power meter driver knows nothing about IPMI. It makes no IPMI
 calls. There's no requirement that a vendor implement it via IPMI.
>>> Yet you claim that IMPI is needed for that, and that's why you made
>>> IMPI default.
>> I claim that the ACPI spec defines the behaviour of IPMI operation
>> regions, and so we should default IPMI to Y in order to (by default)
>> implement the ACPI spec.
>>
>>> So ... do we need dmi-based blacklist?
>> I don't see why.
> Your reasoning for default y was that "power meter depends on
> this". Then, claim that "power meter does not officially depend on it"
> so it would be wrong to have a dependency.

I believe the correct statement is "On some systems power meter depends
on it".

>
> Defaults are not right solution; system should still work if I select
> non-default settings. Which you claim is not a case, but you don't see
> why you should fix it.

If something is implemented using IPMI, then IPMI has to be there to
use it.  Matthew's statement was:

For example, if you load the ACPI power meter driver before you've
installed the ACPI IPMI driver you'll typically get failures (most
vendors implement it via IPMI).

Other things besides the power meter can be implemented using IPMI, it's
up to the vendor.  How will those things work if IPMI is not available?

-corey

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-03-13 Thread Pavel Machek
On Thu 2014-03-13 07:24:36, Matthew Garrett wrote:
> On Thu, 2014-03-13 at 08:22 +0100, Pavel Machek wrote:
> > On Wed 2014-03-12 23:22:49, Matthew Garrett wrote:
> > > No. The power meter driver knows nothing about IPMI. It makes no IPMI
> > > calls. There's no requirement that a vendor implement it via IPMI.
> > 
> > Yet you claim that IMPI is needed for that, and that's why you made
> > IMPI default.
> 
> I claim that the ACPI spec defines the behaviour of IPMI operation
> regions, and so we should default IPMI to Y in order to (by default)
> implement the ACPI spec.
> 
> > So ... do we need dmi-based blacklist?
> 
> I don't see why.

Your reasoning for default y was that "power meter depends on
this". Then, claim that "power meter does not officially depend on it"
so it would be wrong to have a dependency.

Defaults are not right solution; system should still work if I select
non-default settings. Which you claim is not a case, but you don't see
why you should fix it.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-03-13 Thread Matthew Garrett
On Thu, 2014-03-13 at 08:22 +0100, Pavel Machek wrote:
> On Wed 2014-03-12 23:22:49, Matthew Garrett wrote:
> > No. The power meter driver knows nothing about IPMI. It makes no IPMI
> > calls. There's no requirement that a vendor implement it via IPMI.
> 
> Yet you claim that IMPI is needed for that, and that's why you made
> IMPI default.

I claim that the ACPI spec defines the behaviour of IPMI operation
regions, and so we should default IPMI to Y in order to (by default)
implement the ACPI spec.

> So ... do we need dmi-based blacklist?

I don't see why.

-- 
Matthew Garrett 


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-03-13 Thread Pavel Machek
On Wed 2014-03-12 23:22:49, Matthew Garrett wrote:
> On Thu, 2014-03-13 at 00:00 +0100, Pavel Machek wrote:
> > On Tue 2014-02-18 23:15:08, Matthew Garrett wrote:
> > > For example, if you load the ACPI power meter driver before you've
> > > installed the ACPI IPMI driver you'll typically get failures (most
> > > vendors implement it via IPMI).
> > 
> > Would the right solution be to implement dependency between power
> > meter and IMPI?
> 
> No. The power meter driver knows nothing about IPMI. It makes no IPMI
> calls. There's no requirement that a vendor implement it via IPMI.

Yet you claim that IMPI is needed for that, and that's why you made
IMPI default.

So ... do we need dmi-based blacklist?
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-03-12 Thread Matthew Garrett
On Thu, 2014-03-13 at 00:00 +0100, Pavel Machek wrote:
> On Tue 2014-02-18 23:15:08, Matthew Garrett wrote:
> > For example, if you load the ACPI power meter driver before you've
> > installed the ACPI IPMI driver you'll typically get failures (most
> > vendors implement it via IPMI).
> 
> Would the right solution be to implement dependency between power
> meter and IMPI?

No. The power meter driver knows nothing about IPMI. It makes no IPMI
calls. There's no requirement that a vendor implement it via IPMI.

-- 
Matthew Garrett 


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-03-12 Thread Pavel Machek
On Tue 2014-02-18 23:15:08, Matthew Garrett wrote:
> On Wed, 2014-02-19 at 00:26 +0100, Rafael J. Wysocki wrote:
> > On Tuesday, February 18, 2014 11:28:29 AM Matthew Garrett wrote:
> > > 
> > > The ACPI IPMI driver implements IPMI operation region support for the ACPI
> > > core. Systems that declare ACPI operation regions may reference them at 
> > > any
> > > time, including during kernel initialisation. These accesses will fail
> > > unless the ACPI IPMI driver is present, and undesirable system behaviour
> > > may result. Set the default to Y in order to encourage distributions and
> > > users to configure kernels to avoid awkward surprises.
> > 
> > Do you have any examples of problems caused by that or is this just 
> > theoretical
> > at the moment?
> 
> For example, if you load the ACPI power meter driver before you've
> installed the ACPI IPMI driver you'll typically get failures (most
> vendors implement it via IPMI).

Would the right solution be to implement dependency between power
meter and IMPI?
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-23 Thread Zheng, Lv
Hi,

> From: linux-acpi-ow...@vger.kernel.org 
> [mailto:linux-acpi-ow...@vger.kernel.org] On Behalf Of Matthew Garrett
> Sent: Saturday, February 22, 2014 12:13 AM
> 
> On Fri, 2014-02-21 at 02:17 +, Zheng, Lv wrote:
> 
> > In fact there is a workaround solution I've posted here:
> > https://patchwork.kernel.org/patch/2831851/
> > The updated version of this patch can be found at:
> > https://bugzilla.kernel.org/attachment.cgi?id=112611
> > It is the acpi-ipmi13.patch file.
> 
> I don't think that solves the underlying problem? We're still left with
> no guarantee of ordering between ACPI IPMI operation region support
> being available and the loading of a driver that may rely on it.

Yes. As it is a bit far beyond the bug report, I didn't work on it.
But actually, I have solution to achieve this.

> > I think there should be relationship between ACPI region and Linux kernel 
> > modules.
> > In fact on the well-known operating system, _REG is always invoked at the 
> > end of the IRP_PNP_START_DEVICE completions.
> > But we may still be able to return -EPROBE_DEFER in the power meter driver 
> > when it detects failures caused by the readiness state
> of the region handlers.
> 
> We don't have a guarantee that it'll happen at probe time. The
> capabilities list may be static, and so we'll get our first failure on
> an attempted userspace access instead.

This is the issue related to the IPMI operation region, and it can be solved by 
a "request_module()" call.
The -EPROBE_DEFER I mentioned is about the quality of  acpi power meter driver 
itself.

> It may be that Corey's suggestion satisfies the concerns Russ raised.
> Building IPMI in but only probing by default on systems that declare a
> BMC in ACPI or DMI should avoid boot-time delays while still ensuring
> that the functionality is available on systems where there's a real
> chance that the firmware expects the OS to provide support.

If you take a look at this patch (https://patchwork.kernel.org/patch/2831851/), 
you'll see it is possible to add request_module() in 
acpi_region_default_handler().
The callback is invoked when the first IPMI region access happens and it is in 
the process context without any locking.
I'd rather to embed acpi_ipmi into ipmi_si module with this fix:
https://bugzilla.kernel.org/attachment.cgi?id=112611 (acpi-ipmi14.patch).
After that, a request_module() invocation of ipmi_si in 
acpi_region_default_handler() can solve what you are worrying about.

What's your opinion?

Thanks and best regards
-Lv

> --
> Matthew Garrett 
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-21 Thread Corey Minyard
On 02/21/2014 09:51 AM, Matthew Garrett wrote:
> On Fri, 2014-02-21 at 07:37 -0600, Corey Minyard wrote:
>
>> However, the basic problem is that hardware vendors produce hardware
>> that sucks and then expect software to fix all the problems.  Most IPMI
>> interfaces don't have interrupts, so they have to be polled.  Then they
>> add important interfaces on top of it like firmware upgrade and ACPI and
>> expect it to perform well.  If vendors would just have an interrupt for
>> IPMI, 99% of these problems would go away.
> Not going to disagree. The impact on power consumption is also pretty
> awful. I should re-read the spec to figure out whether we can
> legitimately get away with not doing that.

Thinking about this some more, I realized that it may possible to turn
off the driver if nothing at all is waiting.  I'll need to look at the
spec to see if I'm forgetting something.

>
>> One thing we can do is remove the default interface probing for IPMI. 
>> Even though the spec has it, all modern hardware should have it
>> specified in ACPI or device tree.  That should fix all the slow boot
>> problems, at least.  If a user wants to add a default interface, they
>> can use the interface to dynamically add it after boot time.
> Something like this (untested)?

That's exactly what I would have done...

-corey

>
> diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
> index eea8464..5126230 100644
> --- a/drivers/char/ipmi/Kconfig
> +++ b/drivers/char/ipmi/Kconfig
> @@ -52,6 +52,16 @@ config IPMI_SI
>  Currently, only KCS and SMIC are supported.  If
>  you are using IPMI, you should probably say "y" here.
>  
> +config IPMI_PROBE_DEFAULTS
> +   bool 'Probe for all possible IPMI interfaces by default'
> +   help
> + Modern systems will usually expose IPMI interfaces via a 
> discoverable
> +firmware mechanism such as ACPI or DMI. Older systems do not, and so
> +the driver is forced to probe hardware manually. This may cause boot
> +delays. Say "n" here to disable this manual probing. IPMI will then
> +only be available on older systems if the 
> "ipmi_si_intf.trydefaults=1"
> +boot argument is passed.
> +
>  config IPMI_WATCHDOG
> tristate 'IPMI Watchdog Timer'
> help
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c 
> b/drivers/char/ipmi/ipmi_si_intf.c
> index 03f4189..82c7d56 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -1230,7 +1230,11 @@ static bool  si_tryplatform = 1;
>  #ifdef CONFIG_PCI
>  static bool  si_trypci = 1;
>  #endif
> +#ifdef CONFIG_IPMI_PROBE_DEFAULTS
>  static bool  si_trydefaults = 1;
> +#else
> +static bool  si_trydefaults;
> +#endif
>  static char  *si_type[SI_MAX_PARMS];
>  #define MAX_SI_TYPE_STR 30
>  static char  si_type_str[MAX_SI_TYPE_STR];
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-21 Thread Russ Anderson
On Fri, Feb 21, 2014 at 12:13:13AM +, Matthew Garrett wrote:
> On Thu, 2014-02-20 at 17:59 -0600, Russ Anderson wrote:
> > On Thu, Feb 20, 2014 at 11:09:42PM +, Matthew Garrett wrote:
> > > On Thu, 2014-02-20 at 16:45 -0600, Russ Anderson wrote:
> > > > 
> > > > The ACPI spec requires IPMI functionality before a module loads at
> > > > boot time?  And the kernel is *broken* if it does not support ACIP IPMI
> > > > functionality before module load time?  Really?
> > > 
> > > There's no mechanism to ensure that IPMI support will be loaded before
> > > ACPI calls attempt to access IPMI operation regions. Really.
> > 
> > And no mechanism can be added to ensure that ACPI call are
> > not attempted before IPMI is initialized?  A flag or lock
> > or exported symbol indicating IPMI support is ready.
> 
> ACPI functions are a black box to drivers. You make an ACPI call, the
> AML code does something. We could block there, but what's the driver
> supposed to do at that point? The core could call out to a module
> loader, but if the driver is built in and IPMI isn't then you'll end up
> with a 60 second pause in boot and a driver that doesn't work. 

When you say "if the driver is built in" which driver are you
talking about?  I thought the issue was making sure ipmi_si 
driver was loaded before power meter driver.



> > > > > ACPI 4.0 includes support for IPMI operation regions. Modular IPMI 
> > > > > means
> > > > > that the kernel will spend a significant amount of time (potentially
> > > > > until a user manually loads a driver) failing to implement part of the
> > > > > IPMI specification. That's a problem, and the correct fix is to ensure
> > > > > that the kernel always implements IPMI support.
> > > > 
> > > > The ACPI spec says ipmi_si cannot be a driver?  Really?
> > > > What is the real problem you are trying to solve?
> > > 
> > > The most straightforward case is that of an ACPI power meter.
> > 
> > So it is just a matter of making sure ipmi_si modules loads before
> > the ACPI power meter module loads, right?  module dependency issue.
> 
> No, because the power meter driver has no way of knowing that a vendor
> has implemented this interface via IPMI. *Any* ACPI entry point could
> theoretically reference IPMI code, even the _INI method that's called
> during ACPI core init. If it does, and if you don't have built-in ACPI
> support, you'd fail ACPI initialisation and things would go downhill
> from there.

Again, it sounds like as long as ipmi_si driver is loaded before
power meter driver, power meter driver is fine.



-- 
Russ Anderson,  Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc  r...@sgi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-21 Thread Russ Anderson
On Fri, Feb 21, 2014 at 02:17:12AM +, Zheng, Lv wrote:
> Hi,
> 
> Sorry for interrupting you.
> I have some information that may be helpful for your discussion.
> Please find them in the inlined replies.
> Well, I don't want to join the fight, just for your informations. :-)

I don't want to join the fight, either.

I have not looked at your code changes but the description
looks like the right direction.


> > From: linux-acpi-ow...@vger.kernel.org 
> > [mailto:linux-acpi-ow...@vger.kernel.org] On Behalf Of Russ Anderson
> > Sent: Friday, February 21, 2014 7:59 AM
> > 
> > On Thu, Feb 20, 2014 at 11:09:42PM +, Matthew Garrett wrote:
> > > On Thu, 2014-02-20 at 16:45 -0600, Russ Anderson wrote:
> > > > On Thu, Feb 20, 2014 at 10:26:45PM +, Matthew Garrett wrote:
> > >
> > > > > Because I'm trying to ensure that the default behaviour of the kernel 
> > > > > is
> > > > > to *work*. Defaulting to having IPMI be modular means that the default
> > > > > behaviour of the kernel, as far as the ACPI spec goes, is to be 
> > > > > broken.
> > > >
> > > > The ACPI spec requires IPMI functionality before a module loads at
> > > > boot time?  And the kernel is *broken* if it does not support ACIP IPMI
> > > > functionality before module load time?  Really?
> > >
> > > There's no mechanism to ensure that IPMI support will be loaded before
> > > ACPI calls attempt to access IPMI operation regions. Really.
> > 
> > And no mechanism can be added to ensure that ACPI call are
> > not attempted before IPMI is initialized?  A flag or lock
> > or exported symbol indicating IPMI support is ready.
> 
> In fact there is a workaround solution I've posted here:
> https://patchwork.kernel.org/patch/2831851/
> The updated version of this patch can be found at:
> https://bugzilla.kernel.org/attachment.cgi?id=112611
> It is the acpi-ipmi13.patch file.
> 
> This solution may change the meaning of ACPI spec defined _REG.
> So we may need a better solution.
> 
> But after merging this patch, it is safe to unload acpi_ipmi at users' wishes.
> Without solutions to solve region handler uninstallation races, it is not 
> safe to unload acpi_ipmi module.
> You can see crashes in the description of this patch.
> 
> The ipmi_si module is using a different way to unload itself which has not 
> been tested by me.
> You can find it in Documentation/IPMI.txt by searching "hot and remove of 
> interfaces" in this file.
> 
> > 
> > > > > ACPI 4.0 includes support for IPMI operation regions. Modular IPMI 
> > > > > means
> > > > > that the kernel will spend a significant amount of time (potentially
> > > > > until a user manually loads a driver) failing to implement part of the
> > > > > IPMI specification. That's a problem, and the correct fix is to ensure
> > > > > that the kernel always implements IPMI support.
> > > >
> > > > The ACPI spec says ipmi_si cannot be a driver?  Really?
> > > > What is the real problem you are trying to solve?
> > >
> > > The most straightforward case is that of an ACPI power meter.
> > 
> > So it is just a matter of making sure ipmi_si modules loads before
> > the ACPI power meter module loads, right?  module dependency issue.
> 
> I agree.
> I think there should be relationship between ACPI region and Linux kernel 
> modules.
> In fact on the well-known operating system, _REG is always invoked at the end 
> of the IRP_PNP_START_DEVICE completions.
> But we may still be able to return -EPROBE_DEFER in the power meter driver 
> when it detects failures caused by the readiness state of the region handlers.
> 
> I didn't work further on this issue when solving the reported bug:
> https://bugzilla.kernel.org/show_bug.cgi?id=46741
> 
> > 
> > >Several
> > > vendors implement this with an IPMI operation region. Calling any of the
> > > power meter functions will trigger access to that IPMI operation region,
> > > which will fail. This may result in driver initialisation failing. There
> > > is no express dependency between the power meter driver and ipmi_si,
> > > because the spec envisages IPMI support as basic kernel functionality.
> > > It's meant to be there before you start loading any other drivers.
> > 
> > The spec "envisages"?  I get there is a dependency, that IPMI driver
> > needs to be loaded before ACIP power meter.  This isn't the first
> > case of a driver being dependent on another driver.  That doesn't
> > mean IPMI driver must be built into the kernel.
> > 
> > > > > Now, you've described some other problems. I don't disagree that those
> > > > > are problems. The correct thing for us to do with those problems is to
> > > > > fix them, not to simply change the kernel defaults such that it's
> > > > > possible for users to choose between two differently broken states. 
> > > > > I'm
> > > > > absolutely willing to help, as long as you're willing to put some
> > > > > reasonable amount of effort into describing them.
> > > >
> > > > H

Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-21 Thread Matthew Garrett
On Fri, 2014-02-21 at 02:17 +, Zheng, Lv wrote:

> In fact there is a workaround solution I've posted here:
> https://patchwork.kernel.org/patch/2831851/
> The updated version of this patch can be found at:
> https://bugzilla.kernel.org/attachment.cgi?id=112611
> It is the acpi-ipmi13.patch file.

I don't think that solves the underlying problem? We're still left with
no guarantee of ordering between ACPI IPMI operation region support
being available and the loading of a driver that may rely on it.

> I think there should be relationship between ACPI region and Linux kernel 
> modules.
> In fact on the well-known operating system, _REG is always invoked at the end 
> of the IRP_PNP_START_DEVICE completions.
> But we may still be able to return -EPROBE_DEFER in the power meter driver 
> when it detects failures caused by the readiness state of the region handlers.

We don't have a guarantee that it'll happen at probe time. The
capabilities list may be static, and so we'll get our first failure on
an attempted userspace access instead.

It may be that Corey's suggestion satisfies the concerns Russ raised.
Building IPMI in but only probing by default on systems that declare a
BMC in ACPI or DMI should avoid boot-time delays while still ensuring
that the functionality is available on systems where there's a real
chance that the firmware expects the OS to provide support.

-- 
Matthew Garrett 


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-21 Thread Matthew Garrett
On Fri, 2014-02-21 at 07:37 -0600, Corey Minyard wrote:

> However, the basic problem is that hardware vendors produce hardware
> that sucks and then expect software to fix all the problems.  Most IPMI
> interfaces don't have interrupts, so they have to be polled.  Then they
> add important interfaces on top of it like firmware upgrade and ACPI and
> expect it to perform well.  If vendors would just have an interrupt for
> IPMI, 99% of these problems would go away.

Not going to disagree. The impact on power consumption is also pretty
awful. I should re-read the spec to figure out whether we can
legitimately get away with not doing that.

> One thing we can do is remove the default interface probing for IPMI. 
> Even though the spec has it, all modern hardware should have it
> specified in ACPI or device tree.  That should fix all the slow boot
> problems, at least.  If a user wants to add a default interface, they
> can use the interface to dynamically add it after boot time.

Something like this (untested)?

diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
index eea8464..5126230 100644
--- a/drivers/char/ipmi/Kconfig
+++ b/drivers/char/ipmi/Kconfig
@@ -52,6 +52,16 @@ config IPMI_SI
 Currently, only KCS and SMIC are supported.  If
 you are using IPMI, you should probably say "y" here.
 
+config IPMI_PROBE_DEFAULTS
+   bool 'Probe for all possible IPMI interfaces by default'
+   help
+ Modern systems will usually expose IPMI interfaces via a discoverable
+firmware mechanism such as ACPI or DMI. Older systems do not, and so
+the driver is forced to probe hardware manually. This may cause boot
+delays. Say "n" here to disable this manual probing. IPMI will then
+only be available on older systems if the "ipmi_si_intf.trydefaults=1"
+boot argument is passed.
+
 config IPMI_WATCHDOG
tristate 'IPMI Watchdog Timer'
help
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 03f4189..82c7d56 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -1230,7 +1230,11 @@ static bool  si_tryplatform = 1;
 #ifdef CONFIG_PCI
 static bool  si_trypci = 1;
 #endif
+#ifdef CONFIG_IPMI_PROBE_DEFAULTS
 static bool  si_trydefaults = 1;
+#else
+static bool  si_trydefaults;
+#endif
 static char  *si_type[SI_MAX_PARMS];
 #define MAX_SI_TYPE_STR 30
 static char  si_type_str[MAX_SI_TYPE_STR];


-- 
Matthew Garrett 


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-21 Thread Corey Minyard
On 02/20/2014 03:00 PM, Matthew Garrett wrote:
> On Thu, 2014-02-20 at 14:59 -0600, Russ Anderson wrote:
>> On Thu, Feb 20, 2014 at 08:46:04PM +, Matthew Garrett wrote:
>>> On Thu, 2014-02-20 at 14:40 -0600, Russ Anderson wrote:
>>>
 This is also a problem for systems with functional BMCs.  Our
 large cluster systems do all IPMI traffic (monitoring) through
 a system controller back door.  We do not want the kernel
 doing IPMI commands on those systems.
>>> Why not?
>> Because some customers want to use cpu cycles for their
>> application and let the ipmi monitoring go on through
>> the system controller network.
> Why is it generating any significant amount of CPU load? We're not
> talking about a high-bandwidth interface here.
>

I believe that problem is fixed now, at least the one with kipmid using
lots of CPU.

However, the basic problem is that hardware vendors produce hardware
that sucks and then expect software to fix all the problems.  Most IPMI
interfaces don't have interrupts, so they have to be polled.  Then they
add important interfaces on top of it like firmware upgrade and ACPI and
expect it to perform well.  If vendors would just have an interrupt for
IPMI, 99% of these problems would go away.

If there are still issues with lots of CPU being used, then the problem
is most likely non-compliant or just broken hardware.  I've seen enough
of that.

One thing we can do is remove the default interface probing for IPMI. 
Even though the spec has it, all modern hardware should have it
specified in ACPI or device tree.  That should fix all the slow boot
problems, at least.  If a user wants to add a default interface, they
can use the interface to dynamically add it after boot time.

-corey
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Zheng, Lv
Hi,

Sorry for interrupting you.
I have some information that may be helpful for your discussion.
Please find them in the inlined replies.
Well, I don't want to join the fight, just for your informations. :-)

> From: linux-acpi-ow...@vger.kernel.org 
> [mailto:linux-acpi-ow...@vger.kernel.org] On Behalf Of Russ Anderson
> Sent: Friday, February 21, 2014 7:59 AM
> 
> On Thu, Feb 20, 2014 at 11:09:42PM +, Matthew Garrett wrote:
> > On Thu, 2014-02-20 at 16:45 -0600, Russ Anderson wrote:
> > > On Thu, Feb 20, 2014 at 10:26:45PM +, Matthew Garrett wrote:
> >
> > > > Because I'm trying to ensure that the default behaviour of the kernel is
> > > > to *work*. Defaulting to having IPMI be modular means that the default
> > > > behaviour of the kernel, as far as the ACPI spec goes, is to be broken.
> > >
> > > The ACPI spec requires IPMI functionality before a module loads at
> > > boot time?  And the kernel is *broken* if it does not support ACIP IPMI
> > > functionality before module load time?  Really?
> >
> > There's no mechanism to ensure that IPMI support will be loaded before
> > ACPI calls attempt to access IPMI operation regions. Really.
> 
> And no mechanism can be added to ensure that ACPI call are
> not attempted before IPMI is initialized?  A flag or lock
> or exported symbol indicating IPMI support is ready.

In fact there is a workaround solution I've posted here:
https://patchwork.kernel.org/patch/2831851/
The updated version of this patch can be found at:
https://bugzilla.kernel.org/attachment.cgi?id=112611
It is the acpi-ipmi13.patch file.

This solution may change the meaning of ACPI spec defined _REG.
So we may need a better solution.

But after merging this patch, it is safe to unload acpi_ipmi at users' wishes.
Without solutions to solve region handler uninstallation races, it is not safe 
to unload acpi_ipmi module.
You can see crashes in the description of this patch.

The ipmi_si module is using a different way to unload itself which has not been 
tested by me.
You can find it in Documentation/IPMI.txt by searching "hot and remove of 
interfaces" in this file.

> 
> > > > ACPI 4.0 includes support for IPMI operation regions. Modular IPMI means
> > > > that the kernel will spend a significant amount of time (potentially
> > > > until a user manually loads a driver) failing to implement part of the
> > > > IPMI specification. That's a problem, and the correct fix is to ensure
> > > > that the kernel always implements IPMI support.
> > >
> > > The ACPI spec says ipmi_si cannot be a driver?  Really?
> > > What is the real problem you are trying to solve?
> >
> > The most straightforward case is that of an ACPI power meter.
> 
> So it is just a matter of making sure ipmi_si modules loads before
> the ACPI power meter module loads, right?  module dependency issue.

I agree.
I think there should be relationship between ACPI region and Linux kernel 
modules.
In fact on the well-known operating system, _REG is always invoked at the end 
of the IRP_PNP_START_DEVICE completions.
But we may still be able to return -EPROBE_DEFER in the power meter driver when 
it detects failures caused by the readiness state of the region handlers.

I didn't work further on this issue when solving the reported bug:
https://bugzilla.kernel.org/show_bug.cgi?id=46741

> 
> >Several
> > vendors implement this with an IPMI operation region. Calling any of the
> > power meter functions will trigger access to that IPMI operation region,
> > which will fail. This may result in driver initialisation failing. There
> > is no express dependency between the power meter driver and ipmi_si,
> > because the spec envisages IPMI support as basic kernel functionality.
> > It's meant to be there before you start loading any other drivers.
> 
> The spec "envisages"?  I get there is a dependency, that IPMI driver
> needs to be loaded before ACIP power meter.  This isn't the first
> case of a driver being dependent on another driver.  That doesn't
> mean IPMI driver must be built into the kernel.
> 
> > > > Now, you've described some other problems. I don't disagree that those
> > > > are problems. The correct thing for us to do with those problems is to
> > > > fix them, not to simply change the kernel defaults such that it's
> > > > possible for users to choose between two differently broken states. I'm
> > > > absolutely willing to help, as long as you're willing to put some
> > > > reasonable amount of effort into describing them.
> > >
> > > How about ACPI IPMI functionality starts when the ipmi_si
> > > module loads at boot time.

Actually, I have a solution implemented this.
You can find it at:
https://bugzilla.kernel.org/attachment.cgi?id=112611
It is the acpi-ipmi14.patch file.

The patch will hand the maintenance-ship of acpi_ipmi to IPMI community.
I'm not sure it is proper to merge it by Linux upstreams.

Thanks and best regards
-Lv

> >
> > I've repeatedly

Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Matthew Garrett
On Thu, 2014-02-20 at 17:59 -0600, Russ Anderson wrote:
> On Thu, Feb 20, 2014 at 11:09:42PM +, Matthew Garrett wrote:
> > On Thu, 2014-02-20 at 16:45 -0600, Russ Anderson wrote:
> > > 
> > > The ACPI spec requires IPMI functionality before a module loads at
> > > boot time?  And the kernel is *broken* if it does not support ACIP IPMI
> > > functionality before module load time?  Really?
> > 
> > There's no mechanism to ensure that IPMI support will be loaded before
> > ACPI calls attempt to access IPMI operation regions. Really.
> 
> And no mechanism can be added to ensure that ACPI call are
> not attempted before IPMI is initialized?  A flag or lock
> or exported symbol indicating IPMI support is ready.

ACPI functions are a black box to drivers. You make an ACPI call, the
AML code does something. We could block there, but what's the driver
supposed to do at that point? The core could call out to a module
loader, but if the driver is built in and IPMI isn't then you'll end up
with a 60 second pause in boot and a driver that doesn't work. 

> > > > ACPI 4.0 includes support for IPMI operation regions. Modular IPMI means
> > > > that the kernel will spend a significant amount of time (potentially
> > > > until a user manually loads a driver) failing to implement part of the
> > > > IPMI specification. That's a problem, and the correct fix is to ensure
> > > > that the kernel always implements IPMI support.
> > > 
> > > The ACPI spec says ipmi_si cannot be a driver?  Really?
> > > What is the real problem you are trying to solve?
> > 
> > The most straightforward case is that of an ACPI power meter.
> 
> So it is just a matter of making sure ipmi_si modules loads before
> the ACPI power meter module loads, right?  module dependency issue.

No, because the power meter driver has no way of knowing that a vendor
has implemented this interface via IPMI. *Any* ACPI entry point could
theoretically reference IPMI code, even the _INI method that's called
during ACPI core init. If it does, and if you don't have built-in ACPI
support, you'd fail ACPI initialisation and things would go downhill
from there.

(I don't think failure of this magnitude is actually *likely*, but it
would be spec-compliant)

> > I've repeatedly asked for you to provide detailed descriptions of the
> > problems you've seen because I have a genuine interest in fixing them.
> > If you're just going to childishly refuse then this discussion is
> > pointless. 
> 
> The distro cases I would point you at are marked private. 
> And you do not have access to our internal support system.
> A simple google search for "kipmi0" shows a lot of reports of
> high cpu utilization.  

And nobody seems to have put any effort into figuring out what the
underlying cause is. Is it spinning because there are messages? If so,
is it because the BMC would really like some kind of response to those
messages? Is it spinning because the BMC is wedged? If so, can we detect
that case, flag it as broken and cleanly disengage?

We're running systems from a wide range of vendors (including basically
all the Tier 1 server manufacturers, plus some whitebox), use IPMI
functionality heavily and genuinely do not see the described problems. I
don't think there's evidence of widespread breakage, and where it does
exist we should treat it as we would any other bug - diagnose the
underlying problem and fix it.

-- 
Matthew Garrett 
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Russ Anderson
On Thu, Feb 20, 2014 at 11:09:42PM +, Matthew Garrett wrote:
> On Thu, 2014-02-20 at 16:45 -0600, Russ Anderson wrote:
> > On Thu, Feb 20, 2014 at 10:26:45PM +, Matthew Garrett wrote:
> 
> > > Because I'm trying to ensure that the default behaviour of the kernel is
> > > to *work*. Defaulting to having IPMI be modular means that the default
> > > behaviour of the kernel, as far as the ACPI spec goes, is to be broken.
> > 
> > The ACPI spec requires IPMI functionality before a module loads at
> > boot time?  And the kernel is *broken* if it does not support ACIP IPMI
> > functionality before module load time?  Really?
> 
> There's no mechanism to ensure that IPMI support will be loaded before
> ACPI calls attempt to access IPMI operation regions. Really.

And no mechanism can be added to ensure that ACPI call are
not attempted before IPMI is initialized?  A flag or lock
or exported symbol indicating IPMI support is ready.

> > > ACPI 4.0 includes support for IPMI operation regions. Modular IPMI means
> > > that the kernel will spend a significant amount of time (potentially
> > > until a user manually loads a driver) failing to implement part of the
> > > IPMI specification. That's a problem, and the correct fix is to ensure
> > > that the kernel always implements IPMI support.
> > 
> > The ACPI spec says ipmi_si cannot be a driver?  Really?
> > What is the real problem you are trying to solve?
> 
> The most straightforward case is that of an ACPI power meter.

So it is just a matter of making sure ipmi_si modules loads before
the ACPI power meter module loads, right?  module dependency issue.

>Several
> vendors implement this with an IPMI operation region. Calling any of the
> power meter functions will trigger access to that IPMI operation region,
> which will fail. This may result in driver initialisation failing. There
> is no express dependency between the power meter driver and ipmi_si,
> because the spec envisages IPMI support as basic kernel functionality.
> It's meant to be there before you start loading any other drivers.

The spec "envisages"?  I get there is a dependency, that IPMI driver
needs to be loaded before ACIP power meter.  This isn't the first
case of a driver being dependent on another driver.  That doesn't
mean IPMI driver must be built into the kernel.

> > > Now, you've described some other problems. I don't disagree that those
> > > are problems. The correct thing for us to do with those problems is to
> > > fix them, not to simply change the kernel defaults such that it's
> > > possible for users to choose between two differently broken states. I'm
> > > absolutely willing to help, as long as you're willing to put some
> > > reasonable amount of effort into describing them.
> > 
> > How about ACPI IPMI functionality starts when the ipmi_si
> > module loads at boot time.  
> 
> I've repeatedly asked for you to provide detailed descriptions of the
> problems you've seen because I have a genuine interest in fixing them.
> If you're just going to childishly refuse then this discussion is
> pointless. 

The distro cases I would point you at are marked private. 
And you do not have access to our internal support system.
A simple google search for "kipmi0" shows a lot of reports of
high cpu utilization.  

And I'm old enough to appreciate being called childishly.  :-)

-- 
Russ Anderson,  Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc  r...@sgi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Rafael J. Wysocki
On Thursday, February 20, 2014 02:14:58 PM Russ Anderson wrote:
> On Tue, Feb 18, 2014 at 11:28:29AM -0500, Matthew Garrett wrote:
> > The ACPI IPMI driver implements IPMI operation region support for the ACPI
> > core. Systems that declare ACPI operation regions may reference them at any
> > time, including during kernel initialisation. These accesses will fail
> > unless the ACPI IPMI driver is present, and undesirable system behaviour
> > may result. Set the default to Y in order to encourage distributions and
> > users to configure kernels to avoid awkward surprises.
> 
> No, please do not build the ipmi_si driver into the kernel.
> Not all systems want, or need, the ipmi_si driver.  
> 
> The distro that added this change created all sorts of support
> problems.

Which distro was that?

Do you have any pointers to specific bug reports related to that?

Rafael


> Problems include kipmi0 spinning at 100% of cpu
> (creating a performance hit) and long boot delays (as the
> kernel tries to talk to a BMC that will never respond).
> It has been a big mess.
> 
> Nacked-by: Russ Anderson 
> 
> 
> > Signed-off-by: Matthew Garrett 
> > ---
> > Actually, I guess we also want this on CONFIG_IPMI_HANDLER for least
> > surprise
> > 
> >  drivers/acpi/Kconfig  | 2 +-
> >  drivers/char/ipmi/Kconfig | 2 ++
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > index 4770de5..0e6aab9 100644
> > --- a/drivers/acpi/Kconfig
> > +++ b/drivers/acpi/Kconfig
> > @@ -162,7 +162,7 @@ config ACPI_PROCESSOR
> >  config ACPI_IPMI
> > tristate "IPMI"
> > depends on IPMI_SI
> > -   default n
> > +   default y
> > help
> >   This driver enables the ACPI to access the BMC controller. And it
> >   uses the IPMI request/response message to communicate with BMC
> > diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
> > index 0baa8fa..eea8464 100644
> > --- a/drivers/char/ipmi/Kconfig
> > +++ b/drivers/char/ipmi/Kconfig
> > @@ -5,6 +5,7 @@
> >  menuconfig IPMI_HANDLER
> > tristate 'IPMI top-level message handler'
> > depends on HAS_IOMEM
> > +   default y if ACPI
> > help
> >   This enables the central IPMI message handler, required for IPMI
> >  to work.
> > @@ -45,6 +46,7 @@ config IPMI_DEVICE_INTERFACE
> >  
> >  config IPMI_SI
> > tristate 'IPMI System Interface handler'
> > +   default y if ACPI
> > help
> >   Provides a driver for System Interfaces (KCS, SMIC, BT).
> >  Currently, only KCS and SMIC are supported.  If
> 
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Matthew Garrett
On Thu, 2014-02-20 at 16:45 -0600, Russ Anderson wrote:
> On Thu, Feb 20, 2014 at 10:26:45PM +, Matthew Garrett wrote:

> > Because I'm trying to ensure that the default behaviour of the kernel is
> > to *work*. Defaulting to having IPMI be modular means that the default
> > behaviour of the kernel, as far as the ACPI spec goes, is to be broken.
> 
> The ACPI spec requires IPMI functionality before a module loads at
> boot time?  And the kernel is *broken* if it does not support ACIP IPMI
> functionality before module load time?  Really?

There's no mechanism to ensure that IPMI support will be loaded before
ACPI calls attempt to access IPMI operation regions. Really.

> > ACPI 4.0 includes support for IPMI operation regions. Modular IPMI means
> > that the kernel will spend a significant amount of time (potentially
> > until a user manually loads a driver) failing to implement part of the
> > IPMI specification. That's a problem, and the correct fix is to ensure
> > that the kernel always implements IPMI support.
> 
> The ACPI spec says ipmi_si cannot be a driver?  Really?
> What is the real problem you are trying to solve?

The most straightforward case is that of an ACPI power meter. Several
vendors implement this with an IPMI operation region. Calling any of the
power meter functions will trigger access to that IPMI operation region,
which will fail. This may result in driver initialisation failing. There
is no express dependency between the power meter driver and ipmi_si,
because the spec envisages IPMI support as basic kernel functionality.
It's meant to be there before you start loading any other drivers.

> > Now, you've described some other problems. I don't disagree that those
> > are problems. The correct thing for us to do with those problems is to
> > fix them, not to simply change the kernel defaults such that it's
> > possible for users to choose between two differently broken states. I'm
> > absolutely willing to help, as long as you're willing to put some
> > reasonable amount of effort into describing them.
> 
> How about ACPI IPMI functionality starts when the ipmi_si
> module loads at boot time.  

I've repeatedly asked for you to provide detailed descriptions of the
problems you've seen because I have a genuine interest in fixing them.
If you're just going to childishly refuse then this discussion is
pointless. 

-- 
Matthew Garrett 


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Russ Anderson
On Tue, Feb 18, 2014 at 11:28:29AM -0500, Matthew Garrett wrote:
> The ACPI IPMI driver implements IPMI operation region support for the ACPI
> core. Systems that declare ACPI operation regions may reference them at any
> time, including during kernel initialisation. These accesses will fail
> unless the ACPI IPMI driver is present, and undesirable system behaviour
> may result. 

Could you be more specific?  Is the problem kernel code
trying to use IPMI functionality before the ipmi_si driver
is loaded?  Or something else?

What is the "undesirable system behaviour"?


> Set the default to Y in order to encourage distributions and
> users to configure kernels to avoid awkward surprises.

What are the "awkward surprises"?

Thanks,
-- 
Russ Anderson,  Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc  r...@sgi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Russ Anderson
On Thu, Feb 20, 2014 at 10:26:45PM +, Matthew Garrett wrote:
> On Thu, 2014-02-20 at 16:06 -0600, Russ Anderson wrote:
> > On Thu, Feb 20, 2014 at 09:39:23PM +, Matthew Garrett wrote:
> > > On Thu, 2014-02-20 at 15:28 -0600, Russ Anderson wrote:
> > > 
> > > > For some customers _any_ amount is significant, especially
> > > > on large clustered systems where the amount is multiplied
> > > > by tens or hundreds of thousands of nodes.
> > > > 
> > > > You many not think wasting their cpu cycles is important, but they do.
> > > 
> > > Then they should be running locally built kernels in order to ensure
> > 
> > Why don't YOU run a locally built kernel?
> 
> Because I'm trying to ensure that the default behaviour of the kernel is
> to *work*. Defaulting to having IPMI be modular means that the default
> behaviour of the kernel, as far as the ACPI spec goes, is to be broken.

The ACPI spec requires IPMI functionality before a module loads at
boot time?  And the kernel is *broken* if it does not support ACIP IPMI
functionality before module load time?  Really?


> >> If you have specific bug reports, that would be helpful. But you're not
> > > describing actual failure conditions or showing any willingness to
> > > figure out what the underlying problem is.
> > 
> > You can't fix your problem without creating problems for
> > others to fix?
> 
> ACPI 4.0 includes support for IPMI operation regions. Modular IPMI means
> that the kernel will spend a significant amount of time (potentially
> until a user manually loads a driver) failing to implement part of the
> IPMI specification. That's a problem, and the correct fix is to ensure
> that the kernel always implements IPMI support.

The ACPI spec says ipmi_si cannot be a driver?  Really?
What is the real problem you are trying to solve?


> Now, you've described some other problems. I don't disagree that those
> are problems. The correct thing for us to do with those problems is to
> fix them, not to simply change the kernel defaults such that it's
> possible for users to choose between two differently broken states. I'm
> absolutely willing to help, as long as you're willing to put some
> reasonable amount of effort into describing them.

How about ACPI IPMI functionality starts when the ipmi_si
module loads at boot time.  

-- 
Russ Anderson,  Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc  r...@sgi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Matthew Garrett
On Thu, 2014-02-20 at 16:06 -0600, Russ Anderson wrote:
> On Thu, Feb 20, 2014 at 09:39:23PM +, Matthew Garrett wrote:
> > On Thu, 2014-02-20 at 15:28 -0600, Russ Anderson wrote:
> > 
> > > For some customers _any_ amount is significant, especially
> > > on large clustered systems where the amount is multiplied
> > > by tens or hundreds of thousands of nodes.
> > > 
> > > You many not think wasting their cpu cycles is important, but they do.
> > 
> > Then they should be running locally built kernels in order to ensure
> 
> Why don't YOU run a locally built kernel?

Because I'm trying to ensure that the default behaviour of the kernel is
to *work*. Defaulting to having IPMI be modular means that the default
behaviour of the kernel, as far as the ACPI spec goes, is to be broken.

>> If you have specific bug reports, that would be helpful. But you're not
> > describing actual failure conditions or showing any willingness to
> > figure out what the underlying problem is.
> 
> You can't fix your problem without creating problems for
> others to fix?

ACPI 4.0 includes support for IPMI operation regions. Modular IPMI means
that the kernel will spend a significant amount of time (potentially
until a user manually loads a driver) failing to implement part of the
IPMI specification. That's a problem, and the correct fix is to ensure
that the kernel always implements IPMI support.

Now, you've described some other problems. I don't disagree that those
are problems. The correct thing for us to do with those problems is to
fix them, not to simply change the kernel defaults such that it's
possible for users to choose between two differently broken states. I'm
absolutely willing to help, as long as you're willing to put some
reasonable amount of effort into describing them.

-- 
Matthew Garrett 


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Russ Anderson
On Thu, Feb 20, 2014 at 09:39:23PM +, Matthew Garrett wrote:
> On Thu, 2014-02-20 at 15:28 -0600, Russ Anderson wrote:
> 
> > For some customers _any_ amount is significant, especially
> > on large clustered systems where the amount is multiplied
> > by tens or hundreds of thousands of nodes.
> > 
> > You many not think wasting their cpu cycles is important, but they do.
> 
> Then they should be running locally built kernels in order to ensure

Why don't YOU run a locally built kernel?

> that there's no problematic code running at all. Distribution kernels
> will always contain code that some customers won't be interested in, and
> some of that code will end up executing.

We support multiple distros so if one does something the customer
does not like/need we can steer them to other distros.

> If you have specific bug reports, that would be helpful. But you're not
> describing actual failure conditions or showing any willingness to
> figure out what the underlying problem is.

You can't fix your problem without creating problems for
others to fix?


-- 
Russ Anderson,  Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc  r...@sgi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Matthew Garrett
On Thu, 2014-02-20 at 15:49 -0600, Russ Anderson wrote:
> On Thu, Feb 20, 2014 at 08:46:04PM +, Matthew Garrett wrote:
> > You appear to be saying "SGI ship hardware that doesn't work. We don't
> > know why it doesn't work and we're not interested in fixing it, so we'd
> > prefer the default kernel configuration to be broken". That doesn't seem
> > like an especially compelling argument.
> 
> You appear to not understand the mess this causes.  We have hit
> this on 3rd party boards, not just SGI built hardware (much
> of which is industry standard components).  It causes problems
> both for flakey hardware _and_ functioning hardware.

Bug numbers?

-- 
Matthew Garrett 
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Russ Anderson
On Thu, Feb 20, 2014 at 08:46:04PM +, Matthew Garrett wrote:
> On Thu, 2014-02-20 at 14:40 -0600, Russ Anderson wrote:
> 
> > There are any number of reasons why a BMC may not respond.
> > BMCs are notorious for being flakey, with different types
> > of BMCs that may or may not be reliable.  You do not want
> > to make the kernel boot dependent on an unreliable component.
> 
> You appear to be saying "SGI ship hardware that doesn't work. We don't
> know why it doesn't work and we're not interested in fixing it, so we'd
> prefer the default kernel configuration to be broken". That doesn't seem
> like an especially compelling argument.

You appear to not understand the mess this causes.  We have hit
this on 3rd party boards, not just SGI built hardware (much
of which is industry standard components).  It causes problems
both for flakey hardware _and_ functioning hardware.


-- 
Russ Anderson,  Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc  r...@sgi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Matthew Garrett
On Thu, 2014-02-20 at 15:28 -0600, Russ Anderson wrote:

> For some customers _any_ amount is significant, especially
> on large clustered systems where the amount is multiplied
> by tens or hundreds of thousands of nodes.
> 
> You many not think wasting their cpu cycles is important, but they do.

Then they should be running locally built kernels in order to ensure
that there's no problematic code running at all. Distribution kernels
will always contain code that some customers won't be interested in, and
some of that code will end up executing.

If you have specific bug reports, that would be helpful. But you're not
describing actual failure conditions or showing any willingness to
figure out what the underlying problem is. The BMC is generating
messages and handing them off to the host. If that's taking more than a
tiny number of cycles, why? Once we know the answer to that, we can
actually put some effort into fixing it.

-- 
Matthew Garrett 
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Russ Anderson
On Thu, Feb 20, 2014 at 09:00:48PM +, Matthew Garrett wrote:
> On Thu, 2014-02-20 at 14:59 -0600, Russ Anderson wrote:
> > On Thu, Feb 20, 2014 at 08:46:04PM +, Matthew Garrett wrote:
> > > On Thu, 2014-02-20 at 14:40 -0600, Russ Anderson wrote:
> > > 
> > > > This is also a problem for systems with functional BMCs.  Our
> > > > large cluster systems do all IPMI traffic (monitoring) through
> > > > a system controller back door.  We do not want the kernel
> > > > doing IPMI commands on those systems.
> > > 
> > > Why not?
> > 
> > Because some customers want to use cpu cycles for their
> > application and let the ipmi monitoring go on through
> > the system controller network.
> 
> Why is it generating any significant amount of CPU load? We're not
> talking about a high-bandwidth interface here.

For some customers _any_ amount is significant, especially
on large clustered systems where the amount is multiplied
by tens or hundreds of thousands of nodes.

You many not think wasting their cpu cycles is important, but they do.


-- 
Russ Anderson,  Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc  r...@sgi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Matthew Garrett
On Thu, 2014-02-20 at 14:59 -0600, Russ Anderson wrote:
> On Thu, Feb 20, 2014 at 08:46:04PM +, Matthew Garrett wrote:
> > On Thu, 2014-02-20 at 14:40 -0600, Russ Anderson wrote:
> > 
> > > This is also a problem for systems with functional BMCs.  Our
> > > large cluster systems do all IPMI traffic (monitoring) through
> > > a system controller back door.  We do not want the kernel
> > > doing IPMI commands on those systems.
> > 
> > Why not?
> 
> Because some customers want to use cpu cycles for their
> application and let the ipmi monitoring go on through
> the system controller network.

Why is it generating any significant amount of CPU load? We're not
talking about a high-bandwidth interface here.

-- 
Matthew Garrett 
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Russ Anderson
On Thu, Feb 20, 2014 at 08:46:04PM +, Matthew Garrett wrote:
> On Thu, 2014-02-20 at 14:40 -0600, Russ Anderson wrote:
> 
> > This is also a problem for systems with functional BMCs.  Our
> > large cluster systems do all IPMI traffic (monitoring) through
> > a system controller back door.  We do not want the kernel
> > doing IPMI commands on those systems.
> 
> Why not?

Because some customers want to use cpu cycles for their
application and let the ipmi monitoring go on through
the system controller network.

-- 
Russ Anderson,  Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc  r...@sgi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Matthew Garrett
On Thu, 2014-02-20 at 14:40 -0600, Russ Anderson wrote:

> Why build a driver into the kernel?

Because it provides functionality that other drivers may need without
there being any mechanism to provide an explicit dependency. The same
reason we build the ACPI embedded controller driver into the kernel.

> The reason ipmi_si is 
> a driver is so systems that want it can load it and systems
> that do not want it do not have to load it.  Plus you can
> stop/start modules without rebooting.  You can change module
> parameters without rebooting.

You can change module parameters without rebooting anyway - there's an
interface for it in sysfs.

> There are any number of reasons why a BMC may not respond.
> BMCs are notorious for being flakey, with different types
> of BMCs that may or may not be reliable.  You do not want
> to make the kernel boot dependent on an unreliable component.

You appear to be saying "SGI ship hardware that doesn't work. We don't
know why it doesn't work and we're not interested in fixing it, so we'd
prefer the default kernel configuration to be broken". That doesn't seem
like an especially compelling argument.

> This is also a problem for systems with functional BMCs.  Our
> large cluster systems do all IPMI traffic (monitoring) through
> a system controller back door.  We do not want the kernel
> doing IPMI commands on those systems.

Why not?

-- 
Matthew Garrett 
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�&j:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Russ Anderson
On Thu, Feb 20, 2014 at 08:16:22PM +, Matthew Garrett wrote:
> On Thu, 2014-02-20 at 14:14 -0600, Russ Anderson wrote:
> 
> > The distro that added this change created all sorts of support
> > problems.  Problems include kipmi0 spinning at 100% of cpu
> > (creating a performance hit) and long boot delays (as the
> > kernel tries to talk to a BMC that will never respond).
> > It has been a big mess.
> 
> Why is the BMC not responding? Why is kipmi0 at 100%? Why are we not
> fixing those bugs?

Why build a driver into the kernel?  The reason ipmi_si is 
a driver is so systems that want it can load it and systems
that do not want it do not have to load it.  Plus you can
stop/start modules without rebooting.  You can change module
parameters without rebooting.

There are any number of reasons why a BMC may not respond.
BMCs are notorious for being flakey, with different types
of BMCs that may or may not be reliable.  You do not want
to make the kernel boot dependent on an unreliable component.

This is also a problem for systems with functional BMCs.  Our
large cluster systems do all IPMI traffic (monitoring) through
a system controller back door.  We do not want the kernel
doing IPMI commands on those systems.   On those systems we
simply do not load the ipmi_si module.  Building ipmi_si
into the kernel means adding kernel boot line options to
turn ipmi_si back off again.


-- 
Russ Anderson,  Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc  r...@sgi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Matthew Garrett
On Thu, 2014-02-20 at 14:14 -0600, Russ Anderson wrote:

> The distro that added this change created all sorts of support
> problems.  Problems include kipmi0 spinning at 100% of cpu
> (creating a performance hit) and long boot delays (as the
> kernel tries to talk to a BMC that will never respond).
> It has been a big mess.

Why is the BMC not responding? Why is kipmi0 at 100%? Why are we not
fixing those bugs?

-- 
Matthew Garrett 


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-20 Thread Russ Anderson
On Tue, Feb 18, 2014 at 11:28:29AM -0500, Matthew Garrett wrote:
> The ACPI IPMI driver implements IPMI operation region support for the ACPI
> core. Systems that declare ACPI operation regions may reference them at any
> time, including during kernel initialisation. These accesses will fail
> unless the ACPI IPMI driver is present, and undesirable system behaviour
> may result. Set the default to Y in order to encourage distributions and
> users to configure kernels to avoid awkward surprises.

No, please do not build the ipmi_si driver into the kernel.
Not all systems want, or need, the ipmi_si driver.  

The distro that added this change created all sorts of support
problems.  Problems include kipmi0 spinning at 100% of cpu
(creating a performance hit) and long boot delays (as the
kernel tries to talk to a BMC that will never respond).
It has been a big mess.

Nacked-by: Russ Anderson 


> Signed-off-by: Matthew Garrett 
> ---
> Actually, I guess we also want this on CONFIG_IPMI_HANDLER for least
> surprise
> 
>  drivers/acpi/Kconfig  | 2 +-
>  drivers/char/ipmi/Kconfig | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 4770de5..0e6aab9 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -162,7 +162,7 @@ config ACPI_PROCESSOR
>  config ACPI_IPMI
>   tristate "IPMI"
>   depends on IPMI_SI
> - default n
> + default y
>   help
> This driver enables the ACPI to access the BMC controller. And it
> uses the IPMI request/response message to communicate with BMC
> diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig
> index 0baa8fa..eea8464 100644
> --- a/drivers/char/ipmi/Kconfig
> +++ b/drivers/char/ipmi/Kconfig
> @@ -5,6 +5,7 @@
>  menuconfig IPMI_HANDLER
> tristate 'IPMI top-level message handler'
> depends on HAS_IOMEM
> +   default y if ACPI
> help
>   This enables the central IPMI message handler, required for IPMI
>to work.
> @@ -45,6 +46,7 @@ config IPMI_DEVICE_INTERFACE
>  
>  config IPMI_SI
> tristate 'IPMI System Interface handler'
> +   default y if ACPI
> help
>   Provides a driver for System Interfaces (KCS, SMIC, BT).
>Currently, only KCS and SMIC are supported.  If
> -- 
> 1.8.5.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Russ Anderson,  Kernel and Performance Software Team Manager
SGI - Silicon Graphics Inc  r...@sgi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-18 Thread Corey Minyard
I just queued it up, too, but that's fine.  This seems to be a good idea
with the way ACPI is going.

Acked-by: Corey Minyard 

On 02/18/2014 06:45 PM, Rafael J. Wysocki wrote:
> On Tuesday, February 18, 2014 11:25:02 PM Matthew Garrett wrote:
>> On Wed, 2014-02-19 at 00:35 +0100, Rafael J. Wysocki wrote:
>>> On Tuesday, February 18, 2014 11:15:08 PM Matthew Garrett wrote:
 For example, if you load the ACPI power meter driver before you've
 installed the ACPI IPMI driver you'll typically get failures (most
 vendors implement it via IPMI).
>>> Well, any specific machine from any specific vendor?
>> The Dell R720 and HP DL385 both seem to have this configuration. I
>> believe some Ciscos do, too.
> I've queued this up for 3.15 (with the above info added to the changelog).
>
> Thanks!
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-18 Thread Rafael J. Wysocki
On Tuesday, February 18, 2014 11:25:02 PM Matthew Garrett wrote:
> On Wed, 2014-02-19 at 00:35 +0100, Rafael J. Wysocki wrote:
> > On Tuesday, February 18, 2014 11:15:08 PM Matthew Garrett wrote:
> > > For example, if you load the ACPI power meter driver before you've
> > > installed the ACPI IPMI driver you'll typically get failures (most
> > > vendors implement it via IPMI).
> > 
> > Well, any specific machine from any specific vendor?
> 
> The Dell R720 and HP DL385 both seem to have this configuration. I
> believe some Ciscos do, too.

I've queued this up for 3.15 (with the above info added to the changelog).

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-18 Thread Matthew Garrett
On Wed, 2014-02-19 at 00:35 +0100, Rafael J. Wysocki wrote:
> On Tuesday, February 18, 2014 11:15:08 PM Matthew Garrett wrote:
> > For example, if you load the ACPI power meter driver before you've
> > installed the ACPI IPMI driver you'll typically get failures (most
> > vendors implement it via IPMI).
> 
> Well, any specific machine from any specific vendor?

The Dell R720 and HP DL385 both seem to have this configuration. I
believe some Ciscos do, too.
-- 
Matthew Garrett 


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-18 Thread Rafael J. Wysocki
On Tuesday, February 18, 2014 11:15:08 PM Matthew Garrett wrote:
> On Wed, 2014-02-19 at 00:26 +0100, Rafael J. Wysocki wrote:
> > On Tuesday, February 18, 2014 11:28:29 AM Matthew Garrett wrote:
> > > 
> > > The ACPI IPMI driver implements IPMI operation region support for the ACPI
> > > core. Systems that declare ACPI operation regions may reference them at 
> > > any
> > > time, including during kernel initialisation. These accesses will fail
> > > unless the ACPI IPMI driver is present, and undesirable system behaviour
> > > may result. Set the default to Y in order to encourage distributions and
> > > users to configure kernels to avoid awkward surprises.
> > 
> > Do you have any examples of problems caused by that or is this just 
> > theoretical
> > at the moment?
> 
> For example, if you load the ACPI power meter driver before you've
> installed the ACPI IPMI driver you'll typically get failures (most
> vendors implement it via IPMI).

Well, any specific machine from any specific vendor?

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-18 Thread Matthew Garrett
On Wed, 2014-02-19 at 00:26 +0100, Rafael J. Wysocki wrote:
> On Tuesday, February 18, 2014 11:28:29 AM Matthew Garrett wrote:
> > 
> > The ACPI IPMI driver implements IPMI operation region support for the ACPI
> > core. Systems that declare ACPI operation regions may reference them at any
> > time, including during kernel initialisation. These accesses will fail
> > unless the ACPI IPMI driver is present, and undesirable system behaviour
> > may result. Set the default to Y in order to encourage distributions and
> > users to configure kernels to avoid awkward surprises.
> 
> Do you have any examples of problems caused by that or is this just 
> theoretical
> at the moment?

For example, if you load the ACPI power meter driver before you've
installed the ACPI IPMI driver you'll typically get failures (most
vendors implement it via IPMI).

-- 
Matthew Garrett 


Re: [PATCH V2] Change ACPI IPMI support to "default y"

2014-02-18 Thread Rafael J. Wysocki
On Tuesday, February 18, 2014 11:28:29 AM Matthew Garrett wrote:
> 
> The ACPI IPMI driver implements IPMI operation region support for the ACPI
> core. Systems that declare ACPI operation regions may reference them at any
> time, including during kernel initialisation. These accesses will fail
> unless the ACPI IPMI driver is present, and undesirable system behaviour
> may result. Set the default to Y in order to encourage distributions and
> users to configure kernels to avoid awkward surprises.

Do you have any examples of problems caused by that or is this just theoretical
at the moment?

Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/