All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V1 0/3] Add cross-channel support
@ 2015-12-20 10:16 Leon Romanovsky
       [not found] ` <1450606571-15877-1-git-send-email-leon-2ukJVAZIZ/Y@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Leon Romanovsky @ 2015-12-20 10:16 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

This patchset adds cross-channel support.

The cross-channel feature allows to execute WQEs that involve
synchronization of I/O operations’ on different QPs.

This capability enables to program complex flows with a single
function call, hereby significantly reducing overhead associated
with I/O processing.

Changes from v0:
  * Set UAR to be the same for QP and CQ.

Leon Romanovsky (3):
  IB/core: Align coding style of ib_device_cap_flags structure
  IB/core: Add cross-channel support
  IB/mlx5: Add driver cross-channel support

 drivers/infiniband/core/uverbs_cmd.c |  5 ++-
 drivers/infiniband/hw/mlx5/cq.c      |  7 +++-
 drivers/infiniband/hw/mlx5/main.c    |  3 ++
 drivers/infiniband/hw/mlx5/mlx5_ib.h | 16 ++++++++
 drivers/infiniband/hw/mlx5/qp.c      | 54 ++++++++++++++++++++++-----
 include/linux/mlx5/qp.h              |  3 ++
 include/rdma/ib_verbs.h              | 71 +++++++++++++++++++++---------------
 7 files changed, 117 insertions(+), 42 deletions(-)

-- 
1.7.12.4

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH V1 1/3] IB/core: Align coding style of ib_device_cap_flags structure
       [not found] ` <1450606571-15877-1-git-send-email-leon-2ukJVAZIZ/Y@public.gmane.org>
@ 2015-12-20 10:16   ` Leon Romanovsky
       [not found]     ` <1450606571-15877-2-git-send-email-leon-2ukJVAZIZ/Y@public.gmane.org>
  2015-12-20 10:16   ` [PATCH V1 2/3] IB/core: Add cross-channel support Leon Romanovsky
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Leon Romanovsky @ 2015-12-20 10:16 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Modify enum ib_device_cap_flags such that other patches which add new
enum values pass strict checkpatch.pl checks.

Reviewed-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 include/rdma/ib_verbs.h | 60 ++++++++++++++++++++++++-------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 9a68a19..bcf40ad 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -105,24 +105,24 @@ enum rdma_link_layer {
 };
 
 enum ib_device_cap_flags {
-	IB_DEVICE_RESIZE_MAX_WR		= 1,
-	IB_DEVICE_BAD_PKEY_CNTR		= (1<<1),
-	IB_DEVICE_BAD_QKEY_CNTR		= (1<<2),
-	IB_DEVICE_RAW_MULTI		= (1<<3),
-	IB_DEVICE_AUTO_PATH_MIG		= (1<<4),
-	IB_DEVICE_CHANGE_PHY_PORT	= (1<<5),
-	IB_DEVICE_UD_AV_PORT_ENFORCE	= (1<<6),
-	IB_DEVICE_CURR_QP_STATE_MOD	= (1<<7),
-	IB_DEVICE_SHUTDOWN_PORT		= (1<<8),
-	IB_DEVICE_INIT_TYPE		= (1<<9),
-	IB_DEVICE_PORT_ACTIVE_EVENT	= (1<<10),
-	IB_DEVICE_SYS_IMAGE_GUID	= (1<<11),
-	IB_DEVICE_RC_RNR_NAK_GEN	= (1<<12),
-	IB_DEVICE_SRQ_RESIZE		= (1<<13),
-	IB_DEVICE_N_NOTIFY_CQ		= (1<<14),
-	IB_DEVICE_LOCAL_DMA_LKEY	= (1<<15),
-	IB_DEVICE_RESERVED		= (1<<16), /* old SEND_W_INV */
-	IB_DEVICE_MEM_WINDOW		= (1<<17),
+	IB_DEVICE_RESIZE_MAX_WR		= (1 << 0),
+	IB_DEVICE_BAD_PKEY_CNTR		= (1 << 1),
+	IB_DEVICE_BAD_QKEY_CNTR		= (1 << 2),
+	IB_DEVICE_RAW_MULTI		= (1 << 3),
+	IB_DEVICE_AUTO_PATH_MIG		= (1 << 4),
+	IB_DEVICE_CHANGE_PHY_PORT	= (1 << 5),
+	IB_DEVICE_UD_AV_PORT_ENFORCE	= (1 << 6),
+	IB_DEVICE_CURR_QP_STATE_MOD	= (1 << 7),
+	IB_DEVICE_SHUTDOWN_PORT		= (1 << 8),
+	IB_DEVICE_INIT_TYPE		= (1 << 9),
+	IB_DEVICE_PORT_ACTIVE_EVENT	= (1 << 10),
+	IB_DEVICE_SYS_IMAGE_GUID	= (1 << 11),
+	IB_DEVICE_RC_RNR_NAK_GEN	= (1 << 12),
+	IB_DEVICE_SRQ_RESIZE		= (1 << 13),
+	IB_DEVICE_N_NOTIFY_CQ		= (1 << 14),
+	IB_DEVICE_LOCAL_DMA_LKEY	= (1 << 15),
+	IB_DEVICE_RESERVED		= (1 << 16), /* old SEND_W_INV */
+	IB_DEVICE_MEM_WINDOW		= (1 << 17),
 	/*
 	 * Devices should set IB_DEVICE_UD_IP_SUM if they support
 	 * insertion of UDP and TCP checksum on outgoing UD IPoIB
@@ -130,18 +130,18 @@ enum ib_device_cap_flags {
 	 * incoming messages.  Setting this flag implies that the
 	 * IPoIB driver may set NETIF_F_IP_CSUM for datagram mode.
 	 */
-	IB_DEVICE_UD_IP_CSUM		= (1<<18),
-	IB_DEVICE_UD_TSO		= (1<<19),
-	IB_DEVICE_XRC			= (1<<20),
-	IB_DEVICE_MEM_MGT_EXTENSIONS	= (1<<21),
-	IB_DEVICE_BLOCK_MULTICAST_LOOPBACK = (1<<22),
-	IB_DEVICE_MEM_WINDOW_TYPE_2A	= (1<<23),
-	IB_DEVICE_MEM_WINDOW_TYPE_2B	= (1<<24),
-	IB_DEVICE_RC_IP_CSUM		= (1<<25),
-	IB_DEVICE_RAW_IP_CSUM		= (1<<26),
-	IB_DEVICE_MANAGED_FLOW_STEERING = (1<<29),
-	IB_DEVICE_SIGNATURE_HANDOVER	= (1<<30),
-	IB_DEVICE_ON_DEMAND_PAGING	= (1<<31),
+	IB_DEVICE_UD_IP_CSUM		= (1 << 18),
+	IB_DEVICE_UD_TSO		= (1 << 19),
+	IB_DEVICE_XRC			= (1 << 20),
+	IB_DEVICE_MEM_MGT_EXTENSIONS	= (1 << 21),
+	IB_DEVICE_BLOCK_MULTICAST_LOOPBACK = (1 << 22),
+	IB_DEVICE_MEM_WINDOW_TYPE_2A	= (1 << 23),
+	IB_DEVICE_MEM_WINDOW_TYPE_2B	= (1 << 24),
+	IB_DEVICE_RC_IP_CSUM		= (1 << 25),
+	IB_DEVICE_RAW_IP_CSUM		= (1 << 26),
+	IB_DEVICE_MANAGED_FLOW_STEERING = (1 << 29),
+	IB_DEVICE_SIGNATURE_HANDOVER	= (1 << 30),
+	IB_DEVICE_ON_DEMAND_PAGING	= (1 << 31),
 };
 
 enum ib_signature_prot_cap {
-- 
1.7.12.4

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH V1 2/3] IB/core: Add cross-channel support
       [not found] ` <1450606571-15877-1-git-send-email-leon-2ukJVAZIZ/Y@public.gmane.org>
  2015-12-20 10:16   ` [PATCH V1 1/3] IB/core: Align coding style of ib_device_cap_flags structure Leon Romanovsky
@ 2015-12-20 10:16   ` Leon Romanovsky
  2015-12-20 10:16   ` [PATCH V1 3/3] IB/mlx5: Add driver " Leon Romanovsky
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 20+ messages in thread
From: Leon Romanovsky @ 2015-12-20 10:16 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The cross-channel feature allows to execute WQEs that involve
synchronization of I/O operations’ on different QPs.

This capability enables to program complex flows with a single
function call, hereby significantly reducing overhead associated
with I/O processing.

Cross-channel operations support is indicated by HCA capability
information.

The queue pairs can be configured to work as a “sync master queue”
or “sync slave queues”.

The added flags are:

1. Device capability flag IB_DEVICE_CROSS_CHANNEL for the
   devices that can perform cross-channel operations.

2. CQ property flag IB_CQ_FLAGS_IGNORE_OVERRUN to disable CQ overrun
   check. This check is useless in cross-channel scenario.

3. QP property flags to indicate if queues are slave or master:
   * IB_QP_CREATE_MANAGED_SEND indicates that posted send work requests
     will not be executed immediately and requires enabling.
   * IB_QP_CREATE_MANAGED_RECV indicates that posted receive work
     requests will not be executed immediately and requires enabling.
   * IB_QP_CREATE_CROSS_CHANNEL declares the QP to work in cross-channel
     mode. If IB_QP_CREATE_MANAGED_SEND and IB_QP_CREATE_MANAGED_RECV are
     not provided, this QP will be sync master queue, else it will be sync
     slave.

Reviewed-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/uverbs_cmd.c |  5 ++++-
 include/rdma/ib_verbs.h              | 11 +++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 94816ae..0e9711f 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1843,7 +1843,10 @@ static int create_qp(struct ib_uverbs_file *file,
 		      sizeof(cmd->create_flags))
 		attr.create_flags = cmd->create_flags;
 
-	if (attr.create_flags & ~IB_QP_CREATE_BLOCK_MULTICAST_LOOPBACK) {
+	if (attr.create_flags & ~(IB_QP_CREATE_BLOCK_MULTICAST_LOOPBACK |
+				IB_QP_CREATE_CROSS_CHANNEL |
+				IB_QP_CREATE_MANAGED_SEND |
+				IB_QP_CREATE_MANAGED_RECV)) {
 		ret = -EINVAL;
 		goto err_put;
 	}
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index bcf40ad..370fbdf 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -139,6 +139,13 @@ enum ib_device_cap_flags {
 	IB_DEVICE_MEM_WINDOW_TYPE_2B	= (1 << 24),
 	IB_DEVICE_RC_IP_CSUM		= (1 << 25),
 	IB_DEVICE_RAW_IP_CSUM		= (1 << 26),
+	/*
+	 * Devices should set IB_DEVICE_CROSS_CHANNEL if they
+	 * support execution of WQEs that involve synchronization
+	 * of I/O operations with single completion queue managed
+	 * by hardware.
+	 */
+	IB_DEVICE_CROSS_CHANNEL		= (1 << 27),
 	IB_DEVICE_MANAGED_FLOW_STEERING = (1 << 29),
 	IB_DEVICE_SIGNATURE_HANDOVER	= (1 << 30),
 	IB_DEVICE_ON_DEMAND_PAGING	= (1 << 31),
@@ -184,6 +191,7 @@ struct ib_odp_caps {
 
 enum ib_cq_creation_flags {
 	IB_CQ_FLAGS_TIMESTAMP_COMPLETION   = 1 << 0,
+	IB_CQ_FLAGS_IGNORE_OVERRUN	   = 1 << 1,
 };
 
 struct ib_cq_init_attr {
@@ -866,6 +874,9 @@ enum ib_qp_type {
 enum ib_qp_create_flags {
 	IB_QP_CREATE_IPOIB_UD_LSO		= 1 << 0,
 	IB_QP_CREATE_BLOCK_MULTICAST_LOOPBACK	= 1 << 1,
+	IB_QP_CREATE_CROSS_CHANNEL              = 1 << 2,
+	IB_QP_CREATE_MANAGED_SEND               = 1 << 3,
+	IB_QP_CREATE_MANAGED_RECV               = 1 << 4,
 	IB_QP_CREATE_NETIF_QP			= 1 << 5,
 	IB_QP_CREATE_SIGNATURE_EN		= 1 << 6,
 	IB_QP_CREATE_USE_GFP_NOIO		= 1 << 7,
-- 
1.7.12.4

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH V1 3/3] IB/mlx5: Add driver cross-channel support
       [not found] ` <1450606571-15877-1-git-send-email-leon-2ukJVAZIZ/Y@public.gmane.org>
  2015-12-20 10:16   ` [PATCH V1 1/3] IB/core: Align coding style of ib_device_cap_flags structure Leon Romanovsky
  2015-12-20 10:16   ` [PATCH V1 2/3] IB/core: Add cross-channel support Leon Romanovsky
