Re: [Adeos-main] irq masking in ipipe_restore_pipeline_head

[Jan Kiszka <jan.kiszka@domain.hid>]

Philippe Gerum wrote:
> On Fri, 2010-06-18 at 15:46 +0200, Jan Kiszka wrote:
>> Philippe Gerum wrote:
>>> On Fri, 2010-06-18 at 13:26 +0200, Jan Kiszka wrote:
>>>> Hi Philippe,
>>>>
>>>> don't we need this in ipipe_restore_pipeline_head to play safe
>>> I can't see what this is supposed to fix. Any hint?
>> You enter ipipe_restore_pipeline_head with hard IRQs enabled and you
>> don't take the slow path as the domain is not stalled and the passed
>> flags do not change this. Then you leave with hard IRQs disabled. Corner
>> case, but I think it's not per se impossible.
> 
> In fact, the semantics of raw_local_irq_restore() are not that specific,
> looking at various irqflags.h implementations.
> Architectures are allowed to apply a local optimization for that
> routine. They may only re-enable if the flags arg passed says irqs were
> on previously to setting them off (e.g. blackfin), or even test a
> private system variable to know the current hw state and act upon this,
> ignoring the flags argument totally for this purpose (e.g. ppc64). There
> are likely others, since flipping the irq mask is costly for some archs,
> so optimizing makes sense for them.
> 
> In short, __ipipe_restore_pipeline_head() does not guarantee more than
> the kernel does with raw_local_irq_restore(), meaning that it is only
> required to switch irqs on if they were set off by the matching
> __ipipe_test_and_stall_pipeline_head(), but nothing more.
> 
> However, it would make sense to detect such misuse of the interface and
> whine loudly whenever we enter __ipipe_restore_pipeline_head() with irqs
> on.

You mean something like

if (WARN_ON_ONCE(!irqs_disabled_hw()))
	local_irq_disable_hw();

?

(Which leaves the if path even under !CONFIG_BUG. Maybe we want a bit
more optimization.)

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux