* [Patch 1/3] sgi-xp: eliminate '>>>' in comments
2008-06-10 16:24 [Patch 0/3] sgi-xp: response to Andrew's feedback Dean Nelson
@ 2008-06-10 16:28 ` Dean Nelson
2008-06-10 16:30 ` [Patch 2/3] sgi-xp: use standard bitops macros and functions Dean Nelson
2008-06-10 16:31 ` [Patch 3/3] sgi-xp: add 'jiffies' to reserved page's timestamp name Dean Nelson
2 siblings, 0 replies; 4+ messages in thread
From: Dean Nelson @ 2008-06-10 16:28 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel
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);
}
^ permalink raw reply [flat|nested] 4+ messages in thread* [Patch 2/3] sgi-xp: use standard bitops macros and functions
2008-06-10 16:24 [Patch 0/3] sgi-xp: response to Andrew's feedback Dean Nelson
2008-06-10 16:28 ` [Patch 1/3] sgi-xp: eliminate '>>>' in comments Dean Nelson
@ 2008-06-10 16:30 ` Dean Nelson
2008-06-10 16:31 ` [Patch 3/3] sgi-xp: add 'jiffies' to reserved page's timestamp name Dean Nelson
2 siblings, 0 replies; 4+ messages in thread
From: Dean Nelson @ 2008-06-10 16:30 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel
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;
}
^ permalink raw reply [flat|nested] 4+ messages in thread* [Patch 3/3] sgi-xp: add 'jiffies' to reserved page's timestamp name
2008-06-10 16:24 [Patch 0/3] sgi-xp: response to Andrew's feedback Dean Nelson
2008-06-10 16:28 ` [Patch 1/3] sgi-xp: eliminate '>>>' in comments Dean Nelson
2008-06-10 16:30 ` [Patch 2/3] sgi-xp: use standard bitops macros and functions Dean Nelson
@ 2008-06-10 16:31 ` Dean Nelson
2 siblings, 0 replies; 4+ messages in thread
From: Dean Nelson @ 2008-06-10 16:31 UTC (permalink / raw)
To: akpm; +Cc: linux-kernel
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;
}
^ permalink raw reply [flat|nested] 4+ messages in thread