@ 2015-12-20 10:16   ` Leon Romanovsky
  2015-12-24  4:42   ` [PATCH V1 0/3] Add " Doug Ledford
  2015-12-24  8:02   ` Or Gerlitz
  4 siblings, 0 replies; 20+ messages in thread
From: Leon Romanovsky @ 2015-12-20 10:16 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky

From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Add support of cross-channel functionality to mlx5
driver. This includes ability to ignore overrun for CQ
which intended for cross-channel, export device capability and
configure the QP to be sync master/slave queues.

The cross-channel enabled QP supports combination of
three possible properties:
* WQE processing on the receive queue of this QP
* WQE processing on the send queue of this QP
* WQE are supported on the send queue

Reviewed-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/cq.c      |  7 ++++-
 drivers/infiniband/hw/mlx5/main.c    |  3 ++
 drivers/infiniband/hw/mlx5/mlx5_ib.h | 16 +++++++++++
 drivers/infiniband/hw/mlx5/qp.c      | 54 +++++++++++++++++++++++++++++-------
 include/linux/mlx5/qp.h              |  3 ++
 5 files changed, 72 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
index 3dfd287..c363b71 100644
--- a/drivers/infiniband/hw/mlx5/cq.c
+++ b/drivers/infiniband/hw/mlx5/cq.c
@@ -760,7 +760,7 @@ struct ib_cq *mlx5_ib_create_cq(struct ib_device *ibdev,
 	int eqn;
 	int err;
 
-	if (attr->flags)
+	if (check_cq_create_flags(attr->flags))
 		return ERR_PTR(-EINVAL);
 
 	if (entries < 0)
@@ -779,6 +779,7 @@ struct ib_cq *mlx5_ib_create_cq(struct ib_device *ibdev,
 	spin_lock_init(&cq->lock);
 	cq->resize_buf = NULL;
 	cq->resize_umem = NULL;
+	cq->create_flags = attr->flags;
 
 	if (context) {
 		err = create_cq_user(dev, udata, context, cq, entries,
@@ -796,6 +797,10 @@ struct ib_cq *mlx5_ib_create_cq(struct ib_device *ibdev,
 
 	cq->cqe_size = cqe_size;
 	cqb->ctx.cqe_sz_flags = cqe_sz_to_mlx_sz(cqe_size) << 5;
+
+	if (cq->create_flags & IB_CQ_FLAGS_IGNORE_OVERRUN)
+		cqb->ctx.cqe_sz_flags |= (1 << 1);
+
 	cqb->ctx.log_sz_usr_page = cpu_to_be32((ilog2(entries) << 24) | index);
 	err = mlx5_vector2eqn(dev->mdev, vector, &eqn, &irqn);
 	if (err)
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index bdd60a6..be0d994 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -300,6 +300,9 @@ static int mlx5_ib_query_device(struct ib_device *ibdev,
 	props->odp_caps = dev->odp_caps;
 #endif
 
+	if (MLX5_CAP_GEN(mdev, cd))
+		props->device_cap_flags |= IB_DEVICE_CROSS_CHANNEL;
+
 	return 0;
 }
 
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 6333472..fdbd761 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -85,6 +85,10 @@ enum mlx5_ib_mad_ifc_flags {
 	MLX5_MAD_IFC_NET_VIEW		= 4,
 };
 
+enum {
+	MLX5_CROSS_CHANNEL_UUAR         = 0,
+};
+
 struct mlx5_ib_ucontext {
 	struct ib_ucontext	ibucontext;
 	struct list_head	db_page_list;
@@ -242,6 +246,9 @@ struct mlx5_ib_cq_buf {
 enum mlx5_ib_qp_flags {
 	MLX5_IB_QP_BLOCK_MULTICAST_LOOPBACK     = 1 << 0,
 	MLX5_IB_QP_SIGNATURE_HANDLING           = 1 << 1,
+	MLX5_IB_QP_CROSS_CHANNEL		= 1 << 2,
+	MLX5_IB_QP_MANAGED_SEND			= 1 << 3,
+	MLX5_IB_QP_MANAGED_RECV			= 1 << 4,
 };
 
 struct mlx5_umr_wr {
@@ -284,6 +291,7 @@ struct mlx5_ib_cq {
 	struct mlx5_ib_cq_buf  *resize_buf;
 	struct ib_umem	       *resize_umem;
 	int			cqe_size;
+	u32			create_flags;
 };
 
 struct mlx5_ib_srq {
@@ -662,4 +670,12 @@ static inline int is_qp1(enum ib_qp_type qp_type)
 #define MLX5_MAX_UMR_SHIFT 16
 #define MLX5_MAX_UMR_PAGES (1 << MLX5_MAX_UMR_SHIFT)
 
+static inline u32 check_cq_create_flags(u32 flags)
+{
+	/*
+	 * It returns non-zero value for unsupported CQ
+	 * create flags, otherwise it returns zero.
+	 */
+	return (flags & ~IB_CQ_FLAGS_IGNORE_OVERRUN);
+}
 #endif /* MLX5_IB_H */
diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index 307bdbc..c18bf42 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -615,18 +615,23 @@ static int create_user_qp(struct mlx5_ib_dev *dev, struct ib_pd *pd,
 	/*
 	 * TBD: should come from the verbs when we have the API
 	 */
-	uuarn = alloc_uuar(&context->uuari, MLX5_IB_LATENCY_CLASS_HIGH);
-	if (uuarn < 0) {
-		mlx5_ib_dbg(dev, "failed to allocate low latency UUAR\n");
-		mlx5_ib_dbg(dev, "reverting to medium latency\n");
-		uuarn = alloc_uuar(&context->uuari, MLX5_IB_LATENCY_CLASS_MEDIUM);
+	if (qp->flags & MLX5_IB_QP_CROSS_CHANNEL)
+		/* In CROSS_CHANNEL CQ and QP must use the same UAR */
+		uuarn = MLX5_CROSS_CHANNEL_UUAR;
+	else {
+		uuarn = alloc_uuar(&context->uuari, MLX5_IB_LATENCY_CLASS_HIGH);
 		if (uuarn < 0) {
-			mlx5_ib_dbg(dev, "failed to allocate medium latency UUAR\n");
-			mlx5_ib_dbg(dev, "reverting to high latency\n");
-			uuarn = alloc_uuar(&context->uuari, MLX5_IB_LATENCY_CLASS_LOW);
+			mlx5_ib_dbg(dev, "failed to allocate low latency UUAR\n");
+			mlx5_ib_dbg(dev, "reverting to medium latency\n");
+			uuarn = alloc_uuar(&context->uuari, MLX5_IB_LATENCY_CLASS_MEDIUM);
 			if (uuarn < 0) {
-				mlx5_ib_warn(dev, "uuar allocation failed\n");
-				return uuarn;
+				mlx5_ib_dbg(dev, "failed to allocate medium latency UUAR\n");
+				mlx5_ib_dbg(dev, "reverting to high latency\n");
+				uuarn = alloc_uuar(&context->uuari, MLX5_IB_LATENCY_CLASS_LOW);
+				if (uuarn < 0) {
+					mlx5_ib_warn(dev, "uuar allocation failed\n");
+					return uuarn;
+				}
 			}
 		}
 	}
@@ -880,6 +885,21 @@ static int create_qp_common(struct mlx5_ib_dev *dev, struct ib_pd *pd,
 		}
 	}
 
+	if (init_attr->create_flags &
+			(IB_QP_CREATE_CROSS_CHANNEL |
+			 IB_QP_CREATE_MANAGED_SEND |
+			 IB_QP_CREATE_MANAGED_RECV)) {
+		if (!MLX5_CAP_GEN(mdev, cd)) {
+			mlx5_ib_dbg(dev, "cross-channel isn't supported\n");
+			return -EINVAL;
+		}
+		if (init_attr->create_flags & IB_QP_CREATE_CROSS_CHANNEL)
+			qp->flags |= MLX5_IB_QP_CROSS_CHANNEL;
+		if (init_attr->create_flags & IB_QP_CREATE_MANAGED_SEND)
+			qp->flags |= MLX5_IB_QP_MANAGED_SEND;
+		if (init_attr->create_flags & IB_QP_CREATE_MANAGED_RECV)
+			qp->flags |= MLX5_IB_QP_MANAGED_RECV;
+	}
 	if (init_attr->sq_sig_type == IB_SIGNAL_ALL_WR)
 		qp->sq_signal_bits = MLX5_WQE_CTRL_CQ_UPDATE;
 
@@ -954,6 +974,13 @@ static int create_qp_common(struct mlx5_ib_dev *dev, struct ib_pd *pd,
 	if (qp->flags & MLX5_IB_QP_BLOCK_MULTICAST_LOOPBACK)
 		in->ctx.flags_pd |= cpu_to_be32(MLX5_QP_BLOCK_MCAST);
 
+	if (qp->flags & MLX5_IB_QP_CROSS_CHANNEL)
+		in->ctx.params2 |= cpu_to_be32(MLX5_QP_BIT_CC_MASTER);
+	if (qp->flags & MLX5_IB_QP_MANAGED_SEND)
+		in->ctx.params2 |= cpu_to_be32(MLX5_QP_BIT_CC_SLAVE_SEND);
+	if (qp->flags & MLX5_IB_QP_MANAGED_RECV)
+		in->ctx.params2 |= cpu_to_be32(MLX5_QP_BIT_CC_SLAVE_RECV);
+
 	if (qp->scat_cqe && is_connected(init_attr->qp_type)) {
 		int rcqe_sz;
 		int scqe_sz;
@@ -3110,6 +3137,13 @@ int mlx5_ib_query_qp(struct ib_qp *ibqp, struct ib_qp_attr *qp_attr, int qp_attr
 	if (qp->flags & MLX5_IB_QP_BLOCK_MULTICAST_LOOPBACK)
 		qp_init_attr->create_flags |= IB_QP_CREATE_BLOCK_MULTICAST_LOOPBACK;
 
+	if (qp->flags & MLX5_IB_QP_CROSS_CHANNEL)
+		qp_init_attr->create_flags |= IB_QP_CREATE_CROSS_CHANNEL;
+	if (qp->flags & MLX5_IB_QP_MANAGED_SEND)
+		qp_init_attr->create_flags |= IB_QP_CREATE_MANAGED_SEND;
+	if (qp->flags & MLX5_IB_QP_MANAGED_RECV)
+		qp_init_attr->create_flags |= IB_QP_CREATE_MANAGED_RECV;
+
 	qp_init_attr->sq_sig_type = qp->sq_signal_bits & MLX5_WQE_CTRL_CQ_UPDATE ?
 		IB_SIGNAL_ALL_WR : IB_SIGNAL_REQ_WR;
 
diff --git a/include/linux/mlx5/qp.h b/include/linux/mlx5/qp.h
index f079fb1..511e03b 100644
--- a/include/linux/mlx5/qp.h
+++ b/include/linux/mlx5/qp.h
@@ -130,6 +130,9 @@ enum {
 	MLX5_QP_BIT_RWE				= 1 << 14,
 	MLX5_QP_BIT_RAE				= 1 << 13,
 	MLX5_QP_BIT_RIC				= 1 <<	4,
+	MLX5_QP_BIT_CC_SLAVE_RECV		= 1 <<  2,
+	MLX5_QP_BIT_CC_SLAVE_SEND		= 1 <<  1,
+	MLX5_QP_BIT_CC_MASTER			= 1 <<  0
 };
 
 enum {
-- 
1.7.12.4

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH V1 1/3] IB/core: Align coding style of ib_device_cap_flags structure
       [not found]     ` <1450606571-15877-2-git-send-email-leon-2ukJVAZIZ/Y@public.gmane.org>
@ 2015-12-21  6:22       ` ira.weiny
       [not found]         ` <20151221062252.GE3860-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: ira.weiny @ 2015-12-21  6:22 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky

On Sun, Dec 20, 2015 at 12:16:09PM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Modify enum ib_device_cap_flags such that other patches which add new
> enum values pass strict checkpatch.pl checks.
> 
> Reviewed-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>  include/rdma/ib_verbs.h | 60 ++++++++++++++++++++++++-------------------------
>  1 file changed, 30 insertions(+), 30 deletions(-)
> 
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index 9a68a19..bcf40ad 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -105,24 +105,24 @@ enum rdma_link_layer {
>  };
>  
>  enum ib_device_cap_flags {
> -	IB_DEVICE_RESIZE_MAX_WR		= 1,
> -	IB_DEVICE_BAD_PKEY_CNTR		= (1<<1),
> -	IB_DEVICE_BAD_QKEY_CNTR		= (1<<2),
> -	IB_DEVICE_RAW_MULTI		= (1<<3),
> -	IB_DEVICE_AUTO_PATH_MIG		= (1<<4),
> -	IB_DEVICE_CHANGE_PHY_PORT	= (1<<5),
> -	IB_DEVICE_UD_AV_PORT_ENFORCE	= (1<<6),
> -	IB_DEVICE_CURR_QP_STATE_MOD	= (1<<7),
> -	IB_DEVICE_SHUTDOWN_PORT		= (1<<8),
> -	IB_DEVICE_INIT_TYPE		= (1<<9),
> -	IB_DEVICE_PORT_ACTIVE_EVENT	= (1<<10),
> -	IB_DEVICE_SYS_IMAGE_GUID	= (1<<11),
> -	IB_DEVICE_RC_RNR_NAK_GEN	= (1<<12),
> -	IB_DEVICE_SRQ_RESIZE		= (1<<13),
> -	IB_DEVICE_N_NOTIFY_CQ		= (1<<14),
> -	IB_DEVICE_LOCAL_DMA_LKEY	= (1<<15),
> -	IB_DEVICE_RESERVED		= (1<<16), /* old SEND_W_INV */
> -	IB_DEVICE_MEM_WINDOW		= (1<<17),
> +	IB_DEVICE_RESIZE_MAX_WR		= (1 << 0),

NIT: Shouldn't we just use the BIT macro?

	IB_DEVICE_RESIZE_MAX_WR		= BIT(0),

Ira

> +	IB_DEVICE_BAD_PKEY_CNTR		= (1 << 1),
> +	IB_DEVICE_BAD_QKEY_CNTR		= (1 << 2),
> +	IB_DEVICE_RAW_MULTI		= (1 << 3),
> +	IB_DEVICE_AUTO_PATH_MIG		= (1 << 4),
> +	IB_DEVICE_CHANGE_PHY_PORT	= (1 << 5),
> +	IB_DEVICE_UD_AV_PORT_ENFORCE	= (1 << 6),
> +	IB_DEVICE_CURR_QP_STATE_MOD	= (1 << 7),
> +	IB_DEVICE_SHUTDOWN_PORT		= (1 << 8),
> +	IB_DEVICE_INIT_TYPE		= (1 << 9),
> +	IB_DEVICE_PORT_ACTIVE_EVENT	= (1 << 10),
> +	IB_DEVICE_SYS_IMAGE_GUID	= (1 << 11),
> +	IB_DEVICE_RC_RNR_NAK_GEN	= (1 << 12),
> +	IB_DEVICE_SRQ_RESIZE		= (1 << 13),
> +	IB_DEVICE_N_NOTIFY_CQ		= (1 << 14),
> +	IB_DEVICE_LOCAL_DMA_LKEY	= (1 << 15),
> +	IB_DEVICE_RESERVED		= (1 << 16), /* old SEND_W_INV */
> +	IB_DEVICE_MEM_WINDOW		= (1 << 17),
>  	/*
>  	 * Devices should set IB_DEVICE_UD_IP_SUM if they support
>  	 * insertion of UDP and TCP checksum on outgoing UD IPoIB
> @@ -130,18 +130,18 @@ enum ib_device_cap_flags {
>  	 * incoming messages.  Setting this flag implies that the
>  	 * IPoIB driver may set NETIF_F_IP_CSUM for datagram mode.
>  	 */
> -	IB_DEVICE_UD_IP_CSUM		= (1<<18),
> -	IB_DEVICE_UD_TSO		= (1<<19),
> -	IB_DEVICE_XRC			= (1<<20),
> -	IB_DEVICE_MEM_MGT_EXTENSIONS	= (1<<21),
> -	IB_DEVICE_BLOCK_MULTICAST_LOOPBACK = (1<<22),
> -	IB_DEVICE_MEM_WINDOW_TYPE_2A	= (1<<23),
> -	IB_DEVICE_MEM_WINDOW_TYPE_2B	= (1<<24),
> -	IB_DEVICE_RC_IP_CSUM		= (1<<25),
> -	IB_DEVICE_RAW_IP_CSUM		= (1<<26),
> -	IB_DEVICE_MANAGED_FLOW_STEERING = (1<<29),
> -	IB_DEVICE_SIGNATURE_HANDOVER	= (1<<30),
> -	IB_DEVICE_ON_DEMAND_PAGING	= (1<<31),
> +	IB_DEVICE_UD_IP_CSUM		= (1 << 18),
> +	IB_DEVICE_UD_TSO		= (1 << 19),
> +	IB_DEVICE_XRC			= (1 << 20),
> +	IB_DEVICE_MEM_MGT_EXTENSIONS	= (1 << 21),
> +	IB_DEVICE_BLOCK_MULTICAST_LOOPBACK = (1 << 22),
> +	IB_DEVICE_MEM_WINDOW_TYPE_2A	= (1 << 23),
> +	IB_DEVICE_MEM_WINDOW_TYPE_2B	= (1 << 24),
> +	IB_DEVICE_RC_IP_CSUM		= (1 << 25),
> +	IB_DEVICE_RAW_IP_CSUM		= (1 << 26),
> +	IB_DEVICE_MANAGED_FLOW_STEERING = (1 << 29),
> +	IB_DEVICE_SIGNATURE_HANDOVER	= (1 << 30),
> +	IB_DEVICE_ON_DEMAND_PAGING	= (1 << 31),
>  };
>  
>  enum ib_signature_prot_cap {
> -- 
> 1.7.12.4
> 
> --
> 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
--
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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH V1 1/3] IB/core: Align coding style of ib_device_cap_flags structure
       [not found]         ` <20151221062252.GE3860-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
@ 2015-12-21  6:37           ` Leon Romanovsky
       [not found]             ` <CALq1K=J2e=aw1QuJHGhFFcKkY391myz11r7tG-H+fYbQEyr+Gw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Leon Romanovsky @ 2015-12-21  6:37 UTC (permalink / raw)
  To: ira.weiny; +Cc: Doug Ledford, linux-rdma, Leon Romanovsky

On Mon, Dec 21, 2015 at 8:22 AM, ira.weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> On Sun, Dec 20, 2015 at 12:16:09PM +0200, Leon Romanovsky wrote:
>> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>
>> Modify enum ib_device_cap_flags such that other patches which add new
>> enum values pass strict checkpatch.pl checks.
>>
....
>> -     IB_DEVICE_RESERVED              = (1<<16), /* old SEND_W_INV */
>> -     IB_DEVICE_MEM_WINDOW            = (1<<17),
>> +     IB_DEVICE_RESIZE_MAX_WR         = (1 << 0),
>
> NIT: Shouldn't we just use the BIT macro?
>
>         IB_DEVICE_RESIZE_MAX_WR         = BIT(0),
You are right and it is a preferred way for me too, however the
downside of such change will be one of two:
1. Change this structure only => we will have style mix of BITs and
shifts in the same file. IMHO it looks awful.
2. Change the whole file => the work with "git blame" will be less
straightforward.

I will do the change across whole file, If Doug accepts such change.
--
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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH V1 1/3] IB/core: Align coding style of ib_device_cap_flags structure
       [not found]             ` <CALq1K=J2e=aw1QuJHGhFFcKkY391myz11r7tG-H+fYbQEyr+Gw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2015-12-21  6:52               ` Or Gerlitz
       [not found]                 ` <CAJ3xEMjcusZHb5PRT3ziv-sSYM70U+QtNbjhEYNhvL853Q57Qw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2015-12-21  8:03               ` Christoph Hellwig
  1 sibling, 1 reply; 20+ messages in thread
From: Or Gerlitz @ 2015-12-21  6:52 UTC (permalink / raw)
  To: Leon Romanovsky, ira.weiny; +Cc: Doug Ledford, linux-rdma, Leon Romanovsky

On Mon, Dec 21, 2015 at 8:37 AM, Leon Romanovsky <leon-2ukJVAZIZ/Y@public.gmane.org> wrote:
> On Mon, Dec 21, 2015 at 8:22 AM, ira.weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>> On Sun, Dec 20, 2015 at 12:16:09PM +0200, Leon Romanovsky wrote:
>>> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

>>> Modify enum ib_device_cap_flags such that other patches which add new
>>> enum values pass strict checkpatch.pl checks.

>>> -     IB_DEVICE_RESERVED              = (1<<16), /* old SEND_W_INV */
>>> -     IB_DEVICE_MEM_WINDOW            = (1<<17),
>>> +     IB_DEVICE_RESIZE_MAX_WR         = (1 << 0),

> 2. Change the whole file => the work with "git blame" will be less
> straightforward.

Agree.

Leon, I don't think we need to take checkpatch-ing of things to that level.

Indeed, we should make sure that whole new enums and such are done right --
but avoid touching core structs/enums in a manner that disallows
simple git blaming of things, which is very useful for new comers and
old suffers.

For example, the networking community keeps adding IFF_YYY values to
include/linux/netdevice.h :: enum netdev_priv_flags without fixing
checkpatch complaint on missing spaces before/after the << shift sign.

> I will do the change across whole file, If Doug accepts such change.

Please don't... simple git blame is very powerful tool for kernel
developers everyday work.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH V1 1/3] IB/core: Align coding style of ib_device_cap_flags structure
       [not found]                 ` <CAJ3xEMjcusZHb5PRT3ziv-sSYM70U+QtNbjhEYNhvL853Q57Qw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2015-12-21  7:27                   ` Leon Romanovsky
       [not found]                     ` <CALq1K=J-Zmb1zwh-ak+wYxo-xooGGYhCjiCOC_Z-4dYVGgkVsA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Leon Romanovsky @ 2015-12-21  7:27 UTC (permalink / raw)
  To: Or Gerlitz; +Cc: ira.weiny, Doug Ledford, linux-rdma, Leon Romanovsky

