[PATCH 0/2] Re-work UV BAU enable/disable logic, add documentation

[PATCH 0/2] Re-work UV BAU enable/disable logic, add documentation

[Alex Thorlton]

Hey everyone,

This is a fairly simple change to disable the UV BAU by default (see
commit message for reasoning) and to add some documentation to
kernel-parameters.txt to explain the new parameter.

Let me know what you think!

Alex Thorlton (2):
  Disable UV BAU by default
  Add documentation for the bau parameter

 Documentation/kernel-parameters.txt |  8 ++++++++
 arch/x86/platform/uv/tlb_uv.c       | 19 ++++++++++++++-----
 2 files changed, 22 insertions(+), 5 deletions(-)

-- 
1.8.5.6

[PATCH 1/2] Disable UV BAU by default

[Alex Thorlton]

For several years, the common practice has been to boot UVs with the
"nobau" parameter on the command line, to disable the BAU.  We've
decided that it makes more sense to just disable the BAU by default in
the kernel, and provide the option to turn it on, if desired.

Signed-off-by: Alex Thorlton <athorlton@sgi.com>
Reviewed-by: Hedi Berriche <hedi@sgi.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
---
 arch/x86/platform/uv/tlb_uv.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/arch/x86/platform/uv/tlb_uv.c b/arch/x86/platform/uv/tlb_uv.c
index 3b6ec42..a5609a3 100644
--- a/arch/x86/platform/uv/tlb_uv.c
+++ b/arch/x86/platform/uv/tlb_uv.c
@@ -37,7 +37,7 @@ static int timeout_base_ns[] = {
 };
 
 static int timeout_us;
-static int nobau;
+static int nobau = 1;
 static int nobau_perm;
 static cycles_t congested_cycles;
 
@@ -106,13 +106,22 @@ static char *stat_description[] = {
 	"enable:   number times use of the BAU was re-enabled"
 };
 
-static int __init
-setup_nobau(char *arg)
+static int __init setup_bau(char *arg)
 {
-	nobau = 1;
+	if (!arg)
+		return -EINVAL;
+
+	if (!strncmp(arg, "on", 2)) {
+		nobau = 0;
+		pr_info("UV BAU Enabled\n");
+	} else if (!strncmp(arg, "off", 3)) {
+		nobau = 1;
+		pr_info("UV BAU Disabled\n");
+	}
+
 	return 0;
 }
-early_param("nobau", setup_nobau);
+early_param("bau", setup_bau);
 
 /* base pnode in this partition */
 static int uv_base_pnode __read_mostly;
-- 
1.8.5.6

Re: [PATCH 1/2] Disable UV BAU by default

[Thomas Gleixner]

On Mon, 21 Mar 2016, Alex Thorlton wrote:

First of all, please use proper patch prefixes.

x86/platform/uv: ....

And please fold the documentation change into the patch which changes the
parameter.

>  static int timeout_us;
> -static int nobau;
> +static int nobau = 1;
>  static int nobau_perm;
>  static cycles_t congested_cycles;
>  
> @@ -106,13 +106,22 @@ static char *stat_description[] = {
>  	"enable:   number times use of the BAU was re-enabled"
>  };
>  
> -static int __init
> -setup_nobau(char *arg)
> +static int __init setup_bau(char *arg)
>  {
> -	nobau = 1;
> +	if (!arg)
> +		return -EINVAL;
> +
> +	if (!strncmp(arg, "on", 2)) {
> +		nobau = 0;
> +		pr_info("UV BAU Enabled\n");
> +	} else if (!strncmp(arg, "off", 3)) {
> +		nobau = 1;
> +		pr_info("UV BAU Disabled\n");
> +	}
> +
>  	return 0;
>  }
> -early_param("nobau", setup_nobau);
> +early_param("bau", setup_bau);

What's the value of having that extra argument?

The default is off, so we can do with a simple "bau" or "enable_bau" and be
done with it.

Thanks,

	tglx

Re: [PATCH 1/2] Disable UV BAU by default

[Alex Thorlton]

On Wed, Mar 23, 2016 at 12:27:44PM +0100, Thomas Gleixner wrote:
> On Mon, 21 Mar 2016, Alex Thorlton wrote:
> 
> First of all, please use proper patch prefixes.
> 
> x86/platform/uv: ....

Ah - sorry about that!

> And please fold the documentation change into the patch which changes the
> parameter.

Got it.  No problem!

> > +	if (!strncmp(arg, "on", 2)) {
> > +		nobau = 0;
> > +		pr_info("UV BAU Enabled\n");
> > +	} else if (!strncmp(arg, "off", 3)) {
> > +		nobau = 1;
> > +		pr_info("UV BAU Disabled\n");
> > +	}
> 
> What's the value of having that extra argument?
> 
> The default is off, so we can do with a simple "bau" or "enable_bau" and be
> done with it.

This was actually what I initially wrote, but we decided to go with the
on/off switch instead, because, in the UV4 time-frame, we're hoping to
get a few things changed so that we can default to having the bau *on*
for the new UV4 systems.

I left that detail out of the original commit message, as I didn't
figure our future (still tentative) plans were all that important to the
community.  I can add that information to my commit message, if you
would prefer to see it there.

I'll get the other stuff fixed up.  Please let me know if you'd like for
me to give a bit more detail in the commit message about the motivation
for the on/off switch vs. an enable flag.

Thanks for the input!

- Alex

Re: [PATCH 1/2] Disable UV BAU by default

[Thomas Gleixner]

On Wed, 23 Mar 2016, Alex Thorlton wrote:
> This was actually what I initially wrote, but we decided to go with the
> on/off switch instead, because, in the UV4 time-frame, we're hoping to
> get a few things changed so that we can default to having the bau *on*
> for the new UV4 systems.
> 
> I left that detail out of the original commit message, as I didn't
> figure our future (still tentative) plans were all that important to the
> community.  I can add that information to my commit message, if you
> would prefer to see it there.

Yes please.

Please make that parameter boolean type, so you can just use strtobool(). 

That supports "uvbau==[yYnN01]" and you can avoid the custom parser.

Thanks,

	tglx

Re: [PATCH 1/2] Disable UV BAU by default

[Alex Thorlton]

On Wed, Mar 23, 2016 at 06:20:26PM +0100, Thomas Gleixner wrote:
> On Wed, 23 Mar 2016, Alex Thorlton wrote:
> > This was actually what I initially wrote, but we decided to go with the
> > on/off switch instead, because, in the UV4 time-frame, we're hoping to
> > get a few things changed so that we can default to having the bau *on*
> > for the new UV4 systems.
> > 
> > I left that detail out of the original commit message, as I didn't
> > figure our future (still tentative) plans were all that important to the
> > community.  I can add that information to my commit message, if you
> > would prefer to see it there.
> 
> Yes please.

Got it.  Will do.

> Please make that parameter boolean type, so you can just use strtobool(). 
> 
> That supports "uvbau==[yYnN01]" and you can avoid the custom parser.

Makes sense. I'll get that taken care of.

Thanks again!

- Alex

[PATCH 2/2] Add documentation for the bau parameter

[Alex Thorlton]

This commit updates kernel-parameters.txt with some information about
the new "bau" parameter.

Signed-off-by: Alex Thorltlon <athorlton@sgi.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Hedi Berriche <hedi@sgi.com>
Cc: linux-kernel@vger.kernel.org

---
 Documentation/kernel-parameters.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index def4791..c5115e4 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -131,6 +131,7 @@ parameter is applicable:
 			More X86-64 boot options can be found in
 			Documentation/x86/x86_64/boot-options.txt .
 	X86	Either 32-bit or 64-bit x86 (same as X86-32+X86-64)
+	X86_UV	SGI UV support is enabled.
 	XEN	Xen support is enabled
 
 In addition, the following text indicates that the option:
@@ -542,6 +543,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			Format: <int> (must be >=0)
 			Default: 64
 
+	bau=		[X86_UV] Enable the BAU on SGI UV.  The default
+			behavior is to disable the BAU (i.e. bau=off).
+		on
+			Enable the BAU.
+		off
+			Disable the BAU.
+
 	baycom_epp=	[HW,AX25]
 			Format: <io>,<mode>
 
-- 
1.8.5.6