[PATCH] - Patch series for parisc

[PATCH] - Patch series for parisc

[Helge Deller]

The following mails contain patches which I have sent to linux-parisc during 
the last weeks and which have not yet been applied.
I've cleaned them up for Linus' and Kyle's current trees.

Helge

[PATCH] compat_sys_ptrace conversions for parisc

[Helge Deller]

This patch does the compat_sys_ptrace conversion for parisc.
In addition it does convert the parisc ptrace code to use the
architecture-independent ptrace infrastructure instead of own coding.

Signed-off-by: Helge Deller <deller@gmx.de>

diff --git a/arch/parisc/include/asm/ptrace.h b/arch/parisc/include/asm/ptrace.h
index 3e94c5d..afa5333 100644
--- a/arch/parisc/include/asm/ptrace.h
+++ b/arch/parisc/include/asm/ptrace.h
@@ -47,6 +47,16 @@ struct pt_regs {
 
 #define task_regs(task) ((struct pt_regs *) ((char *)(task) + TASK_REGS))
 
+#define __ARCH_WANT_COMPAT_SYS_PTRACE
+
+struct task_struct;
+#define arch_has_single_step()	1
+void user_disable_single_step(struct task_struct *task);
+void user_enable_single_step(struct task_struct *task);
+
+#define arch_has_block_step()	1
+void user_enable_block_step(struct task_struct *task);
+
 /* XXX should we use iaoq[1] or iaoq[0] ? */
 #define user_mode(regs)			(((regs)->iaoq[0] & 3) ? 1 : 0)
 #define user_space(regs)		(((regs)->iasq[1] != 0) ? 1 : 0)
diff --git a/arch/parisc/kernel/ptrace.c b/arch/parisc/kernel/ptrace.c
index 49c6379..90904f9 100644
--- a/arch/parisc/kernel/ptrace.c
+++ b/arch/parisc/kernel/ptrace.c
@@ -4,6 +4,7 @@
  * Copyright (C) 2000 Hewlett-Packard Co, Linuxcare Inc.
  * Copyright (C) 2000 Matthew Wilcox <matthew@wil.cx>
  * Copyright (C) 2000 David Huggins-Daines <dhd@debian.org>
+ * Copyright (C) 2008 Helge Deller <deller@gmx.de>
  */
 
 #include <linux/kernel.h>
@@ -27,15 +28,149 @@
 /* PSW bits we allow the debugger to modify */
 #define USER_PSW_BITS	(PSW_N | PSW_V | PSW_CB)
 
-#undef DEBUG_PTRACE
+/*
+ * Called by kernel/ptrace.c when detaching..
+ *
+ * Make sure single step bits etc are not set.
+ */
+void ptrace_disable(struct task_struct *task)
+{
+	task->ptrace &= ~(PT_SINGLESTEP|PT_BLOCKSTEP);
 
-#ifdef DEBUG_PTRACE
-#define DBG(x...)	printk(x)
-#else
-#define DBG(x...)
-#endif
+	/* make sure the trap bits are not set */
+	pa_psw(task)->r = 0;
+	pa_psw(task)->t = 0;
+	pa_psw(task)->h = 0;
+	pa_psw(task)->l = 0;
+}
+
+/*
+ * The following functions are called by ptrace_resume() when
+ * enabling or disabling single/block tracing.
+ */
+void user_disable_single_step(struct task_struct *task)
+{
+	ptrace_disable(task);
+}
+
+void user_enable_single_step(struct task_struct *task)
+{
+	task->ptrace &= ~PT_BLOCKSTEP;
+	task->ptrace |= PT_SINGLESTEP;
+
+	if (pa_psw(task)->n) {
+		struct siginfo si;
+
+		/* Nullified, just crank over the queue. */
+		task_regs(task)->iaoq[0] = task_regs(task)->iaoq[1];
+		task_regs(task)->iasq[0] = task_regs(task)->iasq[1];
+		task_regs(task)->iaoq[1] = task_regs(task)->iaoq[0] + 4;
+		pa_psw(task)->n = 0;
+		pa_psw(task)->x = 0;
+		pa_psw(task)->y = 0;
+		pa_psw(task)->z = 0;
+		pa_psw(task)->b = 0;
+		ptrace_disable(task);
+		/* Don't wake up the task, but let the
+		   parent know something happened. */
+		si.si_code = TRAP_TRACE;
+		si.si_addr = (void __user *) (task_regs(task)->iaoq[0] & ~3);
+		si.si_signo = SIGTRAP;
+		si.si_errno = 0;
+		force_sig_info(SIGTRAP, &si, task);
+		/* notify_parent(task, SIGCHLD); */
+		return;
+	}
+
+	/* Enable recovery counter traps.  The recovery counter
+	 * itself will be set to zero on a task switch.  If the
+	 * task is suspended on a syscall then the syscall return
+	 * path will overwrite the recovery counter with a suitable
+	 * value such that it traps once back in user space.  We
+	 * disable interrupts in the tasks PSW here also, to avoid
+	 * interrupts while the recovery counter is decrementing.
+	 */
+	pa_psw(task)->r = 1;
+	pa_psw(task)->t = 0;
+	pa_psw(task)->h = 0;
+	pa_psw(task)->l = 0;
+}
+
+void user_enable_block_step(struct task_struct *task)
+{
+	task->ptrace &= ~PT_SINGLESTEP;
+	task->ptrace |= PT_BLOCKSTEP;
+
+	/* Enable taken branch trap. */
+	pa_psw(task)->r = 0;
+	pa_psw(task)->t = 1;
+	pa_psw(task)->h = 0;
+	pa_psw(task)->l = 0;
+}
+
+long arch_ptrace(struct task_struct *child, long request, long addr, long data)
+{
+	unsigned long tmp;
+	long ret = -EIO;
 
-#ifdef CONFIG_64BIT
+	switch (request) {
+
+	/* Read the word at location addr in the USER area.  For ptraced
+	   processes, the kernel saves all regs on a syscall. */
+	case PTRACE_PEEKUSR:
+		if ((addr & (sizeof(long)-1)) ||
+		    (unsigned long) addr >= sizeof(struct pt_regs))
+			break;
+		tmp = *(unsigned long *) ((char *) task_regs(child) + addr);
+		ret = put_user(tmp, (unsigned long *) data);
+		break;
+
+	/* Write the word at location addr in the USER area.  This will need
+	   to change when the kernel no longer saves all regs on a syscall.
+	   FIXME.  There is a problem at the moment in that r3-r18 are only
+	   saved if the process is ptraced on syscall entry, and even then
+	   those values are overwritten by actual register values on syscall
+	   exit. */
+	case PTRACE_POKEUSR:
+		/* Some register values written here may be ignored in
+		 * entry.S:syscall_restore_rfi; e.g. iaoq is written with
+		 * r31/r31+4, and not with the values in pt_regs.
+		 */
+		if (addr == PT_PSW) {
+			/* Allow writing to Nullify, Divide-step-correction,
+			 * and carry/borrow bits.
+			 * BEWARE, if you set N, and then single step, it won't
+			 * stop on the nullified instruction.
+			 */
+			data &= USER_PSW_BITS;
+			task_regs(child)->gr[0] &= ~USER_PSW_BITS;
+			task_regs(child)->gr[0] |= data;
+			ret = 0;
+			break;
+		}
+
+		if ((addr & (sizeof(long)-1)) ||
+		    (unsigned long) addr >= sizeof(struct pt_regs))
+			break;
+		if ((addr >= PT_GR1 && addr <= PT_GR31) ||
+				addr == PT_IAOQ0 || addr == PT_IAOQ1 ||
+				(addr >= PT_FR0 && addr <= PT_FR31 + 4) ||
+				addr == PT_SAR) {
+			*(unsigned long *) ((char *) task_regs(child) + addr) = data;
+			ret = 0;
+		}
+		break;
+
+	default:
+		ret = ptrace_request(child, request, addr, data);
+		break;
+	}
+
+	return ret;
+}
+
+
+#ifdef CONFIG_COMPAT
 
 /* This function is needed to translate 32 bit pt_regs offsets in to
  * 64 bit pt_regs offsets.  For example, a 32 bit gdb under a 64 bit kernel
@@ -61,106 +196,25 @@ static long translate_usr_offset(long offset)
 	else
 		return -1;
 }
-#endif
 
-/*
- * Called by kernel/ptrace.c when detaching..
- *
- * Make sure single step bits etc are not set.
- */
-void ptrace_disable(struct task_struct *child)
+long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
+			compat_ulong_t addr, compat_ulong_t data)
 {
-	/* make sure the trap bits are not set */
-	pa_psw(child)->r = 0;
-	pa_psw(child)->t = 0;
-	pa_psw(child)->h = 0;
-	pa_psw(child)->l = 0;
-}
-
-long arch_ptrace(struct task_struct *child, long request, long addr, long data)
-{
-	long ret;
-#ifdef DEBUG_PTRACE
-	long oaddr=addr, odata=data;
-#endif
+	compat_uint_t tmp;
+	long ret = -EIO;
 
 	switch (request) {
-	case PTRACE_PEEKTEXT: /* read word at location addr. */ 
-	case PTRACE_PEEKDATA: {
-#ifdef CONFIG_64BIT
-		if (__is_compat_task(child)) {
-			int copied;
-			unsigned int tmp;
-
-			addr &= 0xffffffffL;
-			copied = access_process_vm(child, addr, &tmp, sizeof(tmp), 0);
-			ret = -EIO;
-			if (copied != sizeof(tmp))
-				goto out_tsk;
-			ret = put_user(tmp,(unsigned int *) data);
-			DBG("sys_ptrace(PEEK%s, %d, %lx, %lx) returning %ld, data %x\n",
-				request == PTRACE_PEEKTEXT ? "TEXT" : "DATA",
-				pid, oaddr, odata, ret, tmp);
-		}
-		else
-#endif
-			ret = generic_ptrace_peekdata(child, addr, data);
-		goto out_tsk;
-	}
 
-	/* when I and D space are separate, this will have to be fixed. */
-	case PTRACE_POKETEXT: /* write the word at location addr. */
-	case PTRACE_POKEDATA:
-		ret = 0;
-#ifdef CONFIG_64BIT
-		if (__is_compat_task(child)) {
-			unsigned int tmp = (unsigned int)data;
-			DBG("sys_ptrace(POKE%s, %d, %lx, %lx)\n",
-				request == PTRACE_POKETEXT ? "TEXT" : "DATA",
-				pid, oaddr, odata);
-			addr &= 0xffffffffL;
-			if (access_process_vm(child, addr, &tmp, sizeof(tmp), 1) == sizeof(tmp))
-				goto out_tsk;
-		}
-		else
-#endif
-		{
-			if (access_process_vm(child, addr, &data, sizeof(data), 1) == sizeof(data))
-				goto out_tsk;
-		}
-		ret = -EIO;
-		goto out_tsk;
-
-	/* Read the word at location addr in the USER area.  For ptraced
-	   processes, the kernel saves all regs on a syscall. */
-	case PTRACE_PEEKUSR: {
-		ret = -EIO;
-#ifdef CONFIG_64BIT
-		if (__is_compat_task(child)) {
-			unsigned int tmp;
-
-			if (addr & (sizeof(int)-1))
-				goto out_tsk;
-			if ((addr = translate_usr_offset(addr)) < 0)
-				goto out_tsk;
-
-			tmp = *(unsigned int *) ((char *) task_regs(child) + addr);
-			ret = put_user(tmp, (unsigned int *) data);
-			DBG("sys_ptrace(PEEKUSR, %d, %lx, %lx) returning %ld, addr %lx, data %x\n",
-				pid, oaddr, odata, ret, addr, tmp);
-		}
-		else
-#endif
-		{
-			unsigned long tmp;
+	case PTRACE_PEEKUSR:
+		if (addr & (sizeof(compat_uint_t)-1))
+			break;
+		addr = translate_usr_offset(addr);
+		if (addr < 0)
+			break;
 
-			if ((addr & (sizeof(long)-1)) || (unsigned long) addr >= sizeof(struct pt_regs))
-				goto out_tsk;
-			tmp = *(unsigned long *) ((char *) task_regs(child) + addr);
-			ret = put_user(tmp, (unsigned long *) data);
-		}
-		goto out_tsk;
-	}
+		tmp = *(compat_uint_t *) ((char *) task_regs(child) + addr);
+		ret = put_user(tmp, (compat_uint_t *) (unsigned long) data);
+		break;
 
 	/* Write the word at location addr in the USER area.  This will need
 	   to change when the kernel no longer saves all regs on a syscall.
@@ -169,185 +223,46 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 	   those values are overwritten by actual register values on syscall
 	   exit. */
 	case PTRACE_POKEUSR:
-		ret = -EIO;
 		/* Some register values written here may be ignored in
 		 * entry.S:syscall_restore_rfi; e.g. iaoq is written with
 		 * r31/r31+4, and not with the values in pt_regs.
 		 */
-		 /* PT_PSW=0, so this is valid for 32 bit processes under 64
-		 * bit kernels.
-		 */
 		if (addr == PT_PSW) {
-			/* PT_PSW=0, so this is valid for 32 bit processes
-			 * under 64 bit kernels.
-			 *
-			 * Allow writing to Nullify, Divide-step-correction,
-			 * and carry/borrow bits.
-			 * BEWARE, if you set N, and then single step, it won't
-			 * stop on the nullified instruction.
+			/* Since PT_PSW==0, it is valid for 32 bit processes
+			 * under 64 bit kernels as well.
 			 */
-			DBG("sys_ptrace(POKEUSR, %d, %lx, %lx)\n",
-				pid, oaddr, odata);
-			data &= USER_PSW_BITS;
-			task_regs(child)->gr[0] &= ~USER_PSW_BITS;
-			task_regs(child)->gr[0] |= data;
-			ret = 0;
-			goto out_tsk;
-		}
-#ifdef CONFIG_64BIT
-		if (__is_compat_task(child)) {
-			if (addr & (sizeof(int)-1))
-				goto out_tsk;
-			if ((addr = translate_usr_offset(addr)) < 0)
-				goto out_tsk;
-			DBG("sys_ptrace(POKEUSR, %d, %lx, %lx) addr %lx\n",
-				pid, oaddr, odata, addr);
+			ret = arch_ptrace(child, request, addr, data);
+		} else {
+			if (addr & (sizeof(compat_uint_t)-1))
+				break;
+			addr = translate_usr_offset(addr);
+			if (addr < 0)
+				break;
 			if (addr >= PT_FR0 && addr <= PT_FR31 + 4) {
 				/* Special case, fp regs are 64 bits anyway */
-				*(unsigned int *) ((char *) task_regs(child) + addr) = data;
+				*(__u64 *) ((char *) task_regs(child) + addr) = data;
 				ret = 0;
 			}
 			else if ((addr >= PT_GR1+4 && addr <= PT_GR31+4) ||
 					addr == PT_IAOQ0+4 || addr == PT_IAOQ1+4 ||
 					addr == PT_SAR+4) {
 				/* Zero the top 32 bits */
-				*(unsigned int *) ((char *) task_regs(child) + addr - 4) = 0;
-				*(unsigned int *) ((char *) task_regs(child) + addr) = data;
+				*(__u32 *) ((char *) task_regs(child) + addr - 4) = 0;
+				*(__u32 *) ((char *) task_regs(child) + addr) = data;
 				ret = 0;
 			}
-			goto out_tsk;
 		}
-		else
-#endif
-		{
-			if ((addr & (sizeof(long)-1)) || (unsigned long) addr >= sizeof(struct pt_regs))
-				goto out_tsk;
-			if ((addr >= PT_GR1 && addr <= PT_GR31) ||
-					addr == PT_IAOQ0 || addr == PT_IAOQ1 ||
-					(addr >= PT_FR0 && addr <= PT_FR31 + 4) ||
-					addr == PT_SAR) {
-				*(unsigned long *) ((char *) task_regs(child) + addr) = data;
-				ret = 0;
-			}
-			goto out_tsk;
-		}
-
-	case PTRACE_SYSCALL: /* continue and stop at next (return from) syscall */
-	case PTRACE_CONT:
-		ret = -EIO;
-		DBG("sys_ptrace(%s)\n",
-			request == PTRACE_SYSCALL ? "SYSCALL" : "CONT");
-		if (!valid_signal(data))
-			goto out_tsk;
-		child->ptrace &= ~(PT_SINGLESTEP|PT_BLOCKSTEP);
-		if (request == PTRACE_SYSCALL) {
-			set_tsk_thread_flag(child, TIF_SYSCALL_TRACE);
-		} else {
-			clear_tsk_thread_flag(child, TIF_SYSCALL_TRACE);
-		}		
-		child->exit_code = data;
-		goto out_wake_notrap;
-
-	case PTRACE_KILL:
-		/*
-		 * make the child exit.  Best I can do is send it a
-		 * sigkill.  perhaps it should be put in the status
-		 * that it wants to exit.
-		 */
-		ret = 0;
-		DBG("sys_ptrace(KILL)\n");
-		if (child->exit_state == EXIT_ZOMBIE)	/* already dead */
-			goto out_tsk;
-		child->exit_code = SIGKILL;
-		goto out_wake_notrap;
-
-	case PTRACE_SINGLEBLOCK:
-		DBG("sys_ptrace(SINGLEBLOCK)\n");
-		ret = -EIO;
-		if (!valid_signal(data))
-			goto out_tsk;
-		clear_tsk_thread_flag(child, TIF_SYSCALL_TRACE);
-		child->ptrace &= ~PT_SINGLESTEP;
-		child->ptrace |= PT_BLOCKSTEP;
-		child->exit_code = data;
-
-		/* Enable taken branch trap. */
-		pa_psw(child)->r = 0;
-		pa_psw(child)->t = 1;
-		pa_psw(child)->h = 0;
-		pa_psw(child)->l = 0;
-		goto out_wake;
-
-	case PTRACE_SINGLESTEP:
-		DBG("sys_ptrace(SINGLESTEP)\n");
-		ret = -EIO;
-		if (!valid_signal(data))
-			goto out_tsk;
-
-		clear_tsk_thread_flag(child, TIF_SYSCALL_TRACE);
-		child->ptrace &= ~PT_BLOCKSTEP;
-		child->ptrace |= PT_SINGLESTEP;
-		child->exit_code = data;
-
-		if (pa_psw(child)->n) {
-			struct siginfo si;
-
-			/* Nullified, just crank over the queue. */
-			task_regs(child)->iaoq[0] = task_regs(child)->iaoq[1];
-			task_regs(child)->iasq[0] = task_regs(child)->iasq[1];
-			task_regs(child)->iaoq[1] = task_regs(child)->iaoq[0] + 4;
-			pa_psw(child)->n = 0;
-			pa_psw(child)->x = 0;
-			pa_psw(child)->y = 0;
-			pa_psw(child)->z = 0;
-			pa_psw(child)->b = 0;
-			ptrace_disable(child);
-			/* Don't wake up the child, but let the
-			   parent know something happened. */
-			si.si_code = TRAP_TRACE;
-			si.si_addr = (void __user *) (task_regs(child)->iaoq[0] & ~3);
-			si.si_signo = SIGTRAP;
-			si.si_errno = 0;
-			force_sig_info(SIGTRAP, &si, child);
-			//notify_parent(child, SIGCHLD);
-			//ret = 0;
-			goto out_wake;
-		}
-
-		/* Enable recovery counter traps.  The recovery counter
-		 * itself will be set to zero on a task switch.  If the
-		 * task is suspended on a syscall then the syscall return
-		 * path will overwrite the recovery counter with a suitable
-		 * value such that it traps once back in user space.  We
-		 * disable interrupts in the childs PSW here also, to avoid
-		 * interrupts while the recovery counter is decrementing.
-		 */
-		pa_psw(child)->r = 1;
-		pa_psw(child)->t = 0;
-		pa_psw(child)->h = 0;
-		pa_psw(child)->l = 0;
-		/* give it a chance to run. */
-		goto out_wake;
-
-	case PTRACE_GETEVENTMSG:
-                ret = put_user(child->ptrace_message, (unsigned int __user *) data);
-		goto out_tsk;
+		break;
 
 	default:
-		ret = ptrace_request(child, request, addr, data);
-		goto out_tsk;
+		ret = compat_ptrace_request(child, request, addr, data);
+		break;
 	}
 
-out_wake_notrap:
-	ptrace_disable(child);
-out_wake:
-	wake_up_process(child);
-	ret = 0;
-out_tsk:
-	DBG("arch_ptrace(%ld, %d, %lx, %lx) returning %ld\n",
-		request, pid, oaddr, odata, ret);
 	return ret;
 }
