Re: [PATCH 1/3] jump label: add enabled/disabled state to jump label key entries

[David Daney <ddaney@caviumnetworks.com>]

On 11/24/2010 07:42 AM, Jason Baron wrote:
> On Wed, Nov 24, 2010 at 04:24:05PM +0100, Peter Zijlstra wrote:
>> On Wed, 2010-11-24 at 10:19 -0500, Jason Baron wrote:
>>> On Wed, Nov 24, 2010 at 04:11:18PM +0100, Peter Zijlstra wrote:
>>>> On Wed, 2010-11-24 at 09:54 -0500, Jason Baron wrote:
>>>>> On Wed, Nov 24, 2010 at 09:20:09AM +0100, Peter Zijlstra wrote:
>>>>>> On Tue, 2010-11-23 at 16:27 -0500, Jason Baron wrote:
>>>>>>>          struct hlist_head modules;
>>>>>>>          unsigned long key;
>>>>>>> +       u32 nr_entries : 31,
>>>>>>> +              enabled :  1;
>>>>>>>   };
>>>>>>
>>>>>> I still don't see why you do this, why not simply mandate that the key
>>>>>> is of type atomic_t* and use *key as enabled state?
>>>>>>
>>>>>
>>>>> Because I want to use *key as a pointer directly to 'struct jump_label_entry'.
>>>>> In this way jump_label_enable(), jump_label_disable(), become O(1) operations.
>>>>> That way we don't need any hashing.
>>>>
>>>> But but but, you're doing a friggin stop_machine to poke text, that's
>>>> way more expensive than anything else.
>>>>
>>>
>>> Yes, but other arches do not require stop_machine(). Also, there is work
>>> for x86 to make the code patching happen without stop_machine().
>>
>> Even without stop machine you're sending IPIs to all CPUs, that's not
>> free either.
>>
>> And I think the only arch where you can do text pokes without cross-cpu
>> synchronization is one that doesn't have SMP support.
>>
>>
>
> is this really true?
>

For MIPS SMP it is.  ICache invalidation is done per CPU.  If you need 
the change to be visible on all CPUs, you have to do an IPI to get all 
CPUs to do their own ICache invalidation.


> The powerpc implementation uses patch_instruction():
>
>
> arch/powerpc/lib/code-patching.c:
>
> void patch_instruction(unsigned int *addr, unsigned int instr)
> {
>          *addr = instr;
>          asm ("dcbst 0, %0; sync; icbi 0,%0; sync; isync" : : "r"
> (addr));
> }
>
>
> And sparc does uses flushi():
>
>
> include/asm/system_64.h:
>
> #define flushi(addr)    __asm__ __volatile__ ("flush %0" : : "r" (addr)
> : "memory")
>

I don't know how SPARC and PPC work, but does that really work on SMP 
(or even NUMA) machines?

David Daney