On Mon, Dec 21, 2015 at 8:52 AM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, Dec 21, 2015 at 8:37 AM, Leon Romanovsky <leon-2ukJVAZIZ/Y@public.gmane.org> wrote:
>> On Mon, Dec 21, 2015 at 8:22 AM, ira.weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>>> On Sun, Dec 20, 2015 at 12:16:09PM +0200, Leon Romanovsky wrote:
>>>> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
>>>> Modify enum ib_device_cap_flags such that other patches which add new
>>>> enum values pass strict checkpatch.pl checks.
>
>>>> -     IB_DEVICE_RESERVED              = (1<<16), /* old SEND_W_INV */
>>>> -     IB_DEVICE_MEM_WINDOW            = (1<<17),
>>>> +     IB_DEVICE_RESIZE_MAX_WR         = (1 << 0),
>
>> 2. Change the whole file => the work with "git blame" will be less
>> straightforward.
>
> Agree.
>
> Leon, I don't think we need to take checkpatch-ing of things to that level.
>
> Indeed, we should make sure that whole new enums and such are done right --
> but avoid touching core structs/enums in a manner that disallows
> simple git blaming of things, which is very useful for new comers and
> old suffers.
There are no doubts that standalone fixing checkpatch errors is more
suitable to staging subsystem.
In our case, it is part of coming changes in that structure. such
change serves specific goal to minimize
the possibility of error by seeing clean output from static analyser tool.