+#endif
+
 
 void syscall_trace(void)
 {
diff --git a/arch/parisc/kernel/syscall_table.S b/arch/parisc/kernel/syscall_table.S
index 6b5ac38..e719f27 100644
--- a/arch/parisc/kernel/syscall_table.S
+++ b/arch/parisc/kernel/syscall_table.S
@@ -87,7 +87,7 @@
 	ENTRY_SAME(setuid)
 	ENTRY_SAME(getuid)
 	ENTRY_COMP(stime)		/* 25 */
-	ENTRY_SAME(ptrace)
+	ENTRY_COMP(ptrace)
 	ENTRY_SAME(alarm)
 	/* see stat comment */
 	ENTRY_COMP(newfstat)

Re: [PATCH] compat_sys_ptrace conversions for parisc

[Christoph Hellwig]

This should probably get a little review from Rland, I've added him to
the Cc list.  Although I suspect splitting this into at least two
patches for compat_sys_ptrace vs the rest might make sense.

On Sun, Aug 24, 2008 at 08:33:25PM +0200, Helge Deller wrote:
> This patch does the compat_sys_ptrace conversion for parisc.
> In addition it does convert the parisc ptrace code to use the
> architecture-independent ptrace infrastructure instead of own coding.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> 
> diff --git a/arch/parisc/include/asm/ptrace.h b/arch/parisc/include/asm/ptrace.h
> index 3e94c5d..afa5333 100644
> --- a/arch/parisc/include/asm/ptrace.h
> +++ b/arch/parisc/include/asm/ptrace.h
> @@ -47,6 +47,16 @@ struct pt_regs {
>  
>  #define task_regs(task) ((struct pt_regs *) ((char *)(task) + TASK_REGS))
>  
> +#define __ARCH_WANT_COMPAT_SYS_PTRACE
> +
> +struct task_struct;
> +#define arch_has_single_step()	1
> +void user_disable_single_step(struct task_struct *task);
> +void user_enable_single_step(struct task_struct *task);
> +
> +#define arch_has_block_step()	1
> +void user_enable_block_step(struct task_struct *task);
> +
>  /* XXX should we use iaoq[1] or iaoq[0] ? */
>  #define user_mode(regs)			(((regs)->iaoq[0] & 3) ? 1 : 0)
>  #define user_space(regs)		(((regs)->iasq[1] != 0) ? 1 : 0)
> diff --git a/arch/parisc/kernel/ptrace.c b/arch/parisc/kernel/ptrace.c
> index 49c6379..90904f9 100644
> --- a/arch/parisc/kernel/ptrace.c
> +++ b/arch/parisc/kernel/ptrace.c
> @@ -4,6 +4,7 @@
>   * Copyright (C) 2000 Hewlett-Packard Co, Linuxcare Inc.
>   * Copyright (C) 2000 Matthew Wilcox <matthew@wil.cx>
>   * Copyright (C) 2000 David Huggins-Daines <dhd@debian.org>
> + * Copyright (C) 2008 Helge Deller <deller@gmx.de>
>   */
>  
>  #include <linux/kernel.h>
> @@ -27,15 +28,149 @@
>  /* PSW bits we allow the debugger to modify */
>  #define USER_PSW_BITS	(PSW_N | PSW_V | PSW_CB)
>  
> -#undef DEBUG_PTRACE
> +/*
> + * Called by kernel/ptrace.c when detaching..
> + *
> + * Make sure single step bits etc are not set.
> + */
> +void ptrace_disable(struct task_struct *task)
> +{
> +	task->ptrace &= ~(PT_SINGLESTEP|PT_BLOCKSTEP);
>  
> -#ifdef DEBUG_PTRACE
> -#define DBG(x...)	printk(x)
> -#else
> -#define DBG(x...)
> -#endif
> +	/* make sure the trap bits are not set */
> +	pa_psw(task)->r = 0;
> +	pa_psw(task)->t = 0;
> +	pa_psw(task)->h = 0;
> +	pa_psw(task)->l = 0;
> +}
> +
> +/*
> + * The following functions are called by ptrace_resume() when
> + * enabling or disabling single/block tracing.
> + */
> +void user_disable_single_step(struct task_struct *task)
> +{
> +	ptrace_disable(task);
> +}
> +
> +void user_enable_single_step(struct task_struct *task)
> +{
> +	task->ptrace &= ~PT_BLOCKSTEP;
> +	task->ptrace |= PT_SINGLESTEP;
> +
> +	if (pa_psw(task)->n) {
> +		struct siginfo si;
> +
> +		/* Nullified, just crank over the queue. */
> +		task_regs(task)->iaoq[0] = task_regs(task)->iaoq[1];
> +		task_regs(task)->iasq[0] = task_regs(task)->iasq[1];
> +		task_regs(task)->iaoq[1] = task_regs(task)->iaoq[0] + 4;
> +		pa_psw(task)->n = 0;
> +		pa_psw(task)->x = 0;
> +		pa_psw(task)->y = 0;
> +		pa_psw(task)->z = 0;
> +		pa_psw(task)->b = 0;
> +		ptrace_disable(task);
> +		/* Don't wake up the task, but let the
> +		   parent know something happened. */
> +		si.si_code = TRAP_TRACE;
> +		si.si_addr = (void __user *) (task_regs(task)->iaoq[0] & ~3);
> +		si.si_signo = SIGTRAP;
> +		si.si_errno = 0;
> +		force_sig_info(SIGTRAP, &si, task);
> +		/* notify_parent(task, SIGCHLD); */
> +		return;
> +	}
> +
> +	/* Enable recovery counter traps.  The recovery counter
> +	 * itself will be set to zero on a task switch.  If the
> +	 * task is suspended on a syscall then the syscall return
> +	 * path will overwrite the recovery counter with a suitable
> +	 * value such that it traps once back in user space.  We
> +	 * disable interrupts in the tasks PSW here also, to avoid
> +	 * interrupts while the recovery counter is decrementing.
> +	 */
> +	pa_psw(task)->r = 1;
> +	pa_psw(task)->t = 0;
> +	pa_psw(task)->h = 0;
> +	pa_psw(task)->l = 0;
> +}
> +
> +void user_enable_block_step(struct task_struct *task)
> +{
> +	task->ptrace &= ~PT_SINGLESTEP;
> +	task->ptrace |= PT_BLOCKSTEP;
> +
> +	/* Enable taken branch trap. */
> +	pa_psw(task)->r = 0;
> +	pa_psw(task)->t = 1;
> +	pa_psw(task)->h = 0;
> +	pa_psw(task)->l = 0;
> +}
> +
> +long arch_ptrace(struct task_struct *child, long request, long addr, long data)
> +{
> +	unsigned long tmp;
> +	long ret = -EIO;
>  
> -#ifdef CONFIG_64BIT
> +	switch (request) {
> +
> +	/* Read the word at location addr in the USER area.  For ptraced
> +	   processes, the kernel saves all regs on a syscall. */
> +	case PTRACE_PEEKUSR:
> +		if ((addr & (sizeof(long)-1)) ||
> +		    (unsigned long) addr >= sizeof(struct pt_regs))
> +			break;
> +		tmp = *(unsigned long *) ((char *) task_regs(child) + addr);
> +		ret = put_user(tmp, (unsigned long *) data);
> +		break;
> +
> +	/* Write the word at location addr in the USER area.  This will need
> +	   to change when the kernel no longer saves all regs on a syscall.
> +	   FIXME.  There is a problem at the moment in that r3-r18 are only
> +	   saved if the process is ptraced on syscall entry, and even then
> +	   those values are overwritten by actual register values on syscall
> +	   exit. */
> +	case PTRACE_POKEUSR:
> +		/* Some register values written here may be ignored in
> +		 * entry.S:syscall_restore_rfi; e.g. iaoq is written with
> +		 * r31/r31+4, and not with the values in pt_regs.
> +		 */
> +		if (addr == PT_PSW) {
> +			/* Allow writing to Nullify, Divide-step-correction,
> +			 * and carry/borrow bits.
> +			 * BEWARE, if you set N, and then single step, it won't
> +			 * stop on the nullified instruction.
> +			 */
> +			data &= USER_PSW_BITS;
> +			task_regs(child)->gr[0] &= ~USER_PSW_BITS;
> +			task_regs(child)->gr[0] |= data;
> +			ret = 0;
> +			break;
> +		}
> +
> +		if ((addr & (sizeof(long)-1)) ||
> +		    (unsigned long) addr >= sizeof(struct pt_regs))
> +			break;
> +		if ((addr >= PT_GR1 && addr <= PT_GR31) ||
> +				addr == PT_IAOQ0 || addr == PT_IAOQ1 ||
> +				(addr >= PT_FR0 && addr <= PT_FR31 + 4) ||
> +				addr == PT_SAR) {
> +			*(unsigned long *) ((char *) task_regs(child) + addr) = data;
> +			ret = 0;
> +		}
> +		break;
> +
> +	default:
> +		ret = ptrace_request(child, request, addr, data);
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +
> +#ifdef CONFIG_COMPAT
>  
>  /* This function is needed to translate 32 bit pt_regs offsets in to
>   * 64 bit pt_regs offsets.  For example, a 32 bit gdb under a 64 bit kernel
> @@ -61,106 +196,25 @@ static long translate_usr_offset(long offset)
>  	else
>  		return -1;
>  }
> -#endif
>  
> -/*
> - * Called by kernel/ptrace.c when detaching..
> - *
> - * Make sure single step bits etc are not set.
> - */
> -void ptrace_disable(struct task_struct *child)
> +long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
> +			compat_ulong_t addr, compat_ulong_t data)
>  {
> -	/* make sure the trap bits are not set */
> -	pa_psw(child)->r = 0;
> -	pa_psw(child)->t = 0;
> -	pa_psw(child)->h = 0;
> -	pa_psw(child)->l = 0;
> -}
> -
> -long arch_ptrace(struct task_struct *child, long request, long addr, long data)
> -{
> -	long ret;
> -#ifdef DEBUG_PTRACE
> -	long oaddr=addr, odata=data;
> -#endif
> +	compat_uint_t tmp;
> +	long ret = -EIO;
>  
>  	switch (request) {
> -	case PTRACE_PEEKTEXT: /* read word at location addr. */ 
> -	case PTRACE_PEEKDATA: {
> -#ifdef CONFIG_64BIT
> -		if (__is_compat_task(child)) {
> -			int copied;
> -			unsigned int tmp;
> -
> -			addr &= 0xffffffffL;
> -			copied = access_process_vm(child, addr, &tmp, sizeof(tmp), 0);
> -			ret = -EIO;
> -			if (copied != sizeof(tmp))
> -				goto out_tsk;
> -			ret = put_user(tmp,(unsigned int *) data);
> -			DBG("sys_ptrace(PEEK%s, %d, %lx, %lx) returning %ld, data %x\n",
> -				request == PTRACE_PEEKTEXT ? "TEXT" : "DATA",
> -				pid, oaddr, odata, ret, tmp);
> -		}
> -		else
> -#endif
> -			ret = generic_ptrace_peekdata(child, addr, data);
> -		goto out_tsk;
> -	}
>  
> -	/* when I and D space are separate, this will have to be fixed. */
> -	case PTRACE_POKETEXT: /* write the word at location addr. */
> -	case PTRACE_POKEDATA:
> -		ret = 0;
> -#ifdef CONFIG_64BIT
> -		if (__is_compat_task(child)) {
> -			unsigned int tmp = (unsigned int)data;
> -			DBG("sys_ptrace(POKE%s, %d, %lx, %lx)\n",
> -				request == PTRACE_POKETEXT ? "TEXT" : "DATA",
> -				pid, oaddr, odata);
> -			addr &= 0xffffffffL;
> -			if (access_process_vm(child, addr, &tmp, sizeof(tmp), 1) == sizeof(tmp))
> -				goto out_tsk;
> -		}
> -		else
> -#endif
> -		{
> -			if (access_process_vm(child, addr, &data, sizeof(data), 1) == sizeof(data))
> -				goto out_tsk;
> -		}
> -		ret = -EIO;
> -		goto out_tsk;
> -
> -	/* Read the word at location addr in the USER area.  For ptraced
> -	   processes, the kernel saves all regs on a syscall. */
> -	case PTRACE_PEEKUSR: {
> -		ret = -EIO;
> -#ifdef CONFIG_64BIT
> -		if (__is_compat_task(child)) {
> -			unsigned int tmp;
> -
> -			if (addr & (sizeof(int)-1))
> -				goto out_tsk;
> -			if ((addr = translate_usr_offset(addr)) < 0)
> -				goto out_tsk;
> -
> -			tmp = *(unsigned int *) ((char *) task_regs(child) + addr);
> -			ret = put_user(tmp, (unsigned int *) data);
> -			DBG("sys_ptrace(PEEKUSR, %d, %lx, %lx) returning %ld, addr %lx, data %x\n",
> -				pid, oaddr, odata, ret, addr, tmp);
> -		}
> -		else
> -#endif
> -		{
> -			unsigned long tmp;
> +	case PTRACE_PEEKUSR:
> +		if (addr & (sizeof(compat_uint_t)-1))
> +			break;
> +		addr = translate_usr_offset(addr);
> +		if (addr < 0)
> +			break;
>  
> -			if ((addr & (sizeof(long)-1)) || (unsigned long) addr >= sizeof(struct pt_regs))
> -				goto out_tsk;
> -			tmp = *(unsigned long *) ((char *) task_regs(child) + addr);
> -			ret = put_user(tmp, (unsigned long *) data);
> -		}
> -		goto out_tsk;
> -	}
> +		tmp = *(compat_uint_t *) ((char *) task_regs(child) + addr);
> +		ret = put_user(tmp, (compat_uint_t *) (unsigned long) data);
> +		break;
>  
>  	/* Write the word at location addr in the USER area.  This will need
>  	   to change when the kernel no longer saves all regs on a syscall.
> @@ -169,185 +223,46 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
>  	   those values are overwritten by actual register values on syscall
>  	   exit. */
>  	case PTRACE_POKEUSR:
> -		ret = -EIO;
>  		/* Some register values written here may be ignored in
>  		 * entry.S:syscall_restore_rfi; e.g. iaoq is written with
>  		 * r31/r31+4, and not with the values in pt_regs.
>  		 */
> -		 /* PT_PSW=0, so this is valid for 32 bit processes under 64
> -		 * bit kernels.
> -		 */
>  		if (addr == PT_PSW) {
> -			/* PT_PSW=0, so this is valid for 32 bit processes
> -			 * under 64 bit kernels.
> -			 *
> -			 * Allow writing to Nullify, Divide-step-correction,
> -			 * and carry/borrow bits.
> -			 * BEWARE, if you set N, and then single step, it won't
> -			 * stop on the nullified instruction.
> +			/* Since PT_PSW==0, it is valid for 32 bit processes
> +			 * under 64 bit kernels as well.
>  			 */
> -			DBG("sys_ptrace(POKEUSR, %d, %lx, %lx)\n",
> -				pid, oaddr, odata);
> -			data &= USER_PSW_BITS;
> -			task_regs(child)->gr[0] &= ~USER_PSW_BITS;
> -			task_regs(child)->gr[0] |= data;
> -			ret = 0;
> -			goto out_tsk;
> -		}
> -#ifdef CONFIG_64BIT
> -		if (__is_compat_task(child)) {
> -			if (addr & (sizeof(int)-1))
> -				goto out_tsk;
> -			if ((addr = translate_usr_offset(addr)) < 0)
> -				goto out_tsk;
> -			DBG("sys_ptrace(POKEUSR, %d, %lx, %lx) addr %lx\n",
> -				pid, oaddr, odata, addr);
> +			ret = arch_ptrace(child, request, addr, data);
> +		} else {
> +			if (addr & (sizeof(compat_uint_t)-1))
> +				break;
> +			addr = translate_usr_offset(addr);
> +			if (addr < 0)
> +				break;
>  			if (addr >= PT_FR0 && addr <= PT_FR31 + 4) {
>  				/* Special case, fp regs are 64 bits anyway */
> -				*(unsigned int *) ((char *) task_regs(child) + addr) = data;
> +				*(__u64 *) ((char *) task_regs(child) + addr) = data;
>  				ret = 0;
>  			}
>  			else if ((addr >= PT_GR1+4 && addr <= PT_GR31+4) ||
>  					addr == PT_IAOQ0+4 || addr == PT_IAOQ1+4 ||
>  					addr == PT_SAR+4) {
>  				/* Zero the top 32 bits */
> -				*(unsigned int *) ((char *) task_regs(child) + addr - 4) = 0;
> -				*(unsigned int *) ((char *) task_regs(child) + addr) = data;
> +				*(__u32 *) ((char *) task_regs(child) + addr - 4) = 0;
> +				*(__u32 *) ((char *) task_regs(child) + addr) = data;
>  				ret = 0;
>  			}
> -			goto out_tsk;
>  		}
> -		else
> -#endif
> -		{
> -			if ((addr & (sizeof(long)-1)) || (unsigned long) addr >= sizeof(struct pt_regs))
> -				goto out_tsk;
> -			if ((addr >= PT_GR1 && addr <= PT_GR31) ||
> -					addr == PT_IAOQ0 || addr == PT_IAOQ1 ||
> -					(addr >= PT_FR0 && addr <= PT_FR31 + 4) ||
> -					addr == PT_SAR) {
> -				*(unsigned long *) ((char *) task_regs(child) + addr) = data;
> -				ret = 0;
> -			}
> -			goto out_tsk;
> -		}
> -
> -	case PTRACE_SYSCALL: /* continue and stop at next (return from) syscall */
> -	case PTRACE_CONT:
> -		ret = -EIO;
> -		DBG("sys_ptrace(%s)\n",
> -			request == PTRACE_SYSCALL ? "SYSCALL" : "CONT");
> -		if (!valid_signal(data))
> -			goto out_tsk;
> -		child->ptrace &= ~(PT_SINGLESTEP|PT_BLOCKSTEP);
> -		if (request == PTRACE_SYSCALL) {
> -			set_tsk_thread_flag(child, TIF_SYSCALL_TRACE);
> -		} else {
> -			clear_tsk_thread_flag(child, TIF_SYSCALL_TRACE);
> -		}		
> -		child->exit_code = data;
> -		goto out_wake_notrap;
> -
> -	case PTRACE_KILL:
> -		/*
> -		 * make the child exit.  Best I can do is send it a
> -		 * sigkill.  perhaps it should be put in the status
> -		 * that it wants to exit.
> -		 */
> -		ret = 0;
> -		DBG("sys_ptrace(KILL)\n");
> -		if (child->exit_state == EXIT_ZOMBIE)	/* already dead */
> -			goto out_tsk;
> -		child->exit_code = SIGKILL;
> -		goto out_wake_notrap;
> -
> -	case PTRACE_SINGLEBLOCK:
> -		DBG("sys_ptrace(SINGLEBLOCK)\n");
> -		ret = -EIO;
> -		if (!valid_signal(data))
> -			goto out_tsk;
> -		clear_tsk_thread_flag(child, TIF_SYSCALL_TRACE);
> -		child->ptrace &= ~PT_SINGLESTEP;
> -		child->ptrace |= PT_BLOCKSTEP;
> -		child->exit_code = data;
> -
> -		/* Enable taken branch trap. */
> -		pa_psw(child)->r = 0;
> -		pa_psw(child)->t = 1;
> -		pa_psw(child)->h = 0;
> -		pa_psw(child)->l = 0;
> -		goto out_wake;
> -
> -	case PTRACE_SINGLESTEP:
> -		DBG("sys_ptrace(SINGLESTEP)\n");
> -		ret = -EIO;
> -		if (!valid_signal(data))
> -			goto out_tsk;
> -
> -		clear_tsk_thread_flag(child, TIF_SYSCALL_TRACE);
> -		child->ptrace &= ~PT_BLOCKSTEP;
> -		child->ptrace |= PT_SINGLESTEP;
> -		child->exit_code = data;
> -
> -		if (pa_psw(child)->n) {
> -			struct siginfo si;
> -
> -			/* Nullified, just crank over the queue. */
> -			task_regs(child)->iaoq[0] = task_regs(child)->iaoq[1];
> -			task_regs(child)->iasq[0] = task_regs(child)->iasq[1];
> -			task_regs(child)->iaoq[1] = task_regs(child)->iaoq[0] + 4;
> -			pa_psw(child)->n = 0;
> -			pa_psw(child)->x = 0;
> -			pa_psw(child)->y = 0;
> -			pa_psw(child)->z = 0;
> -			pa_psw(child)->b = 0;
> -			ptrace_disable(child);
> -			/* Don't wake up the child, but let the
> -			   parent know something happened. */
> -			si.si_code = TRAP_TRACE;
> -			si.si_addr = (void __user *) (task_regs(child)->iaoq[0] & ~3);
> -			si.si_signo = SIGTRAP;
> -			si.si_errno = 0;
> -			force_sig_info(SIGTRAP, &si, child);
> -			//notify_parent(child, SIGCHLD);
> -			//ret = 0;
> -			goto out_wake;
> -		}
> -
> -		/* Enable recovery counter traps.  The recovery counter
> -		 * itself will be set to zero on a task switch.  If the
> -		 * task is suspended on a syscall then the syscall return
> -		 * path will overwrite the recovery counter with a suitable
> -		 * value such that it traps once back in user space.  We
> -		 * disable interrupts in the childs PSW here also, to avoid
> -		 * interrupts while the recovery counter is decrementing.
> -		 */
> -		pa_psw(child)->r = 1;
> -		pa_psw(child)->t = 0;
> -		pa_psw(child)->h = 0;
> -		pa_psw(child)->l = 0;
> -		/* give it a chance to run. */
> -		goto out_wake;
> -
> -	case PTRACE_GETEVENTMSG:
> -                ret = put_user(child->ptrace_message, (unsigned int __user *) data);
> -		goto out_tsk;
> +		break;
>  
>  	default:
> -		ret = ptrace_request(child, request, addr, data);
> -		goto out_tsk;
> +		ret = compat_ptrace_request(child, request, addr, data);
> +		break;
>  	}
>  
> -out_wake_notrap:
> -	ptrace_disable(child);
> -out_wake:
> -	wake_up_process(child);
> -	ret = 0;
> -out_tsk:
> -	DBG("arch_ptrace(%ld, %d, %lx, %lx) returning %ld\n",
> -		request, pid, oaddr, odata, ret);
>  	return ret;
>  }
> +#endif
> +
>  
>  void syscall_trace(void)
>  {
> diff --git a/arch/parisc/kernel/syscall_table.S b/arch/parisc/kernel/syscall_table.S
> index 6b5ac38..e719f27 100644
> --- a/arch/parisc/kernel/syscall_table.S
> +++ b/arch/parisc/kernel/syscall_table.S
> @@ -87,7 +87,7 @@
>  	ENTRY_SAME(setuid)
>  	ENTRY_SAME(getuid)
>  	ENTRY_COMP(stime)		/* 25 */
> -	ENTRY_SAME(ptrace)
> +	ENTRY_COMP(ptrace)
>  	ENTRY_SAME(alarm)
>  	/* see stat comment */
>  	ENTRY_COMP(newfstat)
---end quoted text---

Re: [PATCH] compat_sys_ptrace conversions for parisc

[Roland McGrath]

> This should probably get a little review from Rland, I've added him to
> the Cc list.  Although I suspect splitting this into at least two
> patches for compat_sys_ptrace vs the rest might make sense.

For the machines where I've done this work, I've certainly been well-served
by breaking it into many smaller incremental patches.  That would make the
meat of each patch far easier to read than this one big patch is.

It seems odd to have user_disable_single_step call ptrace_disable rather
than vice versa.  Otherwise, off hand I don't see anything particularly
suspect in the patch, but I'll admit I didn't look too close because the
one big diff is a pain to read.


Thanks,
Roland

Re: [PATCH] compat_sys_ptrace conversions for parisc

[Kyle McMartin]

On Sun, Aug 24, 2008 at 08:33:25PM +0200, Helge Deller wrote:
> This patch does the compat_sys_ptrace conversion for parisc.
> In addition it does convert the parisc ptrace code to use the
> architecture-independent ptrace infrastructure instead of own coding.
> 

This sounds like it should be two seperate patches, for at least the
reason of making bisecting easier should there be a problem.

Re: [PATCH] compat_sys_ptrace conversions for parisc

[Helge Deller]

Hi Kyle, Hi Christoph,

Kyle McMartin wrote:
> On Sun, Aug 24, 2008 at 08:33:25PM +0200, Helge Deller wrote:
>> This patch does the compat_sys_ptrace conversion for parisc.
>> In addition it does convert the parisc ptrace code to use the
>> architecture-independent ptrace infrastructure instead of own coding.

Both of you said someting like this:

> This sounds like it should be two seperate patches, for at least the
> reason of making bisecting easier should there be a problem.

I agree that my comment suggests that splitting the patch into two parts 
could make sense to track the changes easier, but if you look into the 
original code you'll see, that I would need to duplicate all of the 
arch_ptrace code and then make the copied part 64bit-ready. Then the 
second patch would delete all of this again and replacing it with the 
generic function calls.
IMHO that's a lot of coding and changes without any need. You'll first 
need to understand coding in my first patch which is then deleted 
afterwards with the second patch again. And if you look at the final 
ptrace.c file after applying my patch which I posted here, you'll see 
that arch_ptrace is now really small and simple to understand.

That said, I'm not very motivated to split my patch.

Helge

Re: [PATCH] compat_sys_ptrace conversions for parisc

[Christoph Hellwig]

On Mon, Aug 25, 2008 at 07:49:35PM +0200, Helge Deller wrote:
> I agree that my comment suggests that splitting the patch into two parts 
> could make sense to track the changes easier, but if you look into the 
> original code you'll see, that I would need to duplicate all of the 
> arch_ptrace code and then make the copied part 64bit-ready. Then the 
> second patch would delete all of this again and replacing it with the 
> generic function calls.
> IMHO that's a lot of coding and changes without any need. You'll first 
> need to understand coding in my first patch which is then deleted 
> afterwards with the second patch again. And if you look at the final 
> ptrace.c file after applying my patch which I posted here, you'll see 
> that arch_ptrace is now really small and simple to understand.
> 
> That said, I'm not very motivated to split my patch.

Oh, parisc actually uses sys_ptrace as the 32bit entry point.  Yes, I
see your point and take back the suggestion.

[PATCH] fix crash when trying to unwind user space

[Helge Deller]

Fix kernel to not to try to unwind functions when crash
happens in userspace.
Without this patch kernel may needlessly crash for userspace faults.

Signed-off-by: Kyle McMartin <kyle@mcmartin.ca>
Signed-off-by: Helge Deller <deller@gmx.de>

diff --git a/arch/parisc/kernel/traps.c b/arch/parisc/kernel/traps.c
index 675f1d0..12276c8 100644
--- a/arch/parisc/kernel/traps.c
+++ b/arch/parisc/kernel/traps.c
@@ -121,18 +121,19 @@ static void print_fr(char *level, struct pt_regs *regs)
 
 void show_regs(struct pt_regs *regs)
 {
-	int i;
+	int i, user;
 	char *level;
 	unsigned long cr30, cr31;
 
-	level = user_mode(regs) ? KERN_DEBUG : KERN_CRIT;
+	user = user_mode(regs);
+	level = user ? KERN_DEBUG : KERN_CRIT;
 
 	print_gr(level, regs);
 
 	for (i = 0; i < 8; i += 4)
 		PRINTREGS(level, regs->sr, "sr", RFMT, i);
 
-	if (user_mode(regs))
+	if (user)
 		print_fr(level, regs);
 
 	cr30 = mfctl(30);
@@ -152,7 +153,8 @@ void show_regs(struct pt_regs *regs)
 	printk(level);
 	print_symbol(" RP(r2): %s\n", regs->gr[2]);
 
-	parisc_show_stack(current, NULL, regs);
+	if (!user)
+		parisc_show_stack(current, NULL, regs);
 }
 
 

[PATCH] drop superfluous .align 16

[Helge Deller]

This .align 16 is misplaced and should be in front of the ENTRY(lws_lock_start).
Works because of pure luck since we have a .align PAGE_SIZE before it instead.

Signed-off-by: Helge Deller <deller@gmx.de>

diff --git a/arch/parisc/kernel/syscall.S b/arch/parisc/kernel/syscall.S
index 69b6eeb..c6ff81f 100644
--- a/arch/parisc/kernel/syscall.S
+++ b/arch/parisc/kernel/syscall.S
@@ -640,7 +640,6 @@ END(sys_call_table64)
 	.align	PAGE_SIZE
 ENTRY(lws_lock_start)
 	/* lws locks */
-	.align 16
 	.rept 16
 	/* Keep locks aligned at 16-bytes */
 	.word 1

Re: [PATCH] drop superfluous .align 16

[James Bottomley]

On Sun, 2008-08-24 at 20:51 +0200, Helge Deller wrote:
> This .align 16 is misplaced and should be in front of the ENTRY(lws_lock_start).
> Works because of pure luck since we have a .align PAGE_SIZE before it instead.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> 
> diff --git a/arch/parisc/kernel/syscall.S b/arch/parisc/kernel/syscall.S
> index 69b6eeb..c6ff81f 100644
> --- a/arch/parisc/kernel/syscall.S
> +++ b/arch/parisc/kernel/syscall.S
> @@ -640,7 +640,6 @@ END(sys_call_table64)
>  	.align	PAGE_SIZE
>  ENTRY(lws_lock_start)
>  	/* lws locks */
> -	.align 16
>  	.rept 16
>  	/* Keep locks aligned at 16-bytes */
>  	.word 1

I think I'd really rather keep this.  It may be technically superfluous
because of the .align PAGE_SIZE above it, but that belongs to the
syscall table.  If anyone ever moved this section in head.S, the align
16 ensures nothing goes wrong.

James

Re: [PATCH] drop superfluous .align 16

[Helge Deller]

James Bottomley wrote:
> On Sun, 2008-08-24 at 20:51 +0200, Helge Deller wrote:
>> This .align 16 is misplaced and should be in front of the ENTRY(lws_lock_start).
>> Works because of pure luck since we have a .align PAGE_SIZE before it instead.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>>
>> diff --git a/arch/parisc/kernel/syscall.S b/arch/parisc/kernel/syscall.S
>> index 69b6eeb..c6ff81f 100644
>> --- a/arch/parisc/kernel/syscall.S
>> +++ b/arch/parisc/kernel/syscall.S
>> @@ -640,7 +640,6 @@ END(sys_call_table64)
>>  	.align	PAGE_SIZE
>>  ENTRY(lws_lock_start)
>>  	/* lws locks */
>> -	.align 16
>>  	.rept 16
>>  	/* Keep locks aligned at 16-bytes */
>>  	.word 1
> 
> I think I'd really rather keep this.  It may be technically superfluous
> because of the .align PAGE_SIZE above it, but that belongs to the
> syscall table.  

> If anyone ever moved this section in head.S, the align
> 16 ensures nothing goes wrong.

Hi James,

If the block is moved, then that's exactly what this .align does not 
ensure.
We had this discussion already here on the list. Please read the full 
thread: http://marc.info/?t=121554765000005&r=1&w=2

Helge

Re: [PATCH] drop superfluous .align 16

[Grant Grundler]

On Mon, Aug 25, 2008 at 02:31:40PM +0200, Helge Deller wrote:
> James Bottomley wrote:
>> On Sun, 2008-08-24 at 20:51 +0200, Helge Deller wrote:
>>> This .align 16 is misplaced and should be in front of the 
>>> ENTRY(lws_lock_start).
>>> Works because of pure luck since we have a .align PAGE_SIZE before it 
>>> instead.
>>>
>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>>
>>> diff --git a/arch/parisc/kernel/syscall.S b/arch/parisc/kernel/syscall.S
>>> index 69b6eeb..c6ff81f 100644
>>> --- a/arch/parisc/kernel/syscall.S
>>> +++ b/arch/parisc/kernel/syscall.S
>>> @@ -640,7 +640,6 @@ END(sys_call_table64)
>>>  	.align	PAGE_SIZE
>>>  ENTRY(lws_lock_start)
>>>  	/* lws locks */
>>> -	.align 16
>>>  	.rept 16
>>>  	/* Keep locks aligned at 16-bytes */
>>>  	.word 1
>> I think I'd really rather keep this.  It may be technically superfluous
>> because of the .align PAGE_SIZE above it, but that belongs to the
>> syscall table.  
>
>> If anyone ever moved this section in head.S, the align
>> 16 ensures nothing goes wrong.
>
> Hi James,
>
> If the block is moved, then that's exactly what this .align does not 
> ensure.
> We had this discussion already here on the list. Please read the full 
> thread: http://marc.info/?t=121554765000005&r=1&w=2

Helge,
Thanks for tracking and resubmitting this...Carlos already ACK'd and I think
it would be good if kyle added it to his patch queue.

But I liked two other things proposed in this thread.
1) jejb's preference to document the alignement requirement 
2) jda's proposal for ENTRY_ALIGN() (which accomplishes (1))

So I've hacked your patch to include those two things.

I've only build-tested this for 64-bit builds.

thanks,
grant

Signed-off-by: Helge Deller <deller@gmx.de>
Signed-off-by: Grant Grundler <grundler@parisc-linux.org>


diff --git a/arch/parisc/kernel/syscall.S b/arch/parisc/kernel/syscall.S
index 69b6eeb..faf8b37 100644
--- a/arch/parisc/kernel/syscall.S
+++ b/arch/parisc/kernel/syscall.S
@@ -638,11 +638,10 @@ END(sys_call_table64)
 	*/
 	.section .data
 	.align	PAGE_SIZE
-ENTRY(lws_lock_start)
+
+ENTRY_ALIGN(lws_lock_start,16)
 	/* lws locks */
-	.align 16
 	.rept 16
-	/* Keep locks aligned at 16-bytes */
 	.word 1
 	.word 0 
 	.word 0
diff --git a/include/asm-parisc/linkage.h b/include/asm-parisc/linkage.h
index 0b19a72..2c6cbe2 100644
--- a/include/asm-parisc/linkage.h
+++ b/include/asm-parisc/linkage.h
@@ -17,6 +17,11 @@
 	ALIGN !\
 name:
 
+#define ENTRY_ALIGN(name,alignval) \
+	.export name !\
+	.align alignval !\
+name:
+
 #ifdef CONFIG_64BIT
 #define ENDPROC(name) \
 	END(name)

Re: [PATCH] drop superfluous .align 16

[Carlos O'Donell]

On Tue, Aug 26, 2008 at 12:26 AM, Grant Grundler
<grundler@parisc-linux.org> wrote:
> Thanks for tracking and resubmitting this...Carlos already ACK'd and I think
> it would be good if kyle added it to his patch queue.
>
> But I liked two other things proposed in this thread.
> 1) jejb's preference to document the alignement requirement
> 2) jda's proposal for ENTRY_ALIGN() (which accomplishes (1))
>
> So I've hacked your patch to include those two things.
>
> I've only build-tested this for 64-bit builds.
>
> thanks,
> grant
>
> Signed-off-by: Helge Deller <deller@gmx.de>
> Signed-off-by: Grant Grundler <grundler@parisc-linux.org>
Signed-off-by: Carlos O'Donell <carlos@systemhalted.org>

I agree with both Helge's original patch and this one. Thanks for
making an ENTRY_ALIGN macro. The macro keeps the symbol and alignment
together. Well done.

Cheers,
Carlos.

Re: [PATCH] drop superfluous .align 16

[James Bottomley]

On Tue, 2008-08-26 at 08:42 -0400, Carlos O'Donell wrote:
> On Tue, Aug 26, 2008 at 12:26 AM, Grant Grundler
> <grundler@parisc-linux.org> wrote:
> > Thanks for tracking and resubmitting this...Carlos already ACK'd and I think
> > it would be good if kyle added it to his patch queue.
> >
> > But I liked two other things proposed in this thread.
> > 1) jejb's preference to document the alignement requirement
> > 2) jda's proposal for ENTRY_ALIGN() (which accomplishes (1))
> >
> > So I've hacked your patch to include those two things.
> >
> > I've only build-tested this for 64-bit builds.
> >
> > thanks,
> > grant
> >
> > Signed-off-by: Helge Deller <deller@gmx.de>
> > Signed-off-by: Grant Grundler <grundler@parisc-linux.org>

