[PATCH /3] sh64: remove cli()/sti() from arch/sh64/*

[PATCH /3] sh64: remove cli()/sti() from arch/sh64/*

[James Nelson]

This series of patches is to remove the last cli()/sti() function calls in arch/sh64.

These are the only instances in active code that grep could find.

 kernel/time.c     |    4 ++--
 mach-cayman/irq.c |    8 ++++----
 mm/fault.c        |    2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

[PATCH /3] sh64: remove cli()/sti() in arch/sh64/kernel/time.c

[James Nelson]

Signed-off-by: James Nelson <james4765@gmail.com>

diff -urN --exclude='*~' linux-2.6.10-mm1-original/arch/sh64/kernel/time.c linux-2.6.10-mm1/arch/sh64/kernel/time.c
--- linux-2.6.10-mm1-original/arch/sh64/kernel/time.c	2004-12-24 16:34:58.000000000 -0500
+++ linux-2.6.10-mm1/arch/sh64/kernel/time.c	2005-01-04 21:06:31.232945929 -0500
@@ -424,7 +424,7 @@
 	*/
 	register unsigned long long  __rtc_irq_flag __asm__ ("r3");
 
-	sti();
+	local_irq_enable();
 	do {} while (ctrl_inb(R64CNT) != 0);
 	ctrl_outb(RCR1_CIE, RCR1); /* Enable carry interrupt */
 
@@ -443,7 +443,7 @@
 		     "getcon	" __CTC ", %0\n\t"
 		: "=r"(ctc_val), "=r" (__dummy), "=r" (__rtc_irq_flag)
 		: "0" (0));
-	cli();
+	local_irq_disable();
 	/*
 	 * SH-3:
 	 * CPU clock = 4 stages * loop

Re: [PATCH /3] sh64: remove cli()/sti() from arch/sh64/*

[Al Viro]

On Tue, Jan 04, 2005 at 08:22:47PM -0600, James Nelson wrote:
> This series of patches is to remove the last cli()/sti() function calls in arch/sh64.

Wait a minute.  Is that just a blanket search-and-replace job?  There is
a reason why cli/sti is marked obsolete instead of being silently #define'd
that way.  Namely, in a lot of cases users of cli/sti are actually racy.

For such instances replacing these with local_... would not improve anything
(obviously) *and* would hide a trouble spot by silencing a warning.

I'm not familiar with the architectures in question, so it might very well
be that all replacements so far had been correct.  However, I would really
like to see rationale for each of those warning removals to go along with
the patches.

Note that basically you are doing "remove the warning in foo.c:42 and
keep the current behaviour".  The missing part is "current behaviour is,
in fact, correct in that place and does not deserve a warning because
<list of reasons>".

Re: [PATCH /3] sh64: remove cli()/sti() from arch/sh64/*

[Jim Nelson]

Al Viro wrote:

>On Tue, Jan 04, 2005 at 08:22:47PM -0600, James Nelson wrote:
>  
>
>>This series of patches is to remove the last cli()/sti() function calls in arch/sh64.
>>    
>>
>
>Wait a minute.  Is that just a blanket search-and-replace job?  There is
>a reason why cli/sti is marked obsolete instead of being silently #define'd
>that way.  Namely, in a lot of cases users of cli/sti are actually racy.
>
>For such instances replacing these with local_... would not improve anything
>(obviously) *and* would hide a trouble spot by silencing a warning.
>
>I'm not familiar with the architectures in question, so it might very well
>be that all replacements so far had been correct.  However, I would really
>like to see rationale for each of those warning removals to go along with
>the patches.
>
>Note that basically you are doing "remove the warning in foo.c:42 and
>keep the current behaviour".  The missing part is "current behaviour is,
>in fact, correct in that place and does not deserve a warning because
><list of reasons>".
>
>  
>
Everything I've looked at so far has been for single-processor systems 
AFAICT - embedded processors, evaluation boards, etc.  I do not pretend 
to have intimate familiarity with the hardware in question, and I will 
be much more careful when I reach anything that can be plugged into an 
SMP box, but I was grabbing the low-hanging fruit first.  The nasty 
stuff (drivers/char, for example) will come later.

That's why I cc'd the arch maintainers - figured they'd whack me with a 
cluebat if I'd overlooked something.

Re: [PATCH /3] sh64: remove cli()/sti() from arch/sh64/*

[Paul Mundt]

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

On Tue, Jan 04, 2005 at 10:11:23PM -0500, Jim Nelson wrote:
> Everything I've looked at so far has been for single-processor systems 
> AFAICT - embedded processors, evaluation boards, etc.  I do not pretend 
> to have intimate familiarity with the hardware in question, and I will 
> be much more careful when I reach anything that can be plugged into an 
> SMP box, but I was grabbing the low-hanging fruit first.  The nasty 
> stuff (drivers/char, for example) will come later.
> 
> That's why I cc'd the arch maintainers - figured they'd whack me with a 
> cluebat if I'd overlooked something.

Yes, these are all UP cases, so at least for the sh and sh64 cases these
specific changes are fine.

sh ripped out the save_and_cli() mess a long time ago (during early 2.5
before Linus added the UP compat stuff), so these at least shouldn't
have been there in the first place (mpc1211 wasn't added until after the
fact however and seems to have gotten overlooked).

Anyways, I'll apply these patches to their respective trees, thanks.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]