For the new comers and old suffers checkpatch tool is extremely useful too.

>
>> I will do the change across whole file, If Doug accepts such change.
>
> Please don't... simple git blame is very powerful tool for kernel
> developers everyday work.
It is an open question what we prefer more: history with chance to
sneak a mistake or less history with less chances to make a mistake.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH V1 1/3] IB/core: Align coding style of ib_device_cap_flags structure
       [not found]                     ` <CALq1K=J-Zmb1zwh-ak+wYxo-xooGGYhCjiCOC_Z-4dYVGgkVsA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2015-12-21  7:40                       ` Or Gerlitz
       [not found]                         ` <CAJ3xEMjioX_fFZptJSLLC_yv1NszPwJET3zWj8+1x5fajXfBCQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Or Gerlitz @ 2015-12-21  7:40 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: ira.weiny, Doug Ledford, linux-rdma, Leon Romanovsky

On Mon, Dec 21, 2015 at 9:27 AM, Leon Romanovsky <leon-2ukJVAZIZ/Y@public.gmane.org> wrote:
> On Mon, Dec 21, 2015 at 8:52 AM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Mon, Dec 21, 2015 at 8:37 AM, Leon Romanovsky <leon-2ukJVAZIZ/Y@public.gmane.org> wrote:
>>> On Mon, Dec 21, 2015 at 8:22 AM, ira.weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>>>> On Sun, Dec 20, 2015 at 12:16:09PM +0200, Leon Romanovsky wrote:
>>>>> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>
>>>>> Modify enum ib_device_cap_flags such that other patches which add new
>>>>> enum values pass strict checkpatch.pl checks.
>>
>>>>> -     IB_DEVICE_RESERVED              = (1<<16), /* old SEND_W_INV */
>>>>> -     IB_DEVICE_MEM_WINDOW            = (1<<17),
>>>>> +     IB_DEVICE_RESIZE_MAX_WR         = (1 << 0),
>>
>>> 2. Change the whole file => the work with "git blame" will be less
>>> straightforward.
>>
>> Agree.
>>
>> Leon, I don't think we need to take checkpatch-ing of things to that level.
>>
>> Indeed, we should make sure that whole new enums and such are done right --
>> but avoid touching core structs/enums in a manner that disallows
>> simple git blaming of things, which is very useful for new comers and
>> old suffers.

> There are no doubts that standalone fixing checkpatch errors is more
> suitable to staging subsystem.

Agree

> In our case, it is part of coming changes in that structure. such
> change serves specific goal to minimize the possibility of error
> by seeing clean output from static analyser tool.

Disagree.

What future bugs are you envisioning by let this 10y old header file
keep having some checkpatch issues on few of the major enums?!

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH V1 1/3] IB/core: Align coding style of ib_device_cap_flags structure
       [not found]                         ` <CAJ3xEMjioX_fFZptJSLLC_yv1NszPwJET3zWj8+1x5fajXfBCQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2015-12-21  7:53                           ` Leon Romanovsky
       [not found]                             ` <567A70C9.9090801@mellanox.com>
  0 siblings, 1 reply; 20+ messages in thread
From: Leon Romanovsky @ 2015-12-21  7:53 UTC (permalink / raw)
  To: Or Gerlitz; +Cc: ira.weiny, Doug Ledford, linux-rdma, Leon Romanovsky