Can we get this right, please ... we'll get into awful penguin trouble
if he catches us misusing the DCO:

> Signed-off-by: Carlos O'Donell <carlos@systemhalted.org>

That's not signed off by you.  You only get to add a signed-off-by if
the patch physically passes through your hands.  That means that Grant's
above is correct because he modified the patch and resent it, but yours
isn't because you did nothing with it (and you didn't pass it on).

You can add Acked-by: if you like (that usually means I'm the person in
charge of the relevant code and I approved sending it through someone
else's tree) or Reviewed-by: (I actually looked through the patch and it
seems to be fine to me).

James

Re: [PATCH] drop superfluous .align 16

[Carlos O'Donell]

On Tue, Aug 26, 2008 at 10:10 AM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Tue, 2008-08-26 at 08:42 -0400, Carlos O'Donell wrote:
>> On Tue, Aug 26, 2008 at 12:26 AM, Grant Grundler
>> <grundler@parisc-linux.org> wrote:
>> > Thanks for tracking and resubmitting this...Carlos already ACK'd and I think
>> > it would be good if kyle added it to his patch queue.
>> >
>> > But I liked two other things proposed in this thread.
>> > 1) jejb's preference to document the alignement requirement
>> > 2) jda's proposal for ENTRY_ALIGN() (which accomplishes (1))
>> >
>> > So I've hacked your patch to include those two things.
>> >
>> > I've only build-tested this for 64-bit builds.
>> >
>> > thanks,
>> > grant
>> >
>> > Signed-off-by: Helge Deller <deller@gmx.de>
>> > Signed-off-by: Grant Grundler <grundler@parisc-linux.org>
>
> Can we get this right, please ... we'll get into awful penguin trouble
> if he catches us misusing the DCO:
>
>> Signed-off-by: Carlos O'Donell <carlos@systemhalted.org>
>
> That's not signed off by you.  You only get to add a signed-off-by if
> the patch physically passes through your hands.  That means that Grant's
> above is correct because he modified the patch and resent it, but yours
> isn't because you did nothing with it (and you didn't pass it on).
>
> You can add Acked-by: if you like (that usually means I'm the person in
> charge of the relevant code and I approved sending it through someone
> else's tree) or Reviewed-by: (I actually looked through the patch and it
> seems to be fine to me).

