Adrian wrote: > I've attached modified patch with __attribute__ ((packed)) removed. But > I wonder why is it present in for example i8042_regs_t in i8042 driver? If it's there, it's probably a bug.
Jakub Jermar wrote: > > I wonder if the comment would still have its merrit when we consider all > > the members are effectively volatile. Of course. Volatile means just that GCC is not allowed to optimize the accesses away. It does not do magic - packed structure can be unaligned and so its members cannot be accessed atomically, if they are more than 8 bits wide. > Moreover, all accesses to the structure must be done using the > pio_read/write_n() functions so the proper access width is guaranteed. Yes but at least on some arches these functions are written in C and they're inline. GCC will propagate the information about the packed nature of the structure all the way to the core of the function (no, really!) and generate a non-atomic access. Believe me, I found out the hard way (on ARM when porting to the FreeRunner). You might not get bitten by it all the time, because on some CPU/platform/device a 16-bit I/O access *might* be equivalent to two consecutive 8-bit accesses to subsequent addresses, but many times it just isn't. The only way to be safe when using structs for register definitions is either not to use packed or have all pio_* functions implemented in assembly (and hope that GCC one day does not start to optimize that - or can we prevent that somehow?) Assuming you don't actually have unaligned members in the structure, because that would not work. Either your CPU might not even allow specifying an unaligned port address or it might generate an unaligned access exception. Cheers -Jiri _______________________________________________ HelenOS-devel mailing list [email protected] http://lists.modry.cz/cgi-bin/listinfo/helenos-devel
