On Thu, Apr 12, 2018 at 09:48:49AM -0700, Don Hiatt wrote:
>
>
> On 4/11/2018 9:43 PM, Jason Gunthorpe wrote:
> > On Wed, Apr 11, 2018 at 01:05:51PM -0700, Don Hiatt wrote:
> > > > > diff --git a/drivers/infiniband/hw/hfi1/verbs.h b/drivers/infiniband/hw/hfi1/verbs.h
> > > > > index 2d787b8..3ae26ac 100644
> > > > > +++ b/drivers/infiniband/hw/hfi1/verbs.h
> > > > > @@ -110,6 +110,12 @@ enum {
> > > > > #define LRH_9B_BYTES (FIELD_SIZEOF(struct ib_header, lrh))
> > > > > #define LRH_9B_DWORDS (LRH_9B_BYTES / sizeof(u32))
> > > > > +/* 24Bits for qpn, upper 8Bits reserved */
> > > > > +struct opa_16b_mgmt {
> > > > > + __be32 dest_qpn;
> > > > > + __be32 src_qpn;
> > > > > +} __packed;
> > > > Are all the packed's in this file really necessary?
> > > >
> > > Yes, these packs are necessary as the structures are placed directly on the
> > > wire.
> > That isn't really how you answer this question. Point to where padding
> > is being eliminated or where the struct is used unaligned to justify
> > the __packed.
> >
> > Jason
> Hi Jason,
>
> opa_16b_mgmt is contained within hfi1_16b_header which has ib_other_headers
> that contain
> atomics in ib_ehdrs. Are you saying all the __packed can be removed except
> ib_ehdrs?
Just becuase that one u64 is not aligned right doesn't mean you need
to pack everthing everywhere.
And the correct way to solve that misaligned u64 is not with packed
but with this:
__attribute__((packed, aligned(4)))
On the containing struct.
Which tells the compiler to misalign the u64 and only that u64. Gives
much better codegen and clarity.
The kernel has a macro for this whos spelling I forget right now.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
