> Quoting [EMAIL PROTECTED] <[EMAIL PROTECTED]>:
> Subject: Re: [RFC/PATCH] mthca: ensure alignment of doorbell writes
>
> On Thu, Jul 26, 2007 at 06:39:46AM +0300, Michael S. Tsirkin wrote:
>
> > ....
> > These should be getting 'union mthca_doorbell *db' I think.
> >
>
> Hi Michael;
>
> Want to make sure I understand your point. Are you saying, e.g.,
> that the function:
>
> static inline void mthca_ring_db(union mthca_doorbell db,
> void __iomem *dest,
> spinlock_t *doorbell_lock)
>
> should instead have the prototype:
>
> static inline void mthca_ring_db(union mthca_doorbell* db,
> void __iomem *dest,
> spinlock_t *doorbell_lock)
>
> ?
Yes.
> If so, I'm not sure I agree. The union mthca_doorbell is
> 64 bits so can be passed in a register, but passing a pointer
> requires a few extra operations to calculate the address,
> and dereference the pointer. But maybe I misunderstand you...
This is really coding style thing.
It's usually not a good idea to pass unions/structures by value.
If union size is later changed to be large, gcc might pass it in
a global data section, which fails to be reentrant.
Try compiling both variants and looking at the code - I expect
there won't be difference.
> Now that I look at this again, the __attribute__ ((aligned...))
> thing on union mthca_doorbell is pretty silly - of course the
> alignment is going to be sizeof(__be64)....
>
> +union mthca_doorbell {
> + __be64 val64;
> + __be32 val32[2];
> +} __attribute__ ((aligned (sizeof(__be64))));
> +
Right.
--
MST
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
