Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* Re: [PATCH] i40iw: Add Quality of Service support
From: Leon Romanovsky @ 2016-10-06 16:00 UTC (permalink / raw)
  To: Henry Orosco
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <20161006152128.14940-1-henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

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

On Thu, Oct 06, 2016 at 10:21:28AM -0500, Henry Orosco wrote:
> Add support for QoS on QPs. Upon device initialization,
> a map is created from user priority to queue set
> handles. On QP creation, use ToS to look up the queue
> set handle for use with the QP.
>
> Signed-off-by: Faisal Latif <faisal.latif-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/infiniband/hw/i40iw/i40iw.h       |   9 ++
>  drivers/infiniband/hw/i40iw/i40iw_cm.c    |  30 +++++-
>  drivers/infiniband/hw/i40iw/i40iw_cm.h    |   2 +-
>  drivers/infiniband/hw/i40iw/i40iw_ctrl.c  | 151 +++++++++++++++++++++++++++++-
>  drivers/infiniband/hw/i40iw/i40iw_d.h     |   2 +
>  drivers/infiniband/hw/i40iw/i40iw_hw.c    |  25 ++---
>  drivers/infiniband/hw/i40iw/i40iw_main.c  |  64 +++++++++++--
>  drivers/infiniband/hw/i40iw/i40iw_osdep.h |   2 +
>  drivers/infiniband/hw/i40iw/i40iw_p.h     |   2 +
>  drivers/infiniband/hw/i40iw/i40iw_puda.c  |   3 +-
>  drivers/infiniband/hw/i40iw/i40iw_type.h  |  18 +++-
>  drivers/infiniband/hw/i40iw/i40iw_utils.c |  45 +++++++++
>  drivers/infiniband/hw/i40iw/i40iw_verbs.c |   6 +-
>  13 files changed, 323 insertions(+), 36 deletions(-)

I believe that it can be split to series.

>  mode change 100644 => 100755 drivers/infiniband/hw/i40iw/i40iw_cm.c
>  mode change 100644 => 100755 drivers/infiniband/hw/i40iw/i40iw_ctrl.c

Why did you change file's mode from 644 to 755?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [PATCH] i40iw: Add Quality of Service support
From: Henry Orosco @ 2016-10-06 15:21 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Henry Orosco

Add support for QoS on QPs. Upon device initialization,
a map is created from user priority to queue set
handles. On QP creation, use ToS to look up the queue
set handle for use with the QP.

Signed-off-by: Faisal Latif <faisal.latif-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/hw/i40iw/i40iw.h       |   9 ++
 drivers/infiniband/hw/i40iw/i40iw_cm.c    |  30 +++++-
 drivers/infiniband/hw/i40iw/i40iw_cm.h    |   2 +-
 drivers/infiniband/hw/i40iw/i40iw_ctrl.c  | 151 +++++++++++++++++++++++++++++-
 drivers/infiniband/hw/i40iw/i40iw_d.h     |   2 +
 drivers/infiniband/hw/i40iw/i40iw_hw.c    |  25 ++---
 drivers/infiniband/hw/i40iw/i40iw_main.c  |  64 +++++++++++--
 drivers/infiniband/hw/i40iw/i40iw_osdep.h |   2 +
 drivers/infiniband/hw/i40iw/i40iw_p.h     |   2 +
 drivers/infiniband/hw/i40iw/i40iw_puda.c  |   3 +-
 drivers/infiniband/hw/i40iw/i40iw_type.h  |  18 +++-
 drivers/infiniband/hw/i40iw/i40iw_utils.c |  45 +++++++++
 drivers/infiniband/hw/i40iw/i40iw_verbs.c |   6 +-
 13 files changed, 323 insertions(+), 36 deletions(-)
 mode change 100644 => 100755 drivers/infiniband/hw/i40iw/i40iw_cm.c
 mode change 100644 => 100755 drivers/infiniband/hw/i40iw/i40iw_ctrl.c

diff --git a/drivers/infiniband/hw/i40iw/i40iw.h b/drivers/infiniband/hw/i40iw/i40iw.h
index b738acd..d39ce0a 100644
--- a/drivers/infiniband/hw/i40iw/i40iw.h
+++ b/drivers/infiniband/hw/i40iw/i40iw.h
@@ -210,6 +210,12 @@ struct i40iw_msix_vector {
 	u32 ceq_id;
 };
 