Quite right. Sorry for the cut-n-paste.

Since I wrote the code, I consider myself the maintainer.

Acked-by: Carlos O'Donell <carlos@systemhalted.org>

Cheers,
Carlos.

[PATCH] ldcw inline assembler patch

[Helge Deller]

There are two reasons to expose the memory *a in the asm:

1) To prevent the compiler from discarding a preceeding write to *a, and
2) to prevent it from caching *a in a register over the asm.

The change has had a few days testing with a SMP build of 2.6.22.19
running on a rp3440.

This patch is about the correctness of the __ldcw() macro itself. 
The use of the macro should be confined to small inline functions 
to try to limit the effect of clobbering memory on GCC's optimization 
of loads and stores.

Signed-off-by: Dave Anglin <dave.anglin@nrc-cnrc.gc.ca>
Signed-off-by: Helge Deller <deller@gmx.de>

diff --git a/arch/parisc/include/asm/system.h b/arch/parisc/include/asm/system.h
index ee80c92..d91357b 100644
--- a/arch/parisc/include/asm/system.h
+++ b/arch/parisc/include/asm/system.h
@@ -168,8 +168,8 @@ static inline void set_eiem(unsigned long val)
 /* LDCW, the only atomic read-write operation PA-RISC has. *sigh*.  */
 #define __ldcw(a) ({						\
 	unsigned __ret;						\
