Re: [PATCH] fix libvirt alignment on arm platforms

[Michele Paolino <m.paolino@xxxxxxxxxxxxxxxxxxxxxx>]

On 13/12/2013 08:54, Michal Privoznik wrote:

On 12.12.2013 20:27, Eric Blake wrote:

On 12/12/2013 11:55 AM, Michal Privoznik wrote:

     int detail;
-};
+} ATTRIBUTE_PACKED;

(What an ancient e-mail :-) )

1970 was how many years before libvirt was even started?

Indeed, this is a 43 years old thread :-)

I've had some problem with the ntpd deamon on the OMAP5 yesterday. For this reason I've resent the patch with the prefix [resend].

I've raised the problem here:

https://www.redhat.com/archives/libvir-list/2013-December/msg00635.html

And Eric replied suggesting a fix:

https://www.redhat.com/archives/libvir-list/2013-December/msg00662.html

But I must say I like your approach more. Eric?

I still think ATTRIBUTE_PACKED in a parent class is wrong; it forces the
children to be packed, and may make it impossible to implement atomic
operations on a child member that was supposed to be aligned.  I'd much
rather fix the alignment in the parent class using portable constructs
than a compiler-specific non-alignment directive.

Oh right. The locking could be impossible, because if two struct members
are packed in a word, e.g.:

struct S {
  bool a;
  bool b;
  ...
};

then modifying S.a requires whole struct to be fetched from mem,
modifying S.a a and storing it back. However, even if our locking code
to protect say S.b was written well, S.b can still get changed without
lock. As S.a is being changed, another thread changes S.b, but storing
S.a means overwriting S.b without lock held.

I have not thought of that. So NACK then - we need Eric's approach.

I saw the Eric's patch. I will test it today.

Michal

--
Michele Paolino, Virtualization R&D Engineer
Virtual Open Systems
Open Source  KVM  Virtualization  Developments
Multicore Systems Virtualization Porting Services
Web: www.virtualopensystems.com

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list