On Mon, Dec 21, 2015 at 9:40 AM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, Dec 21, 2015 at 9:27 AM, Leon Romanovsky <leon-2ukJVAZIZ/Y@public.gmane.org> wrote:
>> On Mon, Dec 21, 2015 at 8:52 AM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On Mon, Dec 21, 2015 at 8:37 AM, Leon Romanovsky <leon-2ukJVAZIZ/Y@public.gmane.org> wrote:
>>>> On Mon, Dec 21, 2015 at 8:22 AM, ira.weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>>>>> On Sun, Dec 20, 2015 at 12:16:09PM +0200, Leon Romanovsky wrote:
>>>>>> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>>
>>>>>> Modify enum ib_device_cap_flags such that other patches which add new
>>>>>> enum values pass strict checkpatch.pl checks.
>>>
>>>>>> -     IB_DEVICE_RESERVED              = (1<<16), /* old SEND_W_INV */
>>>>>> -     IB_DEVICE_MEM_WINDOW            = (1<<17),
>>>>>> +     IB_DEVICE_RESIZE_MAX_WR         = (1 << 0),
>>>
>>>> 2. Change the whole file => the work with "git blame" will be less
>>>> straightforward.
>>>
>>> Agree.
>>>
>>> Leon, I don't think we need to take checkpatch-ing of things to that level.
>>>
>>> Indeed, we should make sure that whole new enums and such are done right --
>>> but avoid touching core structs/enums in a manner that disallows
>>> simple git blaming of things, which is very useful for new comers and
>>> old suffers.
>
>> There are no doubts that standalone fixing checkpatch errors is more
>> suitable to staging subsystem.
>
> Agree
>
>> In our case, it is part of coming changes in that structure. such
>> change serves specific goal to minimize the possibility of error
>> by seeing clean output from static analyser tool.
>
> Disagree.
>
> What future bugs are you envisioning by let this 10y old header file
> keep having some checkpatch issues on few of the major enums?!
If I knew the future, I would be able to answer.

I think that you expressed your opinion very clearly, let's wait for
Doug's response on such changes.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH V1 1/3] IB/core: Align coding style of ib_device_cap_flags structure
       [not found]             ` <CALq1K=J2e=aw1QuJHGhFFcKkY391myz11r7tG-H+fYbQEyr+Gw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2015-12-21  6:52               ` Or Gerlitz
@ 2015-12-21  8:03               ` Christoph Hellwig
       [not found]                 ` <20151221080346.GA21779-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
  1 sibling, 1 reply; 20+ messages in thread
From: Christoph Hellwig @ 2015-12-21  8:03 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: ira.weiny, Doug Ledford, linux-rdma, Leon Romanovsky

On Mon, Dec 21, 2015 at 08:37:26AM +0200, Leon Romanovsky wrote:
> You are right and it is a preferred way for me too, however the
> downside of such change will be one of two:
> 1. Change this structure only => we will have style mix of BITs and
> shifts in the same file. IMHO it looks awful.
> 2. Change the whole file => the work with "git blame" will be less
> straightforward.

Honestly, the BIT macros are horribly, and anyone who thinks it's useful
really should read a book on computer architectured and one on C.

Also the capabilities are used by userspace, so they will need to move
to a uapi heder sooner or later, where this stupid macro isn't even
available.
--
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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH V1 1/3] IB/core: Align coding style of ib_device_cap_flags structure
       [not found]                 ` <20151221080346.GA21779-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
@ 2015-12-21 16:36                   ` ira.weiny
       [not found]                     ` <20151221163603.GF3860-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: ira.weiny @ 2015-12-21 16:36 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Leon Romanovsky, Doug Ledford, linux-rdma, Leon Romanovsky

On Mon, Dec 21, 2015 at 12:03:46AM -0800, Christoph Hellwig wrote:
> On Mon, Dec 21, 2015 at 08:37:26AM +0200, Leon Romanovsky wrote:
> > You are right and it is a preferred way for me too, however the
> > downside of such change will be one of two:
> > 1. Change this structure only => we will have style mix of BITs and
> > shifts in the same file. IMHO it looks awful.
> > 2. Change the whole file => the work with "git blame" will be less
> > straightforward.
> 
> Honestly, the BIT macros are horribly, and anyone who thinks it's useful
> really should read a book on computer architectured and one on C.

It would be nice if we were not having to do this for staging then.  Also
perhaps it should be removed from checkpatch --strict?

I'm not a big fan of everything checkpatch does, this being one of them, but
Leon was trying to do the right thing here.

Where are the guidelines for when one can ignore checkpatch and when they can
not?  It would be nice to know when we can "be developers" vs "being robots to
some tool".

I await Dougs guidance.

Ira

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH V1 1/3] IB/core: Align coding style of ib_device_cap_flags structure
       [not found]                     ` <20151221163603.GF3860-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
@ 2015-12-21 20:20                       ` Christoph Hellwig
  2015-12-24  3:31                       ` Doug Ledford
  1 sibling, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2015-12-21 20:20 UTC (permalink / raw)
  To: ira.weiny
  Cc: Christoph Hellwig, Leon Romanovsky, Doug Ledford, linux-rdma,
	Leon Romanovsky

On Mon, Dec 21, 2015 at 11:36:03AM -0500, ira.weiny wrote:
> It would be nice if we were not having to do this for staging then.  Also
> perhaps it should be removed from checkpatch --strict?

Don't use checkpatch --strict ever.  It's full of weird items that
defintively don't apply to the majority of the kernel code base.

> Where are the guidelines for when one can ignore checkpatch and when they can
> not?  It would be nice to know when we can "be developers" vs "being robots to
> some tool".

I think checkpatch is generally useful, and the errors without
--strict are something we I haven't found any false positives.

The warnings are about 90% useful but something are just weird.  For
--strict all bets are off.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH V1 1/3] IB/core: Align coding style of ib_device_cap_flags structure
       [not found]                               ` <567A70C9.9090801-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
@ 2015-12-23 10:23                                 ` Leon Romanovsky
  0 siblings, 0 replies; 20+ messages in thread
From: Leon Romanovsky @ 2015-12-23 10:23 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Or Gerlitz, ira.weiny, Doug Ledford, linux-rdma, Leon Romanovsky

On Wed, Dec 23, 2015 at 12:00:41PM +0200, Or Gerlitz wrote:
> On 12/21/2015 9:53 AM, Leon Romanovsky wrote:
> >On Mon, Dec 21, 2015 at 9:40 AM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>On Mon, Dec 21, 2015 at 9:27 AM, Leon Romanovsky <leon-2ukJVAZIZ/Y@public.gmane.org> wrote:
> >>>On Mon, Dec 21, 2015 at 8:52 AM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>>>On Mon, Dec 21, 2015 at 8:37 AM, Leon Romanovsky <leon-2ukJVAZIZ/Y@public.gmane.org> wrote:
> >>>>>On Mon, Dec 21, 2015 at 8:22 AM, ira.weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> >>>>>>On Sun, Dec 20, 2015 at 12:16:09PM +0200, Leon Romanovsky wrote:
> >>>>>>>From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >>>>>>>Modify enum ib_device_cap_flags such that other patches which add new
> >>>>>>>enum values pass strict checkpatch.pl checks.
> >>>>>>>-     IB_DEVICE_RESERVED              = (1<<16), /* old SEND_W_INV */
> >>>>>>>-     IB_DEVICE_MEM_WINDOW            = (1<<17),
> >>>>>>>+     IB_DEVICE_RESIZE_MAX_WR         = (1 << 0),
> >>>>>2. Change the whole file => the work with "git blame" will be less
> >>>>>straightforward.
> >>>>Agree.
> >>>>
> >>>>Leon, I don't think we need to take checkpatch-ing of things to that level.
> >>>>
> >>>>Indeed, we should make sure that whole new enums and such are done right --
> >>>>but avoid touching core structs/enums in a manner that disallows
> >>>>simple git blaming of things, which is very useful for new comers and
> >>>>old suffers.
> >>>There are no doubts that standalone fixing checkpatch errors is more
> >>>suitable to staging subsystem.
> >>Agree
> >>
> >>>In our case, it is part of coming changes in that structure. such
> >>>change serves specific goal to minimize the possibility of error
> >>>by seeing clean output from static analyser tool.
> >>Disagree.
> >>
> >>What future bugs are you envisioning by let this 10y old header file
> >>keep having some checkpatch issues on few of the major enums?!
> >If I knew the future, I would be able to answer.
> 
> Use your common-sense and experience and maybe even some credit that you can
> give to the 10x larger and super active networking community, you should be
> able to provide some answer if you believe this is the  right way to go.
My common-sense and experience suggest me that the proposed patch
doesn't worth investing so much time. I'll drop it in the next version
of this patchset.