-	__asm__ __volatile__(__LDCW " 0(%1),%0"			\
-		: "=r" (__ret) : "r" (a));			\
+	__asm__ __volatile__(__LDCW " 0(%2),%0"			\
+		: "=r" (__ret), "+m" (*(a)) : "r" (a));		\
 	__ret;							\
 })
 
	

[PATCH] Change cpu_data[] and cpu_devices[] from array sized by NR_CPUS to per_cpu variables

[Helge Deller]

!! Probably needs some testing on a SMP box before it can be applied !!

Signed-off-by: Helge Deller <deller@gmx.de>

diff --git a/arch/parisc/include/asm/processor.h b/arch/parisc/include/asm/processor.h
index 3c9d348..9d64df8 100644
--- a/arch/parisc/include/asm/processor.h
+++ b/arch/parisc/include/asm/processor.h
@@ -17,6 +17,7 @@
 #include <asm/ptrace.h>
 #include <asm/types.h>
 #include <asm/system.h>
+#include <asm/percpu.h>
 #endif /* __ASSEMBLY__ */
 
 #define KERNEL_STACK_SIZE 	(4*PAGE_SIZE)
@@ -109,8 +110,7 @@ struct cpuinfo_parisc {
 };
 
 extern struct system_cpuinfo_parisc boot_cpu_data;
-extern struct cpuinfo_parisc cpu_data[NR_CPUS];
-#define current_cpu_data cpu_data[smp_processor_id()]
+DECLARE_PER_CPU(struct cpuinfo_parisc, cpu_data);
 
 #define CPU_HVERSION ((boot_cpu_data.hversion >> 4) & 0x0FFF)
 
diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c
index 23ef950..3ee6b33 100644
--- a/arch/parisc/kernel/irq.c
+++ b/arch/parisc/kernel/irq.c
@@ -298,7 +298,7 @@ unsigned long txn_affinity_addr(unsigned int irq, int cpu)
 	irq_desc[irq].affinity = cpumask_of_cpu(cpu);
 #endif
 
-	return cpu_data[cpu].txn_addr;
+	return per_cpu(cpu_data, cpu).txn_addr;
 }
 
 
@@ -309,8 +309,9 @@ unsigned long txn_alloc_addr(unsigned int virt_irq)
 	next_cpu++; /* assign to "next" CPU we want this bugger on */
 
 	/* validate entry */
-	while ((next_cpu < NR_CPUS) && (!cpu_data[next_cpu].txn_addr || 
-		!cpu_online(next_cpu)))
+	while ((next_cpu < NR_CPUS) &&
+		(!per_cpu(cpu_data, next_cpu).txn_addr ||
+		 !cpu_online(next_cpu)))
 		next_cpu++;
 
 	if (next_cpu >= NR_CPUS) 
@@ -359,7 +360,7 @@ void do_cpu_irq_mask(struct pt_regs *regs)
 		printk(KERN_DEBUG "redirecting irq %d from CPU %d to %d\n",
 		       irq, smp_processor_id(), cpu);
 		gsc_writel(irq + CPU_IRQ_BASE,
-			   cpu_data[cpu].hpa);
+			   per_cpu(cpu_data, cpu).hpa);
 		goto set_out;
 	}
 #endif
@@ -421,5 +422,5 @@ void __init init_IRQ(void)
 
 void ack_bad_irq(unsigned int irq)
 {
-	printk("unexpected IRQ %d\n", irq);
+	printk(KERN_WARNING "unexpected IRQ %d\n", irq);
 }
diff --git a/arch/parisc/kernel/processor.c b/arch/parisc/kernel/processor.c
index 370086f..678a031 100644
--- a/arch/parisc/kernel/processor.c
+++ b/arch/parisc/kernel/processor.c
@@ -3,7 +3,7 @@
  *    Initial setup-routines for HP 9000 based hardware.
  *
  *    Copyright (C) 1991, 1992, 1995  Linus Torvalds
- *    Modifications for PA-RISC (C) 1999 Helge Deller <deller@gmx.de>
+ *    Modifications for PA-RISC (C) 1999-2008 Helge Deller <deller@gmx.de>
  *    Modifications copyright 1999 SuSE GmbH (Philipp Rumpf)
  *    Modifications copyright 2000 Martin K. Petersen <mkp@mkp.net>
  *    Modifications copyright 2000 Philipp Rumpf <prumpf@tux.org>
@@ -46,7 +46,7 @@
 struct system_cpuinfo_parisc boot_cpu_data __read_mostly;
 EXPORT_SYMBOL(boot_cpu_data);
 
-struct cpuinfo_parisc cpu_data[NR_CPUS] __read_mostly;
+DEFINE_PER_CPU(struct cpuinfo_parisc, cpu_data);
 
 extern int update_cr16_clocksource(void);	/* from time.c */
 
@@ -69,6 +69,23 @@ extern int update_cr16_clocksource(void);	/* from time.c */
 */
 
 /**
+ * init_cpu_profiler - enable/setup per cpu profiling hooks.
+ * @cpunum: The processor instance.
+ *
+ * FIXME: doesn't do much yet...
+ */
+static void __cpuinit
+init_percpu_prof(unsigned long cpunum)
+{
+	struct cpuinfo_parisc *p;
+
+	p = &per_cpu(cpu_data, cpunum);
+	p->prof_counter = 1;
+	p->prof_multiplier = 1;
+}
+
+
+/**
  * processor_probe - Determine if processor driver should claim this device.
  * @dev: The device which has been found.
  *
@@ -147,7 +164,7 @@ static int __cpuinit processor_probe(struct parisc_device *dev)
 	}
 #endif
 
-	p = &cpu_data[cpuid];
+	p = &per_cpu(cpu_data, cpuid);
 	boot_cpu_data.cpu_count++;
 
 	/* initialize counters - CPU 0 gets it_value set in time_init() */
@@ -162,12 +179,9 @@ static int __cpuinit processor_probe(struct parisc_device *dev)
 #ifdef CONFIG_SMP
 	/*
 	** FIXME: review if any other initialization is clobbered
-	**	for boot_cpu by the above memset().
+	**	  for boot_cpu by the above memset().
 	*/
-
-	/* stolen from init_percpu_prof() */
-	cpu_data[cpuid].prof_counter = 1;
-	cpu_data[cpuid].prof_multiplier = 1;
+	init_percpu_prof(cpuid);
 #endif
 
 	/*
@@ -261,19 +275,6 @@ void __init collect_boot_cpu_data(void)
 }
 
 
-/**
- * init_cpu_profiler - enable/setup per cpu profiling hooks.
- * @cpunum: The processor instance.
- *
- * FIXME: doesn't do much yet...
- */
-static inline void __init
-init_percpu_prof(int cpunum)
-{
-	cpu_data[cpunum].prof_counter = 1;
-	cpu_data[cpunum].prof_multiplier = 1;
-}
-
 
 /**
  * init_per_cpu - Handle individual processor initializations.
@@ -293,7 +294,7 @@ init_percpu_prof(int cpunum)
  *
  * o Enable CPU profiling hooks.
  */
