[Patch 0/3] sgi-xp: response to Andrew's feedback

[Patch 0/3] sgi-xp: response to Andrew's feedback

[Dean Nelson]

This patchset is a response to comments made by Andrew Morton concerning my
just recently submitted patchset against /drivers/misc/sgi-xp:

    [Patch 00/18] continued prepartion of XPC/XPNET to support SGI UV

It is intended that that this patchset be applied on top of that patchset.

Again, the following is a false positive from scripts/checkpatch.pl.

    WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
    #25: FILE: misc/sgi-xp/xp_sn2.c:25:
    +EXPORT_SYMBOL_GPL(xp_nofault_PIOR);

[Patch 1/3] sgi-xp: eliminate '>>>' in comments

[Dean Nelson]

Comments in /drivers/misc/sgi-xp has been using '>>>' as a means to draw
attention to something that needs to be done or considered. To avoid colliding
with git rejects, '>>>' will now be replaced by '!!!' to indicate something to
do, and by '???' to indicate something to be considered.

Signed-off-by: Dean Nelson <dcn@sgi.com>

---

On Sun, Jun 08, 2008 at 05:12:35PM -0700, Andrew Morton wrote:
> On Fri, 6 Jun 2008 11:44:55 -0500 Dean Nelson <dcn@sgi.com> wrote:
> 
> > +/* >>> Add this #define to some linux header file some day. */
> 
> The patches fill the code with this ">>>" string - which can cause
> false positives when people are searching for git rejects.  Although I
> (and I suspect most other people) search for "<<<<<<<".

Andrew, I hope that '!!!' and '???' aren't a bad choice to replace '>>>' by.

Thanks for the feedback.

Dean


 drivers/misc/sgi-xp/xp.h            |   11 +++--------
 drivers/misc/sgi-xp/xp_sn2.c        |   10 +++++-----
 drivers/misc/sgi-xp/xp_uv.c         |    2 +-
 drivers/misc/sgi-xp/xpc.h           |   14 +++++++++-----
 drivers/misc/sgi-xp/xpc_channel.c   |    2 +-
 drivers/misc/sgi-xp/xpc_partition.c |    2 +-
 drivers/misc/sgi-xp/xpc_sn2.c       |    8 ++++----
 drivers/misc/sgi-xp/xpc_uv.c        |   32 ++++++++++++++++----------------
 drivers/misc/sgi-xp/xpnet.c         |    6 +++---
 9 files changed, 43 insertions(+), 44 deletions(-)

Index: linux-next/drivers/misc/sgi-xp/xp.h
===================================================================
--- linux-next.orig/drivers/misc/sgi-xp/xp.h	2008-06-10 10:39:39.272140155 -0500
+++ linux-next/drivers/misc/sgi-xp/xp.h	2008-06-10 10:39:42.988598082 -0500
@@ -21,7 +21,7 @@
 #include <asm/sn/arch.h>
 #endif
 
-/* >>> Add this #define to some linux header file some day. */
+/* ??? Add this #define to some linux header file some day? */
 #define BYTES_PER_WORD	sizeof(void *)
 
 #ifdef USE_DBUG_ON
@@ -65,18 +65,13 @@
  * other partition that is currently up. Over these channels, kernel-level
  * `users' can communicate with their counterparts on the other partitions.
  *
->>> The following described limitation of a max of eight channels possible
->>> pertains only to ia64-sn2. THIS ISN'T TRUE SINCE I'M PLANNING TO JUST
->>> TIE INTO THE EXISTING MECHANISM ONCE THE CHANNEL MESSAGES ARE RECEIVED.
->>> THE 128-BYTE CACHELINE PERFORMANCE ISSUE IS TIED TO IA64-SN2.
- *
  * If the need for additional channels arises, one can simply increase
  * XPC_MAX_NCHANNELS accordingly. If the day should come where that number
  * exceeds the absolute MAXIMUM number of channels possible (eight), then one
  * will need to make changes to the XPC code to accommodate for this.
  *
- * The absolute maximum number of channels possible is currently limited to
- * eight for performance reasons. The internal cross partition structures
+ * The absolute maximum number of channels possible is limited to eight for
+ * performance reasons on sn2 hardware. The internal cross partition structures
  * require sixteen bytes per channel, and eight allows all of this
  * interface-shared info to fit in one 128-byte cacheline.
  */
Index: linux-next/drivers/misc/sgi-xp/xp_sn2.c
===================================================================
--- linux-next.orig/drivers/misc/sgi-xp/xp_sn2.c	2008-06-10 10:38:22.734710213 -0500
+++ linux-next/drivers/misc/sgi-xp/xp_sn2.c	2008-06-10 10:39:43.000599561 -0500
@@ -87,11 +87,11 @@ xp_remote_memcpy_sn2(void *vdst, const v
 {
 	bte_result_t ret;
 	u64 pdst = ia64_tpa(vdst);
-	/* >>> What are the rules governing the src and dst addresses passed in?
-	 * >>> Currently we're assuming that dst is a virtual address and src
-	 * >>> is a physical address, is this appropriate? Can we allow them to
-	 * >>> be whatever and we make the change here without damaging the
-	 * >>> addresses?
+	/* ??? What are the rules governing the src and dst addresses passed in?
+	 * ??? Currently we're assuming that dst is a virtual address and src
+	 * ??? is a physical address, is this appropriate? Can we allow them to
+	 * ??? be whatever and we make the change here without damaging the
+	 * ??? addresses?
 	 */
 
 	/*
Index: linux-next/drivers/misc/sgi-xp/xp_uv.c
===================================================================
--- linux-next.orig/drivers/misc/sgi-xp/xp_uv.c	2008-06-10 10:38:22.734710213 -0500
+++ linux-next/drivers/misc/sgi-xp/xp_uv.c	2008-06-10 10:39:43.024602519 -0500
@@ -18,7 +18,7 @@
 static enum xp_retval
 xp_remote_memcpy_uv(void *vdst, const void *psrc, size_t len)
 {
-	/* >>> this function needs fleshing out */
+	/* !!! this function needs fleshing out */
 	return xpUnsupported;
 }
 
Index: linux-next/drivers/misc/sgi-xp/xpc.h
===================================================================
--- linux-next.orig/drivers/misc/sgi-xp/xpc.h	2008-06-10 10:39:39.200131282 -0500
+++ linux-next/drivers/misc/sgi-xp/xpc.h	2008-06-10 10:39:43.040604490 -0500
@@ -276,9 +276,12 @@ struct xpc_notify {
  * There is an array of these structures for each remote partition. It is
  * allocated at the time a partition becomes active. The array contains one
  * of these structures for each potential channel connection to that partition.
+ */
+
+/*
+ * The following is sn2 only.
  *
->>> sn2 only!!!
- * Each of these structures manages two message queues (circular buffers).
+ * Each channel structure manages two message queues (circular buffers).
  * They are allocated at the time a channel connection is made. One of
  * these message queues (local_msgqueue) holds the locally created messages
  * that are destined for the remote partition. The other of these message
@@ -345,6 +348,7 @@ struct xpc_notify {
  *	new messages, by the clearing of the message flags of the acknowledged
  *	messages.
  */
+
 struct xpc_channel_sn2 {
 
 	/* various flavors of local and remote Get/Put values */
@@ -359,7 +363,7 @@ struct xpc_channel_sn2 {
 };
 
 struct xpc_channel_uv {
-	/* >>> code is coming */
+	/* !!! code is coming */
 };
 
 struct xpc_channel {
@@ -500,7 +504,7 @@ xpc_any_msg_chctl_flags_set(union xpc_ch
 }
 
 /*
- * Manages channels on a partition basis. There is one of these structures
+ * Manage channels on a partition basis. There is one of these structures
  * for each partition (a partition will never utilize the structure that
  * represents itself).
  */
@@ -535,7 +539,7 @@ struct xpc_partition_sn2 {
 };
 
 struct xpc_partition_uv {
-	/* >>> code is coming */
+	/* !!! code is coming */
 };
 
 struct xpc_partition {
Index: linux-next/drivers/misc/sgi-xp/xpc_partition.c
===================================================================
--- linux-next.orig/drivers/misc/sgi-xp/xpc_partition.c	2008-06-10 10:39:39.236135718 -0500
+++ linux-next/drivers/misc/sgi-xp/xpc_partition.c	2008-06-10 10:39:43.060606955 -0500
@@ -91,7 +91,7 @@ xpc_get_rsvd_page_pa(int nasid)
 		if (status != SALRET_MORE_PASSES)
 			break;
 
-		/* >>> L1_CACHE_ALIGN() is only a sn2-bte_copy requirement */
+		/* !!! L1_CACHE_ALIGN() is only a sn2-bte_copy requirement */
 		if (L1_CACHE_ALIGN(len) > buf_len) {
 			kfree(buf_base);
 			buf_len = L1_CACHE_ALIGN(len);
Index: linux-next/drivers/misc/sgi-xp/xpc_sn2.c
===================================================================
--- linux-next.orig/drivers/misc/sgi-xp/xpc_sn2.c	2008-06-10 10:39:41.256384645 -0500
+++ linux-next/drivers/misc/sgi-xp/xpc_sn2.c	2008-06-10 10:39:43.080609420 -0500
@@ -75,7 +75,7 @@ xpc_allow_IPI_ops_sn2(void)
 	int node;
 	int nasid;
 
-	/* >>> The following should get moved into SAL. */
+	/* !!! The following should get moved into SAL. */
 	if (is_shub2()) {
 		xpc_sh2_IPI_access0_sn2 =
 		    (u64)HUB_L((u64 *)LOCAL_MMR_ADDR(SH2_IPI_ACCESS0));
@@ -118,7 +118,7 @@ xpc_disallow_IPI_ops_sn2(void)
 	int node;
 	int nasid;
 
-	/* >>> The following should get moved into SAL. */
+	/* !!! The following should get moved into SAL. */
 	if (is_shub2()) {
 		for_each_online_node(node) {
 			nasid = cnodeid_to_nasid(node);
@@ -1360,7 +1360,7 @@ xpc_teardown_infrastructure_sn2(struct x
  * dst must be a cacheline aligned virtual address on this partition.
  * cnt must be cacheline sized
  */
-/* >>> Replace this function by call to xp_remote_memcpy() or bte_copy()? */
+/* ??? Replace this function by call to xp_remote_memcpy() or bte_copy()? */
 static enum xp_retval
 xpc_pull_remote_cachelines_sn2(struct xpc_partition *part, void *dst,
 			       const void *src, size_t cnt)
@@ -2242,7 +2242,7 @@ xpc_send_msg_sn2(struct xpc_channel *ch,
 		notify->key = key;
 		notify->type = notify_type;
 
-		/* >>> is a mb() needed here? */
+		/* ??? Is a mb() needed here? */
 
 		if (ch->flags & XPC_C_DISCONNECTING) {
 			/*
Index: linux-next/drivers/misc/sgi-xp/xpc_uv.c
===================================================================
--- linux-next.orig/drivers/misc/sgi-xp/xpc_uv.c	2008-06-10 10:38:22.738710706 -0500
+++ linux-next/drivers/misc/sgi-xp/xpc_uv.c	2008-06-10 10:39:43.088610405 -0500
@@ -15,8 +15,8 @@
 
 #include <linux/kernel.h>
 
-/* >>> #include <gru/grukservices.h> */
-/* >>> uv_gpa() is defined in <gru/grukservices.h> */
+/* !!! #include <gru/grukservices.h> */
+/* !!! uv_gpa() is defined in <gru/grukservices.h> */
 #define uv_gpa(_a)		((unsigned long)_a)
 
 #include "xpc.h"
@@ -29,16 +29,16 @@ static void
 xpc_send_local_activate_IRQ_uv(struct xpc_partition *part)
 {
 	/*
-	 * >>> make our side think that the remote parition sent an activate
-	 * >>> message our way. Also do what the activate IRQ handler would
-	 * >>> do had one really been sent.
+	 * !!! Make our side think that the remote parition sent an activate
+	 * !!! message our way. Also do what the activate IRQ handler would
+	 * !!! do had one really been sent.
 	 */
 }
 
 static enum xp_retval
 xpc_rsvd_page_init_uv(struct xpc_rsvd_page *rp)
 {
-	/* >>> need to have established xpc_activate_mq earlier */
+	/* !!! need to have established xpc_activate_mq earlier */
 	rp->sn.activate_mq_gpa = uv_gpa(xpc_activate_mq);
 	return xpSuccess;
 }
@@ -46,7 +46,7 @@ xpc_rsvd_page_init_uv(struct xpc_rsvd_pa
 static void
 xpc_increment_heartbeat_uv(void)
 {
-	/* >>> send heartbeat msg to xpc_heartbeating_to_mask partids */
+	/* !!! send heartbeat msg to xpc_heartbeating_to_mask partids */
 }
 
 static void
@@ -59,7 +59,7 @@ xpc_heartbeat_init_uv(void)
 static void
 xpc_heartbeat_exit_uv(void)
 {
-	/* >>> send heartbeat_offline msg to xpc_heartbeating_to_mask partids */
+	/* !!! send heartbeat_offline msg to xpc_heartbeating_to_mask partids */
 }
 
 static void
@@ -70,9 +70,9 @@ xpc_request_partition_activation_uv(stru
 	struct xpc_partition *part = &xpc_partitions[partid];
 
 /*
- * >>> setup part structure with the bits of info we can glean from the rp
- * >>>	part->remote_rp_pa = remote_rp_pa;
- * >>>	part->sn.uv.activate_mq_gpa = remote_rp->sn.activate_mq_gpa;
+ * !!! Setup part structure with the bits of info we can glean from the rp:
+ * !!!	part->remote_rp_pa = remote_rp_pa;
+ * !!!	part->sn.uv.activate_mq_gpa = remote_rp->sn.activate_mq_gpa;
  */
 
 	xpc_send_local_activate_IRQ_uv(part);
@@ -91,7 +91,7 @@ xpc_request_partition_reactivation_uv(st
 static enum xp_retval
 xpc_setup_infrastructure_uv(struct xpc_partition *part)
 {
-	/* >>> this function needs fleshing out */
+	/* !!! this function needs fleshing out */
 	return xpUnsupported;
 }
 
@@ -102,28 +102,28 @@ xpc_setup_infrastructure_uv(struct xpc_p
 static void
 xpc_teardown_infrastructure_uv(struct xpc_partition *part)
 {
-	/* >>> this function needs fleshing out */
+	/* !!! this function needs fleshing out */
 	return;
 }
 
 static enum xp_retval
 xpc_make_first_contact_uv(struct xpc_partition *part)
 {
-	/* >>> this function needs fleshing out */
+	/* !!! this function needs fleshing out */
 	return xpUnsupported;
 }
 
 static u64
 xpc_get_chctl_all_flags_uv(struct xpc_partition *part)
 {
-	/* >>> this function needs fleshing out */
+	/* !!! this function needs fleshing out */
 	return 0UL;
 }
 
 static struct xpc_msg *
 xpc_get_deliverable_msg_uv(struct xpc_channel *ch)
 {
-	/* >>> this function needs fleshing out */
+	/* !!! this function needs fleshing out */
 	return NULL;
 }
 
Index: linux-next/drivers/misc/sgi-xp/xpnet.c
===================================================================
--- linux-next.orig/drivers/misc/sgi-xp/xpnet.c	2008-06-10 10:39:37.147878413 -0500
+++ linux-next/drivers/misc/sgi-xp/xpnet.c	2008-06-10 10:39:43.112613363 -0500
@@ -229,9 +229,9 @@ xpnet_receive(short partid, int channel,
 
 		if (ret != xpSuccess) {
 			/*
-			 * >>> Need better way of cleaning skb.  Currently skb
-			 * >>> appears in_use and we can't just call
-			 * >>> dev_kfree_skb.
+			 * !!! Need better way of cleaning skb.  Currently skb
+			 * !!! appears in_use and we can't just call
+			 * !!! dev_kfree_skb.
 			 */
 			dev_err(xpnet, "xp_remote_memcpy(0x%p, 0x%p, 0x%hx) "
 				"returned error=0x%x\n", (void *)
Index: linux-next/drivers/misc/sgi-xp/xpc_channel.c
===================================================================
--- linux-next.orig/drivers/misc/sgi-xp/xpc_channel.c	2008-06-10 10:39:33.000000000 -0500
+++ linux-next/drivers/misc/sgi-xp/xpc_channel.c	2008-06-10 10:41:12.003567102 -0500
@@ -129,7 +129,7 @@ xpc_process_disconnect(struct xpc_channe
 
 	/* wake those waiting for notify completion */
 	if (atomic_read(&ch->n_to_notify) > 0) {
-		/* >>> we do callout while holding ch->lock */
+		/* we do callout while holding ch->lock, callout can't block */
 		xpc_notify_senders_of_disconnect(ch);
 	}
 

[Patch 2/3] sgi-xp: use standard bitops macros and functions

[Dean Nelson]

Change sgi-xp to use the standard bitops macros and functions instead of trying
to invent its own mechanism.

Signed-off-by: Dean Nelson <dcn@sgi.com>

---

On Sun, Jun 08, 2008 at 05:12:35PM -0700, Andrew Morton wrote:
> On Fri, 6 Jun 2008 11:44:55 -0500 Dean Nelson <dcn@sgi.com> wrote:
> 
> > +#define BYTES_PER_WORD     sizeof(void *)
> 
> Dunno if this is a desirable thing to have, really.  A "word" is a
> somewhat ill-defined thing.  The definition you have here is always
> equal to BYTES_PER_LONG.  If BYTES_PER_LONG is inappropriate then
> BYTES_PER_POINTER would be clearer.

Agreed. In trying to address this issue, I decided to use the bitops macros
and functions already defined. I hope this meets with your approval.

Thanks for the feedback.

Dean

 drivers/misc/sgi-xp/xp.h            |    3 -
 drivers/misc/sgi-xp/xpc.h           |   43 ++++++------------
 drivers/misc/sgi-xp/xpc_partition.c |   43 +++++++++---------
 drivers/misc/sgi-xp/xpc_sn2.c       |   73 +++++++++++++++++---------------
 4 files changed, 76 insertions(+), 86 deletions(-)

Index: linux-next/drivers/misc/sgi-xp/xp.h
===================================================================
--- linux-next.orig/drivers/misc/sgi-xp/xp.h	2008-06-10 10:16:41.272817306 -0500
+++ linux-next/drivers/misc/sgi-xp/xp.h	2008-06-10 10:17:06.051904022 -0500
@@ -21,9 +21,6 @@
 #include <asm/sn/arch.h>
 #endif
 
-/* ??? Add this #define to some linux header file some day? */
-#define BYTES_PER_WORD	sizeof(void *)
-
 #ifdef USE_DBUG_ON
 #define DBUG_ON(condition)	BUG_ON(condition)
 #else
Index: linux-next/drivers/misc/sgi-xp/xpc_partition.c
===================================================================
--- linux-next.orig/drivers/misc/sgi-xp/xpc_partition.c	2008-06-10 10:16:48.593729274 -0500
+++ linux-next/drivers/misc/sgi-xp/xpc_partition.c	2008-06-10 10:17:06.071906513 -0500
@@ -31,11 +31,11 @@ int xpc_exiting;
 
 /* this partition's reserved page pointers */
 struct xpc_rsvd_page *xpc_rsvd_page;
-static u64 *xpc_part_nasids;
-u64 *xpc_mach_nasids;
+static unsigned long *xpc_part_nasids;
+unsigned long *xpc_mach_nasids;
 
-static int xpc_sizeof_nasid_mask;	/* actual size in bytes of nasid mask */
-int xpc_nasid_mask_words;	/* actual size in words of nasid mask */
+static int xpc_nasid_mask_nbytes;	/* #of bytes in nasid mask */
+int xpc_nasid_mask_nlongs;	/* #of longs in nasid mask */
 
 struct xpc_partition *xpc_partitions;
 
@@ -167,9 +167,9 @@ xpc_setup_rsvd_page(void)
 		/* SAL_version 1 didn't set the nasids_size field */
 		rp->SAL_nasids_size = 128;
 	}
-	xpc_sizeof_nasid_mask = rp->SAL_nasids_size;
-	xpc_nasid_mask_words = DIV_ROUND_UP(xpc_sizeof_nasid_mask,
-					    BYTES_PER_WORD);
+	xpc_nasid_mask_nbytes = rp->SAL_nasids_size;
+	xpc_nasid_mask_nlongs = BITS_TO_LONGS(rp->SAL_nasids_size *
+					      BITS_PER_BYTE);
 
 	/* setup the pointers to the various items in the reserved page */
 	xpc_part_nasids = XPC_RP_PART_NASIDS(rp);
@@ -199,10 +199,10 @@ xpc_setup_rsvd_page(void)
  * part_nasids mask.
  */
 enum xp_retval
-xpc_get_remote_rp(int nasid, u64 *discovered_nasids,
+xpc_get_remote_rp(int nasid, unsigned long *discovered_nasids,
 		  struct xpc_rsvd_page *remote_rp, u64 *remote_rp_pa)
 {
-	int i;
+	int l;
 	enum xp_retval ret;
 
 	/* get the reserved page's physical address */
@@ -213,15 +213,16 @@ xpc_get_remote_rp(int nasid, u64 *discov
 
 	/* pull over the reserved page header and part_nasids mask */
 	ret = xp_remote_memcpy(remote_rp, (void *)*remote_rp_pa,
-			       XPC_RP_HEADER_SIZE + xpc_sizeof_nasid_mask);
+			       XPC_RP_HEADER_SIZE + xpc_nasid_mask_nbytes);
 	if (ret != xpSuccess)
 		return ret;
 
 	if (discovered_nasids != NULL) {
-		u64 *remote_part_nasids = XPC_RP_PART_NASIDS(remote_rp);
+		unsigned long *remote_part_nasids =
+		    XPC_RP_PART_NASIDS(remote_rp);
 
-		for (i = 0; i < xpc_nasid_mask_words; i++)
-			discovered_nasids[i] |= remote_part_nasids[i];
+		for (l = 0; l < xpc_nasid_mask_nlongs; l++)
+			discovered_nasids[l] |= remote_part_nasids[l];
 	}
 
 	/* see if the reserved page has been set up by XPC */
@@ -401,16 +402,16 @@ xpc_discovery(void)
 	int max_regions;
 	int nasid;
 	struct xpc_rsvd_page *rp;
-	u64 *discovered_nasids;
+	unsigned long *discovered_nasids;
 	enum xp_retval ret;
 
 	remote_rp = xpc_kmalloc_cacheline_aligned(XPC_RP_HEADER_SIZE +
-						  xpc_sizeof_nasid_mask,
+						  xpc_nasid_mask_nbytes,
 						  GFP_KERNEL, &remote_rp_base);
 	if (remote_rp == NULL)
 		return;
 
-	discovered_nasids = kzalloc(sizeof(u64) * xpc_nasid_mask_words,
+	discovered_nasids = kzalloc(sizeof(long) * xpc_nasid_mask_nlongs,
 				    GFP_KERNEL);
 	if (discovered_nasids == NULL) {
 		kfree(remote_rp_base);
@@ -453,21 +454,21 @@ xpc_discovery(void)
 
 			dev_dbg(xpc_part, "checking nasid %d\n", nasid);
 
-			if (XPC_NASID_IN_ARRAY(nasid, xpc_part_nasids)) {
+			if (test_bit(nasid / 2, xpc_part_nasids)) {
 				dev_dbg(xpc_part, "PROM indicates Nasid %d is "
 					"part of the local partition; skipping "
 					"region\n", nasid);
 				break;
 			}
 
-			if (!(XPC_NASID_IN_ARRAY(nasid, xpc_mach_nasids))) {
+			if (!(test_bit(nasid / 2, xpc_mach_nasids))) {
 				dev_dbg(xpc_part, "PROM indicates Nasid %d was "
 					"not on Numa-Link network at reset\n",
 					nasid);
 				continue;
 			}
 
-			if (XPC_NASID_IN_ARRAY(nasid, discovered_nasids)) {
+			if (test_bit(nasid / 2, discovered_nasids)) {
 				dev_dbg(xpc_part, "Nasid %d is part of a "
 					"partition which was previously "
 					"discovered\n", nasid);
@@ -512,10 +513,10 @@ xpc_initiate_partid_to_nasids(short part
 	if (part->remote_rp_pa == 0)
 		return xpPartitionDown;
 
-	memset(nasid_mask, 0, xpc_sizeof_nasid_mask);
+	memset(nasid_mask, 0, xpc_nasid_mask_nbytes);
 
 	part_nasid_pa = (u64)XPC_RP_PART_NASIDS(part->remote_rp_pa);
 
 	return xp_remote_memcpy(nasid_mask, (void *)part_nasid_pa,
-				xpc_sizeof_nasid_mask);
+				xpc_nasid_mask_nbytes);
 }
Index: linux-next/drivers/misc/sgi-xp/xpc.h
===================================================================
--- linux-next.orig/drivers/misc/sgi-xp/xpc.h	2008-06-10 10:16:48.593729274 -0500
+++ linux-next/drivers/misc/sgi-xp/xpc.h	2008-06-10 10:17:06.091909005 -0500
@@ -35,23 +35,7 @@
 #define XPC_VERSION_MAJOR(_v)		((_v) >> 4)
 #define XPC_VERSION_MINOR(_v)		((_v) & 0xf)
 
-/*
- * The next macros define word or bit representations for given
- * C-brick nasid in either the SAL provided bit array representing
- * nasids in the partition/machine or the array of amo structures used
- * for inter-partition initiation communications.
- *
- * For SN2 machines, C-Bricks are alway even numbered NASIDs.  As
- * such, some space will be saved by insisting that nasid information
- * passed from SAL always be packed for C-Bricks and the
- * cross-partition interrupts use the same packing scheme.
- */
-#define XPC_NASID_W_INDEX(_n)	(((_n) / 64) / 2)
-#define XPC_NASID_B_INDEX(_n)	(((_n) / 2) & (64 - 1))
-#define XPC_NASID_IN_ARRAY(_n, _p) ((_p)[XPC_NASID_W_INDEX(_n)] & \
-				    (1UL << XPC_NASID_B_INDEX(_n)))
-#define XPC_NASID_FROM_W_B(_w, _b) (((_w) * 64 + (_b)) * 2)
-
+/* define frequency of the heartbeat and frequency how often it's checked */
 #define XPC_HB_DEFAULT_INTERVAL		5	/* incr HB every x secs */
 #define XPC_HB_CHECK_DEFAULT_INTERVAL	20	/* check HB every x secs */
 
@@ -86,11 +70,13 @@
  *     the actual nasids in the entire machine (mach_nasids). We're only
  *     interested in the even numbered nasids (which contain the processors
  *     and/or memory), so we only need half as many bits to represent the
- *     nasids. The part_nasids mask is located starting at the first cacheline
- *     following the reserved page header. The mach_nasids mask follows right
- *     after the part_nasids mask. The size in bytes of each mask is reflected
- *     by the reserved page header field 'SAL_nasids_size'. (Local partition's
- *     mask pointers are xpc_part_nasids and xpc_mach_nasids.)
+ *     nasids. When mapping nasid to bit in a mask (or bit to nasid) be sure
+ *     to either divide or multiply by 2. The part_nasids mask is located
+ *     starting at the first cacheline following the reserved page header. The
+ *     mach_nasids mask follows right after the part_nasids mask. The size in
+ *     bytes of each mask is reflected by the reserved page header field
+ *     'SAL_nasids_size'. (Local partition's mask pointers are xpc_part_nasids
+ *     and xpc_mach_nasids.)
  *
  *   vars	(ia64-sn2 only)
  *   vars part	(ia64-sn2 only)
@@ -194,10 +180,11 @@ struct xpc_vars_part_sn2 {
 #define XPC_RP_VARS_SIZE	L1_CACHE_ALIGN(sizeof(struct xpc_vars_sn2))
 
 #define XPC_RP_PART_NASIDS(_rp) ((u64 *)((u8 *)(_rp) + XPC_RP_HEADER_SIZE))
-#define XPC_RP_MACH_NASIDS(_rp) (XPC_RP_PART_NASIDS(_rp) + xpc_nasid_mask_words)
+#define XPC_RP_MACH_NASIDS(_rp) (XPC_RP_PART_NASIDS(_rp) + \
+				 xpc_nasid_mask_nlongs)
 #define XPC_RP_VARS(_rp)	((struct xpc_vars_sn2 *) \
 				 (XPC_RP_MACH_NASIDS(_rp) + \
-				  xpc_nasid_mask_words))
+				  xpc_nasid_mask_nlongs))
 
 /*
  * Functions registered by add_timer() or called by kernel_thread() only
@@ -695,9 +682,9 @@ extern void xpc_exit_uv(void);
 
 /* found in xpc_partition.c */
 extern int xpc_exiting;
-extern int xpc_nasid_mask_words;
+extern int xpc_nasid_mask_nlongs;
 extern struct xpc_rsvd_page *xpc_rsvd_page;
-extern u64 *xpc_mach_nasids;
+extern unsigned long *xpc_mach_nasids;
 extern struct xpc_partition *xpc_partitions;
 extern void *xpc_kmalloc_cacheline_aligned(size_t, gfp_t, void **);
 extern struct xpc_rsvd_page *xpc_setup_rsvd_page(void);
@@ -706,8 +693,8 @@ extern int xpc_partition_disengaged(stru
 extern enum xp_retval xpc_mark_partition_active(struct xpc_partition *);
 extern void xpc_mark_partition_inactive(struct xpc_partition *);
 extern void xpc_discovery(void);
-extern enum xp_retval xpc_get_remote_rp(int, u64 *, struct xpc_rsvd_page *,
-					u64 *);
+extern enum xp_retval xpc_get_remote_rp(int, unsigned long *,
+					struct xpc_rsvd_page *, u64 *);
 extern void xpc_deactivate_partition(const int, struct xpc_partition *,
 				     enum xp_retval);
 extern enum xp_retval xpc_initiate_partid_to_nasids(short, void *);
Index: linux-next/drivers/misc/sgi-xp/xpc_sn2.c
===================================================================
--- linux-next.orig/drivers/misc/sgi-xp/xpc_sn2.c	2008-06-10 10:16:48.593729274 -0500
+++ linux-next/drivers/misc/sgi-xp/xpc_sn2.c	2008-06-10 10:17:06.095909503 -0500
@@ -210,28 +210,26 @@ static void
 xpc_send_activate_IRQ_sn2(u64 amos_page_pa, int from_nasid, int to_nasid,
 			  int to_phys_cpuid)
 {
-	int w_index = XPC_NASID_W_INDEX(from_nasid);
-	int b_index = XPC_NASID_B_INDEX(from_nasid);
 	struct amo *amos = (struct amo *)__va(amos_page_pa +
 					      (XPC_ACTIVATE_IRQ_AMOS_SN2 *
 					      sizeof(struct amo)));
 
-	(void)xpc_send_IRQ_sn2(&amos[w_index], (1UL << b_index), to_nasid,
+	(void)xpc_send_IRQ_sn2(&amos[BIT_WORD(from_nasid / 2)],
+			       BIT_MASK(from_nasid / 2), to_nasid,
 			       to_phys_cpuid, SGI_XPC_ACTIVATE);
 }
 
 static void
 xpc_send_local_activate_IRQ_sn2(int from_nasid)
 {
-	int w_index = XPC_NASID_W_INDEX(from_nasid);
-	int b_index = XPC_NASID_B_INDEX(from_nasid);
 	struct amo *amos = (struct amo *)__va(xpc_vars_sn2->amos_page_pa +
 					      (XPC_ACTIVATE_IRQ_AMOS_SN2 *
 					      sizeof(struct amo)));
 
 	/* fake the sending and receipt of an activate IRQ from remote nasid */
-	FETCHOP_STORE_OP(TO_AMO((u64)&amos[w_index].variable), FETCHOP_OR,
-			 (1UL << b_index));
+	FETCHOP_STORE_OP(TO_AMO((u64)&amos[BIT_WORD(from_nasid / 2)].variable),
+			 FETCHOP_OR, BIT_MASK(from_nasid / 2));
+
 	atomic_inc(&xpc_activate_IRQ_rcvd);
 	wake_up_interruptible(&xpc_activate_IRQ_wq);
 }
@@ -439,7 +437,8 @@ xpc_indicate_partition_engaged_sn2(struc
 
 	/* set bit corresponding to our partid in remote partition's amo */
 	FETCHOP_STORE_OP(TO_AMO((u64)&amo->variable), FETCHOP_OR,
-			 (1UL << sn_partition_id));
+			 BIT(sn_partition_id));
+
 	/*
 	 * We must always use the nofault function regardless of whether we
 	 * are on a Shub 1.1 system or a Shub 1.2 slice 0xc processor. If we
@@ -466,7 +465,8 @@ xpc_indicate_partition_disengaged_sn2(st
 
 	/* clear bit corresponding to our partid in remote partition's amo */
 	FETCHOP_STORE_OP(TO_AMO((u64)&amo->variable), FETCHOP_AND,
-			 ~(1UL << sn_partition_id));
+			 ~BIT(sn_partition_id));
+
 	/*
 	 * We must always use the nofault function regardless of whether we
 	 * are on a Shub 1.1 system or a Shub 1.2 slice 0xc processor. If we
@@ -497,7 +497,7 @@ xpc_partition_engaged_sn2(short partid)
 
 	/* our partition's amo variable ANDed with partid mask */
 	return (FETCHOP_LOAD_OP(TO_AMO((u64)&amo->variable), FETCHOP_LOAD) &
-		(1UL << partid)) != 0;
+		BIT(partid)) != 0;
 }
 
 static int
@@ -518,7 +518,7 @@ xpc_assume_partition_disengaged_sn2(shor
 
 	/* clear bit(s) based on partid mask in our partition's amo */
 	FETCHOP_STORE_OP(TO_AMO((u64)&amo->variable), FETCHOP_AND,
-			 ~(1UL << partid));
+			 ~BIT(partid));
 }
 
 /* original protection values for each node */
@@ -639,7 +639,7 @@ xpc_rsvd_page_init_sn2(struct xpc_rsvd_p
 	       xp_max_npartitions);
 
 	/* initialize the activate IRQ related amo variables */
-	for (i = 0; i < xpc_nasid_mask_words; i++)
+	for (i = 0; i < xpc_nasid_mask_nlongs; i++)
 		(void)xpc_init_IRQ_amo_sn2(XPC_ACTIVATE_IRQ_AMOS_SN2 + i);
 
 	/* initialize the engaged remote partitions related amo variables */
@@ -796,7 +796,8 @@ xpc_request_partition_deactivation_sn2(s
 
 	/* set bit corresponding to our partid in remote partition's amo */
 	FETCHOP_STORE_OP(TO_AMO((u64)&amo->variable), FETCHOP_OR,
-			 (1UL << sn_partition_id));
+			 BIT(sn_partition_id));
+
 	/*
 	 * We must always use the nofault function regardless of whether we
 	 * are on a Shub 1.1 system or a Shub 1.2 slice 0xc processor. If we
@@ -831,7 +832,8 @@ xpc_cancel_partition_deactivation_reques
 
 	/* clear bit corresponding to our partid in remote partition's amo */
 	FETCHOP_STORE_OP(TO_AMO((u64)&amo->variable), FETCHOP_AND,
-			 ~(1UL << sn_partition_id));
+			 ~BIT(sn_partition_id));
+
 	/*
 	 * We must always use the nofault function regardless of whether we
 	 * are on a Shub 1.1 system or a Shub 1.2 slice 0xc processor. If we
@@ -853,7 +855,7 @@ xpc_partition_deactivation_requested_sn2
 
 	/* our partition's amo variable ANDed with partid mask */
 	return (FETCHOP_LOAD_OP(TO_AMO((u64)&amo->variable), FETCHOP_LOAD) &
-		(1UL << partid)) != 0;
+		BIT(partid)) != 0;
 }
 
 /*
@@ -1031,28 +1033,31 @@ xpc_identify_activate_IRQ_req_sn2(int na
 int
 xpc_identify_activate_IRQ_sender_sn2(void)
 {
-	int word, bit;
-	u64 nasid_mask;
+	int l;
+	int b;
+	unsigned long nasid_mask_long;
 	u64 nasid;		/* remote nasid */
 	int n_IRQs_detected = 0;
 	struct amo *act_amos;
 
 	act_amos = xpc_vars_sn2->amos_page + XPC_ACTIVATE_IRQ_AMOS_SN2;
 
-	/* scan through act amo variable looking for non-zero entries */
-	for (word = 0; word < xpc_nasid_mask_words; word++) {
+	/* scan through activate amo variables looking for non-zero entries */
+	for (l = 0; l < xpc_nasid_mask_nlongs; l++) {
 
 		if (xpc_exiting)
 			break;
 
-		nasid_mask = xpc_receive_IRQ_amo_sn2(&act_amos[word]);
-		if (nasid_mask == 0) {
-			/* no IRQs from nasids in this variable */
+		nasid_mask_long = xpc_receive_IRQ_amo_sn2(&act_amos[l]);
+
+		b = find_first_bit(&nasid_mask_long, BITS_PER_LONG);
+		if (b >= BITS_PER_LONG) {
+			/* no IRQs from nasids in this amo variable */
 			continue;
 		}
 
-		dev_dbg(xpc_part, "amo[%d] gave back 0x%lx\n", word,
-			nasid_mask);
+		dev_dbg(xpc_part, "amo[%d] gave back 0x%lx\n", l,
+			nasid_mask_long);
 
 		/*
 		 * If this nasid has been added to the machine since
@@ -1060,19 +1065,19 @@ xpc_identify_activate_IRQ_sender_sn2(voi
 		 * remote nasid in our reserved pages machine mask.
 		 * This is used in the event of module reload.
 		 */
-		xpc_mach_nasids[word] |= nasid_mask;
+		xpc_mach_nasids[l] |= nasid_mask_long;
 
 		/* locate the nasid(s) which sent interrupts */
 
-		for (bit = 0; bit < (8 * sizeof(u64)); bit++) {
-			if (nasid_mask & (1UL << bit)) {
-				n_IRQs_detected++;
-				nasid = XPC_NASID_FROM_W_B(word, bit);
-				dev_dbg(xpc_part, "interrupt from nasid %ld\n",
-					nasid);
-				xpc_identify_activate_IRQ_req_sn2(nasid);
-			}
-		}
+		do {
+			n_IRQs_detected++;
+			nasid = (l * BITS_PER_LONG + b) * 2;
+			dev_dbg(xpc_part, "interrupt from nasid %ld\n", nasid);
+			xpc_identify_activate_IRQ_req_sn2(nasid);
+
+			b = find_next_bit(&nasid_mask_long, BITS_PER_LONG,
+					  b + 1);
+		} while (b < BITS_PER_LONG);
 	}
 	return n_IRQs_detected;
 }

[Patch 3/3] sgi-xp: add 'jiffies' to reserved page's timestamp name

[Dean Nelson]

Rename XPC's reserved page's timestamp member to reflect the units of time
involved.

Signed-off-by: Dean Nelson <dcn@sgi.com>

---

On Sun, Jun 08, 2008 at 05:15:37PM -0700, Andrew Morton wrote:
> On Fri, 6 Jun 2008 11:52:16 -0500 Dean Nelson <dcn@sgi.com> wrote:
> 
> > +   unsigned long stamp;    /* time when reserved page was setup by XPC */
> 
> "time" is a rubbery concept in-kernel.  What are the units of this? 
> microseconds?  jiffies?  seconds?
> 
> At the least, the covering comment should make clear what units this
> variable is using.  Better would be to actually embed the units in the
> variable's identifier.  Because it's awfulyl easy to make mistakes over
> this, and not knowing the units makes the code harder to follow.

Agreed. Thanks for the feedback.

Dean

 drivers/misc/sgi-xp/xpc.h           |    6 +++---
 drivers/misc/sgi-xp/xpc_main.c      |    8 ++++----
 drivers/misc/sgi-xp/xpc_partition.c |   14 +++++++-------
 drivers/misc/sgi-xp/xpc_sn2.c       |   26 ++++++++++++++------------
 4 files changed, 28 insertions(+), 26 deletions(-)

Index: linux-next/drivers/misc/sgi-xp/xpc.h
===================================================================
--- linux-next.orig/drivers/misc/sgi-xp/xpc.h	2008-06-10 10:17:06.091909005 -0500
+++ linux-next/drivers/misc/sgi-xp/xpc.h	2008-06-10 10:17:12.688730757 -0500
@@ -87,7 +87,7 @@
  *     which are partition specific (vars part). These are setup by XPC.
  *     (Local partition's vars pointers are xpc_vars and xpc_vars_part.)
  *
- * Note: Until 'stamp' is set non-zero, the partition XPC code has not been
+ * Note: Until 'ts_jiffies' is set non-zero, the partition XPC code has not been
  *       initialized.
  */
 struct xpc_rsvd_page {
@@ -101,7 +101,7 @@ struct xpc_rsvd_page {
 		u64 vars_pa;	/* physical address of struct xpc_vars */
 		u64 activate_mq_gpa;	/* global phys address of activate_mq */
 	} sn;
-	unsigned long stamp;	/* time when reserved page was setup by XPC */
+	unsigned long ts_jiffies; /* timestamp when rsvd pg was setup by XPC */
 	u64 pad2[10];		/* align to last u64 in 2nd 64-byte cacheline */
 	u64 SAL_nasids_size;	/* SAL: size of each nasid mask in bytes */
 };
@@ -534,7 +534,7 @@ struct xpc_partition {
 	/* XPC HB infrastructure */
 
 	u8 remote_rp_version;	/* version# of partition's rsvd pg */
-	unsigned long remote_rp_stamp; /* time when rsvd pg was initialized */
+	unsigned long remote_rp_ts_jiffies; /* timestamp when rsvd pg setup */
 	u64 remote_rp_pa;	/* phys addr of partition's rsvd pg */
 	u64 last_heartbeat;	/* HB at last read */
 	u32 activate_IRQ_rcvd;	/* IRQs since activation */
Index: linux-next/drivers/misc/sgi-xp/xpc_main.c
===================================================================
--- linux-next.orig/drivers/misc/sgi-xp/xpc_main.c	2008-06-10 10:16:48.593729274 -0500
+++ linux-next/drivers/misc/sgi-xp/xpc_main.c	2008-06-10 10:17:12.812746205 -0500
@@ -862,8 +862,8 @@ xpc_do_exit(enum xp_retval reason)
 	DBUG_ON(xpc_any_partition_engaged());
 	DBUG_ON(xpc_any_hbs_allowed() != 0);
 
-	/* indicate to others that our reserved page is uninitialized */
-	xpc_rsvd_page->stamp = 0;
+	/* a zero timestamp indicates our rsvd page is not initialized */
+	xpc_rsvd_page->ts_jiffies = 0;
 
 	if (reason == xpUnloading) {
 		(void)unregister_die_notifier(&xpc_die_notifier);
@@ -1152,8 +1152,8 @@ xpc_init(void)
 
 	/* initialization was not successful */
 out_3:
-	/* indicate to others that our reserved page is uninitialized */
-	xpc_rsvd_page->stamp = 0;
+	/* a zero timestamp indicates our rsvd page is not initialized */
+	xpc_rsvd_page->ts_jiffies = 0;
 
 	(void)unregister_die_notifier(&xpc_die_notifier);
 	(void)unregister_reboot_notifier(&xpc_reboot_notifier);
Index: linux-next/drivers/misc/sgi-xp/xpc_partition.c
===================================================================
--- linux-next.orig/drivers/misc/sgi-xp/xpc_partition.c	2008-06-10 10:17:06.071906513 -0500
+++ linux-next/drivers/misc/sgi-xp/xpc_partition.c	2008-06-10 10:17:12.976766637 -0500
@@ -133,7 +133,7 @@ xpc_setup_rsvd_page(void)
 {
 	struct xpc_rsvd_page *rp;
 	u64 rp_pa;
-	unsigned long new_stamp;
+	unsigned long new_ts_jiffies;
 
 	/* get the local reserved page's address */
 
@@ -183,10 +183,10 @@ xpc_setup_rsvd_page(void)
 	 * This signifies to the remote partition that our reserved
 	 * page is initialized.
 	 */
-	new_stamp = jiffies;
-	if (new_stamp == 0 || new_stamp == rp->stamp)
-		new_stamp++;
-	rp->stamp = new_stamp;
+	new_ts_jiffies = jiffies;
+	if (new_ts_jiffies == 0 || new_ts_jiffies == rp->ts_jiffies)
+		new_ts_jiffies++;
+	rp->ts_jiffies = new_ts_jiffies;
 
 	return rp;
 }
@@ -225,8 +225,8 @@ xpc_get_remote_rp(int nasid, unsigned lo
 			discovered_nasids[l] |= remote_part_nasids[l];
 	}
 
-	/* see if the reserved page has been set up by XPC */
-	if (remote_rp->stamp == 0)
+	/* zero timestamp indicates the reserved page has not been setup */
+	if (remote_rp->ts_jiffies == 0)
 		return xpRsvdPageNotSet;
 
 	if (XPC_VERSION_MAJOR(remote_rp->version) !=
Index: linux-next/drivers/misc/sgi-xp/xpc_sn2.c
===================================================================
--- linux-next.orig/drivers/misc/sgi-xp/xpc_sn2.c	2008-06-10 10:17:06.095909503 -0500
+++ linux-next/drivers/misc/sgi-xp/xpc_sn2.c	2008-06-10 10:17:13.000769626 -0500
@@ -863,8 +863,8 @@ xpc_partition_deactivation_requested_sn2
  */
 static void
 xpc_update_partition_info_sn2(struct xpc_partition *part, u8 remote_rp_version,
-			      unsigned long *remote_rp_stamp, u64 remote_rp_pa,
-			      u64 remote_vars_pa,
+			      unsigned long *remote_rp_ts_jiffies,
+			      u64 remote_rp_pa, u64 remote_vars_pa,
 			      struct xpc_vars_sn2 *remote_vars)
 {
 	struct xpc_partition_sn2 *part_sn2 = &part->sn.sn2;
@@ -873,9 +873,9 @@ xpc_update_partition_info_sn2(struct xpc
 	dev_dbg(xpc_part, "  remote_rp_version = 0x%016x\n",
 		part->remote_rp_version);
 
-	part->remote_rp_stamp = *remote_rp_stamp;
-	dev_dbg(xpc_part, "  remote_rp_stamp = 0x%016lx\n",
-		part->remote_rp_stamp);
+	part->remote_rp_ts_jiffies = *remote_rp_ts_jiffies;
+	dev_dbg(xpc_part, "  remote_rp_ts_jiffies = 0x%016lx\n",
+		part->remote_rp_ts_jiffies);
 
 	part->remote_rp_pa = remote_rp_pa;
 	dev_dbg(xpc_part, "  remote_rp_pa = 0x%016lx\n", part->remote_rp_pa);
@@ -933,7 +933,7 @@ xpc_identify_activate_IRQ_req_sn2(int na
 	u64 remote_vars_pa;
 	int remote_rp_version;
 	int reactivate = 0;
-	unsigned long remote_rp_stamp = 0;
+	unsigned long remote_rp_ts_jiffies = 0;
 	short partid;
 	struct xpc_partition *part;
 	struct xpc_partition_sn2 *part_sn2;
@@ -952,7 +952,7 @@ xpc_identify_activate_IRQ_req_sn2(int na
 
 	remote_vars_pa = remote_rp->sn.vars_pa;
 	remote_rp_version = remote_rp->version;
-	remote_rp_stamp = remote_rp->stamp;
+	remote_rp_ts_jiffies = remote_rp->ts_jiffies;
 
 	partid = remote_rp->SAL_partid;
 	part = &xpc_partitions[partid];
@@ -981,8 +981,9 @@ xpc_identify_activate_IRQ_req_sn2(int na
 	    part->act_state == XPC_P_INACTIVE) {
 
 		xpc_update_partition_info_sn2(part, remote_rp_version,
-					      &remote_rp_stamp, remote_rp_pa,
-					      remote_vars_pa, remote_vars);
+					      &remote_rp_ts_jiffies,
+					      remote_rp_pa, remote_vars_pa,
+					      remote_vars);
 
 		if (xpc_partition_deactivation_requested_sn2(partid)) {
 			/*
@@ -999,7 +1000,7 @@ xpc_identify_activate_IRQ_req_sn2(int na
 	DBUG_ON(part->remote_rp_version == 0);
 	DBUG_ON(part_sn2->remote_vars_version == 0);
 
-	if (remote_rp_stamp != part->remote_rp_stamp) {
+	if (remote_rp_ts_jiffies != part->remote_rp_ts_jiffies) {
 
 		/* the other side rebooted */
 
@@ -1007,8 +1008,9 @@ xpc_identify_activate_IRQ_req_sn2(int na
 		DBUG_ON(xpc_partition_deactivation_requested_sn2(partid));
 
 		xpc_update_partition_info_sn2(part, remote_rp_version,
-					      &remote_rp_stamp, remote_rp_pa,
-					      remote_vars_pa, remote_vars);
+					      &remote_rp_ts_jiffies,
+					      remote_rp_pa, remote_vars_pa,
+					      remote_vars);
 		reactivate = 1;
 	}