[patch 0/2] x86, UV: fixups for configurations with a large number of nodes.

[patch 0/2] x86, UV: fixups for configurations with a large number of nodes.

[Robin Holt]

We need the __uv_hub_info structure to contain the correct values for
n_val, gpa_mask, and lowmem_remap_*.  The first patch in the series
accomplishes this.   Could this be included in the stable tree as well.
Without this patch, booting a large configuration hits a problem where
the upper bits of the gnode affect the pnode and the bau will not operate.

The second patch cleans up the broadcast assist unit code a small bit.

Thanks,
Robin Holt

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch 1/2] x86, UV: Fix information in __uv_hub_info structure.

[Robin Holt]

[-- Attachment #1: uv_hub_info_fix --]
[-- Type: text/plain, Size: 3123 bytes --]


A few parts of the uv_hub_info structure are initialized incorrectly.

 - n_val is being loaded with m_val.
 - gpa_mask is initialized with a bytes instead of an unsigned long.
 - Handle the case where none of the alias registers are used.

Lastly I converted the bau over to using the uv_hub_info->m_val which
is the correct value.

To: Ingo Molnar <mingo@elte.hu>
To: tglx@linutronix.de
Signed-off-by: Robin Holt <holt@sgi.com>
Signed-off-by: Jack Steiner <steiner@sgi.com>
Cc: stable@kernel.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org

---
 arch/x86/kernel/apic/x2apic_uv_x.c |    8 ++++----
 arch/x86/kernel/tlb_uv.c           |    4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)
Index: linux/arch/x86/kernel/apic/x2apic_uv_x.c
===================================================================
--- linux.orig/arch/x86/kernel/apic/x2apic_uv_x.c	2009-10-15 17:02:33.000000000 -0500
+++ linux/arch/x86/kernel/apic/x2apic_uv_x.c	2009-10-15 17:24:13.000000000 -0500
@@ -352,14 +352,14 @@ static __init void get_lowmem_redirect(u
 
 	for (i = 0; i < ARRAY_SIZE(redir_addrs); i++) {
 		alias.v = uv_read_local_mmr(redir_addrs[i].alias);
-		if (alias.s.base == 0) {
+		if (alias.s.enable && alias.s.base == 0) {
 			*size = (1UL << alias.s.m_alias);
 			redirect.v = uv_read_local_mmr(redir_addrs[i].redirect);
 			*base = (unsigned long)redirect.s.dest_base << DEST_SHIFT;
 			return;
 		}
 	}
-	BUG();
+	*base = *size = 0;
 }
 
 enum map_type {map_wb, map_uc};
@@ -619,12 +619,12 @@ void __init uv_system_init(void)
 		uv_cpu_hub_info(cpu)->lowmem_remap_base = lowmem_redir_base;
 		uv_cpu_hub_info(cpu)->lowmem_remap_top = lowmem_redir_size;
 		uv_cpu_hub_info(cpu)->m_val = m_val;
-		uv_cpu_hub_info(cpu)->n_val = m_val;
+		uv_cpu_hub_info(cpu)->n_val = n_val;
 		uv_cpu_hub_info(cpu)->numa_blade_id = blade;
 		uv_cpu_hub_info(cpu)->blade_processor_id = lcpu;
 		uv_cpu_hub_info(cpu)->pnode = pnode;
 		uv_cpu_hub_info(cpu)->pnode_mask = pnode_mask;
-		uv_cpu_hub_info(cpu)->gpa_mask = (1 << (m_val + n_val)) - 1;
+		uv_cpu_hub_info(cpu)->gpa_mask = (1UL << (m_val + n_val)) - 1;
 		uv_cpu_hub_info(cpu)->gnode_upper = gnode_upper;
 		uv_cpu_hub_info(cpu)->gnode_extra = gnode_extra;
 		uv_cpu_hub_info(cpu)->global_mmr_base = mmr_base;
Index: linux/arch/x86/kernel/tlb_uv.c
===================================================================
--- linux.orig/arch/x86/kernel/tlb_uv.c	2009-10-15 17:02:33.000000000 -0500
+++ linux/arch/x86/kernel/tlb_uv.c	2009-10-15 17:29:32.000000000 -0500
@@ -843,8 +843,8 @@ static int __init uv_bau_init(void)
 				       GFP_KERNEL, cpu_to_node(cur_cpu));
 
 	uv_bau_retry_limit = 1;
