[PATCH 3/4] ARM: perf: check that we have an event in the PMU IRQ handlers

[Ming Lei]

On Tue, Feb 28, 2012 at 6:55 PM, Will Deacon <will.deacon at arm.com> wrote:
> On Tue, Feb 28, 2012 at 12:53:28AM +0000, Ming Lei wrote:
>> On Tue, Feb 28, 2012 at 3:36 AM, Will Deacon <will.deacon at arm.com> wrote:
>> > A single 32-bit store is not interruptible in the same way as load/store
>> > multiple instructions are, so this isn't a problem.
>>
>> In fact, I have tried to look for the similar description in ARMv7 manual, but
>> didn't find it unfortunately, so could you point out where it is
>> stated explicitly?
>
> It's the section about Single-copy atomicity (3.5.3 in revision C). All word
> accesses to word-aligned locations are single-copy atomic. We rely on this
> for our atomic_{read,set} implementations.

Got it.

>
>> >
>> >> So suggest to take the correct way in theory, IMO.
>> >
>> > I don't like checking the used_mask, because then we have to throw in some
>> > barriers to ensure ordering between updates of the event arrays and the
>> > mask. If we do have to change something, I think putting ACCESS_ONCE around
>>
>> Both used_mask and event arrays are defined as percpu variables, so are you sure
>> barriers are required for it?
>
> I expect the compiler could still re-order the statements.

Yes, looks compiler barrier is required if checking with usage_mask.

> I heard back from the tools guys and they say that the store will not be
> split up, but may be coalesced with others in the form of a store-multiple.
> Referring back to the ARM ARM, that's ok since `each 32-bit word access is
> guaranteed to be single-copy atomic'.
>
> So I'm still in favour of the original patch. It's easy to read, fairly minimal
> and will work correctly.

OK, thanks for your detailed explanation.


thanks,
-- 
Ming Lei