> 
> >I think that you expressed your opinion very clearly, let's wait for Doug's response on such changes.
> >
> 
> I expressed my opinion and I ask you Qs. Christoph also made more comments,
> if you think this is the way to go, respond.
--
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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH V1 1/3] IB/core: Align coding style of ib_device_cap_flags structure
       [not found]                     ` <20151221163603.GF3860-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
  2015-12-21 20:20                       ` Christoph Hellwig
@ 2015-12-24  3:31                       ` Doug Ledford
  1 sibling, 0 replies; 20+ messages in thread
From: Doug Ledford @ 2015-12-24  3:31 UTC (permalink / raw)
  To: ira.weiny, Christoph Hellwig; +Cc: Leon Romanovsky, linux-rdma, Leon Romanovsky

[-- Attachment #1: Type: text/plain, Size: 1712 bytes --]

On 12/21/2015 11:36 AM, ira.weiny wrote:
> On Mon, Dec 21, 2015 at 12:03:46AM -0800, Christoph Hellwig wrote:
>> On Mon, Dec 21, 2015 at 08:37:26AM +0200, Leon Romanovsky wrote:
>>> You are right and it is a preferred way for me too, however the
>>> downside of such change will be one of two:
>>> 1. Change this structure only => we will have style mix of BITs and
>>> shifts in the same file. IMHO it looks awful.
>>> 2. Change the whole file => the work with "git blame" will be less
>>> straightforward.
>>
>> Honestly, the BIT macros are horribly, and anyone who thinks it's useful
>> really should read a book on computer architectured and one on C.
> 
> It would be nice if we were not having to do this for staging then.  Also
> perhaps it should be removed from checkpatch --strict?
>
> I'm not a big fan of everything checkpatch does, this being one of them, but
> Leon was trying to do the right thing here.
> 
> Where are the guidelines for when one can ignore checkpatch and when they can
> not?  It would be nice to know when we can "be developers" vs "being robots to
> some tool".
> 
> I await Dougs guidance.

Checkpatch?  What is this thing you speak of?  ;-)

I use it, but not even all the time, and certainly not religiously.  And
I've never used strict mode.  Even in non-strict mode it flags stuff
that I ignore.

As for the BIT macros, I haven't looked at their implementation.  If
Christoph thinks they are crap, then absent my own opinion on the issue,
which I'm not inclined to go form at 10:30pm on Dec. 23rd, I'll trust
his judgment ;-)

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH V1 0/3] Add cross-channel support
       [not found] ` <1450606571-15877-1-git-send-email-leon-2ukJVAZIZ/Y@public.gmane.org>
                     ` (2 preceding siblings ...)
  2015-12-20 10:16   ` [PATCH V1 3/3] IB/mlx5: Add driver " Leon Romanovsky
@ 2015-12-24  4:42   ` Doug Ledford
  2015-12-24  8:02   ` Or Gerlitz
  4 siblings, 0 replies; 20+ messages in thread
From: Doug Ledford @ 2015-12-24  4:42 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky

[-- Attachment #1: Type: text/plain, Size: 1432 bytes --]

On 12/20/2015 05:16 AM, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> This patchset adds cross-channel support.
> 
> The cross-channel feature allows to execute WQEs that involve
> synchronization of I/O operations’ on different QPs.
> 
> This capability enables to program complex flows with a single
> function call, hereby significantly reducing overhead associated
> with I/O processing.
> 
> Changes from v0:
>   * Set UAR to be the same for QP and CQ.
> 
> Leon Romanovsky (3):
>   IB/core: Align coding style of ib_device_cap_flags structure
>   IB/core: Add cross-channel support
>   IB/mlx5: Add driver cross-channel support
> 
>  drivers/infiniband/core/uverbs_cmd.c |  5 ++-
>  drivers/infiniband/hw/mlx5/cq.c      |  7 +++-
>  drivers/infiniband/hw/mlx5/main.c    |  3 ++
>  drivers/infiniband/hw/mlx5/mlx5_ib.h | 16 ++++++++
>  drivers/infiniband/hw/mlx5/qp.c      | 54 ++++++++++++++++++++++-----
>  include/linux/mlx5/qp.h              |  3 ++
>  include/rdma/ib_verbs.h              | 71 +++++++++++++++++++++---------------
>  7 files changed, 117 insertions(+), 42 deletions(-)
> 

I took the series as is.  Please make sure to resubmit the libibverbs
portion of these changes with the requested man page updates.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH V1 0/3] Add cross-channel support
       [not found] ` <1450606571-15877-1-git-send-email-leon-2ukJVAZIZ/Y@public.gmane.org>
                     ` (3 preceding siblings ...)
  2015-12-24  4:42   ` [PATCH V1 0/3] Add " Doug Ledford
@ 2015-12-24  8:02   ` Or Gerlitz
       [not found]     ` <567BA695.8050403-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
  4 siblings, 1 reply; 20+ messages in thread
From: Or Gerlitz @ 2015-12-24  8:02 UTC (permalink / raw)
  To: Leon Romanovsky, dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky

On 12/24/2015 5:31 AM, Doug Ledford wrote:
> On 12/20/2015 12:16 PM, Leon Romanovsky wrote:
>> Leon Romanovsky (3):
>>    IB/core: Align coding style of ib_device_cap_flags structure
>>    IB/core: Add cross-channel support
>>    IB/mlx5: Add driver cross-channel support
>>
>>   drivers/infiniband/core/uverbs_cmd.c |  5 ++-
>>   drivers/infiniband/hw/mlx5/cq.c      |  7 +++-
>>   drivers/infiniband/hw/mlx5/main.c    |  3 ++
>>   drivers/infiniband/hw/mlx5/mlx5_ib.h | 16 ++++++++
>>   drivers/infiniband/hw/mlx5/qp.c      | 54 ++++++++++++++++++++++-----
>>   include/linux/mlx5/qp.h              |  3 ++
>>   include/rdma/ib_verbs.h              | 71 +++++++++++++++++++++---------------
>>   7 files changed, 117 insertions(+), 42 deletions(-)
>>
>
> I took the series as is.  Please make sure to resubmit the libibverbs portion of these changes with the requested man page updates.
>
Doug,

Wait.

We had consensus among the reviewers that the 1st patch ("IB/core: Align 
coding style of ib_device_cap_flags structure") is wrong cleanup which 
basically is (1) unneeded (2) creates more damage (git blame and such,  
non-applicable to uapi, more) than benefit, etc -- finally Leon was 
convinced too [1].

Leon will re-spin in the coming 1-2 hours V2, could please pick it 
instead of V1, when people agree on direction X and you are not against 
it, lets do X and not Y.

thanks,

Or.

[1] http://marc.info/?t=145061068000003&r=1&w=2


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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH V1 0/3] Add cross-channel support
       [not found]     ` <567BA695.8050403-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
@ 2015-12-24 10:00       ` Christoph Hellwig
       [not found]         ` <20151224100001.GA21387-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Christoph Hellwig @ 2015-12-24 10:00 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Leon Romanovsky, dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky

On Thu, Dec 24, 2015 at 10:02:29AM +0200, Or Gerlitz wrote:
> We had consensus among the reviewers that the 1st patch ("IB/core: Align
> coding style of ib_device_cap_flags structure") is wrong cleanup which
> basically is (1) unneeded (2) creates more damage (git blame and such,
> non-applicable to uapi, more) than benefit, etc -- finally Leon was
> convinced too [1].

It's not really an issue vs uapi.  Using the the wierd BIT() macro
would have been, but without it I think this cleanup is ok, even if I
personally wouldn't have done it.  git-blame isn't really a major
issue either, as you can blame past revisions.

> Leon will re-spin in the coming 1-2 hours V2, could please pick it instead
> of V1, when people agree on direction X and you are not against it, lets do
> X and not Y.

It would be great if we could stop rebasing whats already in the tree
for the benefit of everyone building on top of this.  For example just
finished rebasing my series to move many constants includin this one
to the uapi headers, and I'd hate to rebase it once again now that
the dust has settled.
--
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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH V1 0/3] Add cross-channel support
       [not found]         ` <20151224100001.GA21387-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
@ 2015-12-24 10:41           ` Or Gerlitz
       [not found]             ` <567BCBD1.708-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
  0 siblings, 1 reply; 20+ messages in thread
From: Or Gerlitz @ 2015-12-24 10:41 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Leon Romanovsky, dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky

On 12/24/2015 12:00 PM, Christoph Hellwig wrote:
> On Thu, Dec 24, 2015 at 10:02:29AM +0200, Or Gerlitz wrote:
>> We had consensus among the reviewers that the 1st patch ("IB/core: Align
>> coding style of ib_device_cap_flags structure") is wrong cleanup which
>> basically is (1) unneeded (2) creates more damage (git blame and such,
>> non-applicable to uapi, more) than benefit, etc -- finally Leon was
>> convinced too [1].
> It's not really an issue vs uapi.  Using the the wierd BIT() macro
> would have been, but without it I think this cleanup is ok, even if I
> personally wouldn't have done it.  git-blame isn't really a major
> issue either, as you can blame past revisions.

I would personally wouldn't done cleanup either and I managed to 
convinced Leon to drop it, so we had concensus among the developers, the 
maintainer didn't have other opinion and he took the wrong step -- so 
we're asking to fix, that's all.
>
>> Leon will re-spin in the coming 1-2 hours V2, could please pick it instead
>> of V1, when people agree on direction X and you are not against it, lets do
>> X and not Y.
> It would be great if we could stop rebasing whats already in the tree
> for the benefit of everyone building on top of this.  For example just
> finished rebasing my series to move many constants includin this one
> to the uapi headers, and I'd hate to rebase it once again now that
> the dust has settled.

The root issue here is that nothing was picked before 4.4-rc6, so we're 
in a situation where rebases are needed in the own-maintainer tree 
(github) to make things right. No way to avoid that.

We should aim that for 4.6 and onward, code for -next will start getting 
in around rc1-2 and then things will be more robust, etc

Or.

Or.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH V1 0/3] Add cross-channel support
       [not found]             ` <567BCBD1.708-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
@ 2015-12-24 16:55               ` Doug Ledford
  0 siblings, 0 replies; 20+ messages in thread
From: Doug Ledford @ 2015-12-24 16:55 UTC (permalink / raw)
  To: Or Gerlitz, Christoph Hellwig
  Cc: Leon Romanovsky, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Leon Romanovsky

[-- Attachment #1: Type: text/plain, Size: 2724 bytes --]

On 12/24/2015 05:41 AM, Or Gerlitz wrote:
> On 12/24/2015 12:00 PM, Christoph Hellwig wrote:
>> On Thu, Dec 24, 2015 at 10:02:29AM +0200, Or Gerlitz wrote:
>>> We had consensus among the reviewers that the 1st patch ("IB/core: Align
>>> coding style of ib_device_cap_flags structure") is wrong cleanup which
>>> basically is (1) unneeded (2) creates more damage (git blame and such,
>>> non-applicable to uapi, more) than benefit, etc -- finally Leon was
>>> convinced too [1].
>> It's not really an issue vs uapi.  Using the the wierd BIT() macro
>> would have been, but without it I think this cleanup is ok, even if I
>> personally wouldn't have done it.  git-blame isn't really a major
>> issue either, as you can blame past revisions.
> 
> I would personally wouldn't done cleanup either and I managed to
> convinced Leon to drop it, so we had concensus among the developers, the
> maintainer didn't have other opinion and he took the wrong step -- so
> we're asking to fix, that's all.

That's not true.  I didn't bother to speak up in the thread, and I read
all of the comments.  I didn't move to BIT macros for the reason that
Christoph thinks they are crap and I didn't bother to prove him wrong
and took his word for it.  However, just aligning the macros in the area
that the patch touched is reasonable (versus aligning the entire file
just for the fun of it), and git blame will continue working fine.  My
taking of this patch was intentional (in fact, the patch didn't apply, I
had to redo it entirely by hand because the comment changes caused by
Christoph's MR cleanup patches kept this patch from applying at all).
In any case, it wasn't a mistake, and there is nothing to fix up.

>>
>>> Leon will re-spin in the coming 1-2 hours V2, could please pick it
>>> instead
>>> of V1, when people agree on direction X and you are not against it,
>>> lets do
>>> X and not Y.
>> It would be great if we could stop rebasing whats already in the tree
>> for the benefit of everyone building on top of this.  For example just
>> finished rebasing my series to move many constants includin this one
>> to the uapi headers, and I'd hate to rebase it once again now that
>> the dust has settled.
> 
> The root issue here is that nothing was picked before 4.4-rc6, so we're
> in a situation where rebases are needed in the own-maintainer tree
> (github) to make things right. No way to avoid that.
> 
> We should aim that for 4.6 and onward, code for -next will start getting
> in around rc1-2 and then things will be more robust, etc
> 
> Or.
> 
> Or.
> 


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2015-12-24 16:55 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-20 10:16 [PATCH V1 0/3] Add cross-channel support Leon Romanovsky
     [not found] ` <1450606571-15877-1-git-send-email-leon-2ukJVAZIZ/Y@public.gmane.org>
2015-12-20 10:16   ` [PATCH V1 1/3] IB/core: Align coding style of ib_device_cap_flags structure Leon Romanovsky
     [not found]     ` <1450606571-15877-2-git-send-email-leon-2ukJVAZIZ/Y@public.gmane.org>
2015-12-21  6:22       ` ira.weiny
     [not found]         ` <20151221062252.GE3860-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
2015-12-21  6:37           ` Leon Romanovsky
     [not found]             ` <CALq1K=J2e=aw1QuJHGhFFcKkY391myz11r7tG-H+fYbQEyr+Gw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-12-21  6:52               ` Or Gerlitz
     [not found]                 ` <CAJ3xEMjcusZHb5PRT3ziv-sSYM70U+QtNbjhEYNhvL853Q57Qw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-12-21  7:27                   ` Leon Romanovsky
     [not found]                     ` <CALq1K=J-Zmb1zwh-ak+wYxo-xooGGYhCjiCOC_Z-4dYVGgkVsA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-12-21  7:40                       ` Or Gerlitz
     [not found]                         ` <CAJ3xEMjioX_fFZptJSLLC_yv1NszPwJET3zWj8+1x5fajXfBCQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-12-21  7:53                           ` Leon Romanovsky
     [not found]                             ` <567A70C9.9090801@mellanox.com>
     [not found]                               ` <567A70C9.9090801-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2015-12-23 10:23                                 ` Leon Romanovsky
2015-12-21  8:03               ` Christoph Hellwig
     [not found]                 ` <20151221080346.GA21779-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2015-12-21 16:36                   ` ira.weiny
     [not found]                     ` <20151221163603.GF3860-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
2015-12-21 20:20                       ` Christoph Hellwig
2015-12-24  3:31                       ` Doug Ledford
2015-12-20 10:16   ` [PATCH V1 2/3] IB/core: Add cross-channel support Leon Romanovsky
2015-12-20 10:16   ` [PATCH V1 3/3] IB/mlx5: Add driver " Leon Romanovsky
2015-12-24  4:42   ` [PATCH V1 0/3] Add " Doug Ledford
2015-12-24  8:02   ` Or Gerlitz
     [not found]     ` <567BA695.8050403-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2015-12-24 10:00       ` Christoph Hellwig
     [not found]         ` <20151224100001.GA21387-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2015-12-24 10:41           ` Or Gerlitz
     [not found]             ` <567BCBD1.708-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2015-12-24 16:55               ` Doug Ledford

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.