-	uv_nshift = uv_hub_info->n_val;
-	uv_mmask = (1UL << uv_hub_info->n_val) - 1;
+	uv_nshift = uv_hub_info->m_val;
+	uv_mmask = (1UL << uv_hub_info->m_val) - 1;
 	nblades = uv_num_possible_blades();
 
 	uv_bau_table_bases = (struct bau_control **)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch 2/2] x86, UV: Modify bau to use uv_gpa_to_pnode().

[Robin Holt]

[-- Attachment #1: bau_use_gpa_to_pnode --]
[-- Type: text/plain, Size: 3059 bytes --]

Create an inline function to extract the pnode from a global physical
address and then convert the broadcast assist unit to use the newly
created uv_gpa_to_pnode function.

To: Ingo Molnar <mingo@elte.hu>
To: tglx@linutronix.de
Signed-off-by: Robin Holt <holt@sgi.com>
Acked-by: Cliff Whickman <cpw@sgi.com>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org

---
 arch/x86/include/asm/uv/uv_hub.h |    8 +++++++-
 arch/x86/kernel/tlb_uv.c         |    7 ++-----
 2 files changed, 9 insertions(+), 6 deletions(-)
Index: linux/arch/x86/include/asm/uv/uv_hub.h
===================================================================
--- linux.orig/arch/x86/include/asm/uv/uv_hub.h	2009-10-15 17:26:48.000000000 -0500
+++ linux/arch/x86/include/asm/uv/uv_hub.h	2009-10-15 17:28:46.000000000 -0500
@@ -114,7 +114,7 @@
 /*
  * The largest possible NASID of a C or M brick (+ 2)
  */
-#define UV_MAX_NASID_VALUE	(UV_MAX_NUMALINK_NODES * 2)
+#define UV_MAX_NASID_VALUE	(UV_MAX_NUMALINK_BLADES * 2)
 
 struct uv_scir_s {
 	struct timer_list timer;
@@ -230,6 +230,12 @@ static inline unsigned long uv_gpa(void 
 	return uv_soc_phys_ram_to_gpa(__pa(v));
 }
 
+/* gpa -> pnode */
+static inline int uv_gpa_to_pnode(unsigned long gpa)
+{
+	return gpa >> uv_hub_info->m_val & ((1UL << uv_hub_info->n_val) - 1);
+}
+
 /* pnode, offset --> socket virtual */
 static inline void *uv_pnode_offset_to_vaddr(int pnode, unsigned long offset)
 {
Index: linux/arch/x86/kernel/tlb_uv.c
===================================================================
--- linux.orig/arch/x86/kernel/tlb_uv.c	2009-10-15 17:26:48.000000000 -0500
+++ linux/arch/x86/kernel/tlb_uv.c	2009-10-15 17:28:46.000000000 -0500
@@ -23,8 +23,6 @@
 static struct bau_control	**uv_bau_table_bases __read_mostly;
 static int			uv_bau_retry_limit __read_mostly;
 
-/* position of pnode (which is nasid>>1): */
-static int			uv_nshift __read_mostly;
 /* base pnode in this partition */
 static int			uv_partition_base_pnode __read_mostly;
 
@@ -723,7 +721,7 @@ uv_activation_descriptor_init(int node, 
 	BUG_ON(!adp);
 
 	pa = uv_gpa(adp); /* need the real nasid*/
-	n = pa >> uv_nshift;
+	n = uv_gpa_to_pnode(pa);
 	m = pa & uv_mmask;
 
 	uv_write_global_mmr64(pnode, UVH_LB_BAU_SB_DESCRIPTOR_BASE,
@@ -778,7 +776,7 @@ uv_payload_queue_init(int node, int pnod
 	 * need the pnode of where the memory was really allocated
 	 */
 	pa = uv_gpa(pqp);
-	pn = pa >> uv_nshift;
+	pn = uv_gpa_to_pnode(pa);
 	uv_write_global_mmr64(pnode,
 			      UVH_LB_BAU_INTD_PAYLOAD_QUEUE_FIRST,
 			      ((unsigned long)pn << UV_PAYLOADQ_PNODE_SHIFT) |
@@ -843,7 +841,6 @@ static int __init uv_bau_init(void)
 				       GFP_KERNEL, cpu_to_node(cur_cpu));
 
 	uv_bau_retry_limit = 1;
-	uv_nshift = uv_hub_info->m_val;
 	uv_mmask = (1UL << uv_hub_info->m_val) - 1;
 	nblades = uv_num_possible_blades();
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 0/2] x86, UV: fixups for configurations with a large number of nodes.

[Ingo Molnar]

* Robin Holt <holt@sgi.com> wrote:

> We need the __uv_hub_info structure to contain the correct values for 
> n_val, gpa_mask, and lowmem_remap_*.  The first patch in the series 
> accomplishes this.  Could this be included in the stable tree as well. 
> Without this patch, booting a large configuration hits a problem where 
> the upper bits of the gnode affect the pnode and the bau will not 
> operate.

i've applied this one.

> The second patch cleans up the broadcast assist unit code a small bit.

Seems to be more than just a 'cleanup'. It changes:

  uv_nshift = uv_hub_info->m_val;

to (in essence):

              uv_hub_info->m_val & ((1UL << uv_hub_info->n_val) - 1)

which is not the same. Furthermore, the new inline is:

+       return gpa >> uv_hub_info->m_val & ((1UL << uv_hub_info->n_val) - 1);

note that >> has higher priority than bitwise & - is that intended? I 
think the intention was:

+       return gpa >> (uv_hub_info->m_val & ((1UL << uv_hub_info->n_val) - 1));

in any case please do that cleaner by adding a separate mask variable.

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 0/2] x86, UV: fixups for configurations with a large number of nodes.

[Robin Holt]

On Fri, Oct 16, 2009 at 08:34:05AM +0200, Ingo Molnar wrote:
> 
> * Robin Holt <holt@sgi.com> wrote:
> 
> > We need the __uv_hub_info structure to contain the correct values for 
> > n_val, gpa_mask, and lowmem_remap_*.  The first patch in the series 
> > accomplishes this.  Could this be included in the stable tree as well. 
> > Without this patch, booting a large configuration hits a problem where 
> > the upper bits of the gnode affect the pnode and the bau will not 
> > operate.
> 
> i've applied this one.

Thank you for applying this one.

> > The second patch cleans up the broadcast assist unit code a small bit.
> 
> Seems to be more than just a 'cleanup'. It changes:

I am going to rearrange a bit:

> +       return gpa >> uv_hub_info->m_val & ((1UL << uv_hub_info->n_val) - 1);
> 
> note that >> has higher priority than bitwise & - is that intended? I 
> think the intention was:
> 
> +       return gpa >> (uv_hub_info->m_val & ((1UL << uv_hub_info->n_val) - 1));

The intention was (gpa >> m_val) & (n_mask);  I love the clarity of
making it an explicitly stated mask.  Much more readable.

> 
>   uv_nshift = uv_hub_info->m_val;
> 
> to (in essence):
> 
>               uv_hub_info->m_val & ((1UL << uv_hub_info->n_val) - 1)
> 
> which is not the same. Furthermore, the new inline is:

You have an excellent point there.  That was a bug as well.  That may
explain a few of our currently unexplained bau hangs.  The value is
supposed to be a pnode instead of the current gnode.

Robin

---

Create an inline function to extract the pnode from a global physical
address and then convert the broadcast assist unit to use the newly
created uv_gpa_to_pnode function.

To: Ingo Molnar <mingo@elte.hu>
To: tglx@linutronix.de
Signed-off-by: Robin Holt <holt@sgi.com>
Acked-by: Cliff Whickman <cpw@sgi.com>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org

---
 arch/x86/include/asm/uv/uv_hub.h |   16 +++++++++++++++-
 arch/x86/kernel/tlb_uv.c         |    7 ++-----
 2 files changed, 17 insertions(+), 6 deletions(-)
Index: linux/arch/x86/include/asm/uv/uv_hub.h
===================================================================
--- linux.orig/arch/x86/include/asm/uv/uv_hub.h	2009-10-16 06:02:23.000000000 -0500
+++ linux/arch/x86/include/asm/uv/uv_hub.h	2009-10-16 06:07:52.000000000 -0500
@@ -114,7 +114,7 @@
 /*
  * The largest possible NASID of a C or M brick (+ 2)
  */
-#define UV_MAX_NASID_VALUE	(UV_MAX_NUMALINK_NODES * 2)
+#define UV_MAX_NASID_VALUE	(UV_MAX_NUMALINK_BLADES * 2)
 
 struct uv_scir_s {
 	struct timer_list timer;
@@ -230,6 +230,20 @@ static inline unsigned long uv_gpa(void 
 	return uv_soc_phys_ram_to_gpa(__pa(v));
 }
 
+/* gnode -> pnode */
+static inline unsigned long uv_gpa_to_gnode(unsigned long gpa)
+{
+	return gpa >> uv_hub_info->m_val;
+}
+
+/* gpa -> pnode */
+static inline int uv_gpa_to_pnode(unsigned long gpa)
+{
+	unsigned long n_mask = (1UL << uv_hub_info->n_val) - 1;
+
+	return uv_gpa_to_gnode(gpa) & n_mask;
+}
+
 /* pnode, offset --> socket virtual */
 static inline void *uv_pnode_offset_to_vaddr(int pnode, unsigned long offset)
 {
Index: linux/arch/x86/kernel/tlb_uv.c
===================================================================
--- linux.orig/arch/x86/kernel/tlb_uv.c	2009-10-16 06:02:27.000000000 -0500
+++ linux/arch/x86/kernel/tlb_uv.c	2009-10-16 06:02:28.000000000 -0500
@@ -23,8 +23,6 @@
 static struct bau_control	**uv_bau_table_bases __read_mostly;
 static int			uv_bau_retry_limit __read_mostly;
 
-/* position of pnode (which is nasid>>1): */
-static int			uv_nshift __read_mostly;
 /* base pnode in this partition */
 static int			uv_partition_base_pnode __read_mostly;
 
@@ -723,7 +721,7 @@ uv_activation_descriptor_init(int node, 
 	BUG_ON(!adp);
 
 	pa = uv_gpa(adp); /* need the real nasid*/
-	n = pa >> uv_nshift;
+	n = uv_gpa_to_pnode(pa);
 	m = pa & uv_mmask;
 
 	uv_write_global_mmr64(pnode, UVH_LB_BAU_SB_DESCRIPTOR_BASE,
@@ -778,7 +776,7 @@ uv_payload_queue_init(int node, int pnod
 	 * need the pnode of where the memory was really allocated
 	 */
 	pa = uv_gpa(pqp);
-	pn = pa >> uv_nshift;
+	pn = uv_gpa_to_pnode(pa);
 	uv_write_global_mmr64(pnode,
 			      UVH_LB_BAU_INTD_PAYLOAD_QUEUE_FIRST,
 			      ((unsigned long)pn << UV_PAYLOADQ_PNODE_SHIFT) |
@@ -843,7 +841,6 @@ static int __init uv_bau_init(void)
 				       GFP_KERNEL, cpu_to_node(cur_cpu));
 
 	uv_bau_retry_limit = 1;
-	uv_nshift = uv_hub_info->m_val;
 	uv_mmask = (1UL << uv_hub_info->m_val) - 1;
 	nblades = uv_num_possible_blades();
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 0/2] x86, UV: fixups for configurations with a large number of nodes.

[Ingo Molnar]

* Robin Holt <holt@sgi.com> wrote:

> >   uv_nshift = uv_hub_info->m_val;
> > 
> > to (in essence):
> > 
> >               uv_hub_info->m_val & ((1UL << uv_hub_info->n_val) - 1)
> > 
> > which is not the same. Furthermore, the new inline is:
> 
> You have an excellent point there.  That was a bug as well.  That may 
> explain a few of our currently unexplained bau hangs.  The value is 
> supposed to be a pnode instead of the current gnode.

So ... is the commit log message i've put into the commit below correct, 
or is it still only a cleanup patch? You really need to put that kind of 
info into your changelogs - it helps maintainers put it into the right 
kernel release.

	Ingo

------------>

Re: [patch 0/2] x86, UV: fixups for configurations with a large number of nodes.

[Robin Holt]

On Fri, Oct 16, 2009 at 02:53:13PM +0200, Ingo Molnar wrote:
...
> So ... is the commit log message i've put into the commit below correct, 
> or is it still only a cleanup patch? You really need to put that kind of 
> info into your changelogs - it helps maintainers put it into the right 
> kernel release.
> 
> 	Ingo

...

> The open-coded code was wrong as well - it might explain a
> few of our unexplained bau hangs.

Terrific.  Thank you for fixing up my commit message.  I will try to be
more complete next time.

Robin

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>