Re: [Xen-devel] [PATCH] xen: fix alignment for bitops

[Vladimir Murzin <murzin.v@xxxxxxxxx>]

On Tue, Apr 15, 2014 at 11:17 AM, David Vrabel <david.vrabel@xxxxxxxxxx> wrote:
> On 14/04/14 21:43, Vladimir Murzin wrote:
>> Bitops operations like set/clear/change mandate world aligned pointer, mainly
>> because architectures specific implementation.
>>
>> Looks that DEFINE_PER_CPU does required alignment for cpu_control_block;
>> however, local copy used for bitops might not be world aligned.
>>
>> Instruct compiler to keep local copy of "ready" world aligned.
> [...]
>> --- a/drivers/xen/events/events_fifo.c
>> +++ b/drivers/xen/events/events_fifo.c
>> @@ -285,7 +285,7 @@ static void consume_one_event(unsigned cpu,
>>  static void evtchn_fifo_handle_events(unsigned cpu)
>>  {
>>       struct evtchn_fifo_control_block *control_block;
>> -     uint32_t ready;
>> +     uint32_t __aligned(sizeof(long)) ready;
>
> This doesn't look sufficient to me.  The event words within the event
> array are 32-bit sized/aligned and sync_set_bit() and sync_clear_bit()
> etc. are done on these words.
>
> I think arm64 is going to have to provide sync_clear_bit(),
> sync_set_bit(), sync_test_bit() and sync_test_and_set_bit() that work
> with 32-bit sized and aligned words.

Looks reasonable. If I understand correctly these sync_* functions are
used by Xen only (or at least was introduced because of Xen).
I might take rework them for arm64 to operate on 32-bit
size/alignment. Does this sounds reasonable?
... and what to do with this patch?

Vladimir

>
> David

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel