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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11/22/2016 1:24 PM, Jason Gunthorpe wrote:
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..
Yes, will re-order them. Thanks

-	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...

It's that higher warning level that we were a little concerned about.
If you feel very strongly about not having these casts, they can be removed but i would prefer leaving them there to silence certain odd ball compiler configurations.

+#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.
Will change.

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?
They are only used when there is a user-space query into the kernel for lid and sm_lid.

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



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux