-----Original Message----- From: David Laight [mailto:David.Laight@xxxxxxxxxx] Subject: RE: [PATCH] hpsa: fix boot on ia64 (atomic_t alignment) From: Martin K. Petersen > Sent: 17 March 2021 02:26 > > Arnd, > > > Actually that still feels wrong: the annotation of the struct is to > > pack every member, which causes the access to be done in byte units > > on architectures that do not have hardware unaligned load/store > > instructions, at least for things like atomic_read() that does not > > go through a cmpxchg() or ll/sc cycle. > > > This change may fix itanium, but it's still not correct. Other > > architectures would have already been broken before the recent > > change, but that's not a reason against fixing them now. > > I agree. I understand why there are restrictions on fields consumed by > the hardware. But for fields internal to the driver the packing > doesn't make sense to me. Jeepers -- that global #pragma pack(1) is bollocks. I think there are a couple of __u64 that are 32bit aligned. Just marking those field __packed __aligned(4) should have the desired effect. Or use a typedef for '__u64 with 32bit alignment'. (There probably ought to be one in types.h) Then add compile-time asserts that any non-trivial structures the hardware accesses are the right size. David Don: My dilemma is that hpsa is now a maintenance driver and making more packing/alignment changes would trigger a lot of regression testing. My last patch aligns the structure with what has been in place for a long time now. All I did was rename an unused variable. So making any more changes is not high on my todo list...
