Re: [PATCH] IB/core: Fix unaligned accesses

[Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]

On Wed, Apr 29, 2015 at 03:59:20PM -0600, David Ahern wrote:
> On 4/29/15 3:51 PM, Jason Gunthorpe wrote:
> >On Wed, Apr 29, 2015 at 03:38:22PM -0600, David Ahern wrote:
> >
> >>>And dealing with the fairly few resulting changes..
> >>
> >>Confused. That does not deal with the alignment problem. Internal to
> >>cm_mask_copy unsigned longs are used (8-bytes), so why change the
> >>signature to u32?
> >
> >You'd change the loop stride to by u32 as well.
> >
> >This whole thing is just an attempted optimization, but doing copy and
> >mask 8 bytes at a time on unaligned data is not very efficient, even
> >on x86.
> >
> >So either drop the optimization and use u8 as the stride.
> >
> >Or keep the optimization and guarentee alignment, the best we can do
> >is u32.
> >
> >Since this is an optimization, get_unaligned should be avoided,
> >looping over u8 would be faster.
> 
> Sorry, to be dense on this but I still don't see how your proposal
> addresses the underlying problem -- guarantee of 8 byte alignment.
> Be it a u8 or u32 for the input arguments the cast to unsigned long
> says that an extended load operation can be used for access. For
> that to work the address (src, dst, mask) must all be 8-byte
> aligned.

Read carefully:

> >You'd change the loop stride to by u32 as well.

which means this:

static void cm_mask_copy(u32 *dst, const u32 *src, const u32 *mask)
{
	int i;

	for (i = 0; i < IB_CM_COMPARE_SIZE; i++)
		dst[i] = src[i] & mask[i];
}

Divide IB_CM_COMPARE_SIZE by 4, adjust all users.

Make other related type changes the compiler tells you about.

Do not add casts.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html