Re: [PATCH 3/7] percpu: clean up percpu variable definitions

[Jesper Nilsson <Jesper.Nilsson@axis.com>]

On Fri, Jun 05, 2009 at 06:25:30AM +0200, Tejun Heo wrote:
> Benjamin Herrenschmidt wrote:
> > On Mon, 2009-06-01 at 20:36 +0900, Tejun Heo wrote:
> >>> Whether the volatile is actually needed or not, it's bad to have this
> >>> kind of potential behavior changing nugget hidden in this seemingly
> >>> inocuous change.  Especially if you're the poor soul who ends up
> >>> having to debug it :-/
> >> You're right.  Aieee... how do I feed volatile to the DEFINE macro.
> >> I'll think of something.
> > 
> > Or better, work with the cris maintainer to figure out whether it's
> > needed (it probably isn't) and have a pre-requisite patch that removes
> > it before your series :-)
> 
> Yeap, that's worth giving a shot.
> 
> Mikael Starvik, can you please enlighten us why volatile is necessary
> there?

I've talked with Mikael, and we both agreed that this was probably
a legacy from earlier versions, and the volatile is no longer needed.

Confirmed by booting and running some video-streaming on an ARTPEC-3
(CRISv32) board.

You can take the following patch as a pre-requisite, or go the way of
the original patch.

From: Jesper Nilsson <jesper.nilsson@axis.com>
Subject: [PATCH] CRIS: Change DEFINE_PER_CPU of current_pgd to be non volatile.

The DEFINE_PER_CPU of current_pgd was on CRIS defined using volatile,
which is not needed. Remove volatile.

Signed-off-by: Jesper Nilsson <jesper.nilsson@axis.com>
---
 arch/cris/include/asm/mmu_context.h |    3 ++-
 arch/cris/mm/fault.c                |    2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/cris/include/asm/mmu_context.h b/arch/cris/include/asm/mmu_context.h
index 72ba08d..476cd9e 100644
--- a/arch/cris/include/asm/mmu_context.h
+++ b/arch/cris/include/asm/mmu_context.h
@@ -17,7 +17,8 @@ extern void switch_mm(struct mm_struct *prev, struct mm_struct *next,
  * registers like cr3 on the i386
  */
 
-extern volatile DEFINE_PER_CPU(pgd_t *,current_pgd); /* defined in arch/cris/mm/fault.c */
+/* defined in arch/cris/mm/fault.c */
+extern DEFINE_PER_CPU(pgd_t *, current_pgd);
 
 static inline void enter_lazy_tlb(struct mm_struct *mm, struct task_struct *tsk)
 {
diff --git a/arch/cris/mm/fault.c b/arch/cris/mm/fault.c
index c4c76db..84d22ae 100644
--- a/arch/cris/mm/fault.c
+++ b/arch/cris/mm/fault.c
@@ -29,7 +29,7 @@ extern void die_if_kernel(const char *, struct pt_regs *, long);
 
 /* current active page directory */
 
-volatile DEFINE_PER_CPU(pgd_t *,current_pgd);
+DEFINE_PER_CPU(pgd_t *, current_pgd);
 unsigned long cris_signal_return_page;
 
 /*
-- 
1.6.1

> Thanks.
> 
> -- 
> tejun

/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@axis.com

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