lkml.org 
[lkml]   [2013]   [Aug]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] smp.c: Quit unconditionally enabling irqs in on_each_cpu_mask().
On 08/09/2013 11:51 AM, Gilad Ben-Yossef wrote:
> On Thu, Aug 8, 2013 at 11:27 PM, David Daney <ddaney@caviumnetworks.com> wrote:
>>
>> On 08/08/2013 12:25 PM, Christoph Lameter wrote:
>>>
>>> On Thu, 8 Aug 2013, David Daney wrote:
>>>
>>>> I don't know of any bugs currently caused by this unconditional
>>>> local_irq_enable(), but I want to use this function in MIPS/OCTEON
>>>> early boot (when we have early_boot_irqs_disabled). This also makes
>>>> this function have similar semantics to on_each_cpu() which is good in
>>>> itself.
>>>
>>>
>>> smp_call_function_many() wants interrupts enabled.
>>
>>
>> That's what the comments say, but it isn't actually true.
>>
>> The usage introduced by the patch is no different than the existing usage in on_each_cpu() 30 line up in the file.
>>
>
> Regardless of the question of how smp_call_function_many() should be
> called, the IRQ disable/enable pair is actually there to make sure the
> provided function runs on the current CPU at the same conditions as it
> would get called via the IPI.

Yes, I am aware of that.

You will notice that my patch doesn't change in any way the state of the
system while running the provided function, irqs are still disabled,
just as they are in the current implementation.

>
> I would at least consider putting a test there to make sure IRQs
> really are disabled when entering the function,

I think this may be a false predicate. on_each_cpu_mask() is usually
called with IRQs enabled...

> otherwise the bugs
> stemming from incorrect use can be tricky to catch.
>

... all my patch does is allow on_each_cpu_mask() to be called with IRQs
disabled if we are in Early Boot. This is already the case with
smp_call_function(), smp_call_function_many() and on_each_cpu(). I am
arguing that for the sake of consistency and the principle that function
behavior shouldn't be surprising, that we make on_each_cpu_mask() work
the same way.

Any required preconditions on the state of the system when calling
on_each_cpu_mask() are already verified as it unconditionally calls
smp_call_function_many() which has a lot of conditions it warns on.
Additional tests in the callers of smp_call_function_many() would only
be redundant.


David Daney


\
 
 \ /
  Last update: 2013-08-09 21:41    [W:0.054 / U:0.352 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site