-int __init init_per_cpu(int cpunum)
+int __cpuinit init_per_cpu(int cpunum)
 {
 	int ret;
 	struct pdc_coproc_cfg coproc_cfg;
@@ -307,8 +308,8 @@ int __init init_per_cpu(int cpunum)
 		/* FWIW, FP rev/model is a more accurate way to determine
 		** CPU type. CPU rev/model has some ambiguous cases.
 		*/
-		cpu_data[cpunum].fp_rev = coproc_cfg.revision;
-		cpu_data[cpunum].fp_model = coproc_cfg.model;
+		per_cpu(cpu_data, cpunum).fp_rev = coproc_cfg.revision;
+		per_cpu(cpu_data, cpunum).fp_model = coproc_cfg.model;
 
 		printk(KERN_INFO  "FP[%d] enabled: Rev %ld Model %ld\n",
 			cpunum, coproc_cfg.revision, coproc_cfg.model);
@@ -344,16 +345,17 @@ int __init init_per_cpu(int cpunum)
 int
 show_cpuinfo (struct seq_file *m, void *v)
 {
-	int	n;
+	unsigned long cpu;
 
-	for(n=0; n<boot_cpu_data.cpu_count; n++) {
+	for_each_online_cpu(cpu) {
+		const struct cpuinfo_parisc *cpuinfo = &per_cpu(cpu_data, cpu);
 #ifdef CONFIG_SMP
-		if (0 == cpu_data[n].hpa)
+		if (0 == cpuinfo->hpa)
 			continue;
 #endif
-		seq_printf(m, "processor\t: %d\n"
+		seq_printf(m, "processor\t: %lu\n"
 				"cpu family\t: PA-RISC %s\n",
-				 n, boot_cpu_data.family_name);
+				 cpu, boot_cpu_data.family_name);
 
 		seq_printf(m, "cpu\t\t: %s\n",  boot_cpu_data.cpu_name );
 
@@ -365,8 +367,8 @@ show_cpuinfo (struct seq_file *m, void *v)
 		seq_printf(m, "model\t\t: %s\n"
 				"model name\t: %s\n",
 				 boot_cpu_data.pdc.sys_model_name,
-				 cpu_data[n].dev ? 
-				 cpu_data[n].dev->name : "Unknown" );
+				 cpuinfo->dev ?
+				 cpuinfo->dev->name : "Unknown" );
 
 		seq_printf(m, "hversion\t: 0x%08x\n"
 			        "sversion\t: 0x%08x\n",
@@ -377,8 +379,8 @@ show_cpuinfo (struct seq_file *m, void *v)
 		show_cache_info(m);
 
 		seq_printf(m, "bogomips\t: %lu.%02lu\n",
-			     cpu_data[n].loops_per_jiffy / (500000 / HZ),
-			     (cpu_data[n].loops_per_jiffy / (5000 / HZ)) % 100);
+			     cpuinfo->loops_per_jiffy / (500000 / HZ),
+			     (cpuinfo->loops_per_jiffy / (5000 / HZ)) % 100);
 
 		seq_printf(m, "software id\t: %ld\n\n",
 				boot_cpu_data.pdc.model.sw_id);
diff --git a/arch/parisc/kernel/setup.c b/arch/parisc/kernel/setup.c
index a59b71e..eec84df 100644
--- a/arch/parisc/kernel/setup.c
+++ b/arch/parisc/kernel/setup.c
@@ -319,7 +319,7 @@ static int __init parisc_init(void)
 	
 	processor_init();
 	printk(KERN_INFO "CPU(s): %d x %s at %d.%06d MHz\n",
-			boot_cpu_data.cpu_count,
+			num_present_cpus(),
 			boot_cpu_data.cpu_name,
 			boot_cpu_data.cpu_hz / 1000000,
 			boot_cpu_data.cpu_hz % 1000000	);
diff --git a/arch/parisc/kernel/smp.c b/arch/parisc/kernel/smp.c
index d47f397..148667e 100644
--- a/arch/parisc/kernel/smp.c
+++ b/arch/parisc/kernel/smp.c
@@ -56,16 +56,17 @@ static int smp_debug_lvl = 0;
 		if (lvl >= smp_debug_lvl)	\
 			printk(printargs);
 #else
-#define smp_debug(lvl, ...)
+#define smp_debug(lvl, ...)	do { } while(0)
 #endif /* DEBUG_SMP */
 
 DEFINE_SPINLOCK(smp_lock);
 
 volatile struct task_struct *smp_init_current_idle_task;
 
-static volatile int cpu_now_booting __read_mostly = 0;	/* track which CPU is booting */
+/* track which CPU is booting */
+static volatile int cpu_now_booting __cpuinitdata;
 
-static int parisc_max_cpus __read_mostly = 1;
+static int parisc_max_cpus __cpuinitdata = 1;
 
 /* online cpus are ones that we've managed to bring up completely
  * possible cpus are all valid cpu 
@@ -138,7 +139,7 @@ irqreturn_t
 ipi_interrupt(int irq, void *dev_id) 
 {
 	int this_cpu = smp_processor_id();
-	struct cpuinfo_parisc *p = &cpu_data[this_cpu];
+	struct cpuinfo_parisc *p = &per_cpu(cpu_data, this_cpu);
 	unsigned long ops;
 	unsigned long flags;
 
@@ -217,13 +218,13 @@ ipi_interrupt(int irq, void *dev_id)
 static inline void
 ipi_send(int cpu, enum ipi_message_type op)
 {
-	struct cpuinfo_parisc *p = &cpu_data[cpu];
+	struct cpuinfo_parisc *p = &per_cpu(cpu_data, cpu);
 	spinlock_t *lock = &per_cpu(ipi_lock, cpu);
 	unsigned long flags;
 
 	spin_lock_irqsave(lock, flags);
 	p->pending_ipi |= 1 << op;
-	gsc_writel(IPI_IRQ - CPU_IRQ_BASE, cpu_data[cpu].hpa);
+	gsc_writel(IPI_IRQ - CPU_IRQ_BASE, p->hpa);
 	spin_unlock_irqrestore(lock, flags);
 }
 
@@ -239,10 +240,7 @@ send_IPI_mask(cpumask_t mask, enum ipi_message_type op)
 static inline void
 send_IPI_single(int dest_cpu, enum ipi_message_type op)
 {
-	if (dest_cpu == NO_PROC_ID) {
-		BUG();
-		return;
-	}
+	BUG_ON(dest_cpu == NO_PROC_ID);
 
 	ipi_send(dest_cpu, op);
 }
@@ -324,8 +322,7 @@ smp_cpu_init(int cpunum)
 	/* Initialise the idle task for this CPU */
 	atomic_inc(&init_mm.mm_count);
 	current->active_mm = &init_mm;
-	if(current->mm)
-		BUG();
+	BUG_ON(current->mm);
 	enter_lazy_tlb(&init_mm, current);
 
 	init_IRQ();   /* make sure no IRQs are enabled or pending */
@@ -360,6 +357,7 @@ void __init smp_callin(void)
  */
 int __cpuinit smp_boot_one_cpu(int cpuid)
 {
+	const struct cpuinfo_parisc *p = &per_cpu(cpu_data, cpuid);
 	struct task_struct *idle;
 	long timeout;
 
@@ -391,7 +389,7 @@ int __cpuinit smp_boot_one_cpu(int cpuid)
 	smp_init_current_idle_task = idle ;
 	mb();
 
-	printk("Releasing cpu %d now, hpa=%lx\n", cpuid, cpu_data[cpuid].hpa);
+	printk(KERN_INFO "Releasing cpu %d now, hpa=%lx\n", cpuid, p->hpa);
 
 	/*
 	** This gets PDC to release the CPU from a very tight loop.
@@ -402,7 +400,7 @@ int __cpuinit smp_boot_one_cpu(int cpuid)
 	** EIR{0}). MEM_RENDEZ is valid only when it is nonzero and the 
 	** contents of memory are valid."
 	*/
-	gsc_writel(TIMER_IRQ - CPU_IRQ_BASE, cpu_data[cpuid].hpa);
+	gsc_writel(TIMER_IRQ - CPU_IRQ_BASE, p->hpa);
 	mb();
 
 	/* 
@@ -434,12 +432,12 @@ alive:
 	return 0;
 }
 
-void __devinit smp_prepare_boot_cpu(void)
+void __init smp_prepare_boot_cpu(void)
 {
-	int bootstrap_processor=cpu_data[0].cpuid;	/* CPU ID of BSP */
+	int bootstrap_processor = per_cpu(cpu_data, 0).cpuid;
 
 	/* Setup BSP mappings */
-	printk("SMP: bootstrap CPU ID is %d\n",bootstrap_processor);
+	printk(KERN_INFO "SMP: bootstrap CPU ID is %d\n", bootstrap_processor);
 
 	cpu_set(bootstrap_processor, cpu_online_map);
 	cpu_set(bootstrap_processor, cpu_present_map);
diff --git a/arch/parisc/kernel/time.c b/arch/parisc/kernel/time.c
index 24be86b..9db61b4 100644
--- a/arch/parisc/kernel/time.c
+++ b/arch/parisc/kernel/time.c
@@ -59,7 +59,7 @@ irqreturn_t timer_interrupt(int irq, void *dev_id)
 	unsigned long cycles_elapsed, ticks_elapsed;
 	unsigned long cycles_remainder;
 	unsigned int cpu = smp_processor_id();
-	struct cpuinfo_parisc *cpuinfo = &cpu_data[cpu];
+	struct cpuinfo_parisc *cpuinfo = &per_cpu(cpu_data, cpu);
 
 	/* gcc can optimize for "read-only" case with a local clocktick */
 	unsigned long cpt = clocktick;
@@ -212,7 +212,7 @@ void __init start_cpu_itimer(void)
 
 	mtctl(next_tick, 16);		/* kick off Interval Timer (CR16) */
 
-	cpu_data[cpu].it_value = next_tick;
+	per_cpu(cpu_data, cpu).it_value = next_tick;
 }
 
 void __init time_init(void)
diff --git a/arch/parisc/kernel/topology.c b/arch/parisc/kernel/topology.c
index d71cb01..f515938 100644
--- a/arch/parisc/kernel/topology.c
+++ b/arch/parisc/kernel/topology.c
@@ -22,14 +22,14 @@
 #include <linux/cpu.h>
 #include <linux/cache.h>
 
-static struct cpu cpu_devices[NR_CPUS] __read_mostly;
+static DEFINE_PER_CPU(struct cpu, cpu_devices);
 
 static int __init topology_init(void)
 {
 	int num;
 
 	for_each_present_cpu(num) {
-		register_cpu(&cpu_devices[num], num);
+		register_cpu(&per_cpu(cpu_devices, num), num);
 	}
 	return 0;
 }

Re: [PATCH] Change cpu_data[] and cpu_devices[] from array sized by NR_CPUS to per_cpu variables

[Helge Deller]

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

Updated patch attached, which fixes some build problems and section 
mismatches.
Boot-tested on a UP 32bit machine with CONFIG_SMP=y.

Signed-off-by: Helge Deller <deller@gmx.de>

[-- Attachment #2: parisc-per-cpu3.patch --]
[-- Type: text/x-patch, Size: 14161 bytes --]

diff --git a/arch/parisc/include/asm/processor.h b/arch/parisc/include/asm/processor.h
index 3c9d348..9d64df8 100644
--- a/arch/parisc/include/asm/processor.h
+++ b/arch/parisc/include/asm/processor.h
@@ -17,6 +17,7 @@
 #include <asm/ptrace.h>
 #include <asm/types.h>
 #include <asm/system.h>
+#include <asm/percpu.h>
 #endif /* __ASSEMBLY__ */
 
 #define KERNEL_STACK_SIZE 	(4*PAGE_SIZE)
@@ -109,8 +110,7 @@ struct cpuinfo_parisc {
 };
 
 extern struct system_cpuinfo_parisc boot_cpu_data;
-extern struct cpuinfo_parisc cpu_data[NR_CPUS];
-#define current_cpu_data cpu_data[smp_processor_id()]
+DECLARE_PER_CPU(struct cpuinfo_parisc, cpu_data);
 
 #define CPU_HVERSION ((boot_cpu_data.hversion >> 4) & 0x0FFF)
 
diff --git a/arch/parisc/kernel/firmware.c b/arch/parisc/kernel/firmware.c
index 03f26bd..f6d2412 100644
--- a/arch/parisc/kernel/firmware.c
+++ b/arch/parisc/kernel/firmware.c
@@ -151,7 +151,7 @@ static void convert_to_wide(unsigned long *addr)
 }
 
 #ifdef CONFIG_64BIT
-void __init set_firmware_width_unlocked(void)
+void __cpuinit set_firmware_width_unlocked(void)
 {
 	int ret;
 
@@ -168,7 +168,7 @@ void __init set_firmware_width_unlocked(void)
  * This function must be called before any pdc_* function that uses the
  * convert_to_wide function.
  */
-void __init set_firmware_width(void)
+void __cpuinit set_firmware_width(void)
 {
 	unsigned long flags;
 	spin_lock_irqsave(&pdc_lock, flags);
@@ -176,11 +176,11 @@ void __init set_firmware_width(void)
 	spin_unlock_irqrestore(&pdc_lock, flags);
 }
 #else
-void __init set_firmware_width_unlocked(void) {
+void __cpuinit set_firmware_width_unlocked(void) {
 	return;
 }
 
-void __init set_firmware_width(void) {
+void __cpuinit set_firmware_width(void) {
 	return;
 }
 #endif /*CONFIG_64BIT*/
@@ -302,7 +302,7 @@ int pdc_chassis_warn(unsigned long *warn)
 	return retval;
 }
 
-int __init pdc_coproc_cfg_unlocked(struct pdc_coproc_cfg *pdc_coproc_info)
+int __cpuinit pdc_coproc_cfg_unlocked(struct pdc_coproc_cfg *pdc_coproc_info)
 {
 	int ret;
 
@@ -323,7 +323,7 @@ int __init pdc_coproc_cfg_unlocked(struct pdc_coproc_cfg *pdc_coproc_info)
  * This PDC call returns the presence and status of all the coprocessors
  * attached to the processor.
  */
-int __init pdc_coproc_cfg(struct pdc_coproc_cfg *pdc_coproc_info)
+int __cpuinit pdc_coproc_cfg(struct pdc_coproc_cfg *pdc_coproc_info)
 {
 	int ret;
 	unsigned long flags;
diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c
index 23ef950..3ee6b33 100644
--- a/arch/parisc/kernel/irq.c
+++ b/arch/parisc/kernel/irq.c
@@ -298,7 +298,7 @@ unsigned long txn_affinity_addr(unsigned int irq, int cpu)
 	irq_desc[irq].affinity = cpumask_of_cpu(cpu);
 #endif
 
-	return cpu_data[cpu].txn_addr;
+	return per_cpu(cpu_data, cpu).txn_addr;
 }
 
 
@@ -309,8 +309,9 @@ unsigned long txn_alloc_addr(unsigned int virt_irq)
 	next_cpu++; /* assign to "next" CPU we want this bugger on */
 
 	/* validate entry */
-	while ((next_cpu < NR_CPUS) && (!cpu_data[next_cpu].txn_addr || 
-		!cpu_online(next_cpu)))
+	while ((next_cpu < NR_CPUS) &&
+		(!per_cpu(cpu_data, next_cpu).txn_addr ||
+		 !cpu_online(next_cpu)))
 		next_cpu++;
 
 	if (next_cpu >= NR_CPUS) 
@@ -359,7 +360,7 @@ void do_cpu_irq_mask(struct pt_regs *regs)
 		printk(KERN_DEBUG "redirecting irq %d from CPU %d to %d\n",
 		       irq, smp_processor_id(), cpu);
 		gsc_writel(irq + CPU_IRQ_BASE,
-			   cpu_data[cpu].hpa);
+			   per_cpu(cpu_data, cpu).hpa);
 		goto set_out;
 	}
 #endif
@@ -421,5 +422,5 @@ void __init init_IRQ(void)
 
 void ack_bad_irq(unsigned int irq)
 {
-	printk("unexpected IRQ %d\n", irq);
+	printk(KERN_WARNING "unexpected IRQ %d\n", irq);
 }
diff --git a/arch/parisc/kernel/processor.c b/arch/parisc/kernel/processor.c
index 370086f..678a031 100644
--- a/arch/parisc/kernel/processor.c
+++ b/arch/parisc/kernel/processor.c
@@ -3,7 +3,7 @@
  *    Initial setup-routines for HP 9000 based hardware.
  *
  *    Copyright (C) 1991, 1992, 1995  Linus Torvalds
- *    Modifications for PA-RISC (C) 1999 Helge Deller <deller@gmx.de>
+ *    Modifications for PA-RISC (C) 1999-2008 Helge Deller <deller@gmx.de>
  *    Modifications copyright 1999 SuSE GmbH (Philipp Rumpf)
  *    Modifications copyright 2000 Martin K. Petersen <mkp@mkp.net>
  *    Modifications copyright 2000 Philipp Rumpf <prumpf@tux.org>
@@ -46,7 +46,7 @@
 struct system_cpuinfo_parisc boot_cpu_data __read_mostly;
 EXPORT_SYMBOL(boot_cpu_data);
 
-struct cpuinfo_parisc cpu_data[NR_CPUS] __read_mostly;
+DEFINE_PER_CPU(struct cpuinfo_parisc, cpu_data);
 
 extern int update_cr16_clocksource(void);	/* from time.c */
 
@@ -69,6 +69,23 @@ extern int update_cr16_clocksource(void);	/* from time.c */
 */
 
 /**
+ * init_cpu_profiler - enable/setup per cpu profiling hooks.
+ * @cpunum: The processor instance.
+ *
+ * FIXME: doesn't do much yet...
+ */
+static void __cpuinit
+init_percpu_prof(unsigned long cpunum)
+{
+	struct cpuinfo_parisc *p;
+
+	p = &per_cpu(cpu_data, cpunum);
+	p->prof_counter = 1;
+	p->prof_multiplier = 1;
+}
+
+
+/**
  * processor_probe - Determine if processor driver should claim this device.
  * @dev: The device which has been found.
  *
@@ -147,7 +164,7 @@ static int __cpuinit processor_probe(struct parisc_device *dev)
 	}
 #endif
 
-	p = &cpu_data[cpuid];
+	p = &per_cpu(cpu_data, cpuid);
 	boot_cpu_data.cpu_count++;
 
 	/* initialize counters - CPU 0 gets it_value set in time_init() */
@@ -162,12 +179,9 @@ static int __cpuinit processor_probe(struct parisc_device *dev)
 #ifdef CONFIG_SMP
 	/*
 	** FIXME: review if any other initialization is clobbered
-	**	for boot_cpu by the above memset().
+	**	  for boot_cpu by the above memset().
 	*/
-
-	/* stolen from init_percpu_prof() */
-	cpu_data[cpuid].prof_counter = 1;
-	cpu_data[cpuid].prof_multiplier = 1;
+	init_percpu_prof(cpuid);
 #endif
 
 	/*
@@ -261,19 +275,6 @@ void __init collect_boot_cpu_data(void)
 }
 
 
-/**
- * init_cpu_profiler - enable/setup per cpu profiling hooks.
- * @cpunum: The processor instance.
- *
- * FIXME: doesn't do much yet...
- */
-static inline void __init
-init_percpu_prof(int cpunum)
-{
-	cpu_data[cpunum].prof_counter = 1;
-	cpu_data[cpunum].prof_multiplier = 1;
-}
-
 
 /**
  * init_per_cpu - Handle individual processor initializations.
@@ -293,7 +294,7 @@ init_percpu_prof(int cpunum)
  *
  * o Enable CPU profiling hooks.
  */
-int __init init_per_cpu(int cpunum)
+int __cpuinit init_per_cpu(int cpunum)
 {
 	int ret;
 	struct pdc_coproc_cfg coproc_cfg;
@@ -307,8 +308,8 @@ int __init init_per_cpu(int cpunum)
 		/* FWIW, FP rev/model is a more accurate way to determine
 		** CPU type. CPU rev/model has some ambiguous cases.
 		*/
-		cpu_data[cpunum].fp_rev = coproc_cfg.revision;
-		cpu_data[cpunum].fp_model = coproc_cfg.model;
+		per_cpu(cpu_data, cpunum).fp_rev = coproc_cfg.revision;
+		per_cpu(cpu_data, cpunum).fp_model = coproc_cfg.model;
 
 		printk(KERN_INFO  "FP[%d] enabled: Rev %ld Model %ld\n",
 			cpunum, coproc_cfg.revision, coproc_cfg.model);
@@ -344,16 +345,17 @@ int __init init_per_cpu(int cpunum)
 int
 show_cpuinfo (struct seq_file *m, void *v)
 {
-	int	n;
+	unsigned long cpu;
 
-	for(n=0; n<boot_cpu_data.cpu_count; n++) {
+	for_each_online_cpu(cpu) {
+		const struct cpuinfo_parisc *cpuinfo = &per_cpu(cpu_data, cpu);
 #ifdef CONFIG_SMP
-		if (0 == cpu_data[n].hpa)
+		if (0 == cpuinfo->hpa)
 			continue;
 #endif
-		seq_printf(m, "processor\t: %d\n"
+		seq_printf(m, "processor\t: %lu\n"
 				"cpu family\t: PA-RISC %s\n",
-				 n, boot_cpu_data.family_name);
+				 cpu, boot_cpu_data.family_name);
 
 		seq_printf(m, "cpu\t\t: %s\n",  boot_cpu_data.cpu_name );
 
@@ -365,8 +367,8 @@ show_cpuinfo (struct seq_file *m, void *v)
 		seq_printf(m, "model\t\t: %s\n"
 				"model name\t: %s\n",
 				 boot_cpu_data.pdc.sys_model_name,
-				 cpu_data[n].dev ? 
-				 cpu_data[n].dev->name : "Unknown" );
+				 cpuinfo->dev ?
+				 cpuinfo->dev->name : "Unknown" );
 
 		seq_printf(m, "hversion\t: 0x%08x\n"
 			        "sversion\t: 0x%08x\n",
@@ -377,8 +379,8 @@ show_cpuinfo (struct seq_file *m, void *v)
 		show_cache_info(m);
 
 		seq_printf(m, "bogomips\t: %lu.%02lu\n",
-			     cpu_data[n].loops_per_jiffy / (500000 / HZ),
-			     (cpu_data[n].loops_per_jiffy / (5000 / HZ)) % 100);
+			     cpuinfo->loops_per_jiffy / (500000 / HZ),
+			     (cpuinfo->loops_per_jiffy / (5000 / HZ)) % 100);
 
 		seq_printf(m, "software id\t: %ld\n\n",
 				boot_cpu_data.pdc.model.sw_id);
diff --git a/arch/parisc/kernel/setup.c b/arch/parisc/kernel/setup.c
index a59b71e..3965fd4 100644
--- a/arch/parisc/kernel/setup.c
+++ b/arch/parisc/kernel/setup.c
@@ -319,7 +319,7 @@ static int __init parisc_init(void)
 	
 	processor_init();
 	printk(KERN_INFO "CPU(s): %d x %s at %d.%06d MHz\n",
-			boot_cpu_data.cpu_count,
+			num_present_cpus(),
 			boot_cpu_data.cpu_name,
 			boot_cpu_data.cpu_hz / 1000000,
 			boot_cpu_data.cpu_hz % 1000000	);
@@ -385,8 +385,8 @@ void start_parisc(void)
 	if (ret >= 0 && coproc_cfg.ccr_functional) {
 		mtctl(coproc_cfg.ccr_functional, 10);
 
-		cpu_data[cpunum].fp_rev = coproc_cfg.revision;
-		cpu_data[cpunum].fp_model = coproc_cfg.model;
+		per_cpu(cpu_data, cpunum).fp_rev = coproc_cfg.revision;
+		per_cpu(cpu_data, cpunum).fp_model = coproc_cfg.model;
 
 		asm volatile ("fstd	%fr0,8(%sp)");
 	} else {
diff --git a/arch/parisc/kernel/smp.c b/arch/parisc/kernel/smp.c
index d47f397..148667e 100644
--- a/arch/parisc/kernel/smp.c
+++ b/arch/parisc/kernel/smp.c
@@ -56,16 +56,17 @@ static int smp_debug_lvl = 0;
 		if (lvl >= smp_debug_lvl)	\
 			printk(printargs);
 #else
-#define smp_debug(lvl, ...)
+#define smp_debug(lvl, ...)	do { } while(0)
 #endif /* DEBUG_SMP */
 
 DEFINE_SPINLOCK(smp_lock);
 
 volatile struct task_struct *smp_init_current_idle_task;
 
-static volatile int cpu_now_booting __read_mostly = 0;	/* track which CPU is booting */
+/* track which CPU is booting */
+static volatile int cpu_now_booting __cpuinitdata;
 
-static int parisc_max_cpus __read_mostly = 1;
+static int parisc_max_cpus __cpuinitdata = 1;
 
 /* online cpus are ones that we've managed to bring up completely
  * possible cpus are all valid cpu 
@@ -138,7 +139,7 @@ irqreturn_t
 ipi_interrupt(int irq, void *dev_id) 
 {
 	int this_cpu = smp_processor_id();
-	struct cpuinfo_parisc *p = &cpu_data[this_cpu];
+	struct cpuinfo_parisc *p = &per_cpu(cpu_data, this_cpu);
 	unsigned long ops;
 	unsigned long flags;
 
@@ -217,13 +218,13 @@ ipi_interrupt(int irq, void *dev_id)
 static inline void
 ipi_send(int cpu, enum ipi_message_type op)
 {
-	struct cpuinfo_parisc *p = &cpu_data[cpu];
+	struct cpuinfo_parisc *p = &per_cpu(cpu_data, cpu);
 	spinlock_t *lock = &per_cpu(ipi_lock, cpu);
 	unsigned long flags;
 
 	spin_lock_irqsave(lock, flags);
 	p->pending_ipi |= 1 << op;
-	gsc_writel(IPI_IRQ - CPU_IRQ_BASE, cpu_data[cpu].hpa);
+	gsc_writel(IPI_IRQ - CPU_IRQ_BASE, p->hpa);
 	spin_unlock_irqrestore(lock, flags);
 }
 
@@ -239,10 +240,7 @@ send_IPI_mask(cpumask_t mask, enum ipi_message_type op)
 static inline void
 send_IPI_single(int dest_cpu, enum ipi_message_type op)
 {
-	if (dest_cpu == NO_PROC_ID) {
-		BUG();
-		return;
-	}
+	BUG_ON(dest_cpu == NO_PROC_ID);
 
 	ipi_send(dest_cpu, op);
 }
@@ -324,8 +322,7 @@ smp_cpu_init(int cpunum)
 	/* Initialise the idle task for this CPU */
 	atomic_inc(&init_mm.mm_count);
 	current->active_mm = &init_mm;
-	if(current->mm)
-		BUG();
+	BUG_ON(current->mm);
 	enter_lazy_tlb(&init_mm, current);
 
 	init_IRQ();   /* make sure no IRQs are enabled or pending */
@@ -360,6 +357,7 @@ void __init smp_callin(void)
  */
 int __cpuinit smp_boot_one_cpu(int cpuid)
 {
+	const struct cpuinfo_parisc *p = &per_cpu(cpu_data, cpuid);
 	struct task_struct *idle;
 	long timeout;
 
@@ -391,7 +389,7 @@ int __cpuinit smp_boot_one_cpu(int cpuid)
 	smp_init_current_idle_task = idle ;
 	mb();
 
-	printk("Releasing cpu %d now, hpa=%lx\n", cpuid, cpu_data[cpuid].hpa);
+	printk(KERN_INFO "Releasing cpu %d now, hpa=%lx\n", cpuid, p->hpa);
 
 	/*
 	** This gets PDC to release the CPU from a very tight loop.
@@ -402,7 +400,7 @@ int __cpuinit smp_boot_one_cpu(int cpuid)
 	** EIR{0}). MEM_RENDEZ is valid only when it is nonzero and the 
 	** contents of memory are valid."
 	*/
-	gsc_writel(TIMER_IRQ - CPU_IRQ_BASE, cpu_data[cpuid].hpa);
+	gsc_writel(TIMER_IRQ - CPU_IRQ_BASE, p->hpa);
 	mb();
 
 	/* 
@@ -434,12 +432,12 @@ alive:
 	return 0;
 }
 
-void __devinit smp_prepare_boot_cpu(void)
+void __init smp_prepare_boot_cpu(void)
 {
-	int bootstrap_processor=cpu_data[0].cpuid;	/* CPU ID of BSP */
+	int bootstrap_processor = per_cpu(cpu_data, 0).cpuid;
 
 	/* Setup BSP mappings */
-	printk("SMP: bootstrap CPU ID is %d\n",bootstrap_processor);
+	printk(KERN_INFO "SMP: bootstrap CPU ID is %d\n", bootstrap_processor);
 
 	cpu_set(bootstrap_processor, cpu_online_map);
 	cpu_set(bootstrap_processor, cpu_present_map);
diff --git a/arch/parisc/kernel/time.c b/arch/parisc/kernel/time.c
index 24be86b..9db61b4 100644
--- a/arch/parisc/kernel/time.c
+++ b/arch/parisc/kernel/time.c
@@ -59,7 +59,7 @@ irqreturn_t timer_interrupt(int irq, void *dev_id)
 	unsigned long cycles_elapsed, ticks_elapsed;
 	unsigned long cycles_remainder;
 	unsigned int cpu = smp_processor_id();
-	struct cpuinfo_parisc *cpuinfo = &cpu_data[cpu];
+	struct cpuinfo_parisc *cpuinfo = &per_cpu(cpu_data, cpu);
 
 	/* gcc can optimize for "read-only" case with a local clocktick */
 	unsigned long cpt = clocktick;
@@ -212,7 +212,7 @@ void __init start_cpu_itimer(void)
 
 	mtctl(next_tick, 16);		/* kick off Interval Timer (CR16) */
 
-	cpu_data[cpu].it_value = next_tick;
+	per_cpu(cpu_data, cpu).it_value = next_tick;
 }
 
 void __init time_init(void)
diff --git a/arch/parisc/kernel/topology.c b/arch/parisc/kernel/topology.c
index d71cb01..f515938 100644
--- a/arch/parisc/kernel/topology.c
+++ b/arch/parisc/kernel/topology.c
@@ -22,14 +22,14 @@
 #include <linux/cpu.h>
 #include <linux/cache.h>
 
-static struct cpu cpu_devices[NR_CPUS] __read_mostly;
+static DEFINE_PER_CPU(struct cpu, cpu_devices);
 
 static int __init topology_init(void)
 {
 	int num;
 
 	for_each_present_cpu(num) {
-		register_cpu(&cpu_devices[num], num);
+		register_cpu(&per_cpu(cpu_devices, num), num);
 	}
 	return 0;
 }

Re: [PATCH] Change cpu_data[] and cpu_devices[] from array sized by NR_CPUS to per_cpu variables

[Grant Grundler]

On Sun, Aug 24, 2008 at 11:24:21PM +0200, Helge Deller wrote:
> Updated patch attached, which fixes some build problems and section 
> mismatches.
> Boot-tested on a UP 32bit machine with CONFIG_SMP=y.

Fixed this build failure (64-bit):
  CC      arch/parisc/kernel/perf.o
arch/parisc/kernel/perf.c: In function 'perf_init':
arch/parisc/kernel/perf.c:544: error: 'cpu_data' undeclared (first use in this function)
arch/parisc/kernel/perf.c:544: error: (Each undeclared identifier is reported only once
arch/parisc/kernel/perf.c:544: error: for each function it appears in.)
make[1]: *** [arch/parisc/kernel/perf.o] Error 1

Signed-off-by: Grant Grundler <grundler@parisc-linux.org>

diff --git a/arch/parisc/kernel/perf.c b/arch/parisc/kernel/perf.c
index f696f57..c661db7 100644
--- a/arch/parisc/kernel/perf.c
+++ b/arch/parisc/kernel/perf.c
@@ -55,6 +55,7 @@
 #include <asm/processor.h>
 #include <asm/runway.h>
 #include <asm/io.h>		/* for __raw_read() */
+#include <asm/percpu.h>
 
 #include "perf_images.h"
 
@@ -541,9 +542,9 @@ static int __init perf_init(void)
 	spin_lock_init(&perf_lock);
 
 	/* TODO: this only lets us access the first cpu.. what to do for SMP? */
-	cpu_device = cpu_data[0].dev;
+	cpu_device = per_cpu(cpu_data, 0).dev;
 	printk("Performance monitoring counters enabled for %s\n",
-		cpu_data[0].dev->name);
+		cpu_device->name);
 
 	return 0;
 }

Re: [PATCH] Change cpu_data[] and cpu_devices[] from array sized by NR_CPUS to per_cpu variables

[Helge Deller]

Grant Grundler wrote:
> On Sun, Aug 24, 2008 at 11:24:21PM +0200, Helge Deller wrote:
>> Updated patch attached, which fixes some build problems and section 
>> mismatches.
>> Boot-tested on a UP 32bit machine with CONFIG_SMP=y.
> 
> Fixed this build failure (64-bit):
>   CC      arch/parisc/kernel/perf.o
> arch/parisc/kernel/perf.c: In function 'perf_init':
> arch/parisc/kernel/perf.c:544: error: 'cpu_data' undeclared (first use in this function)
> arch/parisc/kernel/perf.c:544: error: (Each undeclared identifier is reported only once
> arch/parisc/kernel/perf.c:544: error: for each function it appears in.)
> make[1]: *** [arch/parisc/kernel/perf.o] Error 1
> 
> Signed-off-by: Grant Grundler <grundler@parisc-linux.org>

Thanks!

Acked-by: Helge Deller <deller@gmx.de>


> diff --git a/arch/parisc/kernel/perf.c b/arch/parisc/kernel/perf.c
> index f696f57..c661db7 100644
> --- a/arch/parisc/kernel/perf.c
> +++ b/arch/parisc/kernel/perf.c
> @@ -55,6 +55,7 @@
>  #include <asm/processor.h>
>  #include <asm/runway.h>
>  #include <asm/io.h>		/* for __raw_read() */
> +#include <asm/percpu.h>
>  
>  #include "perf_images.h"
>  
> @@ -541,9 +542,9 @@ static int __init perf_init(void)
>  	spin_lock_init(&perf_lock);
>  
>  	/* TODO: this only lets us access the first cpu.. what to do for SMP? */
> -	cpu_device = cpu_data[0].dev;
> +	cpu_device = per_cpu(cpu_data, 0).dev;
>  	printk("Performance monitoring counters enabled for %s\n",
> -		cpu_data[0].dev->name);
> +		cpu_device->name);
>  
>  	return 0;
>  }

Re: [PATCH] - Patch series for parisc

[Helge Deller]

On Sunday 24 August 2008, Helge Deller wrote:
> The following mails contain patches which I have sent to linux-parisc during 
> the last weeks and which have not yet been applied.

Kyle, in addition to my patches it would be nice if you could apply
a final LWS documentation patch on which Carlos and John still want 
to agree on.
For reference here is the thread:
http://marc.info/?t=121612540800004&r=1&w=2

Helge

[PATCH] drop superfluous .align 16

[Helge Deller]

This .align 16 is misplaced and should be in front of the ENTRY(lws_lock_start).
Works because of pure luck since we have a .align PAGE_SIZE before it instead.

Signed-off-by: Helge Deller <deller@gmx.de>

--- a/arch/parisc/kernel/syscall.S
+++ b/arch/parisc/kernel/syscall.S
@@ -640,7 +640,6 @@ END(sys_call_table64)
 	.align	PAGE_SIZE
 ENTRY(lws_lock_start)
 	/* lws locks */
-	.align 16
 	.rept 16
 	/* Keep locks aligned at 16-bytes */
 	.word 1

Re: [PATCH] drop superfluous .align 16

[Carlos O'Donell]

On Tue, Jul 8, 2008 at 4:06 PM, Helge Deller <deller@gmx.de> wrote:
> This .align 16 is misplaced and should be in front of the ENTRY(lws_lock_start).
> Works because of pure luck since we have a .align PAGE_SIZE before it instead.

This is not true, the .align 16 aligns almost any object in the
subsection including .word.

However, it is superfluous since we have a .align PAGE_SIZE, but I was
probably being safe in the event that someone put an object before
lws_lock_start.

> Signed-off-by: Helge Deller <deller@gmx.de>
>
> --- a/arch/parisc/kernel/syscall.S
> +++ b/arch/parisc/kernel/syscall.S
> @@ -640,7 +640,6 @@ END(sys_call_table64)
>        .align  PAGE_SIZE
>  ENTRY(lws_lock_start)
>        /* lws locks */
> -       .align 16
>        .rept 16
>        /* Keep locks aligned at 16-bytes */
>        .word 1

Cheers,
Carlos.

Re: [PATCH] drop superfluous .align 16

[Helge Deller]

Carlos O'Donell wrote:
> On Tue, Jul 8, 2008 at 4:06 PM, Helge Deller <deller@gmx.de> wrote:
>> This .align 16 is misplaced and should be in front of the ENTRY(lws_lock_start).
>> Works because of pure luck since we have a .align PAGE_SIZE before it instead.
> 
> This is not true, the .align 16 aligns almost any object in the
> subsection including .word.

Sure, but it does not take care of lws_lock_start itself.

> However, it is superfluous since we have a .align PAGE_SIZE, but I was
> probably being safe in the event that someone put an object before
> lws_lock_start.

What I mean is, that the lws_lock_start label should start at 16byte 
boundary, right? (it is due to the .align PAGE_SIZE).
Now, if e.g. the .word555 (see below) would be in there, then
lws_lock_start would be at PAGE_SIZE+4, while the .word1 would start at 
PAGE_SIZE+16, which is wrong for accessing the lws_lock_start variables, 
where the first entry should be "1".

Example:
        .align  PAGE_SIZE
        .word 5555  <<<- think this would be here.
ENTRY(lws_lock_start)
       /* lws locks */
      .align 16
      .rept 16
      /* Keep locks aligned at 16-bytes */
      .word 1

So, I still think it would be better to remove the .align16, or 
alternatively to move it in front of ENTRY(lws_lock_start) [to be on the 
safe side].

Am I really that wrong?

Helge

>> Signed-off-by: Helge Deller <deller@gmx.de>
>>
>> --- a/arch/parisc/kernel/syscall.S
>> +++ b/arch/parisc/kernel/syscall.S
>> @@ -640,7 +640,6 @@ END(sys_call_table64)
>>        .align  PAGE_SIZE
>>  ENTRY(lws_lock_start)
>>        /* lws locks */
>> -       .align 16
>>        .rept 16
>>        /* Keep locks aligned at 16-bytes */
>>        .word 1
> 
> Cheers,
> Carlos.

Re: [PATCH] drop superfluous .align 16

[Carlos O'Donell]

On Wed, Jul 9, 2008 at 3:50 PM, Helge Deller <deller@gmx.de> wrote:
> So, I still think it would be better to remove the .align16, or
> alternatively to move it in front of ENTRY(lws_lock_start) [to be on the
> safe side].
>
> Am I really that wrong?

No, you are absolutely right. I forgot the locks are referenced via
lws_lock_start, and the symbol needs to be aligned. Sorry, I thought
there was another symbol elsewhere.

Signed-off-by: Carlos O'Donell <carlos@systemhalted.org>

Cheers,
Carlos.

Re: [PATCH] drop superfluous .align 16

[John David Anglin]

> On Wed, Jul 9, 2008 at 3:50 PM, Helge Deller <deller@gmx.de> wrote:
> > So, I still think it would be better to remove the .align16, or
> > alternatively to move it in front of ENTRY(lws_lock_start) [to be on the
> > safe side].
> >
> > Am I really that wrong?
> 
> No, you are absolutely right. I forgot the locks are referenced via
> lws_lock_start, and the symbol needs to be aligned. Sorry, I thought
> there was another symbol elsewhere.

This would be clearer if there was an ENTRY_ALIGNED macro (e.g.,
ENTRY_ALIGNED(lws_lock_start, 16)).  It's probably overkill though.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)