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

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

 



On 5/1/15 2:45 AM, Yann Droneaud wrote:
  static int cm_compare_data(struct ib_cm_compare_data *src_data,
  			   struct ib_cm_compare_data *dst_data)
  {
-	u8 src[IB_CM_COMPARE_SIZE];
-	u8 dst[IB_CM_COMPARE_SIZE];
+	u32 src[IB_CM_COMPARE_SIZE];
+	u32 dst[IB_CM_COMPARE_SIZE];


You have to change IB_CM_COMPARE_SIZE, as it's going to allocate more
bytes than necessary.

d'oh. yes, oversight. Thanks for pointing out.


Perhaps renaming IB_CM_COMPARE_SIZE to IB_CM_COMPARE_COUNT would be
good, then remove the division by sizeof(u32).

I feel like I stepped into a tar pit... It's a bit odd that IB_CM_COMPARE_SIZE is part of an enum rather than a #define.

It seems like the number of bytes being compared is what is relevant here; comparing by u32's or unsigned long's is the optimization. So, I'd prefer to leave it as SIZE.

If there are no objections I think the better thing to do is pull it out as:

/* compare's are done u32 at a time */
#define IB_CM_COMPARE_SIZE 64/sizeof(u32)



diff --git a/include/rdma/ib_cm.h b/include/rdma/ib_cm.h
index 0e3ff30647d5..0dd576f40c83 100644
--- a/include/rdma/ib_cm.h
+++ b/include/rdma/ib_cm.h
@@ -337,8 +337,8 @@ void ib_destroy_cm_id(struct ib_cm_id *cm_id);
  #define IB_SDP_SERVICE_ID_MASK	cpu_to_be64(0xFFFFFFFFFFFF0000ULL)

  struct ib_cm_compare_data {
-	u8  data[IB_CM_COMPARE_SIZE];
-	u8  mask[IB_CM_COMPARE_SIZE];
+	u32  data[IB_CM_COMPARE_SIZE];
+	u32  mask[IB_CM_COMPARE_SIZE];
  };

  /**

Anyway, you might want to compile this part of the kernel with
-Wcast-align and ignore the false positive (in particular
every other use of container_of()).

Why?

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