Re: [PATCH v2 02/11] IB/core: Change port_attr.sm_lid from 16 to 32 bits

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

On Tue, Nov 22, 2016 at 01:13:26PM -0800, Chandramouli, Dasaratharaman wrote:
> 
> 
> On 11/22/2016 12:57 PM, Jason Gunthorpe wrote:
> >On Tue, Nov 22, 2016 at 02:38:43PM -0500, Dasaratharaman Chandramouli wrote:
> >>+++ b/drivers/infiniband/core/sa_query.c
> >>@@ -958,7 +958,7 @@ static void update_sm_ah(struct work_struct *work)
> >> 		pr_err("Couldn't find index for default PKey\n");
> >>
> >> 	memset(&ah_attr, 0, sizeof ah_attr);
> >>-	ah_attr.dlid     = port_attr.sm_lid;
> >>+	ah_attr.dlid     = (u16)port_attr.sm_lid;
> >
> >Why are we dropping bits here?
> 
> Patch 3 increases ah_attr.dlid to 32 bits and the typecast is dropped.
> We added it in this patch just to avoid compiler warnings, if any.

It would be alot better to fix this series so adding typecasts and
then dropping them didn't happen so extensively. Just reodering some
patched would do the trick. That would make it alot less churny and
easy to read..

> >>-	sport->sm_lid = port_attr.sm_lid;
> >>+	sport->sm_lid = (u16)port_attr.sm_lid;
> >> 	sport->lid = port_attr.lid;
> >
> >And here..

> Patch 11 increases lid and sm_lid fields in struct srpt_port and the
> typecast is dropped.  We added it in this patch just to avoid
> compiler warnings, if any.

Eg if you do patch 11 first this hunk goes away.

I also don't really care for all the added u16 casts, the compiler
doesn't warn on implicit demotion without a higher warning level...

> >
> >>+#define OPA_TO_IB_UCAST_LID(x)	(((x) >= be16_to_cpu(IB_MULTICAST_LID_BASE)) \
> >>+				 ? 0 : x)
> >
> >static inline function please.
> Will change. Thanks.

All of them please.

And I think you should re-think how this is being used, the pattern
around OPA_TO_IB_UCAST_LID is copied too many times for my liking, and
isn't this UAPI compatability only?

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