linux-kernel - Re: [PATCH 1/4] Blackfin: arch patch for 2.6.18

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <200609271337.53485.arnd@arndb.de>
Date:	Wed, 27 Sep 2006 13:37:52 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Aubrey <aubreylee@...il.com>
Cc:	"Luke Yang" <luke.adi@...il.com>, linux-kernel@...r.kernel.org,
	"Andrew Morton" <akpm@...l.org>,
	"Getz, Robin" <Robin.Getz@...log.com>
Subject: Re: [PATCH 1/4] Blackfin: arch patch for 2.6.18

On Wednesday 27 September 2006 12:04, Aubrey wrote:
> inline static void default_idle(void)
> {
>        int flag;
> 
>        while (!need_resched()) {
>                leds_switch(LED_OFF);
>                local_irq_save(flag);
>                if ( likely(!need_resched()) {
> #if defined (ANOMALY_05000244) && defined (CONFIG_BLKFIN_CACHE)
>                      __asm__("nop; nop;\n");
> #endif
>                      __asm__(".align 64;\n STI %0; IDLE;\n"
>                              : %0 (flag): :"cc");
>                }
>                local_irq_restore(flag);
>                leds_switch(LED_ON);
>        }
> }======================================
> 
> Here, according to design, it's not possible that interrupt occurs
> between "STI %0"(enable interrupt) and "IDLE".
> 
> __asm__(".align 64; STI %0; IDLE;" : %0 (x):  :"cc");
> 
> Robin can explain more details.

Ok, looks good now. Just a few details that don't impact the 
functionality:

- Always use 'static inline', not 'inline static', because of C99
- In the kernel, it's more common to use 'asm' than '__asm__'.
- It should probably be 'asm volatile', since gcc might notice
  that the output (flag) is not used anywhere and it can therefore
  eliminate the asm.
- Usually, I recommend using local_irq_disable() instead of
  local_irq_save(flags) when you know that interrupts are enabled
  before. It uses one less local variable, which makes it more
  efficient on some architectures.
- I'd insert the two NOPs unconditionally here for better
  readability.

	Arnd <><
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/