[PATCH 1/5] ppc64: make current preempt safe

[PATCH 1/5] ppc64: make current preempt safe

[Hugh Dickins]

Repeated -j20 kernel builds on a G5 Quad running an SMP PREEMPT kernel
would often collapse within a day, some exec failing with "Bad address".
In each case examined, load_elf_binary was doing a kernel_read, but
generic_file_aio_read's access_ok saw current->thread.fs.seg as USER_DS
instead of KERNEL_DS.

objdump of filemap.o shows gcc 4.1.0 emitting "mr r5,r13 ... ld r9,416(r5)"
here for get_paca()->__current, instead of the expected and much more usual
"ld r9,416(r13)"; I've seen other gcc4s do the same, but perhaps not gcc3s.

So, if the task is preempted and rescheduled on a different cpu in between
the mr and the ld, r5 will be looking at a different paca_struct from the
one it's now on, pick up the wrong __current, and perhaps the wrong seg.
Presumably much worse could happen elsewhere, though that split is rare.

Other architectures appear to be safe (x86_64's read_pda is more limiting
than get_paca), but ppc64 needs to force "current" into one instruction.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
---
I think this patch should go into both 2.6.19 and 2.6.18-stable.

 include/asm-powerpc/current.h |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

--- 2.6.19-rc4/include/asm-powerpc/current.h	2006-03-20 05:53:29.000000000 +0000
+++ linux/include/asm-powerpc/current.h	2006-10-30 19:27:05.000000000 +0000
@@ -14,7 +14,17 @@ struct task_struct;
 #ifdef __powerpc64__
 #include <asm/paca.h>
 
-#define current		(get_paca()->__current)
+static inline struct task_struct *get_current(void)
+{
+	struct task_struct *task;
+
+	__asm__ __volatile__("ld %0,%1(13)"
+	: "=r" (task)
+	: "i" (offsetof(struct paca_struct, __current)));
+
+	return task;
+}
+#define current	get_current()
 
 #else
 

[PATCH 2/5] ppc64: make high hugepage areas preempt safe

[Hugh Dickins]

Checking source for other get_paca()->field preemption dangers found that
open_high_hpage_areas does a structure copy into its paca while preemption
is enabled: unsafe however gcc accomplishes it.  Just remove that copy:
it's done safely afterwards by on_each_cpu, as in open_low_hpage_areas.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
---
I think this patch should go into 2.6.19 (but no reports to justify 18-stable).

 arch/powerpc/mm/hugetlbpage.c |    3 ---
 1 file changed, 3 deletions(-)

