All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.