On Mon, Feb 25, 2019 at 06:54:07AM -0800, Bart Van Assche wrote:
> On 2/24/19 10:52 PM, Leon Romanovsky wrote:
> > diff --git a/drivers/infiniband/core/cm_msgs.h b/drivers/infiniband/core/cm_msgs.h
> > index 476d4309576d..3d16d614aff6 100644
> > +++ b/drivers/infiniband/core/cm_msgs.h
> > @@ -98,7 +98,7 @@ struct cm_req_msg {
> >
> > u32 private_data[IB_CM_REQ_PRIVATE_DATA_SIZE / sizeof(u32)];
> >
> > -} __attribute__ ((packed));
> > +} __packed;
>
> Hi Leon,
>
> A previous RDMA maintainer once wrote a blog in which he explained why it is
> better to apply the __packed attribute only to data members that need it
> instead of to data structures. The reason is that applying __packed to data
> structures does not allow compilers to generate optimal code for
> architectures that cannot perform non-aligned multi-byte reads efficiently.
> Does that concern apply to the data structures modified by this patch?
__packed is a very big hammer. It sets the required min alignment of
everything inside the struct to 1. This is required if the struct is
going to start at any point in memory..
But if you know, for instance, that the struct is always 4 byte
aligned then there are better options that continue to allow the
compiler to optimize. ie set 8 byte fields to aligned on 4 bytes.
Jason