+struct l2params_work {
+	struct work_struct work;
+	struct i40iw_device *iwdev;
+	struct i40iw_l2params l2params;
+};
+
 #define I40IW_MSIX_TABLE_SIZE   65
 
 struct virtchnl_work {
@@ -514,6 +520,9 @@ void i40iw_add_pdusecount(struct i40iw_pd *iwpd);
 void i40iw_hw_modify_qp(struct i40iw_device *iwdev, struct i40iw_qp *iwqp,
 			struct i40iw_modify_qp_info *info, bool wait);
 
+void i40iw_qp_suspend_resume(struct i40iw_sc_dev *dev,
+			     struct i40iw_sc_qp *qp,
+			     bool suspend);
 enum i40iw_status_code i40iw_manage_qhash(struct i40iw_device *iwdev,
 					  struct i40iw_cm_info *cminfo,
 					  enum i40iw_quad_entry_type etype,
diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c b/drivers/infiniband/hw/i40iw/i40iw_cm.c
old mode 100644
new mode 100755
index c490f8d..06fd9ae
--- a/drivers/infiniband/hw/i40iw/i40iw_cm.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c
@@ -221,6 +221,7 @@ static void i40iw_get_addr_info(struct i40iw_cm_node *cm_node,
 	memcpy(cm_info->rem_addr, cm_node->rem_addr, sizeof(cm_info->rem_addr));
 	cm_info->loc_port = cm_node->loc_port;
 	cm_info->rem_port = cm_node->rem_port;
+	cm_info->user_pri = cm_node->user_pri;
 }
 
 /**
@@ -396,6 +397,7 @@ static struct i40iw_puda_buf *i40iw_form_cm_frame(struct i40iw_cm_node *cm_node,
 	u32 opts_len = 0;
 	u32 pd_len = 0;
 	u32 hdr_len = 0;
+	u16 vtag;
 
 	sqbuf = i40iw_puda_get_bufpool(dev->ilq);
 	if (!sqbuf)
@@ -445,7 +447,8 @@ static struct i40iw_puda_buf *i40iw_form_cm_frame(struct i40iw_cm_node *cm_node,
 		ether_addr_copy(ethh->h_source, cm_node->loc_mac);
 		if (cm_node->vlan_id < VLAN_TAG_PRESENT) {
 			((struct vlan_ethhdr *)ethh)->h_vlan_proto = htons(ETH_P_8021Q);
-			((struct vlan_ethhdr *)ethh)->h_vlan_TCI = htons(cm_node->vlan_id);
+			vtag = (cm_node->user_pri << VLAN_PRIO_SHIFT) | cm_node->vlan_id;
+			((struct vlan_ethhdr *)ethh)->h_vlan_TCI = htons(vtag);
 
 			((struct vlan_ethhdr *)ethh)->h_vlan_encapsulated_proto = htons(ETH_P_IP);
 		} else {
@@ -474,7 +477,8 @@ static struct i40iw_puda_buf *i40iw_form_cm_frame(struct i40iw_cm_node *cm_node,
 		ether_addr_copy(ethh->h_source, cm_node->loc_mac);
 		if (cm_node->vlan_id < VLAN_TAG_PRESENT) {
 			((struct vlan_ethhdr *)ethh)->h_vlan_proto = htons(ETH_P_8021Q);
-			((struct vlan_ethhdr *)ethh)->h_vlan_TCI = htons(cm_node->vlan_id);
+			vtag = (cm_node->user_pri << VLAN_PRIO_SHIFT) | cm_node->vlan_id;
+			((struct vlan_ethhdr *)ethh)->h_vlan_TCI = htons(vtag);
 			((struct vlan_ethhdr *)ethh)->h_vlan_encapsulated_proto = htons(ETH_P_IPV6);
 		} else {
 			ethh->h_proto = htons(ETH_P_IPV6);
@@ -1880,6 +1884,7 @@ static int i40iw_dec_refcnt_listen(struct i40iw_cm_core *cm_core,
 			nfo.loc_port = listener->loc_port;
 			nfo.ipv4 = listener->ipv4;
 			nfo.vlan_id = listener->vlan_id;
+			nfo.user_pri = listener->user_pri;
 
 			if (!list_empty(&listener->child_listen_list)) {
 				i40iw_del_multiple_qhash(listener->iwdev, &nfo, listener);
@@ -2138,6 +2143,11 @@ static struct i40iw_cm_node *i40iw_make_cm_node(
 	/* set our node specific transport info */
 	cm_node->ipv4 = cm_info->ipv4;
 	cm_node->vlan_id = cm_info->vlan_id;
+	if ((cm_node->vlan_id == I40IW_NO_VLAN) && iwdev->dcb)
+		cm_node->vlan_id = 0;
+	cm_node->user_pri = cm_info->user_pri;
+	if (listener)
+		cm_node->user_pri = listener->user_pri;
 	memcpy(cm_node->loc_addr, cm_info->loc_addr, sizeof(cm_node->loc_addr));
 	memcpy(cm_node->rem_addr, cm_info->rem_addr, sizeof(cm_node->rem_addr));
 	cm_node->loc_port = cm_info->loc_port;
@@ -3055,6 +3065,7 @@ void i40iw_receive_ilq(struct i40iw_sc_dev *dev, struct i40iw_puda_buf *rbuf)
 	struct i40iw_device *iwdev = (struct i40iw_device *)dev->back_dev;
 	struct i40iw_cm_core *cm_core = &iwdev->cm_core;
 	struct vlan_ethhdr *ethh;
+	u16 vtag;
 
 	/* if vlan, then maclen = 18 else 14 */
 	iph = (struct iphdr *)rbuf->iph;
@@ -3068,7 +3079,9 @@ void i40iw_receive_ilq(struct i40iw_sc_dev *dev, struct i40iw_puda_buf *rbuf)
 	ethh = (struct vlan_ethhdr *)rbuf->mem.va;
 
 	if (ethh->h_vlan_proto == htons(ETH_P_8021Q)) {
-		cm_info.vlan_id = ntohs(ethh->h_vlan_TCI) & VLAN_VID_MASK;
+		vtag = ntohs(ethh->h_vlan_TCI);
+		cm_info.user_pri = (vtag & VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
+		cm_info.vlan_id = vtag & VLAN_VID_MASK;
 		i40iw_debug(cm_core->dev,
 			    I40IW_DEBUG_CM,
 			    "%s vlan_id=%d\n",
@@ -3309,6 +3322,8 @@ static void i40iw_cm_init_tsa_conn(struct i40iw_qp *iwqp,
 
 	ctx_info->tcp_info_valid = true;
 	ctx_info->iwarp_info_valid = true;
+	ctx_info->add_to_qoslist = true;
+	ctx_info->user_pri = cm_node->user_pri;
 
 	i40iw_init_tcp_ctx(cm_node, &tcp_info, iwqp);
 	if (cm_node->snd_mark_en) {
@@ -3326,6 +3341,7 @@ static void i40iw_cm_init_tsa_conn(struct i40iw_qp *iwqp,
 	/* once tcp_info is set, no need to do it again */
 	ctx_info->tcp_info_valid = false;
 	ctx_info->iwarp_info_valid = false;
+	ctx_info->add_to_qoslist = false;
 }
 
 /**
@@ -3779,6 +3795,9 @@ int i40iw_connect(struct iw_cm_id *cm_id, struct iw_cm_conn_param *conn_param)
 		i40iw_netdev_vlan_ipv6(cm_info.loc_addr, &cm_info.vlan_id, NULL);
 	}
 	cm_info.cm_id = cm_id;
+	cm_info.user_pri = rt_tos2priority(cm_id->tos);
+	i40iw_debug(&iwdev->sc_dev, I40IW_DEBUG_DCB, "%s TOS:[%d] UP:[%d]\n",
+		    __func__, cm_id->tos, cm_info.user_pri);
 	if ((cm_info.ipv4 && (laddr->sin_addr.s_addr != raddr->sin_addr.s_addr)) ||
 	    (!cm_info.ipv4 && memcmp(laddr6->sin6_addr.in6_u.u6_addr32,
 				     raddr6->sin6_addr.in6_u.u6_addr32,
@@ -3924,6 +3943,11 @@ int i40iw_create_listen(struct iw_cm_id *cm_id, int backlog)
 
 	cm_id->provider_data = cm_listen_node;
 
+	cm_listen_node->user_pri = rt_tos2priority(cm_id->tos);
+	cm_info.user_pri = cm_listen_node->user_pri;
+	i40iw_debug(&iwdev->sc_dev, I40IW_DEBUG_DCB, "%s TOS:[%d] UP:[%d]\n",
+		    __func__, cm_id->tos, cm_listen_node->user_pri);
+
 	if (!cm_listen_node->reused_node) {
 		if (wildcard) {
 			if (cm_info.ipv4)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.h b/drivers/infiniband/hw/i40iw/i40iw_cm.h
index e9046d9..945ed26 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_cm.h
+++ b/drivers/infiniband/hw/i40iw/i40iw_cm.h
@@ -368,7 +368,7 @@ struct i40iw_cm_info {
 	u32 rem_addr[4];
 	u16 vlan_id;
 	int backlog;
-	u16 user_pri;
+	u8 user_pri;
 	bool ipv4;
 };
 
diff --git a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
old mode 100644
new mode 100755
index 2c4b4d0..31c4a0c
--- a/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_ctrl.c
@@ -223,6 +223,133 @@ static enum i40iw_status_code i40iw_sc_parse_fpm_query_buf(
 }
 
 /**
+ * i40iw_fill_qos_list - Change all unknown qs handles to available ones
+ * @qs_list: list of qs_handles to be fixed with valid qs_handles
+ */
+static void i40iw_fill_qos_list(u16 *qs_list)
+{
+	u16 qshandle = qs_list[0];
+	int i;
+
+	for (i = 0; i < I40IW_MAX_USER_PRIORITY; i++) {
+		if (qs_list[i] == QS_HANDLE_UNKNOWN)
+			qs_list[i] = qshandle;
+		else
+			qshandle = qs_list[i];
+	}
+}
+
+/**
+ * i40iw_qp_from_entry - Given entry, get to the qp structure
+ * @entry: Points to list of qp structure
+ */
+static struct i40iw_sc_qp *i40iw_qp_from_entry(struct list_head *entry)
+{
+	if (!entry)
+		return NULL;
+
+	return (struct i40iw_sc_qp *)((char *)entry - offsetof(struct i40iw_sc_qp, list));
+}
+
+/**
+ * i40iw_get_qp - get the next qp from the list given current qp
+ * @head: Listhead of qp's
+ * @qp: current qp
+ */
+static struct i40iw_sc_qp *i40iw_get_qp(struct list_head *head, struct i40iw_sc_qp *qp)
+{
+	struct list_head *entry = NULL;
+	struct list_head *lastentry;
+
+	if (list_empty(head))
+		return NULL;
+
+	if (!qp) {
+		entry = head->next;
+	} else {
+		lastentry = &qp->list;
+		entry = (lastentry != head) ? lastentry->next : NULL;
+	}
+
+	return i40iw_qp_from_entry(entry);
+}
+
+/**
+ * i40iw_change_l2params - given the new l2 parameters, change all qp
+ * @dev: IWARP device pointer
+ * @l2params: New paramaters from l2
+ */
+void i40iw_change_l2params(struct i40iw_sc_dev *dev, struct i40iw_l2params *l2params)
+{
+	struct i40iw_sc_qp *qp = NULL;
+	bool qs_handle_change = false;
+	bool mss_change = false;
+	unsigned long flags;
+	u16 qs_handle;
+	int i;
+
+	if (dev->mss != l2params->mss) {
+		mss_change = true;
+		dev->mss = l2params->mss;
+	}
+
+	i40iw_fill_qos_list(l2params->qs_handle_list);
+	for (i = 0; i < I40IW_MAX_USER_PRIORITY; i++) {
+		qs_handle = l2params->qs_handle_list[i];
+		if (dev->qos[i].qs_handle != qs_handle)
+			qs_handle_change = true;
+		else if (!mss_change)
+			continue;       /* no MSS nor qs handle change */
+		spin_lock_irqsave(&dev->qos[i].lock, flags);
+		qp = i40iw_get_qp(&dev->qos[i].qplist, qp);
+		while (qp) {
+			if (mss_change)
+				i40iw_qp_mss_modify(dev, qp);
+			if (qs_handle_change) {
+				qp->qs_handle = qs_handle;
+				/* issue cqp suspend command */
+				i40iw_qp_suspend_resume(dev, qp, true);
+			}
+			qp = i40iw_get_qp(&dev->qos[i].qplist, qp);
+		}
+		spin_unlock_irqrestore(&dev->qos[i].lock, flags);
+		dev->qos[i].qs_handle = qs_handle;
+	}
+}
+
+/**
+ * i40iw_qp_rem_qos - remove qp from qos lists during destroy qp
+ * @dev: IWARP device pointer
+ * @qp: qp to be removed from qos
+ */
+static void i40iw_qp_rem_qos(struct i40iw_sc_dev *dev, struct i40iw_sc_qp *qp)
+{
+	unsigned long flags;
+
+	if (!qp->on_qoslist)
+		return;
+	spin_lock_irqsave(&dev->qos[qp->user_pri].lock, flags);
+	list_del(&qp->list);
+	spin_unlock_irqrestore(&dev->qos[qp->user_pri].lock, flags);
+}
+
+/**
+ * i40iw_qp_add_qos - called during setctx fot qp to be added to qos
+ * @dev: IWARP device pointer
+ * @qp: qp to be added to qos
+ */
+void i40iw_qp_add_qos(struct i40iw_sc_dev *dev, struct i40iw_sc_qp *qp)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->qos[qp->user_pri].lock, flags);
+	qp->qs_handle = dev->qos[qp->user_pri].qs_handle;
+	list_add(&qp->list, &dev->qos[qp->user_pri].qplist);
+	qp->on_qoslist = true;
+	spin_unlock_irqrestore(&dev->qos[qp->user_pri].lock, flags);
+}
+
+/**
  * i40iw_sc_pd_init - initialize sc pd struct
  * @dev: sc device struct
  * @pd: sc pd ptr
@@ -1082,7 +1209,7 @@ static enum i40iw_status_code i40iw_sc_manage_qhash_table_entry(
 			      LS_64(info->dest_ip[2], I40IW_CQPSQ_QHASH_ADDR2) |
 			      LS_64(info->dest_ip[3], I40IW_CQPSQ_QHASH_ADDR3));
 	}
-	qw2 = LS_64(cqp->dev->qs_handle, I40IW_CQPSQ_QHASH_QS_HANDLE);
+	qw2 = LS_64(cqp->dev->qos[info->user_pri].qs_handle, I40IW_CQPSQ_QHASH_QS_HANDLE);
 	if (info->vlan_valid)
 		qw2 |= LS_64(info->vlan_id, I40IW_CQPSQ_QHASH_VLANID);
 	set_64bit_val(wqe, 16, qw2);
@@ -2151,7 +2278,7 @@ static enum i40iw_status_code i40iw_sc_qp_init(struct i40iw_sc_qp *qp,
 	qp->rq_tph_en = info->rq_tph_en;
 	qp->rcv_tph_en = info->rcv_tph_en;
 	qp->xmit_tph_en = info->xmit_tph_en;
-	qp->qs_handle = qp->pd->dev->qs_handle;
+	qp->qs_handle = qp->pd->dev->qos[qp->user_pri].qs_handle;
 	qp->exception_lan_queue = qp->pd->dev->exception_lan_queue;
 
 	return 0;
@@ -2296,6 +2423,7 @@ static enum i40iw_status_code i40iw_sc_qp_destroy(
 	struct i40iw_sc_cqp *cqp;
 	u64 header;
 
+	i40iw_qp_rem_qos(qp->pd->dev, qp);
 	cqp = qp->pd->dev->cqp;
 	wqe = i40iw_sc_cqp_get_next_send_wqe(cqp, scratch);
 	if (!wqe)
@@ -2447,6 +2575,12 @@ static enum i40iw_status_code i40iw_sc_qp_setctx(
 
 	iw = info->iwarp_info;
 	tcp = info->tcp_info;
+	if (info->add_to_qoslist) {
+		qp->user_pri = info->user_pri;
+		i40iw_qp_add_qos(qp->pd->dev, qp);
+		i40iw_debug(qp->dev, I40IW_DEBUG_DCB, "%s qp[%d] UP[%d] qset[%d]\n",
+			    __func__, qp->qp_uk.qp_id, qp->user_pri, qp->qs_handle);
+	}
 	qw0 = LS_64(qp->qp_uk.rq_wqe_size, I40IWQPC_RQWQESIZE) |
 	      LS_64(info->err_rq_idx_valid, I40IWQPC_ERR_RQ_IDX_VALID) |
 	      LS_64(qp->rcv_tph_en, I40IWQPC_RCVTPHEN) |
@@ -3959,7 +4093,7 @@ enum i40iw_status_code i40iw_process_cqp_cmd(struct i40iw_sc_dev *dev,
 					     struct cqp_commands_info *pcmdinfo)
 {
 	enum i40iw_status_code status = 0;
-	unsigned long	flags;
+	unsigned long flags;
 
 	spin_lock_irqsave(&dev->cqp_lock, flags);
 	if (list_empty(&dev->cqp_cmd_head) && !i40iw_ring_full(dev->cqp))
@@ -3978,7 +4112,7 @@ enum i40iw_status_code i40iw_process_bh(struct i40iw_sc_dev *dev)
 {
 	enum i40iw_status_code status = 0;
 	struct cqp_commands_info *pcmdinfo;
-	unsigned long	flags;
+	unsigned long flags;
 
 	spin_lock_irqsave(&dev->cqp_lock, flags);
 	while (!list_empty(&dev->cqp_cmd_head) && !i40iw_ring_full(dev->cqp)) {
@@ -4742,6 +4876,7 @@ enum i40iw_status_code i40iw_device_init(struct i40iw_sc_dev *dev,
 	u16 hmc_fcn = 0;
 	enum i40iw_status_code ret_code = 0;
 	u8 db_size;
+	int i;
 
 	spin_lock_init(&dev->cqp_lock);
 	INIT_LIST_HEAD(&dev->cqp_cmd_head);             /* for the cqp commands backlog. */
@@ -4757,7 +4892,13 @@ enum i40iw_status_code i40iw_device_init(struct i40iw_sc_dev *dev,
 		return ret_code;
 	}
 	dev->hmc_fn_id = info->hmc_fn_id;
-	dev->qs_handle = info->qs_handle;
+	i40iw_fill_qos_list(info->l2params.qs_handle_list);
+	for (i = 0; i < I40IW_MAX_USER_PRIORITY; i++) {
+		dev->qos[i].qs_handle = info->l2params.qs_handle_list[i];
+		i40iw_debug(dev, I40IW_DEBUG_DCB, "qset[%d]: %d\n", i, dev->qos[i].qs_handle);
+		spin_lock_init(&dev->qos[i].lock);
+		INIT_LIST_HEAD(&dev->qos[i].qplist);
+	}
 	dev->exception_lan_queue = info->exception_lan_queue;
 	dev->is_pf = info->is_pf;
 
diff --git a/drivers/infiniband/hw/i40iw/i40iw_d.h b/drivers/infiniband/hw/i40iw/i40iw_d.h
index 2fac1db..e184c0e 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_d.h
+++ b/drivers/infiniband/hw/i40iw/i40iw_d.h
@@ -74,6 +74,8 @@
 #define RS_32_1(val, bits)      (u32)(val >> bits)
 #define I40E_HI_DWORD(x)        ((u32)((((x) >> 16) >> 16) & 0xFFFFFFFF))
 
+#define QS_HANDLE_UNKNOWN       0xffff
+
 #define LS_64(val, field) (((u64)val << field ## _SHIFT) & (field ## _MASK))
 
 #define RS_64(val, field) ((u64)(val & field ## _MASK) >> field ## _SHIFT)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_hw.c b/drivers/infiniband/hw/i40iw/i40iw_hw.c
index 3ee0cad..4a7619f 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_hw.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_hw.c
@@ -358,6 +358,9 @@ void i40iw_process_aeq(struct i40iw_device *iwdev)
 				continue;
 			i40iw_cm_disconn(iwqp);
 			break;
+		case I40IW_AE_QP_SUSPEND_COMPLETE:
+			i40iw_qp_suspend_resume(dev, &iwqp->sc_qp, false);
+			break;
 		case I40IW_AE_TERMINATE_SENT:
 			i40iw_terminate_send_fin(qp);
 			break;
@@ -403,19 +406,18 @@ void i40iw_process_aeq(struct i40iw_device *iwdev)
 		case I40IW_AE_LCE_CQ_CATASTROPHIC:
 		case I40IW_AE_UDA_XMIT_DGRAM_TOO_LONG:
 		case I40IW_AE_UDA_XMIT_IPADDR_MISMATCH:
-		case I40IW_AE_QP_SUSPEND_COMPLETE:
 			ctx_info->err_rq_idx_valid = false;
 		default:
-				if (!info->sq && ctx_info->err_rq_idx_valid) {
-					ctx_info->err_rq_idx = info->wqe_idx;
-					ctx_info->tcp_info_valid = false;
-					ctx_info->iwarp_info_valid = false;
-					ret = dev->iw_priv_qp_ops->qp_setctx(&iwqp->sc_qp,
-									     iwqp->host_ctx.va,
-									     ctx_info);
-				}
-				i40iw_terminate_connection(qp, info);
-				break;
+			if (!info->sq && ctx_info->err_rq_idx_valid) {
+				ctx_info->err_rq_idx = info->wqe_idx;
+				ctx_info->tcp_info_valid = false;
+				ctx_info->iwarp_info_valid = false;
+				ret = dev->iw_priv_qp_ops->qp_setctx(&iwqp->sc_qp,
+								     iwqp->host_ctx.va,
+								     ctx_info);
+			}
+			i40iw_terminate_connection(qp, info);
+			break;
 		}
 		if (info->qp)
 			i40iw_rem_ref(&iwqp->ibqp);
@@ -559,6 +561,7 @@ enum i40iw_status_code i40iw_manage_qhash(struct i40iw_device *iwdev,
 	}
 
 	info->ipv4_valid = cminfo->ipv4;
+	info->user_pri = cminfo->user_pri;
 	ether_addr_copy(info->mac_addr, iwdev->netdev->dev_addr);
 	info->qp_num = cpu_to_le32(dev->ilq->qp_id);
 	info->dest_port = cpu_to_le16(cminfo->loc_port);
diff --git a/drivers/infiniband/hw/i40iw/i40iw_main.c b/drivers/infiniband/hw/i40iw/i40iw_main.c
index 798335f..6746000 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_main.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_main.c
@@ -939,7 +939,7 @@ static enum i40iw_status_code i40iw_initialize_ilq(struct i40iw_device *iwdev)
 	info.rq_size = 8192;
 	info.buf_size = 1024;
 	info.tx_buf_cnt = 16384;
-	info.mss = iwdev->mss;
+	info.mss = iwdev->sc_dev.mss;
 	info.receive = i40iw_receive_ilq;
 	info.xmit_complete = i40iw_free_sqbuf;
 	status = i40iw_puda_create_rsrc(&iwdev->sc_dev, &info);
@@ -967,7 +967,7 @@ static enum i40iw_status_code i40iw_initialize_ieq(struct i40iw_device *iwdev)
 	info.sq_size = 8192;
 	info.rq_size = 8192;
 	info.buf_size = 2048;
-	info.mss = iwdev->mss;
+	info.mss = iwdev->sc_dev.mss;
 	info.tx_buf_cnt = 16384;
 	status = i40iw_puda_create_rsrc(&iwdev->sc_dev, &info);
 	if (status)
@@ -1296,6 +1296,9 @@ static enum i40iw_status_code i40iw_initialize_dev(struct i40iw_device *iwdev,
 	struct i40iw_device_init_info info;
 	struct i40iw_dma_mem mem;
 	u32 size;
+	u16 last_qset = I40IW_NO_QSET;
+	u16 qset;
+	u32 i;
 
 	memset(&info, 0, sizeof(info));
 	size = sizeof(struct i40iw_hmc_pble_rsrc) + sizeof(struct i40iw_hmc_info) +
@@ -1325,7 +1328,16 @@ static enum i40iw_status_code i40iw_initialize_dev(struct i40iw_device *iwdev,
 	info.bar0 = ldev->hw_addr;
 	info.hw = &iwdev->hw;
 	info.debug_mask = debug;
-	info.qs_handle = ldev->params.qos.prio_qos[0].qs_handle;
+	info.l2params.mss =
+		(ldev->params.mtu) ? ldev->params.mtu - I40IW_MTU_TO_MSS : I40IW_DEFAULT_MSS;
+	for (i = 0; i < I40E_CLIENT_MAX_USER_PRIORITY; i++) {
+		qset = ldev->params.qos.prio_qos[i].qs_handle;
+		info.l2params.qs_handle_list[i] = qset;
+		if (last_qset == I40IW_NO_QSET)
+			last_qset = qset;
+		else if ((qset != last_qset) && (qset != I40IW_NO_QSET))
+			iwdev->dcb = true;
+	}
 	info.exception_lan_queue = 1;
 	info.vchnl_send = i40iw_virtchnl_send;
 	status = i40iw_device_init(&iwdev->sc_dev, &info);
@@ -1417,6 +1429,8 @@ static void i40iw_deinit_device(struct i40iw_device *iwdev, bool reset, bool del
 	struct i40iw_sc_dev *dev = &iwdev->sc_dev;
 
 	i40iw_pr_info("state = %d\n", iwdev->init_state);
+	if (iwdev->param_wq)
+		destroy_workqueue(iwdev->param_wq);
 
 	switch (iwdev->init_state) {
 	case RDMA_DEV_REGISTERED:
@@ -1628,6 +1642,7 @@ static int i40iw_open(struct i40e_info *ldev, struct i40e_client *client)
 		iwdev->init_state = RDMA_DEV_REGISTERED;
 		iwdev->iw_status = 1;
 		i40iw_port_ibevent(iwdev);
+		iwdev->param_wq = create_singlethread_workqueue("l2params");
 		i40iw_pr_info("i40iw_open completed\n");
 		return 0;
 	} while (0);
@@ -1638,25 +1653,58 @@ static int i40iw_open(struct i40e_info *ldev, struct i40e_client *client)
 }
 
 /**
- * i40iw_l2param_change : handle qs handles for qos and mss change
+ * i40iw_l2params_worker - worker for l2 params change
+ * @work: work pointer for l2 params
+ */
+static void i40iw_l2params_worker(struct work_struct *work)
+{
+	struct l2params_work *dwork =
+	    container_of(work, struct l2params_work, work);
+	struct i40iw_device *iwdev = dwork->iwdev;
+
+	i40iw_change_l2params(&iwdev->sc_dev, &dwork->l2params);
+	atomic_dec(&iwdev->params_busy);
+	kfree(work);
+}
+
+/**
+ * i40iw_l2param_change - handle qs handles for qos and mss change
  * @ldev: lan device information
  * @client: client for paramater change
  * @params: new parameters from L2
  */
-static void i40iw_l2param_change(struct i40e_info *ldev,
-				 struct i40e_client *client,
+static void i40iw_l2param_change(struct i40e_info *ldev, struct i40e_client *client,
 				 struct i40e_params *params)
 {
 	struct i40iw_handler *hdl;
+	struct i40iw_l2params *l2params;
+	struct l2params_work *work;
 	struct i40iw_device *iwdev;
+	int i;
 
 	hdl = i40iw_find_i40e_handler(ldev);
 	if (!hdl)
 		return;
 
 	iwdev = &hdl->device;
-	if (params->mtu)
-		iwdev->mss = params->mtu - I40IW_MTU_TO_MSS;
+
+	if (atomic_read(&iwdev->params_busy))
+		return;
+
+
+	work = kzalloc(sizeof(*work), GFP_ATOMIC);
+	if (!work)
+		return;
+
+	atomic_inc(&iwdev->params_busy);
+
+	work->iwdev = iwdev;
+	l2params = &work->l2params;
+	for (i = 0; i < I40E_CLIENT_MAX_USER_PRIORITY; i++)
+		l2params->qs_handle_list[i] = params->qos.prio_qos[i].qs_handle;
+
+	INIT_WORK(&work->work, i40iw_l2params_worker);
+	queue_work(iwdev->param_wq, &work->work);
 }
 
 /**
diff --git a/drivers/infiniband/hw/i40iw/i40iw_osdep.h b/drivers/infiniband/hw/i40iw/i40iw_osdep.h
index 80f422b..a6b18cd 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_osdep.h
+++ b/drivers/infiniband/hw/i40iw/i40iw_osdep.h
@@ -198,6 +198,8 @@ enum i40iw_status_code i40iw_cqp_manage_vf_pble_bp(struct i40iw_sc_dev *dev,
 void i40iw_cqp_spawn_worker(struct i40iw_sc_dev *dev,
 			    struct i40iw_virtchnl_work_info *work_info, u32 iw_vf_idx);
 void *i40iw_remove_head(struct list_head *list);
+void i40iw_qp_suspend_resume(struct i40iw_sc_dev *dev, struct i40iw_sc_qp *qp, bool suspend);
+void i40iw_qp_mss_modify(struct i40iw_sc_dev *dev, struct i40iw_sc_qp *qp);
 
 void i40iw_term_modify_qp(struct i40iw_sc_qp *qp, u8 next_state, u8 term, u8 term_len);
 void i40iw_terminate_done(struct i40iw_sc_qp *qp, int timeout_occurred);
diff --git a/drivers/infiniband/hw/i40iw/i40iw_p.h b/drivers/infiniband/hw/i40iw/i40iw_p.h
index a0b8ca1..c9e8cb8 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_p.h
+++ b/drivers/infiniband/hw/i40iw/i40iw_p.h
@@ -65,6 +65,8 @@ enum i40iw_status_code i40iw_pf_init_vfhmc(struct i40iw_sc_dev *dev, u8 vf_hmc_f
 					   u32 *vf_cnt_array);
 
 /* cqp misc functions */
+void i40iw_change_l2params(struct i40iw_sc_dev *dev, struct i40iw_l2params *l2params);
+void i40iw_qp_add_qos(struct i40iw_sc_dev *dev, struct i40iw_sc_qp *qp);
 
 void i40iw_terminate_send_fin(struct i40iw_sc_qp *qp);
 
diff --git a/drivers/infiniband/hw/i40iw/i40iw_puda.c b/drivers/infiniband/hw/i40iw/i40iw_puda.c
index c62d354..7541b0d 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_puda.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_puda.c
@@ -608,7 +608,8 @@ static enum i40iw_status_code i40iw_puda_qp_create(struct i40iw_puda_rsrc *rsrc)
 		ukqp->wqe_alloc_reg = (u32 __iomem *)(i40iw_get_hw_addr(qp->pd->dev) +
 						    I40E_VFPE_WQEALLOC1);
 
-	qp->qs_handle = qp->dev->qs_handle;
+	qp->user_pri = 0;
+	i40iw_qp_add_qos(rsrc->dev, qp);
 	i40iw_puda_qp_setctx(rsrc);
 	ret = i40iw_puda_qp_wqe(rsrc);
 	if (ret)
diff --git a/drivers/infiniband/hw/i40iw/i40iw_type.h b/drivers/infiniband/hw/i40iw/i40iw_type.h
index 2b1a04e..b6f448a 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_type.h
+++ b/drivers/infiniband/hw/i40iw/i40iw_type.h
@@ -397,6 +397,9 @@ struct i40iw_sc_qp {
 	bool virtual_map;
 	bool flush_sq;
 	bool flush_rq;
+	u8 user_pri;
+	struct list_head list;
+	bool on_qoslist;
 	bool sq_flush;
 	enum i40iw_flush_opcode flush_code;
 	enum i40iw_term_eventtypes eventtype;
@@ -424,6 +427,12 @@ struct i40iw_vchnl_vf_msg_buffer {
 	char parm_buffer[I40IW_VCHNL_MAX_VF_MSG_SIZE - 1];
 };
 
+struct i40iw_qos {
+	struct list_head qplist;
+	spinlock_t lock;	/* qos list */
+	u16 qs_handle;
+};
+
 struct i40iw_vfdev {
 	struct i40iw_sc_dev *pf_dev;
 	u8 *hmc_info_mem;
@@ -482,7 +491,8 @@ struct i40iw_sc_dev {
 	const struct i40iw_vf_cqp_ops *iw_vf_cqp_ops;
 
 	struct i40iw_hmc_fpm_misc hmc_fpm_misc;
-	u16 qs_handle;
+	struct i40iw_qos qos[I40IW_MAX_USER_PRIORITY];
+	u16 mss;
 	u32 debug_mask;
 	u16 exception_lan_queue;
 	u8 hmc_fn_id;
@@ -564,7 +574,7 @@ struct i40iw_device_init_info {
 	struct i40iw_hw *hw;
 	void __iomem *bar0;
 	enum i40iw_status_code (*vchnl_send)(struct i40iw_sc_dev *, u32, u8 *, u16);
-	u16 qs_handle;
+	struct i40iw_l2params l2params;
 	u16 exception_lan_queue;
 	u8 hmc_fn_id;
 	bool is_pf;
@@ -722,6 +732,8 @@ struct i40iw_qp_host_ctx_info {
 	bool iwarp_info_valid;
 	bool err_rq_idx_valid;
 	u16 err_rq_idx;
+	bool add_to_qoslist;
+	u8 user_pri;
 };
 
 struct i40iw_aeqe_info {
@@ -886,7 +898,7 @@ struct i40iw_qhash_table_info {
 	bool ipv4_valid;
 	u8 mac_addr[6];
 	u16 vlan_id;
-	u16 qs_handle;
+	u8 user_pri;
 	u32 qp_num;
 	u32 dest_ip[4];
 	u32 src_ip[4];
diff --git a/drivers/infiniband/hw/i40iw/i40iw_utils.c b/drivers/infiniband/hw/i40iw/i40iw_utils.c
index 0e8db0a..218e9fd 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_utils.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_utils.c
@@ -709,6 +709,51 @@ enum i40iw_status_code i40iw_cqp_sds_cmd(struct i40iw_sc_dev *dev,
 }
 
 /**
+ * i40iw_qp_suspend_resume - cqp command for suspend/resume
+ * @dev: hardware control device structure
+ * @qp: hardware control qp
+ * @suspend: flag if suspend or resume
+ */
+void i40iw_qp_suspend_resume(struct i40iw_sc_dev *dev, struct i40iw_sc_qp *qp, bool suspend)
+{
+	struct i40iw_device *iwdev = (struct i40iw_device *)dev->back_dev;
+	struct i40iw_cqp_request *cqp_request;
+	struct i40iw_sc_cqp *cqp = dev->cqp;
+	struct cqp_commands_info *cqp_info;
+	enum i40iw_status_code status;
+
+	cqp_request = i40iw_get_cqp_request(&iwdev->cqp, false);
+	if (!cqp_request)
+		return;
+
+	cqp_info = &cqp_request->info;
+	cqp_info->cqp_cmd = (suspend) ? OP_SUSPEND : OP_RESUME;
+	cqp_info->in.u.suspend_resume.cqp = cqp;
+	cqp_info->in.u.suspend_resume.qp = qp;
+	cqp_info->in.u.suspend_resume.scratch = (uintptr_t)cqp_request;
+	status = i40iw_handle_cqp_op(iwdev, cqp_request);
+	if (status)
+		i40iw_pr_err("CQP-OP QP Suspend/Resume fail");
+}
+
+/**
+ * i40iw_qp_mss_modify - modify mss for qp
+ * @dev: hardware control device structure
+ * @qp: hardware control qp
+ */
+void i40iw_qp_mss_modify(struct i40iw_sc_dev *dev, struct i40iw_sc_qp *qp)
+{
+	struct i40iw_device *iwdev = (struct i40iw_device *)dev->back_dev;
+	struct i40iw_qp *iwqp = (struct i40iw_qp *)qp->back_qp;
+	struct i40iw_modify_qp_info info;
+
+	memset(&info, 0, sizeof(info));
+	info.mss_change = true;
+	info.new_mss = dev->mss;
+	i40iw_hw_modify_qp(iwdev, iwqp, &info, false);
+}
+
+/**
  * i40iw_term_modify_qp - modify qp for term message
  * @qp: hardware control qp
  * @next_state: qp's next state
diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
index 2360338..423edad 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
@@ -254,7 +254,6 @@ static void i40iw_alloc_push_page(struct i40iw_device *iwdev, struct i40iw_sc_qp
 {
 	struct i40iw_cqp_request *cqp_request;
 	struct cqp_commands_info *cqp_info;
-	struct i40iw_sc_dev *dev = &iwdev->sc_dev;
 	enum i40iw_status_code status;
 
 	if (qp->push_idx != I40IW_INVALID_PUSH_PAGE_INDEX)
@@ -270,7 +269,7 @@ static void i40iw_alloc_push_page(struct i40iw_device *iwdev, struct i40iw_sc_qp
 	cqp_info->cqp_cmd = OP_MANAGE_PUSH_PAGE;
 	cqp_info->post_sq = 1;
 
-	cqp_info->in.u.manage_push_page.info.qs_handle = dev->qs_handle;
+	cqp_info->in.u.manage_push_page.info.qs_handle = qp->qs_handle;
 	cqp_info->in.u.manage_push_page.info.free_page = 0;
 	cqp_info->in.u.manage_push_page.cqp = &iwdev->cqp.sc_cqp;
 	cqp_info->in.u.manage_push_page.scratch = (uintptr_t)cqp_request;
@@ -292,7 +291,6 @@ static void i40iw_dealloc_push_page(struct i40iw_device *iwdev, struct i40iw_sc_
 {
 	struct i40iw_cqp_request *cqp_request;
 	struct cqp_commands_info *cqp_info;
-	struct i40iw_sc_dev *dev = &iwdev->sc_dev;
 	enum i40iw_status_code status;
 
 	if (qp->push_idx == I40IW_INVALID_PUSH_PAGE_INDEX)
@@ -307,7 +305,7 @@ static void i40iw_dealloc_push_page(struct i40iw_device *iwdev, struct i40iw_sc_
 	cqp_info->post_sq = 1;
 
 	cqp_info->in.u.manage_push_page.info.push_idx = qp->push_idx;
-	cqp_info->in.u.manage_push_page.info.qs_handle = dev->qs_handle;
+	cqp_info->in.u.manage_push_page.info.qs_handle = qp->qs_handle;
 	cqp_info->in.u.manage_push_page.info.free_page = 1;
 	cqp_info->in.u.manage_push_page.cqp = &iwdev->cqp.sc_cqp;
 	cqp_info->in.u.manage_push_page.scratch = (uintptr_t)cqp_request;
-- 
2.8.3

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

* Re: [PATCHv12 0/3] rdmacg: IB/core: rdma controller support
From: Parav Pandit @ 2016-10-06 13:49 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: cgroups-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	Tejun Heo, Li Zefan, Johannes Weiner, Doug Ledford,
	Christoph Hellwig, Liran Liss, Hefty, Sean, Jason Gunthorpe,
	Haggai Eran, james.l.morris-QHcLZuEGTsvQT0dZR+AlfA, Or Gerlitz,
	Matan Barak
In-Reply-To: <20161005112206.GC9282-2ukJVAZIZ/Y@public.gmane.org>

Hi Leon,

On Wed, Oct 5, 2016 at 4:52 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Aug 31, 2016 at 02:07:24PM +0530, Parav Pandit wrote:
>> rdmacg: IB/core: rdma controller support
>>
>> Overview:
>> Currently user space applications can easily take away all the rdma
>> device specific resources such as AH, CQ, QP, MR etc. Due to which other
>> applications in other cgroup or kernel space ULPs may not even get chance
>> to allocate any rdma resources. This results into service unavailibility.
>>
>> RDMA cgroup addresses this issue by allowing resource accounting,
>> limit enforcement on per cgroup, per rdma device basis.
>>
>> RDMA uverbs layer will enforce limits on well defined RDMA verb
>> resources without any HCA vendor device driver involvement.
>>
>> RDMA uverbs layer will not do limit enforcement of HCA hw vendor
>> specific resources. Instead rdma cgroup provides set of APIs
>> through which vendor specific drivers can do resource accounting
>> by making use of rdma cgroup.
>
> Hi Parav,
> I want to propose an extension to the RDMA cgroup which can be done as
> follow-up patches.
>
> Let's add new global type, which will control whole HCA (for example in percentages). It will
> allow natively define new objects without need to introduce them to the user.
>
In other cgroup such as CPU, this is done using cpu.weight API. Where
percentage or weight is configured by the user.
In this mode, resources taken away from other cgroup proportionately.
It works for cpu because its mainly stateless resource unlike rdma
resources.
So if we want to simplify user configuration similarly,
percentage/weight configuration can be extended.
This way they need not be introduced to users.
I hope your definition of "user" is actual end-user and not rdma cgroup. Right?
In other words, new object should be still added as new enum value in
rdma_cgroup.h?
Only than it can be overwritten by specific UVERBs type as you
described below. I think thats what you meant as you described below.

Otherwise charging/uncharging this new percentage resource can get messy.

> This HCA share will be overwritten by specific UVERBS types which you
> already defined.
>
> What do you think?

So to refine your proposal from cgroup perspective, instead of adding
new resource type in rdma_cgroup.h for percentage, I prefer to have

Existing
1. rdma.max
2. rdma.current
New,
3. rdma.weight
This ABI will have similar API to say
echo "mlx4_0 50" > rdma.weight.
Where 50 is weight of the resources.
For example,
for one cgroup instance weight=sum=100% resource for a given cgroup.
for three cgroup instances percentage=(weight/sum)% = 50/(50+50+50) = 33%.
One cgroup gets 33% resource.

Weight can be in range of 1 to 10,000 similar to cpu cgroup.

This might work if applications running in all cgroups are similar.
But weight doesn't do justice, when there are different type of
applications running in each cgroup. Such as few running libfabric
based apps, few running MPI, others directly using ibverbs.
So as you said rdma.max configuration would be required for management
plane to override weight (percentage) for certain resources.


>
> Except this proposal,
> Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> Thanks.

^ permalink raw reply

* Re: [PATCH qedr 04/10] qedr: Add support for PD,PKEY and CQ verbs
From: Leon Romanovsky @ 2016-10-06 13:33 UTC (permalink / raw)
  To: Ram Amrani
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	Ariel.Elior-YGCgFSpz5w/QT0dZR+AlfA,
	Michal.Kalderon-YGCgFSpz5w/QT0dZR+AlfA,
	Yuval.Mintz-YGCgFSpz5w/QT0dZR+AlfA,
	rajesh.borundia-YGCgFSpz5w/QT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1475682483-9878-5-git-send-email-Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>

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

On Wed, Oct 05, 2016 at 06:47:57PM +0300, Ram Amrani wrote:
> Add support for protection domain and completion queue verbs.
>
> Signed-off-by: Rajesh Borundia <rajesh.borundia-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Ram Amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/infiniband/hw/qedr/main.c          |  48 ++-
>  drivers/infiniband/hw/qedr/qedr.h          |  78 +++++
>  drivers/infiniband/hw/qedr/qedr_hsi_rdma.h |  79 +++++
>  drivers/infiniband/hw/qedr/verbs.c         | 543 +++++++++++++++++++++++++++++
>  drivers/infiniband/hw/qedr/verbs.h         |  14 +
>  include/uapi/rdma/qedr-abi.h               |  19 +
>  6 files changed, 780 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/hw/qedr/main.c b/drivers/infiniband/hw/qedr/main.c
> index 7c6d8a7..dc7f072 100644
> --- a/drivers/infiniband/hw/qedr/main.c
> +++ b/drivers/infiniband/hw/qedr/main.c
> @@ -87,7 +87,14 @@ static int qedr_register_device(struct qedr_dev *dev)
>
>  	dev->ibdev.uverbs_cmd_mask = QEDR_UVERBS(GET_CONTEXT) |
>  				     QEDR_UVERBS(QUERY_DEVICE) |
> -				     QEDR_UVERBS(QUERY_PORT);
> +				     QEDR_UVERBS(QUERY_PORT) |
> +				     QEDR_UVERBS(ALLOC_PD) |
> +				     QEDR_UVERBS(DEALLOC_PD) |
> +				     QEDR_UVERBS(CREATE_COMP_CHANNEL) |
> +				     QEDR_UVERBS(CREATE_CQ) |
> +				     QEDR_UVERBS(RESIZE_CQ) |
> +				     QEDR_UVERBS(DESTROY_CQ) |
> +				     QEDR_UVERBS(REQ_NOTIFY_CQ);
>
>  	dev->ibdev.phys_port_cnt = 1;
>  	dev->ibdev.num_comp_vectors = dev->num_cnq;
> @@ -105,6 +112,16 @@ static int qedr_register_device(struct qedr_dev *dev)
>  	dev->ibdev.dealloc_ucontext = qedr_dealloc_ucontext;
>  	dev->ibdev.mmap = qedr_mmap;
>
> +	dev->ibdev.alloc_pd = qedr_alloc_pd;
> +	dev->ibdev.dealloc_pd = qedr_dealloc_pd;
> +
> +	dev->ibdev.create_cq = qedr_create_cq;
> +	dev->ibdev.destroy_cq = qedr_destroy_cq;
> +	dev->ibdev.resize_cq = qedr_resize_cq;
> +	dev->ibdev.req_notify_cq = qedr_arm_cq;
> +
> +	dev->ibdev.query_pkey = qedr_query_pkey;
> +
>  	dev->ibdev.dma_device = &dev->pdev->dev;
>
>  	dev->ibdev.get_link_layer = qedr_link_layer;
> @@ -324,6 +341,8 @@ static irqreturn_t qedr_irq_handler(int irq, void *handle)
>  {
>  	u16 hw_comp_cons, sw_comp_cons;
>  	struct qedr_cnq *cnq = handle;
> +	struct regpair *cq_handle;
> +	struct qedr_cq *cq;
>
>  	qed_sb_ack(cnq->sb, IGU_INT_DISABLE, 0);
>
> @@ -336,7 +355,34 @@ static irqreturn_t qedr_irq_handler(int irq, void *handle)
>  	rmb();
>
>  	while (sw_comp_cons != hw_comp_cons) {
> +		cq_handle = (struct regpair *)qed_chain_consume(&cnq->pbl);
> +		cq = (struct qedr_cq *)(uintptr_t)HILO_U64(cq_handle->hi,
> +				cq_handle->lo);
> +
> +		if (cq == NULL) {
> +			DP_ERR(cnq->dev,
> +			       "Received NULL CQ cq_handle->hi=%d cq_handle->lo=%d sw_comp_cons=%d hw_comp_cons=%d\n",
> +			       cq_handle->hi, cq_handle->lo, sw_comp_cons,
> +			       hw_comp_cons);
> +
> +			break;
> +		}
> +
> +		if (cq->sig != QEDR_CQ_MAGIC_NUMBER) {
> +			DP_ERR(cnq->dev,
> +			       "Problem with cq signature, cq_handle->hi=%d ch_handle->lo=%d cq=%p\n",
> +			       cq_handle->hi, cq_handle->lo, cq);
> +			break;
> +		}
> +
> +		cq->arm_flags = 0;
> +
> +		if (cq->ibcq.comp_handler)
> +			(*cq->ibcq.comp_handler)
> +				(&cq->ibcq, cq->ibcq.cq_context);
> +
>  		sw_comp_cons = qed_chain_get_cons_idx(&cnq->pbl);
> +
>  		cnq->n_comp++;
>  	}
>
> diff --git a/drivers/infiniband/hw/qedr/qedr.h b/drivers/infiniband/hw/qedr/qedr.h
> index 2091c0d..557b9e0 100644
> --- a/drivers/infiniband/hw/qedr/qedr.h
> +++ b/drivers/infiniband/hw/qedr/qedr.h
> @@ -50,6 +50,10 @@
>
>  #define QEDR_MSG_INIT "INIT"
>  #define QEDR_MSG_MISC "MISC"
> +#define QEDR_MSG_CQ   "  CQ"
> +#define QEDR_MSG_MR   "  MR"
> +
> +#define QEDR_CQ_MAGIC_NUMBER   (0x11223344)
>
>  struct qedr_dev;
>
> @@ -181,6 +185,12 @@ struct qedr_dev {
>  #define QEDR_ROCE_PKEY_TABLE_LEN 1
>  #define QEDR_ROCE_PKEY_DEFAULT 0xffff
>
> +struct qedr_pbl {
> +	struct list_head list_entry;
> +	void *va;
> +	dma_addr_t pa;
> +};
> +
>  struct qedr_ucontext {
>  	struct ib_ucontext ibucontext;
>  	struct qedr_dev *dev;
> @@ -196,6 +206,64 @@ struct qedr_ucontext {
>  	struct mutex mm_list_lock;
>  };
>
> +union db_prod64 {
> +	struct rdma_pwm_val32_data data;
> +	u64 raw;
> +};
> +
> +enum qedr_cq_type {
> +	QEDR_CQ_TYPE_GSI,
> +	QEDR_CQ_TYPE_KERNEL,
> +	QEDR_CQ_TYPE_USER
> +};
> +
> +struct qedr_pbl_info {
> +	u32 num_pbls;
> +	u32 num_pbes;
> +	u32 pbl_size;
> +	u32 pbe_size;
> +	bool two_layered;
> +};
> +
> +struct qedr_userq {
> +	struct ib_umem *umem;
> +	struct qedr_pbl_info pbl_info;
> +	struct qedr_pbl *pbl_tbl;
> +	u64 buf_addr;
> +	size_t buf_len;
> +};
> +
> +struct qedr_cq {
> +	struct ib_cq ibcq;
> +
> +	enum qedr_cq_type cq_type;
> +	u32 sig;
> +
> +	u16 icid;
> +
> +	/* Lock to protect multiplem CQ's */
> +	spinlock_t cq_lock;
> +	u8 arm_flags;
> +	struct qed_chain pbl;
> +
> +	void __iomem *db_addr;
> +	union db_prod64 db;
> +
> +	u8 pbl_toggle;
> +	union rdma_cqe *latest_cqe;
> +	union rdma_cqe *toggle_cqe;
> +
> +	u32 cq_cons;
> +
> +	struct qedr_userq q;
> +};
> +
> +struct qedr_pd {
> +	struct ib_pd ibpd;
> +	u32 pd_id;
> +	struct qedr_ucontext *uctx;
> +};
> +
>  struct qedr_mm {
>  	struct {
>  		u64 phy_addr;
> @@ -215,4 +283,14 @@ static inline struct qedr_dev *get_qedr_dev(struct ib_device *ibdev)
>  	return container_of(ibdev, struct qedr_dev, ibdev);
>  }
>
> +static inline struct qedr_pd *get_qedr_pd(struct ib_pd *ibpd)
> +{
> +	return container_of(ibpd, struct qedr_pd, ibpd);
> +}
> +
> +static inline struct qedr_cq *get_qedr_cq(struct ib_cq *ibcq)
> +{
> +	return container_of(ibcq, struct qedr_cq, ibcq);
> +}
> +
>  #endif
> diff --git a/drivers/infiniband/hw/qedr/qedr_hsi_rdma.h b/drivers/infiniband/hw/qedr/qedr_hsi_rdma.h
> index 3e508fb..84f6520 100644
> --- a/drivers/infiniband/hw/qedr/qedr_hsi_rdma.h
> +++ b/drivers/infiniband/hw/qedr/qedr_hsi_rdma.h
> @@ -47,6 +47,19 @@ struct rdma_cqe_responder {
>  	__le32 imm_data_hi;
>  	__le16 rq_cons;
>  	u8 flags;
> +#define RDMA_CQE_RESPONDER_TOGGLE_BIT_MASK  0x1
> +#define RDMA_CQE_RESPONDER_TOGGLE_BIT_SHIFT 0
> +#define RDMA_CQE_RESPONDER_TYPE_MASK        0x3
> +#define RDMA_CQE_RESPONDER_TYPE_SHIFT       1
> +#define RDMA_CQE_RESPONDER_INV_FLG_MASK     0x1
> +#define RDMA_CQE_RESPONDER_INV_FLG_SHIFT    3
> +#define RDMA_CQE_RESPONDER_IMM_FLG_MASK     0x1
> +#define RDMA_CQE_RESPONDER_IMM_FLG_SHIFT    4
> +#define RDMA_CQE_RESPONDER_RDMA_FLG_MASK    0x1
> +#define RDMA_CQE_RESPONDER_RDMA_FLG_SHIFT   5
> +#define RDMA_CQE_RESPONDER_RESERVED2_MASK   0x3
> +#define RDMA_CQE_RESPONDER_RESERVED2_SHIFT  6
> +	u8 status;
>  };
>
>  struct rdma_cqe_requester {
> @@ -58,6 +71,12 @@ struct rdma_cqe_requester {
>  	__le32 reserved3;
>  	__le16 reserved4;
>  	u8 flags;
> +#define RDMA_CQE_REQUESTER_TOGGLE_BIT_MASK  0x1
> +#define RDMA_CQE_REQUESTER_TOGGLE_BIT_SHIFT 0
> +#define RDMA_CQE_REQUESTER_TYPE_MASK        0x3
> +#define RDMA_CQE_REQUESTER_TYPE_SHIFT       1
> +#define RDMA_CQE_REQUESTER_RESERVED5_MASK   0x1F
> +#define RDMA_CQE_REQUESTER_RESERVED5_SHIFT  3
>  	u8 status;
>  };
>
> @@ -66,6 +85,12 @@ struct rdma_cqe_common {
>  	struct regpair qp_handle;
>  	__le16 reserved1[7];
>  	u8 flags;
> +#define RDMA_CQE_COMMON_TOGGLE_BIT_MASK  0x1
> +#define RDMA_CQE_COMMON_TOGGLE_BIT_SHIFT 0
> +#define RDMA_CQE_COMMON_TYPE_MASK        0x3
> +#define RDMA_CQE_COMMON_TYPE_SHIFT       1
> +#define RDMA_CQE_COMMON_RESERVED2_MASK   0x1F
> +#define RDMA_CQE_COMMON_RESERVED2_SHIFT  3
>  	u8 status;

It is VERY uncommon to mix defines and structs together.
Please don't do it, it confuses a lot and doesn't help to
readability/debug.

>  };
>
> @@ -76,6 +101,45 @@ union rdma_cqe {
>  	struct rdma_cqe_common cmn;
>  };
>
> +/* * CQE requester status enumeration */
> +enum rdma_cqe_requester_status_enum {
> +	RDMA_CQE_REQ_STS_OK,
> +	RDMA_CQE_REQ_STS_BAD_RESPONSE_ERR,
> +	RDMA_CQE_REQ_STS_LOCAL_LENGTH_ERR,
> +	RDMA_CQE_REQ_STS_LOCAL_QP_OPERATION_ERR,
> +	RDMA_CQE_REQ_STS_LOCAL_PROTECTION_ERR,
> +	RDMA_CQE_REQ_STS_MEMORY_MGT_OPERATION_ERR,
> +	RDMA_CQE_REQ_STS_REMOTE_INVALID_REQUEST_ERR,
> +	RDMA_CQE_REQ_STS_REMOTE_ACCESS_ERR,
> +	RDMA_CQE_REQ_STS_REMOTE_OPERATION_ERR,
> +	RDMA_CQE_REQ_STS_RNR_NAK_RETRY_CNT_ERR,
> +	RDMA_CQE_REQ_STS_TRANSPORT_RETRY_CNT_ERR,
> +	RDMA_CQE_REQ_STS_WORK_REQUEST_FLUSHED_ERR,
> +	MAX_RDMA_CQE_REQUESTER_STATUS_ENUM

Please add "," at the last line of enums, it will allow future changes
to these enums with minimal churn.

> +};
> +
> +/* CQE responder status enumeration */
> +enum rdma_cqe_responder_status_enum {
> +	RDMA_CQE_RESP_STS_OK,
> +	RDMA_CQE_RESP_STS_LOCAL_ACCESS_ERR,
> +	RDMA_CQE_RESP_STS_LOCAL_LENGTH_ERR,
> +	RDMA_CQE_RESP_STS_LOCAL_QP_OPERATION_ERR,
> +	RDMA_CQE_RESP_STS_LOCAL_PROTECTION_ERR,
> +	RDMA_CQE_RESP_STS_MEMORY_MGT_OPERATION_ERR,
> +	RDMA_CQE_RESP_STS_REMOTE_INVALID_REQUEST_ERR,
> +	RDMA_CQE_RESP_STS_WORK_REQUEST_FLUSHED_ERR,
> +	MAX_RDMA_CQE_RESPONDER_STATUS_ENUM
> +};
> +
> +/* CQE type enumeration */
> +enum rdma_cqe_type {
> +	RDMA_CQE_TYPE_REQUESTER,
> +	RDMA_CQE_TYPE_RESPONDER_RQ,
> +	RDMA_CQE_TYPE_RESPONDER_SRQ,
> +	RDMA_CQE_TYPE_INVALID,
> +	MAX_RDMA_CQE_TYPE
> +};
> +
>  struct rdma_sq_sge {
>  	__le32 length;
>  	struct regpair	addr;
> @@ -93,4 +157,19 @@ struct rdma_srq_sge {
>  	__le32 length;
>  	__le32 l_key;
>  };
> +
> +/* Rdma doorbell data for CQ */
> +struct rdma_pwm_val32_data {
> +	__le16 icid;
> +	u8 agg_flags;
> +	u8 params;
> +#define RDMA_PWM_VAL32_DATA_AGG_CMD_MASK    0x3
> +#define RDMA_PWM_VAL32_DATA_AGG_CMD_SHIFT   0
> +#define RDMA_PWM_VAL32_DATA_BYPASS_EN_MASK  0x1
> +#define RDMA_PWM_VAL32_DATA_BYPASS_EN_SHIFT 2
> +#define RDMA_PWM_VAL32_DATA_RESERVED_MASK   0x1F
> +#define RDMA_PWM_VAL32_DATA_RESERVED_SHIFT  3
> +	__le32 value;
> +};
> +
>  #endif /* __QED_HSI_RDMA__ */
> diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
> index ab5f11a..bff4ece 100644
> --- a/drivers/infiniband/hw/qedr/verbs.c
> +++ b/drivers/infiniband/hw/qedr/verbs.c
> @@ -49,6 +49,17 @@
>  #include "verbs.h"
>  #include <rdma/qedr-abi.h>
>
> +#define DB_ADDR_SHIFT(addr)		((addr) << DB_PWM_ADDR_OFFSET_SHIFT)
> +
> +int qedr_query_pkey(struct ib_device *ibdev, u8 port, u16 index, u16 *pkey)
> +{
> +	if (index > QEDR_ROCE_PKEY_TABLE_LEN)
> +		return -EINVAL;
> +
> +	*pkey = QEDR_ROCE_PKEY_DEFAULT;
> +	return 0;
> +}
> +
>  int qedr_query_gid(struct ib_device *ibdev, u8 port, int index,
>  		   union ib_gid *sgid)
>  {
> @@ -453,3 +464,535 @@ int qedr_mmap(struct ib_ucontext *context, struct vm_area_struct *vma)
>  	DP_DEBUG(dev, QEDR_MSG_INIT, "qedr_mmap return code: %d\n", rc);
>  	return rc;
>  }
> +
> +struct ib_pd *qedr_alloc_pd(struct ib_device *ibdev,
> +			    struct ib_ucontext *context, struct ib_udata *udata)
> +{
> +	struct qedr_dev *dev = get_qedr_dev(ibdev);
> +	struct qedr_ucontext *uctx = NULL;
> +	struct qedr_alloc_pd_uresp uresp;
> +	struct qedr_pd *pd;
> +	u16 pd_id;
> +	int rc;
> +
> +	DP_DEBUG(dev, QEDR_MSG_INIT, "Function called from: %s\n",
> +		 (udata && context) ? "User Lib" : "Kernel");
> +	if (udata && context)
> +		uctx = get_qedr_ucontext(context);
> +
> +	pd = kzalloc(sizeof(*pd), GFP_KERNEL);
> +	if (!pd) {
> +		DP_ERR(dev, "failed to alloce PD\n");
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	if (!dev->rdma_ctx) {
> +		DP_ERR(dev, "invlaid RDMA context\n");
> +		return ERR_PTR(-EINVAL);

Don't you want to free pd which was allocated a couple of lines before?

> +	}
> +
> +	dev->ops->rdma_alloc_pd(dev->rdma_ctx, &pd_id);
> +
> +	uresp.pd_id = pd_id;
> +	pd->pd_id = pd_id;
> +
> +	if (uctx) {
> +		rc = ib_copy_to_udata(udata, &uresp, sizeof(uresp));
> +		if (rc)
> +			DP_ERR(dev, "copy error pd_id=0x%x.\n", pd_id);
> +		uctx->pd = pd;
> +		pd->uctx = uctx;
> +	}
> +
> +	return &pd->ibpd;
> +}
> +
> +int qedr_dealloc_pd(struct ib_pd *ibpd)
> +{
> +	struct qedr_dev *dev = get_qedr_dev(ibpd->device);
> +	struct qedr_pd *pd = get_qedr_pd(ibpd);
> +
> +	if (!pd)
> +		pr_err("Invalid PD received in dealloc_pd\n");
> +
> +	DP_DEBUG(dev, QEDR_MSG_INIT, "Deallocating PD %d\n", pd->pd_id);
> +	dev->ops->rdma_dealloc_pd(dev->rdma_ctx, pd->pd_id);
> +
> +	kfree(pd);
> +
> +	return 0;
> +}
> +
> +static void qedr_free_pbl(struct qedr_dev *dev,
> +			  struct qedr_pbl_info *pbl_info, struct qedr_pbl *pbl)
> +{
> +	struct pci_dev *pdev = dev->pdev;
> +	int i;
> +
> +	for (i = 0; i < pbl_info->num_pbls; i++) {
> +		if (!pbl[i].va)
> +			continue;
> +		dma_free_coherent(&pdev->dev, pbl_info->pbl_size,
> +				  pbl[i].va, pbl[i].pa);
> +	}
> +
> +	kfree(pbl);
> +}
> +
> +#define MIN_FW_PBL_PAGE_SIZE (4 * 1024)
> +#define MAX_FW_PBL_PAGE_SIZE (64 * 1024)
> +
> +#define NUM_PBES_ON_PAGE(_page_size) (_page_size / sizeof(u64))
> +#define MAX_PBES_ON_PAGE NUM_PBES_ON_PAGE(MAX_FW_PBL_PAGE_SIZE)
> +#define MAX_PBES_TWO_LAYER (MAX_PBES_ON_PAGE * MAX_PBES_ON_PAGE)
> +
> +static struct qedr_pbl *qedr_alloc_pbl_tbl(struct qedr_dev *dev,
> +					   struct qedr_pbl_info *pbl_info,
> +					   gfp_t flags)
> +{
> +	struct pci_dev *pdev = dev->pdev;
> +	struct qedr_pbl *pbl_table;
> +	dma_addr_t *pbl_main_tbl;
> +	dma_addr_t pa;
> +	int rc = 0;
> +	void *va;
> +	int i;
> +
> +	pbl_table = kcalloc(pbl_info->num_pbls, sizeof(*pbl_table), flags);
> +
> +	if (!pbl_table) {
> +		DP_ERR(dev, "pbl table is NULL\n");

No, there is no need to print this error, kcalloc will print it for you.

> +		return NULL;

??? return ERR_PTR(-ENOMEM) ????

> +	}
> +
> +	for (i = 0; i < pbl_info->num_pbls; i++) {
> +		va = dma_alloc_coherent(&pdev->dev, pbl_info->pbl_size,
> +					&pa, flags);
> +		if (!va) {
> +			DP_ERR(dev, "Failed to allocate pbl#%d\n", i);
> +			rc = -ENOMEM;
> +			goto err;
> +		}
> +		memset(va, 0, pbl_info->pbl_size);
> +		pbl_table[i].va = va;
> +		pbl_table[i].pa = pa;
> +	}
> +
> +	/* Two-Layer PBLs, if we have more than one pbl we need to initialize
> +	 * the first one with physical pointers to all of the rest
> +	 */
> +	pbl_main_tbl = (dma_addr_t *)pbl_table[0].va;
> +	for (i = 0; i < pbl_info->num_pbls - 1; i++)
> +		pbl_main_tbl[i] = pbl_table[i + 1].pa;
> +
> +	return pbl_table;
> +
> +err:
> +	qedr_free_pbl(dev, pbl_info, pbl_table);
> +	return NULL;
> +}
> +
> +static int qedr_prepare_pbl_tbl(struct qedr_dev *dev,
> +				struct qedr_pbl_info *pbl_info,
> +				u32 num_pbes, int two_layer_capable)
> +{
> +	u32 pbl_capacity;
> +	u32 pbl_size;
> +	u32 num_pbls;
> +
> +	if ((num_pbes > MAX_PBES_ON_PAGE) && two_layer_capable) {
> +		if (num_pbes > MAX_PBES_TWO_LAYER) {
> +			DP_ERR(dev, "prepare pbl table: too many pages %d\n",
> +			       num_pbes);
> +			return -EINVAL;
> +		}
> +
> +		/* calculate required pbl page size */
> +		pbl_size = MIN_FW_PBL_PAGE_SIZE;
> +		pbl_capacity = NUM_PBES_ON_PAGE(pbl_size) *
> +			       NUM_PBES_ON_PAGE(pbl_size);
> +
> +		while (pbl_capacity < num_pbes) {
> +			pbl_size *= 2;
> +			pbl_capacity = pbl_size / sizeof(u64);
> +			pbl_capacity = pbl_capacity * pbl_capacity;
> +		}
> +
> +		num_pbls = DIV_ROUND_UP(num_pbes, NUM_PBES_ON_PAGE(pbl_size));
> +		num_pbls++;	/* One for the layer0 ( points to the pbls) */
> +		pbl_info->two_layered = true;
> +	} else {
> +		/* One layered PBL */
> +		num_pbls = 1;
> +		pbl_size = max_t(u32, MIN_FW_PBL_PAGE_SIZE,
> +				 roundup_pow_of_two((num_pbes * sizeof(u64))));
> +		pbl_info->two_layered = false;
> +	}
> +
> +	pbl_info->num_pbls = num_pbls;
> +	pbl_info->pbl_size = pbl_size;
> +	pbl_info->num_pbes = num_pbes;
> +
> +	DP_DEBUG(dev, QEDR_MSG_MR,
> +		 "prepare pbl table: num_pbes=%d, num_pbls=%d, pbl_size=%d\n",
> +		 pbl_info->num_pbes, pbl_info->num_pbls, pbl_info->pbl_size);
> +
> +	return 0;
> +}
> +
> +static void qedr_populate_pbls(struct qedr_dev *dev, struct ib_umem *umem,
> +			       struct qedr_pbl *pbl,
> +			       struct qedr_pbl_info *pbl_info)
> +{
> +	int shift, pg_cnt, pages, pbe_cnt, total_num_pbes = 0;
> +	struct qedr_pbl *pbl_tbl;
> +	struct scatterlist *sg;
> +	struct regpair *pbe;
> +	int entry;
> +	u32 addr;
> +
> +	if (!pbl_info->num_pbes)
> +		return;
> +
> +	/* If we have a two layered pbl, the first pbl points to the rest
> +	 * of the pbls and the first entry lays on the second pbl in the table
> +	 */
> +	if (pbl_info->two_layered)
> +		pbl_tbl = &pbl[1];
> +	else
> +		pbl_tbl = pbl;
> +
> +	pbe = (struct regpair *)pbl_tbl->va;
> +	if (!pbe) {
> +		DP_ERR(dev, "pbe is NULL\n");

????

> +		return;
> +	}
> +
> +	pbe_cnt = 0;
> +
> +	shift = ilog2(umem->page_size);
> +
> +	for_each_sg(umem->sg_head.sgl, sg, umem->nmap, entry) {
> +		pages = sg_dma_len(sg) >> shift;
> +		for (pg_cnt = 0; pg_cnt < pages; pg_cnt++) {
> +			/* store the page address in pbe */
> +			pbe->lo = cpu_to_le32(sg_dma_address(sg) +
> +					      umem->page_size * pg_cnt);
> +			addr = upper_32_bits(sg_dma_address(sg) +
> +					     umem->page_size * pg_cnt);
> +			pbe->hi = cpu_to_le32(addr);
> +			pbe_cnt++;
> +			total_num_pbes++;
> +			pbe++;
> +
> +			if (total_num_pbes == pbl_info->num_pbes)
> +				return;
> +
> +			/* If the given pbl is full storing the pbes,
> +			 * move to next pbl.
> +			 */
> +			if (pbe_cnt == (pbl_info->pbl_size / sizeof(u64))) {
> +				pbl_tbl++;
> +				pbe = (struct regpair *)pbl_tbl->va;
> +				pbe_cnt = 0;
> +			}
> +		}
> +	}
> +}
> +
> +static int qedr_copy_cq_uresp(struct qedr_dev *dev,
> +			      struct qedr_cq *cq, struct ib_udata *udata)
> +{
> +	struct qedr_create_cq_uresp uresp;
> +	int rc;
> +
> +	memset(&uresp, 0, sizeof(uresp));
> +
> +	uresp.db_offset = DB_ADDR_SHIFT(DQ_PWM_OFFSET_UCM_RDMA_CQ_CONS_32BIT);
> +	uresp.icid = cq->icid;
> +
> +	rc = ib_copy_to_udata(udata, &uresp, sizeof(uresp));
> +	if (rc)
> +		DP_ERR(dev, "copy error cqid=0x%x.\n", cq->icid);
> +
> +	return rc;
> +}
> +
> +static void consume_cqe(struct qedr_cq *cq)
> +{
> +	if (cq->latest_cqe == cq->toggle_cqe)
> +		cq->pbl_toggle ^= RDMA_CQE_REQUESTER_TOGGLE_BIT_MASK;
> +
> +	cq->latest_cqe = qed_chain_consume(&cq->pbl);
> +}
> +
> +static inline int qedr_align_cq_entries(int entries)
> +{
> +	u64 size, aligned_size;
> +
> +	/* We allocate an extra entry that we don't report to the FW. */
> +	size = (entries + 1) * QEDR_CQE_SIZE;
> +	aligned_size = ALIGN(size, PAGE_SIZE);
> +
> +	return aligned_size / QEDR_CQE_SIZE;
> +}
> +
> +static inline int qedr_init_user_queue(struct ib_ucontext *ib_ctx,
> +				       struct qedr_dev *dev,
> +				       struct qedr_userq *q,
> +				       u64 buf_addr, size_t buf_len,
> +				       int access, int dmasync)
> +{
> +	int page_cnt;
> +	int rc;
> +
> +	q->buf_addr = buf_addr;
> +	q->buf_len = buf_len;
> +	q->umem = ib_umem_get(ib_ctx, q->buf_addr, q->buf_len, access, dmasync);
> +	if (IS_ERR(q->umem)) {
> +		DP_ERR(dev, "create user queue: failed ib_umem_get, got %ld\n",
> +		       PTR_ERR(q->umem));
> +		return PTR_ERR(q->umem);
> +	}
> +
> +	page_cnt = ib_umem_page_count(q->umem);
> +	rc = qedr_prepare_pbl_tbl(dev, &q->pbl_info, page_cnt, 0);
> +	if (rc)
> +		goto err0;
> +
> +	q->pbl_tbl = qedr_alloc_pbl_tbl(dev, &q->pbl_info, GFP_KERNEL);
> +	if (!q->pbl_tbl)
> +		goto err0;
> +
> +	qedr_populate_pbls(dev, q->umem, q->pbl_tbl, &q->pbl_info);
> +
> +	return 0;
> +
> +err0:
> +	ib_umem_release(q->umem);
> +
> +	return rc;
> +}
> +
> +static inline void qedr_init_cq_params(struct qedr_cq *cq,
> +				       struct qedr_ucontext *ctx,
> +				       struct qedr_dev *dev, int vector,
> +				       int chain_entries, int page_cnt,
> +				       u64 pbl_ptr,
> +				       struct qed_rdma_create_cq_in_params
> +				       *params)
> +{
> +	memset(params, 0, sizeof(*params));
> +	params->cq_handle_hi = upper_32_bits((uintptr_t)cq);
> +	params->cq_handle_lo = lower_32_bits((uintptr_t)cq);
> +	params->cnq_id = vector;
> +	params->cq_size = chain_entries - 1;
> +	params->dpi = (ctx) ? ctx->dpi : dev->dpi;
> +	params->pbl_num_pages = page_cnt;
> +	params->pbl_ptr = pbl_ptr;
> +	params->pbl_two_level = 0;
> +}
> +
> +static void doorbell_cq(struct qedr_cq *cq, u32 cons, u8 flags)
> +{
> +	/* Flush data before signalling doorbell */
> +	wmb();
> +	cq->db.data.agg_flags = flags;
> +	cq->db.data.value = cpu_to_le32(cons);
> +	writeq(cq->db.raw, cq->db_addr);
> +
> +	/* Make sure write would stick */
> +	mmiowb();
> +}
> +
> +int qedr_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify_flags flags)
> +{
> +	struct qedr_cq *cq = get_qedr_cq(ibcq);
> +	unsigned long sflags;
> +
> +	if (cq->cq_type == QEDR_CQ_TYPE_GSI)
> +		return 0;
> +
> +	spin_lock_irqsave(&cq->cq_lock, sflags);
> +
> +	cq->arm_flags = 0;
> +
> +	if (flags & IB_CQ_SOLICITED)
> +		cq->arm_flags |= DQ_UCM_ROCE_CQ_ARM_SE_CF_CMD;
> +
> +	if (flags & IB_CQ_NEXT_COMP)
> +		cq->arm_flags |= DQ_UCM_ROCE_CQ_ARM_CF_CMD;
> +
> +	doorbell_cq(cq, cq->cq_cons - 1, cq->arm_flags);
> +
> +	spin_unlock_irqrestore(&cq->cq_lock, sflags);
> +
> +	return 0;
> +}
> +
> +struct ib_cq *qedr_create_cq(struct ib_device *ibdev,
> +			     const struct ib_cq_init_attr *attr,
> +			     struct ib_ucontext *ib_ctx, struct ib_udata *udata)
> +{
> +	struct qedr_ucontext *ctx = get_qedr_ucontext(ib_ctx);
> +	struct qed_rdma_destroy_cq_out_params destroy_oparams;
> +	struct qed_rdma_destroy_cq_in_params destroy_iparams;
> +	struct qedr_dev *dev = get_qedr_dev(ibdev);
> +	struct qed_rdma_create_cq_in_params params;
> +	struct qedr_create_cq_ureq ureq;
> +	int vector = attr->comp_vector;
> +	int entries = attr->cqe;
> +	struct qedr_cq *cq;
> +	int chain_entries;
> +	int page_cnt;
> +	u64 pbl_ptr;
> +	u16 icid;
> +	int rc;
> +
> +	DP_DEBUG(dev, QEDR_MSG_INIT,
> +		 "create_cq: called from %s. entries=%d, vector=%d\n",
> +		 udata ? "User Lib" : "Kernel", entries, vector);
> +
> +	if (entries > QEDR_MAX_CQES) {
> +		DP_ERR(dev,
> +		       "create cq: the number of entries %d is too high. Must be equal or below %d.\n",
> +		       entries, QEDR_MAX_CQES);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	chain_entries = qedr_align_cq_entries(entries);
> +	chain_entries = min_t(int, chain_entries, QEDR_MAX_CQES);
> +
> +	cq = kzalloc(sizeof(*cq), GFP_KERNEL);
> +	if (!cq)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (udata) {
> +		memset(&ureq, 0, sizeof(ureq));
> +		if (ib_copy_from_udata(&ureq, udata, sizeof(ureq))) {
> +			DP_ERR(dev,
> +			       "create cq: problem copying data from user space\n");
> +			goto err0;
> +		}
> +
> +		if (!ureq.len) {
> +			DP_ERR(dev,
> +			       "create cq: cannot create a cq with 0 entries\n");
> +			goto err0;
> +		}
> +
> +		cq->cq_type = QEDR_CQ_TYPE_USER;
> +
> +		rc = qedr_init_user_queue(ib_ctx, dev, &cq->q, ureq.addr,
> +					  ureq.len, IB_ACCESS_LOCAL_WRITE, 1);
> +		if (rc)
> +			goto err0;
> +
> +		pbl_ptr = cq->q.pbl_tbl->pa;
> +		page_cnt = cq->q.pbl_info.num_pbes;
> +	} else {
> +		cq->cq_type = QEDR_CQ_TYPE_KERNEL;
> +
> +		rc = dev->ops->common->chain_alloc(dev->cdev,
> +						   QED_CHAIN_USE_TO_CONSUME,
> +						   QED_CHAIN_MODE_PBL,
> +						   QED_CHAIN_CNT_TYPE_U32,
> +						   chain_entries,
> +						   sizeof(union rdma_cqe),
> +						   &cq->pbl);
> +		if (rc)
> +			goto err1;
> +
> +		page_cnt = qed_chain_get_page_cnt(&cq->pbl);
> +		pbl_ptr = qed_chain_get_pbl_phys(&cq->pbl);
> +	}
> +
> +	qedr_init_cq_params(cq, ctx, dev, vector, chain_entries, page_cnt,
> +			    pbl_ptr, &params);
> +
> +	rc = dev->ops->rdma_create_cq(dev->rdma_ctx, &params, &icid);
> +	if (rc)
> +		goto err2;
> +
> +	cq->icid = icid;
> +	cq->sig = QEDR_CQ_MAGIC_NUMBER;
> +	spin_lock_init(&cq->cq_lock);
> +
> +	if (ib_ctx) {
> +		rc = qedr_copy_cq_uresp(dev, cq, udata);
> +		if (rc)
> +			goto err3;
> +	} else {
> +		/* Generate doorbell address. */
> +		cq->db_addr = dev->db_addr +
> +		    DB_ADDR_SHIFT(DQ_PWM_OFFSET_UCM_RDMA_CQ_CONS_32BIT);
> +		cq->db.data.icid = cq->icid;
> +		cq->db.data.params = DB_AGG_CMD_SET <<
> +		    RDMA_PWM_VAL32_DATA_AGG_CMD_SHIFT;
> +
> +		/* point to the very last element, passing it we will toggle */
> +		cq->toggle_cqe = qed_chain_get_last_elem(&cq->pbl);
> +		cq->pbl_toggle = RDMA_CQE_REQUESTER_TOGGLE_BIT_MASK;
> +		cq->latest_cqe = NULL;
> +		consume_cqe(cq);
> +		cq->cq_cons = qed_chain_get_cons_idx_u32(&cq->pbl);
> +	}
> +
> +	DP_DEBUG(dev, QEDR_MSG_CQ,
> +		 "create cq: icid=0x%0x, addr=%p, size(entries)=0x%0x\n",
> +		 cq->icid, cq, params.cq_size);
> +
> +	return &cq->ibcq;
> +
> +err3:
> +	destroy_iparams.icid = cq->icid;
> +	dev->ops->rdma_destroy_cq(dev->rdma_ctx, &destroy_iparams,
> +				  &destroy_oparams);
> +err2:
> +	if (udata)
> +		qedr_free_pbl(dev, &cq->q.pbl_info, cq->q.pbl_tbl);
> +	else
> +		dev->ops->common->chain_free(dev->cdev, &cq->pbl);
> +err1:
> +	if (udata)
> +		ib_umem_release(cq->q.umem);
> +err0:
> +	kfree(cq);
> +	return ERR_PTR(-EINVAL);
> +}
> +
> +int qedr_resize_cq(struct ib_cq *ibcq, int new_cnt, struct ib_udata *udata)
> +{
> +	struct qedr_dev *dev = get_qedr_dev(ibcq->device);
> +	struct qedr_cq *cq = get_qedr_cq(ibcq);
> +
> +	DP_ERR(dev, "cq %p RESIZE NOT SUPPORTED\n", cq);
> +
> +	return 0;
> +}
> +
> +int qedr_destroy_cq(struct ib_cq *ibcq)
> +{
> +	struct qedr_dev *dev = get_qedr_dev(ibcq->device);
> +	struct qed_rdma_destroy_cq_out_params oparams;
> +	struct qed_rdma_destroy_cq_in_params iparams;
> +	struct qedr_cq *cq = get_qedr_cq(ibcq);
> +
> +	DP_DEBUG(dev, QEDR_MSG_CQ, "destroy cq: cq_id %d", cq->icid);
> +
> +	/* GSIs CQs are handled by driver, so they don't exist in the FW */
> +	if (cq->cq_type != QEDR_CQ_TYPE_GSI) {
> +		iparams.icid = cq->icid;
> +		dev->ops->rdma_destroy_cq(dev->rdma_ctx, &iparams, &oparams);
> +		dev->ops->common->chain_free(dev->cdev, &cq->pbl);
> +	}
> +
> +	if (ibcq->uobject && ibcq->uobject->context) {
> +		qedr_free_pbl(dev, &cq->q.pbl_info, cq->q.pbl_tbl);
> +		ib_umem_release(cq->q.umem);
> +	}
> +
> +	kfree(cq);
> +
> +	return 0;
> +}
> diff --git a/drivers/infiniband/hw/qedr/verbs.h b/drivers/infiniband/hw/qedr/verbs.h
> index 9472044..36c8a69 100644
> --- a/drivers/infiniband/hw/qedr/verbs.h
> +++ b/drivers/infiniband/hw/qedr/verbs.h
> @@ -40,6 +40,8 @@ int qedr_modify_port(struct ib_device *, u8 port, int mask,
>
>  int qedr_query_gid(struct ib_device *, u8 port, int index, union ib_gid *gid);
>
> +int qedr_query_pkey(struct ib_device *, u8 port, u16 index, u16 *pkey);
> +
>  struct ib_ucontext *qedr_alloc_ucontext(struct ib_device *, struct ib_udata *);
>  int qedr_dealloc_ucontext(struct ib_ucontext *);
>
> @@ -49,4 +51,16 @@ int qedr_del_gid(struct ib_device *device, u8 port_num,
>  int qedr_add_gid(struct ib_device *device, u8 port_num,
>  		 unsigned int index, const union ib_gid *gid,
>  		 const struct ib_gid_attr *attr, void **context);
> +struct ib_pd *qedr_alloc_pd(struct ib_device *,
> +			    struct ib_ucontext *, struct ib_udata *);
> +int qedr_dealloc_pd(struct ib_pd *pd);
> +
> +struct ib_cq *qedr_create_cq(struct ib_device *ibdev,
> +			     const struct ib_cq_init_attr *attr,
> +			     struct ib_ucontext *ib_ctx,
> +			     struct ib_udata *udata);
> +int qedr_resize_cq(struct ib_cq *, int cqe, struct ib_udata *);
> +int qedr_destroy_cq(struct ib_cq *);
> +int qedr_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify_flags flags);
> +
>  #endif
> diff --git a/include/uapi/rdma/qedr-abi.h b/include/uapi/rdma/qedr-abi.h
> index f7c7fff..b0fc5f2 100644
> --- a/include/uapi/rdma/qedr-abi.h
> +++ b/include/uapi/rdma/qedr-abi.h
> @@ -50,4 +50,23 @@ struct qedr_alloc_ucontext_resp {
>  	__u32 sges_per_srq_wr;
>  	__u32 max_cqes;
>  };
> +
> +struct qedr_alloc_pd_ureq {
> +	__u64 rsvd1;
> +};
> +
> +struct qedr_alloc_pd_uresp {
> +	__u32 pd_id;
> +};
> +
> +struct qedr_create_cq_ureq {
> +	__u64 addr;
> +	__u64 len;
> +};
> +
> +struct qedr_create_cq_uresp {
> +	__u32 db_offset;
> +	__u16 icid;
> +};
> +
>  #endif /* __QEDR_USER_H__ */
> --
> 1.8.3.1
>
> --
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCHv12 0/3] rdmacg: IB/core: rdma controller support
From: Parav Pandit @ 2016-10-06 12:59 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: cgroups-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA,
	Linux Kernel Mailing List, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	Tejun Heo, Li Zefan, Johannes Weiner, Doug Ledford,
	Christoph Hellwig, Liran Liss, Hefty, Sean, Jason Gunthorpe,
	Haggai Eran, Jonathan Corbet,
	james.l.morris-QHcLZuEGTsvQT0dZR+AlfA,
	serge-A9i7LUbDfNHQT0dZR+AlfA, Or Gerlitz, Matan Barak,
	Andrew Morton, linux-security-module-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161005112206.GC9282-2ukJVAZIZ/Y@public.gmane.org>

Hi Leon,

On Wed, Oct 5, 2016 at 4:52 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Aug 31, 2016 at 02:07:24PM +0530, Parav Pandit wrote:
>> rdmacg: IB/core: rdma controller support
>>
>> Patch is generated and tested against below Doug's linux-rdma
>> git tree.
>>
>> URL: git://git.kernel.org/pub/scm/linux/kernel/git/dledford/rdma.git
>> Branch: master
>>
>> Patchset is also compiled and tested against below Tejun's cgroup tree
>> using cgroup v2 mode.
>> URL: git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git
>> Branch: master
>>
>> Overview:
>> Currently user space applications can easily take away all the rdma
>> device specific resources such as AH, CQ, QP, MR etc. Due to which other
>> applications in other cgroup or kernel space ULPs may not even get chance
>> to allocate any rdma resources. This results into service unavailibility.
>>
>> RDMA cgroup addresses this issue by allowing resource accounting,
>> limit enforcement on per cgroup, per rdma device basis.
>>
>> RDMA uverbs layer will enforce limits on well defined RDMA verb
>> resources without any HCA vendor device driver involvement.
>>
>> RDMA uverbs layer will not do limit enforcement of HCA hw vendor
>> specific resources. Instead rdma cgroup provides set of APIs
>> through which vendor specific drivers can do resource accounting
>> by making use of rdma cgroup.
>
> Hi Parav,
> I want to propose an extension to the RDMA cgroup which can be done as
> follow-up patches.
>

To bring logical end to this feature/patch discussion and to progress
towards merging it, Lets discuss this new feature in follow-on email
right after this email between these two mailing list and I will drop
linux kernel and docs mailing list.

> Let's add new global type, which will control whole HCA (for example in percentages). It will
> allow natively define new objects without need to introduce them to the user.
>
> This HCA share will be overwritten by specific UVERBS types which you
> already defined.
>
> What do you think?
>
> Except this proposal,
> Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Thanks a lot for review.

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

* Re: [PATCHv12 1/3] rdmacg: Added rdma cgroup controller
From: Parav Pandit @ 2016-10-06 12:55 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Tejun Heo, Leon Romanovsky, Jason Gunthorpe, Matan Barak,
	cgroups-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA,
	Linux Kernel Mailing List, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	Li Zefan, Johannes Weiner, Doug Ledford, Liran Liss, Hefty, Sean,
	Haggai Eran, Jonathan Corbet,
	james.l.morris-QHcLZuEGTsvQT0dZR+AlfA,
	serge-A9i7LUbDfNHQT0dZR+AlfA, Or Gerlitz, Andrew Morton,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161005063735.GC3086-jcswGhMUV9g@public.gmane.org>

Hi Christoph,

On Wed, Oct 5, 2016 at 12:07 PM, Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org> wrote:
> FYI, the patches look fine to me:
>
> Acked-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
>
Thanks a lot for review.

Parav

^ permalink raw reply

* Re: [PATCH 8/9] Remove most checks of __BYTE_ORDER
From: Yishai Hadas @ 2016-10-06 12:09 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Steve Wise, Steve Wise, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	Tatyana Nikolova, Hal Rosenstock, Doug Ledford, Devesh Sharma,
	Sean Hefty, Bart Van Assche,
	yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
In-Reply-To: <20161005174449.GG18636-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

On 10/5/2016 8:44 PM, Jason Gunthorpe wrote:
> On Wed, Oct 05, 2016 at 06:16:45PM +0300, Yishai Hadas wrote:
>
>> Upon QP creation the code uses htonl for setting qp->doorbell_qpn (see
>> mlx4_create_qp_ex), later on this value is used without any change upon
>> writing a door bell. (see mmio_writel). No PCI swap is expected.
>
> Thanks, so the mlx drivers are doing what the kernel calls writel_be()?

It's logically the same. You can look also in mlx4 driver code in the 
kernel [1], writel() is used on qp->doorbell_qpn which was previously 
set to be BE as the hardware expects.

[1] 
http://lxr.free-electrons.com/source/drivers/infiniband/hw/mlx4/qp.c#L3223


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

* Re: [PATCH 8/9] Remove most checks of __BYTE_ORDER
From: Christoph Hellwig @ 2016-10-06 11:40 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Christoph Hellwig, Steve Wise, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	'Tatyana Nikolova', 'Hal Rosenstock',
	'Doug Ledford', 'Yishai Hadas',
	'Devesh Sharma', 'Sean Hefty',
	'Bart Van Assche'
In-Reply-To: <20161005173359.GF18636-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

On Wed, Oct 05, 2016 at 11:33:59AM -0600, Jason Gunthorpe wrote:
> Right, stated differently, the kernel requires that writel()/etc
> always produce the same PCI-E packet on the wire. (eg writel(1)
> produces a TLP with bit 0 of the data payload set)

Exactly.

> Not going through the kernel's writel is the whole problem. The writel
> helper generates the arch-specific instruction sequence required to
> issue generate the required PCI-E packet.

All the architectures (including your quoted above ARM example) seem
to do the byte swap in software.  That seems to be important for
accesses like memcpy_{to,from}_io, which would be painful to handle.

But yes, in theory an architecture could do it any way it wants.

> Today (at best, some drivers do not even do this) our userspace
> assumes all archs implement writel as:
> 
>    *(u32 *)reg = cpu_to_le32(val);
> 
> Which is a good start, but not portable to every arch the kernel
> supports.

It should do the right thing for every architecture that matters.
Thay beeing said having an iomem abstraction certainly makes sense
for various reasons, and handling any oddball architecture (or rather
PCI hostbridge implemtation, I would not expect something this
broken to be universal) would come as a bonus.

> Yeah, it would be nice to get that working too. I guess we need to
> standardize on the cpu_to_xx macro style as a first step?

Any style will work as long as it separate the swap directions,
but for a low-level Linux projects using the kernel style certainly
makes sene.

> The __iomem annotation would be nice as well.

I can look into that as well.  Usuaully the first step before adding
sparse annotations is fixing all the misc sparse warnings, as typical
userspace projects have a not too stellar code quality.
--
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

* [PATCH rdma-core V1] Remove unused HIDDEN gcc attribute
From: Leon Romanovsky @ 2016-10-06  9:33 UTC (permalink / raw)
  To: dledford-H+wXaHxf7aLQT0dZR+AlfA; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

The attribute HIDDEN was declared in almost every provider, but it wasn't
used, except in libibverbs. This attribute is configured by linker map
now and can be safely removed.

Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Reviewed-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
Changelog v0->v1:
 * Add Jason's RB tag
 * Remove hidden parameter from libibverbs too
 * Improve commit message
---
 libhfi1verbs/src/hfiverbs.h    | 2 --
 libi40iw/src/i40iw_umain.h     | 2 --
 libibverbs/src/ibverbs.h       | 6 ++----
 libibverbs/src/init.c          | 4 ++--
 libipathverbs/src/ipathverbs.h | 2 --
 libmlx4/src/mlx4.h             | 2 --
 libmlx5/src/mlx5.h             | 2 --
 libmthca/src/mthca.h           | 2 --
 libnes/src/nes_umain.h         | 2 --
 9 files changed, 4 insertions(+), 20 deletions(-)

diff --git a/libhfi1verbs/src/hfiverbs.h b/libhfi1verbs/src/hfiverbs.h
index deebc65..e82ba2b 100644
--- a/libhfi1verbs/src/hfiverbs.h
+++ b/libhfi1verbs/src/hfiverbs.h
@@ -67,8 +67,6 @@
 #include <infiniband/arch.h>
 #include <infiniband/verbs.h>

-#define HIDDEN		__attribute__((visibility ("hidden")))
-
 #define PFX		"hfi1: "

 struct hfi1_device {
diff --git a/libi40iw/src/i40iw_umain.h b/libi40iw/src/i40iw_umain.h
index 13d3da8..719aefc 100644
--- a/libi40iw/src/i40iw_umain.h
+++ b/libi40iw/src/i40iw_umain.h
@@ -54,8 +54,6 @@
 #define unlikely(x) __builtin_expect((x), 0)
 #endif

-#define HIDDEN __attribute__((visibility("hidden")))
-
 #define PFX "libi40iw-"

 #define  I40IW_BASE_PUSH_PAGE	1
diff --git a/libibverbs/src/ibverbs.h b/libibverbs/src/ibverbs.h
index a01dff2..dd2dacf 100644
--- a/libibverbs/src/ibverbs.h
+++ b/libibverbs/src/ibverbs.h
@@ -40,8 +40,6 @@

 #include <valgrind/memcheck.h>

-#define HIDDEN		__attribute__((visibility ("hidden")))
-
 #define INIT		__attribute__((constructor))
 #define FINI		__attribute__((destructor))

@@ -58,9 +56,9 @@ struct ibv_abi_compat_v2 {
 	pthread_mutex_t		in_use;
 };

-extern HIDDEN int abi_ver;
+extern int abi_ver;

-HIDDEN int ibverbs_init(struct ibv_device ***list);
+int ibverbs_init(struct ibv_device ***list);

 struct verbs_ex_private {
 	struct ibv_cq_ex *(*create_cq_ex)(struct ibv_context *context,
diff --git a/libibverbs/src/init.c b/libibverbs/src/init.c
index 7797569..03f3c42 100644
--- a/libibverbs/src/init.c
+++ b/libibverbs/src/init.c
@@ -48,7 +48,7 @@

 #include "ibverbs.h"

-HIDDEN int abi_ver;
+int abi_ver;

 struct ibv_sysfs_dev {
 	char		        sysfs_name[IBV_SYSFS_NAME_MAX];
@@ -498,7 +498,7 @@ static void add_device(struct ibv_device *dev,
 	(*dev_list)[(*num_devices)++] = dev;
 }

-HIDDEN int ibverbs_init(struct ibv_device ***list)
+int ibverbs_init(struct ibv_device ***list)
 {
 	const char *sysfs_path;
 	struct ibv_sysfs_dev *sysfs_dev, *next_dev;
diff --git a/libipathverbs/src/ipathverbs.h b/libipathverbs/src/ipathverbs.h
index 05e1c71..ff25854 100644
--- a/libipathverbs/src/ipathverbs.h
+++ b/libipathverbs/src/ipathverbs.h
@@ -47,8 +47,6 @@
 #include <infiniband/arch.h>
 #include <infiniband/verbs.h>

-#define HIDDEN		__attribute__((visibility ("hidden")))
-
 #define PFX		"ipath: "

 struct ipath_device {
diff --git a/libmlx4/src/mlx4.h b/libmlx4/src/mlx4.h
index 1855cfb..95a6521 100644
--- a/libmlx4/src/mlx4.h
+++ b/libmlx4/src/mlx4.h
@@ -49,8 +49,6 @@

 #include <valgrind/memcheck.h>

-#define HIDDEN		__attribute__((visibility ("hidden")))
-
 #define PFX		"mlx4: "

 enum {
diff --git a/libmlx5/src/mlx5.h b/libmlx5/src/mlx5.h
index c209a79..f8674c7 100644
--- a/libmlx5/src/mlx5.h
+++ b/libmlx5/src/mlx5.h
@@ -54,8 +54,6 @@

 #include <valgrind/memcheck.h>

-#define HIDDEN		__attribute__((visibility("hidden")))
-
 #ifdef HAVE_FUNC_ATTRIBUTE_ALWAYS_INLINE
 #define ALWAYS_INLINE __attribute__((always_inline))
 #else
diff --git a/libmthca/src/mthca.h b/libmthca/src/mthca.h
index da53010..b4b0b6a 100644
--- a/libmthca/src/mthca.h
+++ b/libmthca/src/mthca.h
@@ -41,8 +41,6 @@

 #include <valgrind/memcheck.h>

-#define HIDDEN		__attribute__((visibility ("hidden")))
-
 #define PFX		"mthca: "

 enum mthca_hca_type {
diff --git a/libnes/src/nes_umain.h b/libnes/src/nes_umain.h
index 9129982..f3cc348 100644
--- a/libnes/src/nes_umain.h
+++ b/libnes/src/nes_umain.h
@@ -48,8 +48,6 @@
 #define unlikely(x) __builtin_expect((x),0)
 #endif

-#define HIDDEN __attribute__((visibility ("hidden")))
-
 #define PFX	"libnes: "

 #define  NES_QP_MMAP		1
--
2.7.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

* Introduction of libqedr to the Consolidated Userspace RDMA Library Repo
From: Amrani, Ram @ 2016-10-06  8:08 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Elior, Ariel,
	Kalderon, Michal, Borundia, Rajesh,
	dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

Hi Jason,
Yesterday the qedr (Qlogic Everest Driver for RDMA) patch was sent.
Even before it being accepted I would like to start working on preparing and adding our user space library for qedr (libqedr) to the consolidated userspace RDMA library repo.
I see that a lot of work is being done by you and so I want to understand how to prepare the code, what is the process and when is a good time to join.

Thanks,
Ram
--
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

* Re: [PATCH] MAINTAINERS: Update Intel RDMA RNIC driver maintainers
From: Leon Romanovsky @ 2016-10-06  7:10 UTC (permalink / raw)
  To: Faisal Latif
  Cc: Henry Orosco, dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
In-Reply-To: <20161005153530.GA11148@flatif-MOBL1>

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

On Wed, Oct 05, 2016 at 10:35:30AM -0500, Faisal Latif wrote:
> On Wed, Oct 05, 2016 at 07:30:54AM -0700, Henry Orosco wrote:
> Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > ---
> >  MAINTAINERS | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index cc77b9a..32f535e 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -6188,6 +6188,7 @@ R:     Chien Tin Tung <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >  R:     Mustafa Ismail <mustafa.ismail-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >  R:     Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >  R:     Tatyana Nikolova <tatyana.e.nikolova-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > +R:     Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >  L:     linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >  S:     Supported
> >  F:     drivers/infiniband/hw/i40iw/
> > --
> > 2.8.3
>
> Acked-by: Faisal Latif <faisal.latif-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Faisal,

Do you really expect to see 9 people in TO and CC field for every patch in i40iw area?
3 comes from drivers/infiniband/
6 comes from drivers/infiniband/hw/i40iw/

IMHO, it is worth to shrink the list and limit it to the active developers.

Thanks

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH rdma-core] Add a .travis.yml file
From: Leon Romanovsky @ 2016-10-06  4:58 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Jason Gunthorpe, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford
In-Reply-To: <0de7bc15-5e2e-8240-ba84-a5a95dcb2834-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>

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

On Wed, Oct 05, 2016 at 09:39:42AM -0700, Bart Van Assche wrote:
> On 10/05/2016 04:51 AM, Leon Romanovsky wrote:
> >On Wed, Sep 28, 2016 at 05:28:57PM -0600, Jason Gunthorpe wrote:
> >>Necessary to use the Travis CI service.
> >>
> >>Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> >>---
> >> .travis.yml | 28 ++++++++++++++++++++++++++++
> >> 1 file changed, 28 insertions(+)
> >> create mode 100644 .travis.yml
> >>
> >>Now that travis is enabled on repo, we need a yml file to make it run.
> >>
> >>Here is a basic one.
> >>
> >>I don't test with distros as old as precise and I don't expect that glibc to
> >>be new enough to build this (eg missing timerfd, O_CLOEXEC, etc), and the
> >>cmake certainly isn't. So we use the trusty 'beta' environment and install
> >>gcc 6.2.
> >
> >I'm using 16.04 and it has gcc 5.4.0 as a default. Don't you think that
> >6.2 is too "new" for us and other distributions?
>
> Hi Leon,
>
> The rolling release distro that is running on my laptop includes gcc 6.2.1.

I'm not sure if there are many users of this bundle who are running
rolling distros.

Anyway, let's start from the simple (one, latest gcc) and increase
complexity later on.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH rdma-core] Add a .travis.yml file
From: Leon Romanovsky @ 2016-10-06  4:53 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford
In-Reply-To: <20161005163250.GB18636-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

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

On Wed, Oct 05, 2016 at 10:32:50AM -0600, Jason Gunthorpe wrote:
> On Wed, Oct 05, 2016 at 02:51:32PM +0300, Leon Romanovsky wrote:
>
> > I'm using 16.04 and it has gcc 5.4.0 as a default. Don't you think that
> > 6.2 is too "new" for us and other distributions?
>
> I don't understand how a compiler can be too new? The next round of
> rolling release distros are likely to use this compiler, and those are
> the distros that will first pick to up rdma-core.

I have an issue in front of my eyes where kernel fails to build with old
gcc version, while new gcc version works.

I don't want the same situation here.

>
> > gcc (Ubuntu 5.4.0-6ubuntu1~16.04.2) 5.4.0 20160609
>
> This is exactly why the automatic builds should run the latest
> compiler, because people are not going to install it on their desktop.
>
> People will test with older compilers on their own. If we want to run
> automatic tests from travis with old compilers we can, but my results
> basically show that older compilers are just buggy and produce
> incorrect warnings.

My experience shows the same, however I want to minimize the test burden
on the users.

>
> Jason

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH v2] IB/{core,hw}: Add constant for node_desc
From: Yuval Shaia @ 2016-10-06  4:46 UTC (permalink / raw)
  To: Doug Ledford
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
	hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <c39be0ef-d170-d40d-aac4-0c157fc843b7-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Fri, Aug 26, 2016 at 09:57:06AM -0400, Doug Ledford wrote:
> On 8/25/2016 1:57 PM, Yuval Shaia wrote:
> > Signed-off-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > ---
> > v0 -> v1:
> > 	* Exclude some files from commit based on Jason's comment
> > v1 -> v2:
> > 	* Re-include all the excluded files with BUILD_BUG_ON
> 
> v2 applied, thanks.

I have a bad feeling that this one slipped out.

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



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

* Re: [PATCH rdma-core] Improve global COPYING files
From: Leon Romanovsky @ 2016-10-06  4:30 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford
In-Reply-To: <20161005124939.GF9282-2ukJVAZIZ/Y@public.gmane.org>

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

On Wed, Oct 05, 2016 at 03:49:39PM +0300, Leon Romanovsky wrote:
> On Fri, Sep 23, 2016 at 11:34:21AM -0600, Jason Gunthorpe wrote:
> > On Fri, Sep 23, 2016 at 08:10:03PM +0300, Leon Romanovsky wrote:
> >
> > > I sure that this question will sound dumb for you, but can we unify all
> > > code under OFA's licenses (dual-license)? I don't feel comfortable with
> > > this multi-license situation.
> >
> > It is an excellent question.
> >
> > To do this we would need the OFA to talk to each of the member
> > companies and get them to sign some kind of legal change of copyright
> > document. Assuming all of the member companies agree, and all the
> > member companies are the exhaustive copyright owners then the code can
> > be placed under a single uniform license.
> >
> > As I understand it all OFA members were required to agree to use a
> > specific licensing scheme, including specific license text when they
> > signed the OFA membership agreement. What we are seeing here is that
> > the corporate legal side agreed to something but the developers made
> > small errors along the way, and those errors were later copied by
> > other developers and spread widely. So the OFA has a basis for
> > requests of this nature.
> >
> > For instance, a legal statement from Mellanox that all code they
> > contributed is available under the GPLv2 or *either* MIT or FreeBSD
> > license varient would allow immediately placing all Mellanox
> > copyrighted code under the single Default License. I expect this is
> > what Mellanox intended to do anyhow, the fact that ibverbs and all
> > their providers had an error in the COPYING file is simply
> > an unfortunate mistake.
> >
> > These sorts of license issues are typical in historical code bases. I
> > would say we are in pretty good shape, from what I can tell absolutely
> > everything is unambiguously licensed under at least the GPLv2, or a
> > compatible license.
> >
> > Almost everything is alternatively licensed under some kind of BSD
> > license. The notable exception is ipathverbs and rxe. It is also
> > unfortunate we have so many BSD variants.
> >
> > Further, almost all C code is licensed under the dual GPLv2/OpenIB.org
> > (MIT) license. The kernel is similar, almost all C code is using the
> > MIT variant. Hence my desire to make that the license for new code in
> > the tree.
> >
> > I would say this is a pretty good result.
> >
> > My ultimate suggestion is that we push the non-default copyright into
> > the impacted files, eg add short licenses headers to the man pages,
> > etc, and then delete the extraneous COPYING files once every single
> > file has a correct license statement. From that point we can look at
> > switching individual files based on the above Legal process to the
> > Default License, or just leave them as is - a historical quirk.
>
> It looks like a lot of work to do and it can be handled by anyone.
> Can OPA handle this?

Sorry,
OFA and not OPA.

>
> >
> > Jason



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH v2 4/7] blk-mq: Introduce blk_quiesce_queue() and blk_resume_queue()
From: Bart Van Assche @ 2016-10-05 23:00 UTC (permalink / raw)
  To: Ming Lei
  Cc: Sagi Grimberg, Jens Axboe, Christoph Hellwig, James Bottomley,
	Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
	linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org
In-Reply-To: <CACVXFVMwQnOuUukEMO8CyG3sPOg6DxuKAZx+5FT+LS+hGAcWVQ@mail.gmail.com>

On 10/05/2016 03:49 PM, Ming Lei wrote:
> We can use srcu read lock for BLOCKING and rcu read lock for non-BLOCKING,
> by putting *_read_lock() and *_read_unlock() into two wrappers, which
> should minimize the cost of srcu read lock & unlock and the code is still easy
> to read & verify.

Hello Ming,

The lock checking algorithms in the sparse and smatch static checkers 
are unable to deal with code of the type "if (condition) (un)lock()". So 
unless someone has a better proposal my preference is to use the 
approach from the patch at the start of this e-mail thread.

Thanks,

Bart.


^ permalink raw reply

* Re: [PATCH v2 4/7] blk-mq: Introduce blk_quiesce_queue() and blk_resume_queue()
From: Ming Lei @ 2016-10-05 22:49 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Sagi Grimberg, Jens Axboe, Christoph Hellwig, James Bottomley,
	Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
	linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <07628c2f-3a4e-b12e-663a-b0711ce20cc9-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>

On Thu, Oct 6, 2016 at 5:08 AM, Bart Van Assche
<bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:
> On 10/05/2016 12:11 PM, Sagi Grimberg wrote:
>>
>> I was referring to weather we can take srcu in the submission path
>> conditional of the hctx being STOPPED?
>
>
> Hello Sagi,
>
> Regarding run-time overhead:
> * rcu_read_lock() is a no-op on CONFIG_PREEMPT_NONE kernels and is
>   translated into preempt_disable() with preemption enabled. The latter
>   function modifies a per-cpu variable.
> * Checking BLK_MQ_S_STOPPED before taking an rcu or srcu lock is only
>   safe if the BLK_MQ_S_STOPPED flag is tested in such a way that the
>   compiler is told to reread the hctx flags (READ_ONCE()) and if the
>   compiler and CPU are told not to reorder test_bit() with the
>   memory accesses in (s)rcu_read_lock(). To avoid races
>   BLK_MQ_S_STOPPED will have to be tested a second time after the lock
>   has been obtained, similar to the double-checked-locking pattern.
> * srcu_read_lock() reads a word from the srcu structure, disables
>   preemption, calls __srcu_read_lock() and re-enables preemption. The
>   latter function increments two CPU-local variables and triggers a
>   memory barrier (smp_mp()).

We can use srcu read lock for BLOCKING and rcu read lock for non-BLOCKING,
by putting *_read_lock() and *_read_unlock() into two wrappers, which
should minimize the cost of srcu read lock & unlock and the code is still easy
to read & verify.

>
> Swapping srcu_read_lock() and the BLK_MQ_S_STOPPED flag test will make the
> code more complicated. Going back to the implementation that calls
> rcu_read_lock() if .queue_rq() won't sleep will result in an implementation
> that is easier to read and to verify.

Yeah, I agree.

Thanks,
Ming Lei
--
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

* Re: [PATCH v2 6/7] SRP transport: Port srp_wait_for_queuecommand() to scsi-mq
From: Bart Van Assche @ 2016-10-05 21:51 UTC (permalink / raw)
  To: Sagi Grimberg, Jens Axboe
  Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
	Mike Snitzer, Doug Ledford, Keith Busch,
	linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org
In-Reply-To: <8ce2910d-fde0-724f-a61f-8fbd7fbf499d@grimberg.me>

On 10/05/2016 10:38 AM, Sagi Grimberg wrote:
>> +static void srp_mq_wait_for_queuecommand(struct Scsi_Host *shost)
>> +{
>> +    struct scsi_device *sdev;
>> +    struct request_queue *q;
>> +
>> +    shost_for_each_device(sdev, shost) {
>> +        q = sdev->request_queue;
>> +
>> +        blk_mq_quiesce_queue(q);
>> +        blk_mq_resume_queue(q);
>> +    }
>> +}
>> +
>
> This *should* live in scsi_lib.c. I suspect that
> various drivers would really want this functionality.

Hello Sagi,

There are multiple direct blk_*() calls in other SCSI transport drivers. 
So my proposal is to wait with moving this code into scsi_lib.c until 
there is a second user of this code.

Bart.

^ permalink raw reply

* Re: [PATCH v2 4/7] blk-mq: Introduce blk_quiesce_queue() and blk_resume_queue()
From: Bart Van Assche @ 2016-10-05 21:08 UTC (permalink / raw)
  To: Sagi Grimberg, Ming Lei
  Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
	Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
	linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <732bf5d6-d7a8-6ff0-6501-8a3d46f12fab-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>

On 10/05/2016 12:11 PM, Sagi Grimberg wrote:
> I was referring to weather we can take srcu in the submission path
> conditional of the hctx being STOPPED?

Hello Sagi,

Regarding run-time overhead:
* rcu_read_lock() is a no-op on CONFIG_PREEMPT_NONE kernels and is
   translated into preempt_disable() with preemption enabled. The latter
   function modifies a per-cpu variable.
* Checking BLK_MQ_S_STOPPED before taking an rcu or srcu lock is only
   safe if the BLK_MQ_S_STOPPED flag is tested in such a way that the
   compiler is told to reread the hctx flags (READ_ONCE()) and if the
   compiler and CPU are told not to reorder test_bit() with the
   memory accesses in (s)rcu_read_lock(). To avoid races
   BLK_MQ_S_STOPPED will have to be tested a second time after the lock
   has been obtained, similar to the double-checked-locking pattern.
* srcu_read_lock() reads a word from the srcu structure, disables
   preemption, calls __srcu_read_lock() and re-enables preemption. The
   latter function increments two CPU-local variables and triggers a
   memory barrier (smp_mp()).

Swapping srcu_read_lock() and the BLK_MQ_S_STOPPED flag test will make 
the code more complicated. Going back to the implementation that calls 
rcu_read_lock() if .queue_rq() won't sleep will result in an 
implementation that is easier to read and to verify. If I overlooked 
something, please let me know.

Bart.


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

* Re: [PATCH rdma-core] Remove unused HIDDEN gcc attribute
From: Jason Gunthorpe @ 2016-10-05 19:23 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1475665714-5885-1-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On Wed, Oct 05, 2016 at 02:08:34PM +0300, Leon Romanovsky wrote:
> The attribute HIDDEN was declared is almost every provider, but it wasn't
> used, except in libibverbs. Let's remove it.

There is no reason for libibverbs to use this either, please remove
it completely.

All libraries and providers use a linker map file which superceeds
the hidden attribute.

Reviewed-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

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

^ permalink raw reply

* Re: [PATCH] MAINTAINERS: Update Intel RDMA RNIC driver maintainers
From: Jason Gunthorpe @ 2016-10-05 19:12 UTC (permalink / raw)
  To: Faisal Latif
  Cc: Henry Orosco, dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
In-Reply-To: <20161005153530.GA11148@flatif-MOBL1>

On Wed, Oct 05, 2016 at 10:35:30AM -0500, Faisal Latif wrote:
> On Wed, Oct 05, 2016 at 07:30:54AM -0700, Henry Orosco wrote:
> Signed-off-by: Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >  MAINTAINERS | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index cc77b9a..32f535e 100644
> > +++ b/MAINTAINERS
> > @@ -6188,6 +6188,7 @@ R:     Chien Tin Tung <chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >  R:     Mustafa Ismail <mustafa.ismail-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >  R:     Shiraz Saleem <shiraz.saleem-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >  R:     Tatyana Nikolova <tatyana.e.nikolova-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > +R:     Henry Orosco <henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >  L:     linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >  S:     Supported
> >  F:     drivers/infiniband/hw/i40iw/
> 
> Acked-by: Faisal Latif <faisal.latif-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Are any of you going to continue supporting the user space component
that now lives in rdma-core?

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

^ permalink raw reply

* Re: [PATCH v2 4/7] blk-mq: Introduce blk_quiesce_queue() and blk_resume_queue()
From: Sagi Grimberg @ 2016-10-05 19:10 UTC (permalink / raw)
  To: Bart Van Assche, Ming Lei
  Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
	Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
	linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org
In-Reply-To: <6d612be9-7ba7-888e-f9c6-f8aa4ef1606a@sandisk.com>


>>> Hello Ming,
>>>
>>> Can you have a look at the attached patch? That patch uses an srcu read
>>> lock for all queue types, whether or not the BLK_MQ_F_BLOCKING flag has
>>> been set. Additionally, I have dropped the QUEUE_FLAG_QUIESCING flag.
>>> Just like previous versions, this patch has been tested.
>>
>> Hey Bart,
>>
>> Do we care about the synchronization of queue_rq and/or
>> blk_mq_run_hw_queue of the hctx is not stopped?
>>
>> I'm wandering if we can avoid introducing new barriers in the
>> submission path of its not absolutely needed.
>
> Hello Sagi,

Hey Bart,

>
> I'm not sure whether the new blk_quiesce_queue() function is useful
> without stopping all hardware contexts first. In other words, in my view
> setting BLK_MQ_F_BLOCKING flag before calling blk_quiesce_queue() is
> sufficient and I don't think that a new QUEUE_FLAG_QUIESCING flag is
> necessary.

I was referring to weather we can take srcu in the submission path
conditional of the hctx being STOPPED?

^ permalink raw reply

* Re: [PATCH v2 4/7] blk-mq: Introduce blk_quiesce_queue() and blk_resume_queue()
From: Bart Van Assche @ 2016-10-05 19:05 UTC (permalink / raw)
  To: Sagi Grimberg, Ming Lei
  Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
	Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
	linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org
In-Reply-To: <09238e87-7d60-89c9-98b0-e51caa0c0839@grimberg.me>

On 10/05/2016 11:14 AM, Sagi Grimberg wrote:
>> Hello Ming,
>>
>> Can you have a look at the attached patch? That patch uses an srcu read
>> lock for all queue types, whether or not the BLK_MQ_F_BLOCKING flag has
>> been set. Additionally, I have dropped the QUEUE_FLAG_QUIESCING flag.
>> Just like previous versions, this patch has been tested.
>
> Hey Bart,
>
> Do we care about the synchronization of queue_rq and/or
> blk_mq_run_hw_queue of the hctx is not stopped?
>
> I'm wandering if we can avoid introducing new barriers in the
> submission path of its not absolutely needed.

Hello Sagi,

I'm not sure whether the new blk_quiesce_queue() function is useful 
without stopping all hardware contexts first. In other words, in my view 
setting BLK_MQ_F_BLOCKING flag before calling blk_quiesce_queue() is 
sufficient and I don't think that a new QUEUE_FLAG_QUIESCING flag is 
necessary.

Bart.

^ permalink raw reply

* Re: RDMA developer gatherings around Kernel Summit and Linux Plumbers in Santa Fe
From: Jason Gunthorpe @ 2016-10-05 18:31 UTC (permalink / raw)
  To: Atchley, Scott
  Cc: Christoph Hellwig, Christoph Lameter, Linux RDMA Mailing List,
	ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	john.fleck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Doug Ledford,
	Coulter, Susan K, shewa-YOWKrPYUwWM@public.gmane.org
In-Reply-To: <96C6AF13-B968-41C2-9AB3-CAB5F91D72A1-1Heg1YXhbW8@public.gmane.org>

On Wed, Oct 05, 2016 at 01:49:56PM +0000, Atchley, Scott wrote:

> I am trying to understand the issue from both points of view. Again,
> I ask you to help me understand how requesting dual-licensing hurts
> the adoption of RDMA technology. I can understand the vendor???s
> concerns about having to write software twice for GPL and non-GPL
> environments.

The opposing point of view is that every time an opensource project is
forked or taken propriety by a company the opensource project itself
suffers. (and this is by no means a universal view, but let us
explore it)

A good example of this harm in our community would be opensm. Many
sites are now running a build of 'opensm' that does not come with
source.

>From the OFA perspective, many of the companies using the code outside
Linux are not OFA members, do not contribute financially, and do not
contribute their bugfixes or other code. Yet, they get a 'free ride'
when the OFA encourages Linux-focused member companies to use the dual
license.

I can understand why the OFA adopted this position at the start, but
IMHO, it is time to stop. The OFA should not be spending its limited
resources to benefit the non-Linux community.

Further, it is hard to see why the OFA board should burn what little
good will it has in the Linux community by acting to the benifit of a
nebulous sub-group of non-Linux members (that may not even exist today).

In the end I hope the bylaws will be revised to allow for
GPLv2/LGPLv2-only as a legtimiate license choice for OFA projects.

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

^ permalink raw reply

* Re: iscsi_trx going into D state
From: Robert LeBlanc @ 2016-10-05 18:19 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Hannes Reinecke, Johannes Thumshirn, linux-rdma, linux-scsi
In-Reply-To: <20161005180305.GA2900@infradead.org>

Thanks, we will apply that too. We'd like to get this stable. We'll
report back on what we find with these patches.
----------------
Robert LeBlanc
PGP Fingerprint 79A2 9CA4 6CC4 45DD A904  C70E E654 3BB2 FA62 B9F1


On Wed, Oct 5, 2016 at 12:03 PM, Christoph Hellwig <hch@infradead.org> wrote:
> Hi Robert,
>
> I actually got the name wrong, the patch wasn't from Lee, but from Zhu,
> another SuSE engineer.  This is the one:
>
> http://www.spinics.net/lists/target-devel/msg13463.html

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox