linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86/UV: Fix conditional in gru_exit
@ 2014-03-20 20:36 Dimitri Sivanich
  2014-03-21  7:55 ` Ingo Molnar
  0 siblings, 1 reply; 6+ messages in thread
From: Dimitri Sivanich @ 2014-03-20 20:36 UTC (permalink / raw)
  To: linux-kernel; +Cc: x86, Ingo Molnar

Fix conditional in gru_exit to match gru_init.

Signed-off-by: Dimitri Sivanich <sivanich@sgi.com>
---
 drivers/misc/sgi-gru/grufile.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/misc/sgi-gru/grufile.c
===================================================================
--- linux.orig/drivers/misc/sgi-gru/grufile.c
+++ linux/drivers/misc/sgi-gru/grufile.c
@@ -573,7 +573,7 @@ exit0:
 
 static void __exit gru_exit(void)
 {
-	if (!is_uv_system())
+	if (!is_uv_system() || (is_uvx_hub() && !is_uv2_hub()))
 		return;
 
 	gru_teardown_tlb_irqs();

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/UV: Fix conditional in gru_exit
  2014-03-20 20:36 [PATCH] x86/UV: Fix conditional in gru_exit Dimitri Sivanich
@ 2014-03-21  7:55 ` Ingo Molnar
  2014-03-21 16:13   ` Dimitri Sivanich
  0 siblings, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2014-03-21  7:55 UTC (permalink / raw)
  To: Dimitri Sivanich; +Cc: linux-kernel, x86, Ingo Molnar


* Dimitri Sivanich <sivanich@sgi.com> wrote:

> Fix conditional in gru_exit to match gru_init.
> 
> Signed-off-by: Dimitri Sivanich <sivanich@sgi.com>
> ---
>  drivers/misc/sgi-gru/grufile.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux/drivers/misc/sgi-gru/grufile.c
> ===================================================================
> --- linux.orig/drivers/misc/sgi-gru/grufile.c
> +++ linux/drivers/misc/sgi-gru/grufile.c
> @@ -573,7 +573,7 @@ exit0:
>  
>  static void __exit gru_exit(void)
>  {
> -	if (!is_uv_system())
> +	if (!is_uv_system() || (is_uvx_hub() && !is_uv2_hub()))
>  		return;

Such an amalgation of three system specific conditionals is 
disgusting, please at minimum factor out a helper routine so that such 
mismatches cannot happen.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/UV: Fix conditional in gru_exit
  2014-03-21  7:55 ` Ingo Molnar
@ 2014-03-21 16:13   ` Dimitri Sivanich
  2014-03-31  7:25     ` Ingo Molnar
  0 siblings, 1 reply; 6+ messages in thread
From: Dimitri Sivanich @ 2014-03-21 16:13 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, x86, Ingo Molnar

On Fri, Mar 21, 2014 at 08:55:49AM +0100, Ingo Molnar wrote:
> > -	if (!is_uv_system())
> > +	if (!is_uv_system() || (is_uvx_hub() && !is_uv2_hub()))
> >  		return;
> 
> Such an amalgation of three system specific conditionals is 
> disgusting, please at minimum factor out a helper routine so that such 
> mismatches cannot happen.
>
Agreed.  Here's a new patch.


Fix supported system conditional in gru_exit.

Signed-off-by: Dimitri Sivanich <sivanich@sgi.com>
---
 drivers/misc/sgi-gru/grufile.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Index: linux/drivers/misc/sgi-gru/grufile.c
===================================================================
--- linux.orig/drivers/misc/sgi-gru/grufile.c
+++ linux/drivers/misc/sgi-gru/grufile.c
@@ -58,6 +58,10 @@ static int max_user_cbrs, max_user_dsr_b
 
 static struct miscdevice gru_miscdev;
 
+static int gru_unsupported(void)
+{
+	return !is_uv_system() || (is_uvx_hub() && !is_uv2_hub());
+}
 
 /*
  * gru_vma_close
@@ -518,7 +522,7 @@ static int __init gru_init(void)
 {
 	int ret;
 
-	if (!is_uv_system() || (is_uvx_hub() && !is_uv2_hub()))
+	if (gru_unsupported())
 		return 0;
 
 #if defined CONFIG_IA64
@@ -573,7 +577,7 @@ exit0:
 
 static void __exit gru_exit(void)
 {
-	if (!is_uv_system())
+	if (gru_unsupported())
 		return;
 
 	gru_teardown_tlb_irqs();

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/UV: Fix conditional in gru_exit
  2014-03-21 16:13   ` Dimitri Sivanich
@ 2014-03-31  7:25     ` Ingo Molnar
  2014-03-31 15:23       ` Dimitri Sivanich
  0 siblings, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2014-03-31  7:25 UTC (permalink / raw)
  To: Dimitri Sivanich; +Cc: linux-kernel, x86, Ingo Molnar


* Dimitri Sivanich <sivanich@sgi.com> wrote:

> On Fri, Mar 21, 2014 at 08:55:49AM +0100, Ingo Molnar wrote:
> > > -	if (!is_uv_system())
> > > +	if (!is_uv_system() || (is_uvx_hub() && !is_uv2_hub()))
> > >  		return;
> > 
> > Such an amalgation of three system specific conditionals is 
> > disgusting, please at minimum factor out a helper routine so that such 
> > mismatches cannot happen.
> >
> Agreed.  Here's a new patch.
> 
> 
> Fix supported system conditional in gru_exit.
> 
> Signed-off-by: Dimitri Sivanich <sivanich@sgi.com>
> ---
>  drivers/misc/sgi-gru/grufile.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> Index: linux/drivers/misc/sgi-gru/grufile.c
> ===================================================================
> --- linux.orig/drivers/misc/sgi-gru/grufile.c
> +++ linux/drivers/misc/sgi-gru/grufile.c
> @@ -58,6 +58,10 @@ static int max_user_cbrs, max_user_dsr_b
>  
>  static struct miscdevice gru_miscdev;
>  
> +static int gru_unsupported(void)
> +{
> +	return !is_uv_system() || (is_uvx_hub() && !is_uv2_hub());
> +}

So the usual pattern is to introduce simple patterns, without logic 
operations in their name. I.e. "gru_supported()" would be more natural 
than "gru_not_supported()" or gru_unsupported()".

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/UV: Fix conditional in gru_exit
  2014-03-31  7:25     ` Ingo Molnar
@ 2014-03-31 15:23       ` Dimitri Sivanich
  2014-04-01 11:07         ` [tip:x86/uv] x86/UV: Fix conditional in gru_exit() tip-bot for Dimitri Sivanich
  0 siblings, 1 reply; 6+ messages in thread
From: Dimitri Sivanich @ 2014-03-31 15:23 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, x86, Ingo Molnar

On Mon, Mar 31, 2014 at 09:25:05AM +0200, Ingo Molnar wrote:
> * Dimitri Sivanich <sivanich@sgi.com> wrote:
> 
> > +static int gru_unsupported(void)
> > +{
> > +	return !is_uv_system() || (is_uvx_hub() && !is_uv2_hub());
> > +}
> 
> So the usual pattern is to introduce simple patterns, without logic 
> operations in their name. I.e. "gru_supported()" would be more natural 
> than "gru_not_supported()" or gru_unsupported()".
>

OK.  Here's a revised version.



Fix supported system conditional in gru_exit.

Signed-off-by: Dimitri Sivanich <sivanich@sgi.com>
---
 drivers/misc/sgi-gru/grufile.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Index: linux/drivers/misc/sgi-gru/grufile.c
===================================================================
--- linux.orig/drivers/misc/sgi-gru/grufile.c
+++ linux/drivers/misc/sgi-gru/grufile.c
@@ -6,7 +6,7 @@
  * This file supports the user system call for file open, close, mmap, etc.
  * This also incudes the driver initialization code.
  *
- *  Copyright (c) 2008 Silicon Graphics, Inc.  All Rights Reserved.
+ *  Copyright (c) 2008-2014 Silicon Graphics, Inc.  All Rights Reserved.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -58,6 +58,11 @@ static int max_user_cbrs, max_user_dsr_b
 
 static struct miscdevice gru_miscdev;
 
+static int gru_supported(void)
+{
+	return is_uv_system() &&
+		(uv_hub_info->hub_revision < UV3_HUB_REVISION_BASE);
+}
 
 /*
  * gru_vma_close
@@ -518,7 +523,7 @@ static int __init gru_init(void)
 {
 	int ret;
 
-	if (!is_uv_system() || (is_uvx_hub() && !is_uv2_hub()))
+	if (!gru_supported())
 		return 0;
 
 #if defined CONFIG_IA64
@@ -573,7 +578,7 @@ exit0:
 
 static void __exit gru_exit(void)
 {
-	if (!is_uv_system())
+	if (!gru_supported())
 		return;
 
 	gru_teardown_tlb_irqs();

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [tip:x86/uv] x86/UV: Fix conditional in gru_exit()
  2014-03-31 15:23       ` Dimitri Sivanich
@ 2014-04-01 11:07         ` tip-bot for Dimitri Sivanich
  0 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Dimitri Sivanich @ 2014-04-01 11:07 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, tglx, sivanich

Commit-ID:  fe455b17de6c881eecf4f9784c3b0483a5e3d19e
Gitweb:     http://git.kernel.org/tip/fe455b17de6c881eecf4f9784c3b0483a5e3d19e
Author:     Dimitri Sivanich <sivanich@sgi.com>
AuthorDate: Mon, 31 Mar 2014 10:23:20 -0500
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 1 Apr 2014 12:10:45 +0200

x86/UV: Fix conditional in gru_exit()

Fix supported system conditional in gru_exit(), in preparation for UV3.

Signed-off-by: Dimitri Sivanich <sivanich@sgi.com>
Link: http://lkml.kernel.org/r/20140331152320.GA31495@sgi.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/misc/sgi-gru/grufile.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/sgi-gru/grufile.c b/drivers/misc/sgi-gru/grufile.c
index 0535d1e..104a05f 100644
--- a/drivers/misc/sgi-gru/grufile.c
+++ b/drivers/misc/sgi-gru/grufile.c
@@ -6,7 +6,7 @@
  * This file supports the user system call for file open, close, mmap, etc.
  * This also incudes the driver initialization code.
  *
- *  Copyright (c) 2008 Silicon Graphics, Inc.  All Rights Reserved.
+ *  Copyright (c) 2008-2014 Silicon Graphics, Inc.  All Rights Reserved.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -58,6 +58,11 @@ static int max_user_cbrs, max_user_dsr_bytes;
 
 static struct miscdevice gru_miscdev;
 
+static int gru_supported(void)
+{
+	return is_uv_system() &&
+		(uv_hub_info->hub_revision < UV3_HUB_REVISION_BASE);
+}
 
 /*
  * gru_vma_close
@@ -518,7 +523,7 @@ static int __init gru_init(void)
 {
 	int ret;
 
-	if (!is_uv_system() || (is_uvx_hub() && !is_uv2_hub()))
+	if (!gru_supported())
 		return 0;
 
 #if defined CONFIG_IA64
@@ -573,7 +578,7 @@ exit0:
 
 static void __exit gru_exit(void)
 {
-	if (!is_uv_system())
+	if (!gru_supported())
 		return;
 
 	gru_teardown_tlb_irqs();

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-04-01 11:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-20 20:36 [PATCH] x86/UV: Fix conditional in gru_exit Dimitri Sivanich
2014-03-21  7:55 ` Ingo Molnar
2014-03-21 16:13   ` Dimitri Sivanich
2014-03-31  7:25     ` Ingo Molnar
2014-03-31 15:23       ` Dimitri Sivanich
2014-04-01 11:07         ` [tip:x86/uv] x86/UV: Fix conditional in gru_exit() tip-bot for Dimitri Sivanich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).