--- 2.6.19-rc4/arch/powerpc/mm/hugetlbpage.c	2006-09-20 04:42:06.000000000 +0100
+++ linux/arch/powerpc/mm/hugetlbpage.c	2006-10-30 19:27:05.000000000 +0000
@@ -480,9 +480,6 @@ static int open_high_hpage_areas(struct 
 
 	mm->context.high_htlb_areas |= newareas;
 
-	/* update the paca copy of the context struct */
-	get_paca()->context = mm->context;
-
 	/* the context change must make it to memory before the flush,
 	 * so that further SLB misses do the right thing. */
 	mb();

[PATCH 3/5] ppc64: make mmiowb's io_sync preempt safe

[Hugh Dickins]

If mmiowb() is always used prior to releasing spinlock as Doc suggests,
then it's safe against preemption; but I'm not convinced that's always
the case.  If preemption occurs between sync and get_paca()->io_sync = 0,
I believe there's no problem.  But in the unlikely event that gcc does
the store relative to another register than r13 (as it did with current),
then there's a small danger of setting another cpu's io_sync to 0, after
it had just set it to 1.  Rewrite ppc64 mmiowb to prevent that.

The remaining io_sync assignments in io.h all get_paca()->io_sync = 1,
which is harmless even if preempted to the wrong cpu (the context switch
itself syncs); and those in spinlock.h are while preemption is disabled.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
---
I'm clueless with powerpc and inline asm, this patch is likely to be
nonsense: more a placeholder to provoke whatever is the correct patch.
But I think the resulting patch should probably go into 2.6.19.

 include/asm-powerpc/io.h |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

--- 2.6.19-rc4/include/asm-powerpc/io.h	2006-10-24 04:34:33.000000000 +0100
+++ linux/include/asm-powerpc/io.h	2006-10-30 19:27:05.000000000 +0000
@@ -163,8 +163,11 @@ extern void _outsl_ns(volatile u32 __iom
 
 static inline void mmiowb(void)
 {
-	__asm__ __volatile__ ("sync" : : : "memory");
-	get_paca()->io_sync = 0;
+	unsigned long tmp;
+
+	__asm__ __volatile__("sync; li %0,0; stb %0,%1(13)"
+	: "=&r" (tmp) : "i" (offsetof(struct paca_struct, io_sync))
+	: "memory");
 }
 
 /*

[PATCH 4/5] ppc64: make soft_enabled irqs preempt safe

[Hugh Dickins]

Rewrite local_get_flags and local_irq_disable to use r13 explicitly,
to avoid the risk that gcc will split get_paca()->soft_enabled into
a sequence unsafe against preemption.

local_irq_restore be careful to access hard_enabled and lppaca before
setting soft_enabled, which may well permit preemption.  Use local_paca
instead of get_paca() when setting hard_enabled: looking ahead to the
DEBUG_PREEMPT patch, which uses local_paca as the raw unchecked version.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
---
As before, my powerpc and inline asm may be nonsense: please do fix it.
This patch is to powerpc git code, not intended for 2.6.19.

 arch/powerpc/kernel/irq.c    |   20 +++++++++++++++-----
 include/asm-powerpc/hw_irq.h |   20 +++++++++++++++-----
 2 files changed, 30 insertions(+), 10 deletions(-)

--- git-powerpc/arch/powerpc/kernel/irq.c	2006-10-30 19:30:57.000000000 +0000
+++ linux/arch/powerpc/kernel/irq.c	2006-10-30 19:32:38.000000000 +0000
@@ -99,20 +99,30 @@ int distribute_irqs = 1;
 
 void local_irq_restore(unsigned long en)
 {
-	get_paca()->soft_enabled = en;
-	if (!en)
+	unsigned long hard_enabled;
+
+	if (!en) {
+		get_paca()->soft_enabled = en;
 		return;
+	}
 
 	if (firmware_has_feature(FW_FEATURE_ISERIES)) {
-		if (get_paca()->lppaca_ptr->int_dword.any_int)
+		unsigned long any_int;
+
+		any_int = get_lppaca()->int_dword.any_int;
+		get_paca()->soft_enabled = en;
+		if (any_int)
 			iseries_handle_interrupts();
 		return;
 	}
 
-	if (get_paca()->hard_enabled)
+	hard_enabled = get_paca()->hard_enabled;
+	get_paca()->soft_enabled = en;
+	if (hard_enabled)
 		return;
+
 	/* need to hard-enable interrupts here */
-	get_paca()->hard_enabled = en;
+	local_paca->hard_enabled = en;
 	if ((int)mfspr(SPRN_DEC) < 0)
 		mtspr(SPRN_DEC, 1);
 	hard_irq_enable();
--- git-powerpc/include/asm-powerpc/hw_irq.h	2006-10-30 19:30:57.000000000 +0000
+++ linux/include/asm-powerpc/hw_irq.h	2006-10-30 19:32:38.000000000 +0000
@@ -18,15 +18,25 @@ extern void timer_interrupt(struct pt_re
 
 static inline unsigned long local_get_flags(void)
 {
-	return get_paca()->soft_enabled;
+	unsigned long flags;
+
+	__asm__ __volatile__("lbz %0,%1(13)"
+	: "=r" (flags)
+	: "i" (offsetof(struct paca_struct, soft_enabled)));
+
+	return flags;
 }
 
 static inline unsigned long local_irq_disable(void)
 {
-	unsigned long flag = get_paca()->soft_enabled;
-	get_paca()->soft_enabled = 0;
-	barrier();
-	return flag;
+	unsigned long flags, zero;
+
+	__asm__ __volatile__("li %1,0; lbz %0,%2(13); stb %1,%2(13)"
+	: "=r" (flags), "=&r" (zero)
+	: "i" (offsetof(struct paca_struct, soft_enabled))
+	: "memory");
+
+	return flags;
 }
 
 extern void local_irq_restore(unsigned long);

[PATCH 5/5] ppc64: support CONFIG_DEBUG_PREEMPT

[Hugh Dickins]

Add CONFIG_DEBUG_PREEMPT support to ppc64: it was useful for testing
get_paca() preemption.  Cheat a little, just use debug_smp_processor_id()
in the debug version of get_paca(): it contains all the right checks and
reporting, though get_paca() doesn't really use smp_processor_id().

Use local_paca for what might have been called __raw_get_paca().
Silence harmless warnings from io.h and lparcfg.c with local_paca - I
assume it is okay for iseries_lparcfg_data to be referencing shared_proc
with preemption enabled: all cpus show the same value for shared_proc?

Why do other architectures need TRACE_IRQFLAGS_SUPPORT for DEBUG_PREEMPT?
I don't know, ppc64 appears to get along fine without it.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
---
I've only tested this on one particular PowerMac G5 configuration:
it's quite likely that once integrated, defaulting to y when PREEMPT,
it'll show up a bunch of noisy warnings on other configurations:
not something to rush into mainline.

 arch/powerpc/kernel/lparcfg.c |    2 +-
 include/asm-powerpc/io.h      |   14 +++++++-------
 include/asm-powerpc/paca.h    |   11 +++++++++++
 include/asm-powerpc/percpu.h  |    2 +-
 include/asm-powerpc/smp.h     |    2 +-
 lib/Kconfig.debug             |    2 +-
 6 files changed, 22 insertions(+), 11 deletions(-)

--- git-powerpc/arch/powerpc/kernel/lparcfg.c	2006-10-24 04:34:08.000000000 +0100
+++ linux/arch/powerpc/kernel/lparcfg.c	2006-10-30 19:50:54.000000000 +0000
@@ -77,7 +77,7 @@ static int iseries_lparcfg_data(struct s
 	int processors, max_processors;
 	unsigned long purr = get_purr();
 
-	shared = (int)(get_lppaca()->shared_proc);
+	shared = (int)(local_paca->lppaca_ptr->shared_proc);
 
 	seq_printf(m, "system_active_processors=%d\n",
 		   (int)HvLpConfig_getSystemPhysicalProcessors());
--- git-powerpc/include/asm-powerpc/io.h	2006-10-24 04:34:33.000000000 +0100
+++ linux/include/asm-powerpc/io.h	2006-10-30 19:50:54.000000000 +0000
@@ -282,7 +282,7 @@ static inline void __out_8(volatile unsi
 {
 	__asm__ __volatile__("sync; stb%U0%X0 %1,%0"
 			     : "=m" (*addr) : "r" (val));
-	get_paca()->io_sync = 1;
+	local_paca->io_sync = 1;
 }
 
 static inline int __in_le16(const volatile unsigned short __iomem *addr)
@@ -307,14 +307,14 @@ static inline void __out_le16(volatile u
 {
 	__asm__ __volatile__("sync; sthbrx %1,0,%2"
 			     : "=m" (*addr) : "r" (val), "r" (addr));
-	get_paca()->io_sync = 1;
+	local_paca->io_sync = 1;
 }
 
 static inline void __out_be16(volatile unsigned short __iomem *addr, int val)
 {
 	__asm__ __volatile__("sync; sth%U0%X0 %1,%0"
 			     : "=m" (*addr) : "r" (val));
-	get_paca()->io_sync = 1;
+	local_paca->io_sync = 1;
 }
 
 static inline unsigned __in_le32(const volatile unsigned __iomem *addr)
@@ -339,14 +339,14 @@ static inline void __out_le32(volatile u
 {
 	__asm__ __volatile__("sync; stwbrx %1,0,%2" : "=m" (*addr)
 			     : "r" (val), "r" (addr));
-	get_paca()->io_sync = 1;
+	local_paca->io_sync = 1;
 }
 
 static inline void __out_be32(volatile unsigned __iomem *addr, int val)
 {
 	__asm__ __volatile__("sync; stw%U0%X0 %1,%0"
 			     : "=m" (*addr) : "r" (val));
-	get_paca()->io_sync = 1;
+	local_paca->io_sync = 1;
 }
 
 static inline unsigned long __in_le64(const volatile unsigned long __iomem *addr)
@@ -393,13 +393,13 @@ static inline void __out_le64(volatile u
 			     "sync\n"
 			     "std %0,0(%3)"
 			     : "=&r" (tmp) , "=&r" (val) : "1" (val) , "b" (addr) , "m" (*addr));
-	get_paca()->io_sync = 1;
+	local_paca->io_sync = 1;
 }
 
 static inline void __out_be64(volatile unsigned long __iomem *addr, unsigned long val)
 {
 	__asm__ __volatile__("sync; std%U0%X0 %1,%0" : "=m" (*addr) : "r" (val));
-	get_paca()->io_sync = 1;
+	local_paca->io_sync = 1;
 }
 
 #include <asm/eeh.h>
--- git-powerpc/include/asm-powerpc/paca.h	2006-10-30 19:30:57.000000000 +0000
+++ linux/include/asm-powerpc/paca.h	2006-10-30 19:50:54.000000000 +0000
@@ -21,7 +21,18 @@
 #include	<asm/mmu.h>
 
 register struct paca_struct *local_paca asm("r13");
+
+#if defined(CONFIG_DEBUG_PREEMPT) && defined(CONFIG_SMP)
+extern unsigned int debug_smp_processor_id(void); /* from linux/smp.h */
+/*
+ * Add standard checks that preemption cannot occur when using get_paca():
+ * otherwise the paca_struct it points to may be the wrong one just after.
+ */
+#define get_paca()	((void) debug_smp_processor_id(), local_paca)
+#else
 #define get_paca()	local_paca
+#endif
+
 #define get_lppaca()	(get_paca()->lppaca_ptr)
 #define get_slb_shadow()	(get_paca()->slb_shadow_ptr)
 
--- git-powerpc/include/asm-powerpc/percpu.h	2006-09-20 04:42:06.000000000 +0100
+++ linux/include/asm-powerpc/percpu.h	2006-10-30 19:50:54.000000000 +0000
@@ -23,7 +23,7 @@
 /* var is in discarded region: offset to particular copy we want */
 #define per_cpu(var, cpu) (*RELOC_HIDE(&per_cpu__##var, __per_cpu_offset(cpu)))
 #define __get_cpu_var(var) (*RELOC_HIDE(&per_cpu__##var, __my_cpu_offset()))
-#define __raw_get_cpu_var(var) (*RELOC_HIDE(&per_cpu__##var, __my_cpu_offset()))
+#define __raw_get_cpu_var(var) (*RELOC_HIDE(&per_cpu__##var, local_paca->data_offset))
 
 /* A macro to avoid #include hell... */
 #define percpu_modcopy(pcpudst, src, size)			\
--- git-powerpc/include/asm-powerpc/smp.h	2006-10-24 04:34:33.000000000 +0100
+++ linux/include/asm-powerpc/smp.h	2006-10-30 19:50:54.000000000 +0000
@@ -45,7 +45,7 @@ void generic_mach_cpu_die(void);
 #endif
 
 #ifdef CONFIG_PPC64
-#define raw_smp_processor_id()	(get_paca()->paca_index)
+#define raw_smp_processor_id()	(local_paca->paca_index)
 #define hard_smp_processor_id() (get_paca()->hw_cpu_id)
 #else
 /* 32-bit */
--- git-powerpc/lib/Kconfig.debug	2006-10-30 19:15:49.000000000 +0000
+++ linux/lib/Kconfig.debug	2006-10-30 19:50:54.000000000 +0000
@@ -114,7 +114,7 @@ config DEBUG_SLAB_LEAK
 
 config DEBUG_PREEMPT
 	bool "Debug preemptible kernel"
-	depends on DEBUG_KERNEL && PREEMPT && TRACE_IRQFLAGS_SUPPORT
+	depends on DEBUG_KERNEL && PREEMPT && (TRACE_IRQFLAGS_SUPPORT || PPC64)
 	default y
 	help
 	  If you say Y here then the kernel will use a debug variant of the

Re: [PATCH 2/5] ppc64: make high hugepage areas preempt safe

[David Gibson]

On Tue, Oct 31, 2006 at 06:40:39PM +0000, Hugh Dickins wrote:
> Checking source for other get_paca()->field preemption dangers found that
> open_high_hpage_areas does a structure copy into its paca while preemption
> is enabled: unsafe however gcc accomplishes it.  Just remove that copy:
> it's done safely afterwards by on_each_cpu, as in open_low_hpage_areas.

Oops, when I fixed the other problems with the PACA loading here, I
guess I missed this redundant assignment.

> Signed-off-by: Hugh Dickins <hugh@veritas.com>
Acked-by: David Gibson <dwg@au1.ibm.com>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 4/5] ppc64: make soft_enabled irqs preempt safe

[Paul Mackerras]

Hugh Dickins writes:

> local_irq_restore be careful to access hard_enabled and lppaca before
> setting soft_enabled, which may well permit preemption.  Use local_paca

The reason for testing hard_enabled *after* setting soft_enabled is to
avoid a race.  If we load hard_enabled first, as your patch does, then
an interrupt could come in between that load and the store that sets
soft_enabled, and we would miss that and not hard-enable as we should.

I'm not sure what the solution is.  I suppose we could disable
preemption, although I'd rather something lighter-weight if possible.

Paul.

Re: [PATCH 4/5] ppc64: make soft_enabled irqs preempt safe

[Hugh Dickins]

On Fri, 10 Nov 2006, Paul Mackerras wrote:
> Hugh Dickins writes:
> 
> > local_irq_restore be careful to access hard_enabled and lppaca before
> > setting soft_enabled, which may well permit preemption.  Use local_paca
> 
> The reason for testing hard_enabled *after* setting soft_enabled is to
> avoid a race.  If we load hard_enabled first, as your patch does, then
> an interrupt could come in between that load and the store that sets
> soft_enabled, and we would miss that and not hard-enable as we should.

Ah, yes, of course.

> 
> I'm not sure what the solution is.  I suppose we could disable
> preemption, although I'd rather something lighter-weight if possible.

On reflection, considering what can actually happen in those cases
where local_irq_restore might get preempted - what happens when it
gets it wrong - it now looks to me like disabling preemption there
would be overkill: we just need to make sure that we're looking at
our own cpu in a couple of places i.e. stop gcc from doing one of
those "mr r5,r13" kind of things.  (If hard_enabled is actually 0,
we won't be preempted anyway; if it's 1, then all we have to do is
make sure we're looking at ours rather than someone else's 0.)

That's assuming iseries_handle_interrupts does no harm if called
without an interrupt pending, I hope that's the case (otherwise,
maybe that block would need preempt_disable/preempt_enable, since
get_lppaca() references are inevitably non-atomic, never mind the
gcc oddity - though I'm sure you'd work out how to avoid it later).

I must emphasize again that my asm is likely to be nonsense: seems
to work, but as likely to be introducing its own bugs as fixing
the preemption issue: please hack it right, thanks!



Rewrite local_get_flags and local_irq_disable to use r13 explicitly,
to avoid the risk that gcc will split get_paca()->soft_enabled into a
sequence unsafe against preemption.  Similar care in local_irq_restore.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
---

 arch/powerpc/kernel/irq.c    |   57 +++++++++++++++++++++++++++++++++++++++----
 include/asm-powerpc/hw_irq.h |   20 +++++++++++----
 2 files changed, 67 insertions(+), 10 deletions(-)

--- 2.6.19-rc5-mm1/arch/powerpc/kernel/irq.c	2006-11-08 12:39:06.000000000 +0000
+++ linux/arch/powerpc/kernel/irq.c	2006-11-10 20:12:10.000000000 +0000
@@ -97,22 +97,69 @@ EXPORT_SYMBOL(irq_desc);
 
 int distribute_irqs = 1;
 
+static inline unsigned long get_hard_enabled(void)
+{
+	unsigned long enabled;
+
+	__asm__ __volatile__("lbz %0,%1(13)"
+	: "=r" (enabled) : "i" (offsetof(struct paca_struct, hard_enabled)));
+
+	return enabled;
+}
+
+static inline void set_soft_enabled(unsigned long enable)
+{
+	__asm__ __volatile__("stb %0,%1(13)"
+	: : "r" (enable), "i" (offsetof(struct paca_struct, soft_enabled)));
+}
+
 void local_irq_restore(unsigned long en)
 {
-	get_paca()->soft_enabled = en;
+	/*
+	 * get_paca()->soft_enabled = en;
+	 * Is it ever valid to use local_irq_restore(0) when soft_enabled is 1?
+	 * That was allowed before, and in such a case we do need to take care
+	 * that gcc will set soft_enabled directly via r13, not choose to use
+	 * an intermediate register, lest we're preempted to a different cpu.
+	 */
+	set_soft_enabled(en);
 	if (!en)
 		return;
 
 	if (firmware_has_feature(FW_FEATURE_ISERIES)) {
-		if (get_paca()->lppaca_ptr->int_dword.any_int)
+		/*
+		 * Do we need to disable preemption here?  Not really: in the
+		 * unlikely event that we're preempted to a different cpu in
+		 * between getting r13, loading its lppaca_ptr, and loading
+		 * its any_int, we might call iseries_handle_interrupts without
+		 * an interrupt pending on the new cpu, but that's no disaster,
+		 * is it?  And the business of preempting us off the old cpu
+		 * would itself involve a local_irq_restore which handles the
+		 * interrupt to that cpu.
+		 *
+		 * But use "local_paca->lppaca_ptr" instead of "get_lppaca()"
+		 * to avoid any preemption checking added into get_paca().
+		 */
+		if (local_paca->lppaca_ptr->int_dword.any_int)
 			iseries_handle_interrupts();
 		return;
 	}
 
-	if (get_paca()->hard_enabled)
+	/*
+	 * if (get_paca()->hard_enabled) return;
+	 * But again we need to take care that gcc gets hard_enabled directly
+	 * via r13, not choose to use an intermediate register, lest we're
+	 * preempted to a different cpu in between the two instructions.
+	 */
+	if (get_hard_enabled())
 		return;
-	/* need to hard-enable interrupts here */
-	get_paca()->hard_enabled = en;
+
+	/*
+	 * Need to hard-enable interrupts here.  Since currently disabled,
+	 * no need to take further asm precautions against preemption; but
+	 * use local_paca instead of get_paca() to avoid preemption checking.
+	 */
+	local_paca->hard_enabled = en;
 	if ((int)mfspr(SPRN_DEC) < 0)
 		mtspr(SPRN_DEC, 1);
 	hard_irq_enable();
--- 2.6.19-rc5-mm1/include/asm-powerpc/hw_irq.h	2006-11-08 12:39:15.000000000 +0000
+++ linux/include/asm-powerpc/hw_irq.h	2006-11-10 13:21:39.000000000 +0000
@@ -18,15 +18,25 @@ extern void timer_interrupt(struct pt_re
 
 static inline unsigned long local_get_flags(void)
 {
-	return get_paca()->soft_enabled;
+	unsigned long flags;
+
+	__asm__ __volatile__("lbz %0,%1(13)"
+	: "=r" (flags)
+	: "i" (offsetof(struct paca_struct, soft_enabled)));
+
+	return flags;
 }
 
 static inline unsigned long local_irq_disable(void)
 {
-	unsigned long flag = get_paca()->soft_enabled;
-	get_paca()->soft_enabled = 0;
-	barrier();
-	return flag;
+	unsigned long flags, zero;
+
+	__asm__ __volatile__("li %1,0; lbz %0,%2(13); stb %1,%2(13)"
+	: "=r" (flags), "=&r" (zero)
+	: "i" (offsetof(struct paca_struct, soft_enabled))
+	: "memory");
+
+	return flags;
 }
 
 extern void local_irq_restore(unsigned long);

[PATCH] include stddef.h in asm-powerpc/current.h to get definition of offsetof

[Olaf Hering]

On Tue, Oct 31, Hugh Dickins wrote:

> +++ linux/include/asm-powerpc/current.h	2006-10-30 19:27:05.000000000 +0000

> +static inline struct task_struct *get_current(void)
> +{
> +	struct task_struct *task;
> +
> +	__asm__ __volatile__("ld %0,%1(13)"
> +	: "=r" (task)
> +	: "i" (offsetof(struct paca_struct, __current)));

This breaks compile of 2.6.18.8:

  CC [M]  drivers/media/video/pwc/pwc-uncompress.o
In file included from /home/olaf/kernel/linux-2.6.18.8/drivers/media/video/pwc/pwc-uncompress.c:29:
include2/asm/current.h: In function 'get_current':
include2/asm/current.h:23: warning: implicit declaration of function 'offsetof'
include2/asm/current.h:23: error: expected expression before 'struct'
make[5]: *** [drivers/media/video/pwc/pwc-uncompress.o] Error 1


Signed-off-by: Olaf Hering <olaf@aepfle.de>

---
 include/asm-powerpc/current.h |    1 +
 1 file changed, 1 insertion(+)

Index: linux-2.6.18.8/include/asm-powerpc/current.h
===================================================================
--- linux-2.6.18.8.orig/include/asm-powerpc/current.h
+++ linux-2.6.18.8/include/asm-powerpc/current.h
@@ -12,6 +12,7 @@
 struct task_struct;
 
 #ifdef __powerpc64__
+#include <linux/stddef.h>
 #include <asm/paca.h>
 
 static inline struct task_struct *get_current(void)

Re: [PATCH] include stddef.h in asm-powerpc/current.h to get definition of offsetof

[Hugh Dickins]

On Sun, 25 Feb 2007, Olaf Hering wrote:
> On Tue, Oct 31, Hugh Dickins wrote:
> 
> > +++ linux/include/asm-powerpc/current.h	2006-10-30 19:27:05.000000000 +0000
> 
> > +static inline struct task_struct *get_current(void)
> > +{
> > +	struct task_struct *task;
> > +
> > +	__asm__ __volatile__("ld %0,%1(13)"
> > +	: "=r" (task)
> > +	: "i" (offsetof(struct paca_struct, __current)));
> 
> This breaks compile of 2.6.18.8:
> 
>   CC [M]  drivers/media/video/pwc/pwc-uncompress.o
> In file included from /home/olaf/kernel/linux-2.6.18.8/drivers/media/video/pwc/pwc-uncompress.c:29:
> include2/asm/current.h: In function 'get_current':
> include2/asm/current.h:23: warning: implicit declaration of function 'offsetof'
> include2/asm/current.h:23: error: expected expression before 'struct'
> make[5]: *** [drivers/media/video/pwc/pwc-uncompress.o] Error 1

Ow!  I'm very sorry for that.  My fault.

(I wonder what changed between 2.6.18 and 2.6.19,
to make 2.6.19 and 2.6.20 okay, yet this error in 2.6.18.8?)

Your patch is clearly correct, and satisfies the "any header should
include whatever it needs" rule; though my own preference (desperate
attempt to fool people into thinking my blame lies elsewhere?) would
be for the patch at the bottom - it seems that pwc-uncompress.c's
inclusion of asm/current.h is just bogus, probably a leftover of
some private debugging session around 2.6.8.

Hugh

> 
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> ---
>  include/asm-powerpc/current.h |    1 +
>  1 file changed, 1 insertion(+)
> 
> Index: linux-2.6.18.8/include/asm-powerpc/current.h
> ===================================================================
> --- linux-2.6.18.8.orig/include/asm-powerpc/current.h
> +++ linux-2.6.18.8/include/asm-powerpc/current.h
> @@ -12,6 +12,7 @@
>  struct task_struct;
>  
>  #ifdef __powerpc64__
> +#include <linux/stddef.h>
>  #include <asm/paca.h>
>  
>  static inline struct task_struct *get_current(void)

--- 2.6.18.8/drivers/media/video/pwc/pwc-uncompress.c	2006-09-20 04:42:06.000000000 +0100
+++ 2.6.18.9/drivers/media/video/pwc/pwc-uncompress.c	2007-02-26 10:59:40.000000000 +0000
@@ -26,7 +26,6 @@
    vim: set ts=8:
 */
 
-#include <asm/current.h>
 #include <asm/types.h>
 
 #include "pwc.h"