Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* [PATCH 11/11] qedr: Add events support and register IB device
From: Ram Amrani @ 2016-09-26 14:16 UTC (permalink / raw)
  To: davem, dledford
  Cc: Ariel.Elior, Michal.Kalderon, Yuval.Mintz, rajesh.borundia,
	linux-rdma, netdev, Ram Amrani
In-Reply-To: <1474899407-12635-1-git-send-email-Ram.Amrani@cavium.com>

Add error handling support.
Register ib device with ib stack.

Signed-off-by: Rajesh Borundia <rajesh.borundia@cavium.com>
Signed-off-by: Ram Amrani <Ram.Amrani@cavium.com>
---
 drivers/infiniband/hw/qedr/main.c  | 114 ++++++++++++++++++++++++++++++++++++-
 drivers/infiniband/hw/qedr/verbs.c |  37 ++++++++++++
 drivers/infiniband/hw/qedr/verbs.h |   9 +++
 include/linux/qed/common_hsi.h     |   1 +
 4 files changed, 159 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/qedr/main.c b/drivers/infiniband/hw/qedr/main.c
index 735bc70..fc2afd0 100644
--- a/drivers/infiniband/hw/qedr/main.c
+++ b/drivers/infiniband/hw/qedr/main.c
@@ -79,10 +79,25 @@ static void qedr_get_dev_fw_str(struct ib_device *ibdev, char *str,
 		 (fw_ver >> 8) & 0xFF, fw_ver & 0xFF);
 }
 
+static struct net_device *qedr_get_netdev(struct ib_device *dev, u8 port_num)
+{
+	struct qedr_dev *qdev;
+
+	qdev = get_qedr_dev(dev);
+	dev_hold(qdev->ndev);
+
+	/* The HW vendor's device driver must guarantee
+	 * that this function returns NULL before the net device reaches
+	 * NETDEV_UNREGISTER_FINAL state.
+	 */
+	return qdev->ndev;
+}
+
 static int qedr_register_device(struct qedr_dev *dev)
 {
 	strlcpy(dev->ibdev.name, "qedr%d", IB_DEVICE_NAME_MAX);
 
+	dev->ibdev.node_guid = dev->attr.node_guid;
 	memcpy(dev->ibdev.node_desc, QEDR_NODE_DESC, sizeof(QEDR_NODE_DESC));
 	dev->ibdev.owner = THIS_MODULE;
 	dev->ibdev.uverbs_abi_ver = QEDR_ABI_VERSION;
@@ -151,11 +166,15 @@ static int qedr_register_device(struct qedr_dev *dev)
 	dev->ibdev.post_send = qedr_post_send;
 	dev->ibdev.post_recv = qedr_post_recv;
 
+	dev->ibdev.process_mad = qedr_process_mad;
+	dev->ibdev.get_port_immutable = qedr_port_immutable;
+	dev->ibdev.get_netdev = qedr_get_netdev;
+
 	dev->ibdev.dma_device = &dev->pdev->dev;
 	dev->ibdev.get_link_layer = qedr_link_layer;
 	dev->ibdev.get_dev_fw_str = qedr_get_dev_fw_str;
 
-	return 0;
+	return ib_register_device(&dev->ibdev, NULL);
 }
 
 /* This function allocates fast-path status block memory */
@@ -530,6 +549,92 @@ static int qedr_set_device_attr(struct qedr_dev *dev)
 
 	return 0;
 }
+void qedr_unaffiliated_event(void *context,
+			     u8 event_code)
+{
+	pr_err("unaffiliated event not implemented yet\n");
+}
+
+
+void qedr_affiliated_event(void *context, u8 e_code, void *fw_handle)
+{
+#define EVENT_TYPE_NOT_DEFINED	0
+#define EVENT_TYPE_CQ		1
+#define EVENT_TYPE_QP		2
+	struct qedr_dev *dev = (struct qedr_dev *)context;
+	union event_ring_data *data = fw_handle;
+	u64 roce_handle64 = ((u64)data->roce_handle.hi << 32) +
+			    data->roce_handle.lo;
+	u8 event_type = EVENT_TYPE_NOT_DEFINED;
+	struct ib_event event;
+	struct ib_cq *ibcq;
+	struct ib_qp *ibqp;
+	struct qedr_cq *cq;
+	struct qedr_qp *qp;
+
+	switch (e_code) {
+	case ROCE_ASYNC_EVENT_CQ_OVERFLOW_ERR:
+		event.event = IB_EVENT_CQ_ERR;
+		event_type = EVENT_TYPE_CQ;
+		break;
+	case ROCE_ASYNC_EVENT_SQ_DRAINED:
+		event.event = IB_EVENT_SQ_DRAINED;
+		event_type = EVENT_TYPE_QP;
+		break;
+	case ROCE_ASYNC_EVENT_QP_CATASTROPHIC_ERR:
+		event.event = IB_EVENT_QP_FATAL;
+		event_type = EVENT_TYPE_QP;
+		break;
+	case ROCE_ASYNC_EVENT_LOCAL_INVALID_REQUEST_ERR:
+		event.event = IB_EVENT_QP_REQ_ERR;
+		event_type = EVENT_TYPE_QP;
+		break;
+	case ROCE_ASYNC_EVENT_LOCAL_ACCESS_ERR:
+		event.event = IB_EVENT_QP_ACCESS_ERR;
+		event_type = EVENT_TYPE_QP;
+		break;
+	default:
+		DP_ERR(dev, "unsupported event %d on handle=%llx\n", e_code,
+		       roce_handle64);
+	}
+
+	switch (event_type) {
+	case EVENT_TYPE_CQ:
+		cq = (struct qedr_cq *)(uintptr_t)roce_handle64;
+		if (cq) {
+			ibcq = &cq->ibcq;
+			if (ibcq->event_handler) {
+				event.device = ibcq->device;
+				event.element.cq = ibcq;
+				ibcq->event_handler(&event, ibcq->cq_context);
+			}
+		} else {
+			WARN(1,
+			     "Error: CQ event with NULL pointer ibcq. Handle=%llx\n",
+			     roce_handle64);
+		}
+		DP_ERR(dev, "CQ event %d on hanlde %p\n", e_code, cq);
+		break;
+	case EVENT_TYPE_QP:
+		qp = (struct qedr_qp *)(uintptr_t)roce_handle64;
+		if (qp) {
+			ibqp = &qp->ibqp;
+			if (ibqp->event_handler) {
+				event.device = ibqp->device;
+				event.element.qp = ibqp;
+				ibqp->event_handler(&event, ibqp->qp_context);
+			}
+		} else {
+			WARN(1,
+			     "Error: QP event with NULL pointer ibqp. Handle=%llx\n",
+			     roce_handle64);
+		}
+		DP_ERR(dev, "QP event %d on hanlde %p\n", e_code, qp);
+		break;
+	default:
+		break;
+	}
+}
 
 static int qedr_init_hw(struct qedr_dev *dev)
 {
@@ -559,6 +664,8 @@ static int qedr_init_hw(struct qedr_dev *dev)
 		cur_pbl->pbl_ptr = (u64)p_phys_table;
 	}
 
+	events.affiliated_event = qedr_affiliated_event;
+	events.unaffiliated_event = qedr_unaffiliated_event;
 	events.context = dev;
 
 	in_params->events = &events;
@@ -657,11 +764,13 @@ static struct qedr_dev *qedr_add(struct qed_dev *cdev, struct pci_dev *pdev,
 
 	for (i = 0; i < ARRAY_SIZE(qedr_attributes); i++)
 		if (device_create_file(&dev->ibdev.dev, qedr_attributes[i]))
-			goto reg_err;
+			goto sysfs_err;
 
 	DP_DEBUG(dev, QEDR_MSG_INIT, "qedr driver loaded successfully\n");
 	return dev;
 
+sysfs_err:
+	ib_unregister_device(&dev->ibdev);
 reg_err:
 	qedr_sync_free_irqs(dev);
 irq_err:
@@ -681,6 +790,7 @@ static void qedr_remove(struct qedr_dev *dev)
 	 * of the registered clients.
 	 */
 	qedr_remove_sysfiles(dev);
+	ib_unregister_device(&dev->ibdev);
 
 	qedr_stop_hw(dev);
 	qedr_sync_free_irqs(dev);
diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
index 9f0a72f..36424ec 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -3522,3 +3522,40 @@ int qedr_poll_cq(struct ib_cq *ibcq, int num_entries, struct ib_wc *wc)
 	spin_unlock_irqrestore(&cq->cq_lock, flags);
 	return done;
 }
+
+int qedr_process_mad(struct ib_device *ibdev, int process_mad_flags,
+		     u8 port_num,
+		     const struct ib_wc *in_wc,
+		     const struct ib_grh *in_grh,
+		     const struct ib_mad_hdr *mad_hdr,
+		     size_t in_mad_size, struct ib_mad_hdr *out_mad,
+		     size_t *out_mad_size, u16 *out_mad_pkey_index)
+{
+	struct qedr_dev *dev = get_qedr_dev(ibdev);
+
+	DP_DEBUG(dev, QEDR_MSG_GSI,
+		 "QEDR_PROCESS_MAD in_mad %x %x %x %x %x %x %x %x\n",
+		 mad_hdr->attr_id, mad_hdr->base_version, mad_hdr->attr_mod,
+		 mad_hdr->class_specific, mad_hdr->class_version,
+		 mad_hdr->method, mad_hdr->mgmt_class, mad_hdr->status);
+	return IB_MAD_RESULT_SUCCESS;
+}
+
+int qedr_port_immutable(struct ib_device *ibdev, u8 port_num,
+			struct ib_port_immutable *immutable)
+{
+	struct ib_port_attr attr;
+	int err;
+
+	err = qedr_query_port(ibdev, port_num, &attr);
+	if (err)
+		return err;
+
+	immutable->pkey_tbl_len = attr.pkey_tbl_len;
+	immutable->gid_tbl_len = attr.gid_tbl_len;
+	immutable->core_cap_flags = RDMA_CORE_PORT_IBA_ROCE |
+				    RDMA_CORE_PORT_IBA_ROCE_UDP_ENCAP;
+	immutable->max_mad_size = IB_MGMT_MAD_SIZE;
+
+	return 0;
+}
diff --git a/drivers/infiniband/hw/qedr/verbs.h b/drivers/infiniband/hw/qedr/verbs.h
index 9760fd1..89c21a9 100644
--- a/drivers/infiniband/hw/qedr/verbs.h
+++ b/drivers/infiniband/hw/qedr/verbs.h
@@ -89,4 +89,13 @@ int qedr_post_send(struct ib_qp *, struct ib_send_wr *,
 		   struct ib_send_wr **bad_wr);
 int qedr_post_recv(struct ib_qp *, struct ib_recv_wr *,
 		   struct ib_recv_wr **bad_wr);
+int qedr_process_mad(struct ib_device *ibdev, int process_mad_flags,
+		     u8 port_num, const struct ib_wc *in_wc,
+		     const struct ib_grh *in_grh,
+		     const struct ib_mad_hdr *in_mad,
+		     size_t in_mad_size, struct ib_mad_hdr *out_mad,
+		     size_t *out_mad_size, u16 *out_mad_pkey_index);
+
+int qedr_port_immutable(struct ib_device *ibdev, u8 port_num,
+			struct ib_port_immutable *immutable);
 #endif
diff --git a/include/linux/qed/common_hsi.h b/include/linux/qed/common_hsi.h
index 1902763..734deb0 100644
--- a/include/linux/qed/common_hsi.h
+++ b/include/linux/qed/common_hsi.h
@@ -674,6 +674,7 @@ union event_ring_data {
 	struct iscsi_eqe_data iscsi_info;
 	struct malicious_vf_eqe_data malicious_vf;
 	struct initial_cleanup_eqe_data vf_init_cleanup;
+	struct regpair roce_handle;
 };
 
 /* Event Ring Entry */
-- 
1.8.3.1

^ permalink raw reply related

* OFED-4.8 daily build with RHEL7.2 support
From: Vladimir Sokolovsky @ 2016-09-26 15:04 UTC (permalink / raw)
  To: ewg-ZwoEplunGu1OwGhvXhtEPSCwEArCW2h5@public.gmane.org
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

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

Hi,
The IB core stuff and ConnectX-3 IB and Ethernet drivers were backported 
for RHEL7.2.
Available kernel components on RHEL7.2 are ib_core, mlx4_core, mlx4_ib 
and mlx4_en.

https://www.openfabrics.org/downloads/OFED/ofed-4.8-daily/OFED-4.8-20160926-0542.tgz

To install this OFED version on RHEL7.2 use the attached ofed.conf file:
# cd OFED-4.8-20160926-0542
# ./install.pl -c ofed.conf

Compat-rdma development environment:

1. Clone the relevant git trees
git://git.openfabrics.org/compat-rdma/compat-rdma.git master
git://git.openfabrics.org/compat-rdma/linux-4.8.git master (includes the 
relevant upstream linux sub trees based on 4.8-rc8)
git://git.openfabrics.org/compat-rdma/compat.git ofed

2. Prepare compilation environment
# cd <workdir>/compat-rdma
# GIT_TREE=<workdir>/linux-4.8 GIT_COMPAT_TREE=<workdir>/compat 
./scripts/admin_rdma.sh -n -p

- This is what applies all the patches in the following order:
     1. backports from patches directory
     2. linux-next-cherry-picks
     3. linux-next-pending

Patches from the tech-preview directory will be applied by the configure 
script if the corresponding configuration parameter is set.

3. To add support for the new kernel module need to update configure, 
makefile, Makefile, compat-rdma.spec and ofed_scripts/openibd under 
compat-rdma.

4. Compilation procedure:
Note: Procedure to build/install OFED kernel modules on your local machine.
# ./configure --with-core-mod --with-user_mad-mod --with-user_access-mod 
--with-addr_trans-mod --with-mlx4-mod --with-mlx4_en-mod 
--with-cxgb3-mod --with-cxgb4-mod --with-nes-mod --with-qib-mod 
--with-ipoib-mod --with-srp-mod --with-nfsrdma-mod
# make
# make install

Notes:
Use "compat-rdma/ofed_scripts/ofed_format_patch.sh" to create backport 
patches.
All patches should be applicable using "git am"

OFED installation environment:
git://git.openfabrics.org/~vlad/ofed_scripts.git master

OFED build environment:
git://git.openfabrics.org/~vlad/build.git master

Next steps:
- Add ConnectX-4 backports for RHEL7.2
- Add ULPs backports for RHEL7.2
- Creating backports for SLES12 SP1
- Add support for other versions of RHEL7.x and SLES12
- Replace user space libraries with rdma-core package

Regards,
Vladimir



[-- Attachment #2: ofed.conf --]
[-- Type: text/plain, Size: 1346 bytes --]

compat-rdma=y
compat-rdma-devel=y
libibverbs=y
libibverbs-devel=y
libibverbs-devel-static=y
libibverbs-utils=y
libibverbs-debuginfo=y
libmthca=y
libmthca-devel-static=y
libmthca-debuginfo=y
libmlx4=y
libmlx4-devel=y
libmlx4-debuginfo=y
libmlx5=y
libmlx5-devel=y
libmlx5-debuginfo=y
libcxgb3=y
libcxgb3-devel=y
libcxgb3-debuginfo=y
libcxgb4=y
libcxgb4-devel=y
libcxgb4-debuginfo=y
libnes=y
libnes-devel-static=y
libnes-debuginfo=y
libocrdma=y
libocrdma-devel=y
libocrdma-debuginfo=y
libipathverbs=y
libipathverbs-devel=y
libipathverbs-debuginfo=y
libibcm=y
libibcm-devel=y
libibcm-debuginfo=y
libibumad=y
libibumad-devel=y
libibumad-static=y
libibumad-debuginfo=y
libibmad=y
libibmad-devel=y
libibmad-static=y
libibmad-debuginfo=y
ibsim=y
ibsim-debuginfo=y
ibacm=y
librdmacm=y
librdmacm-utils=y
librdmacm-devel=y
librdmacm-debuginfo=y
opensm=y
opensm-libs=y
opensm-devel=y
opensm-debuginfo=y
opensm-static=y
dapl=y
dapl-devel=y
dapl-devel-static=y
dapl-utils=y
dapl-debuginfo=y
perftest=y
mstflint=y
libiwpm=y
srptools=y
rds-tools=y
rds-devel=y
ibutils=y
infiniband-diags=y
qperf=y
qperf-debuginfo=y
ofed-docs=y
ofed-scripts=y
libfabric=y
libfabric-devel=y
libfabric-debuginfo=y
fabtests=y
fabtests-debuginfo=y
infinipath-psm=y
infinipath-psm-devel=y
core=y
mthca=
mlx4=y
mlx4_en=y
mlx5=
cxgb3=
cxgb4=
nes=
qib=
ocrdma=
ipoib=
srp=
iser=
nfsrdma=

^ permalink raw reply

* RE: [PATCH 05/13] Have cmake run man pages through text substitution
From: Hefty, Sean @ 2016-09-26 15:19 UTC (permalink / raw)
  To: Weiny, Ira, Jason Gunthorpe
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <2807E5FD2F6FDA4886F6618EAC48510E24EEBF1A-8k97q/ur5Z2krb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org>

> Infiniband-diags uses rst through rst2man.  But if I had to do it over
> I would probably do it different.  rst2man is not always installed and
> has caused build issues for some people.  Whatever tool we use I would
> recommend it be something common.  I'm not too familiar with markdown.
> Is this really common now?

Only by those who want to provide actual documentation.
 
Libfabric uses a daemon that checks for updates to the markdown pages.  If it finds any, it generates the man page and commits those changes to the source tree.  So the tar file always has the actual man page.  Our tool selection was based on what worked well with github and web page generation.
--
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 3/5] utils: Remove container_of and offset local declarations
From: Hefty, Sean @ 2016-09-26 15:32 UTC (permalink / raw)
  To: Leon Romanovsky, dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
  Cc: jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org,
	yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
In-Reply-To: <1474786207-2149-4-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

For ibacm/librdmacm:

Acked-by: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
--
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: [RFC v3 00/11] QLogic RDMA Driver (qedr) RFC
From: Amrani, Ram @ 2016-09-26 15:45 UTC (permalink / raw)
  To: davem@davemloft.net, dledford@redhat.com
  Cc: Elior, Ariel, Kalderon, Michal, Mintz, Yuval, Borundia, Rajesh,
	linux-rdma@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <1474899407-12635-1-git-send-email-Ram.Amrani@cavium.com>

> The series adds on top of RFC v2:
> * fix licensing header to dual license
> * remove 'debug' module paramter and make use of pr_debug
> * relocation of qedr user API to include/rdma/uapi/
> * use the __u32/64 in uapi and include types.h
> * advance ABI version (since shifting to __u32/64 changed the ABI)
> * remove the check for all drivers that IB_ACCESS_MW_BIND isn't set
>   in ib_get_dma_mr. It will be sent in a dedicated patch.
> * misc.: fixed typos, removed redundant includes
> 
> For RFC v2 visit http://marc.info/?l=linux-rdma&m=147436779328618&w=2

I see that the following e-mails were sent with "PATCH" in the subject rather than "RFC v3".
This is a mistake. All e-mails are intended to be "RFC v3".
I don't think it should matter much for reading and reviewing purposes, but don't hesitate if 
you would like me to re-send with fixed headers.

Ram

^ permalink raw reply

* [PATCH] IB/core: Improve ib_map_mr_sg() documentation
From: Bart Van Assche @ 2016-09-26 16:09 UTC (permalink / raw)
  To: Doug Ledford; +Cc: Sagi Grimberg, Christoph Hellwig, linux-rdma

Document that ib_map_mr_sg() is able to map physically discontiguous
sg-lists as a single MR. Change IB_MR_TYPE_SG_GAPS_REG into
IB_MR_TYPE_SG_GAPS.

Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
Cc: Sagi Grimberg <sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>
Cc: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
 drivers/infiniband/core/verbs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index f2b776efab3a..ff75be35cf28 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -1812,13 +1812,13 @@ EXPORT_SYMBOL(ib_set_vf_guid);
  *
  * Constraints:
  * - The first sg element is allowed to have an offset.
- * - Each sg element must be aligned to page_size (or physically
- *   contiguous to the previous element). In case an sg element has a
- *   non contiguous offset, the mapping prefix will not include it.
+ * - Each sg element must either be aligned to page_size or virtually
+ *   contiguous to the previous element. In case an sg element has a
+ *   non-contiguous offset, the mapping prefix will not include it.
  * - The last sg element is allowed to have length less than page_size.
  * - If sg_nents total byte length exceeds the mr max_num_sge * page_size
  *   then only max_num_sg entries will be mapped.
- * - If the MR was allocated with type IB_MR_TYPE_SG_GAPS_REG, non of these
+ * - If the MR was allocated with type IB_MR_TYPE_SG_GAPS, none of these
  *   constraints holds and the page_size argument is ignored.
  *
  * Returns the number of sg elements that were mapped to the memory region.
-- 
2.10.0

--
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: [PATCH] IB/core: Improve ib_map_mr_sg() documentation
From: Christoph Hellwig @ 2016-09-26 16:14 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Doug Ledford, Sagi Grimberg, Christoph Hellwig, linux-rdma
In-Reply-To: <b75ad680-88e5-e725-80cd-9a0b41bbb2ec-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>

On Mon, Sep 26, 2016 at 09:09:42AM -0700, Bart Van Assche wrote:
> Document that ib_map_mr_sg() is able to map physically discontiguous
> sg-lists as a single MR. Change IB_MR_TYPE_SG_GAPS_REG into
> IB_MR_TYPE_SG_GAPS.

Thanks a lot Bart, this look fine:

Reviewed-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
--
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 v5 00/16] Add Paravirtual RDMA Driver
From: Jason Gunthorpe @ 2016-09-26 16:51 UTC (permalink / raw)
  To: Adit Ranadive
  Cc: Leon Romanovsky, dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	pv-drivers-pghWNbHTmq7QT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-pci-u79uwXL29TY76Z2rM5mHXA, jhansen-pghWNbHTmq7QT0dZR+AlfA,
	asarwade-pghWNbHTmq7QT0dZR+AlfA,
	georgezhang-pghWNbHTmq7QT0dZR+AlfA,
	bryantan-pghWNbHTmq7QT0dZR+AlfA
In-Reply-To: <9f65ab5c-d8c2-e8e3-9334-5d1865a20dc9-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>

On Sun, Sep 25, 2016 at 10:25:12PM -0700, Adit Ranadive wrote:
> > As Jason said, you need a very good reason to split and create number of
> > files per-driver in UAPI folder.
> 
> I can move the pvrdma-uapi.h back to the pvrdma driver folder.

Just don't copy any of the content into your user space provider.

Thanks
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 1/4] IB/hfi1: Restore EPROM read ability
From: Jason Gunthorpe @ 2016-09-26 16:52 UTC (permalink / raw)
  To: Dennis Dalessandro
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, Easwar Hariharan, Dean Luick
In-Reply-To: <20160925144926.13602.47716.stgit-9QXIwq+3FY+1XWohqUldA0EOCMrvLtNR@public.gmane.org>

On Sun, Sep 25, 2016 at 07:49:29AM -0700, Dennis Dalessandro wrote:
> +	for (offset = 0; offset < len; offset += EP_PAGE_SIZE) {
> +		read_page(dd, start + offset, buffer);
> +		if (copy_to_user((void __user *)(addr + offset),

If it is not being used by userpsace then do not call copy_to_user...

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 1/4] IB/hfi1: Restore EPROM read ability
From: Luick, Dean @ 2016-09-26 17:11 UTC (permalink / raw)
  To: Jason Gunthorpe, Dalessandro, Dennis
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Hariharan, Easwar
In-Reply-To: <20160926165247.GB12011-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>



> -----Original Message-----
> From: Jason Gunthorpe [mailto:jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org]
> Sent: Monday, September 26, 2016 11:53 AM
> To: Dalessandro, Dennis <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Hariharan, Easwar
> <easwar.hariharan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; Luick, Dean <dean.luick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Subject: Re: [PATCH 1/4] IB/hfi1: Restore EPROM read ability
> 
> On Sun, Sep 25, 2016 at 07:49:29AM -0700, Dennis Dalessandro wrote:
> > +	for (offset = 0; offset < len; offset += EP_PAGE_SIZE) {
> > +		read_page(dd, start + offset, buffer);
> > +		if (copy_to_user((void __user *)(addr + offset),
> 
> If it is not being used by userpsace then do not call copy_to_user...

The first patch in the series is a straight (partial) restore of the old code.  This is what you are commenting on.  The second patch does a "fixup" of the restored code and removes this call.  Should we squash patches 1 and 2 so that this never shows up?  It is a question of how much "show your work" is desired.

Dean
--
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 2/5] utils: Create utils directory to put all common code and move min/max into it
From: Jason Gunthorpe @ 2016-09-26 17:17 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, yishaih-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <20160926070520.GJ4088-2ukJVAZIZ/Y@public.gmane.org>

On Mon, Sep 26, 2016 at 10:05:20AM +0300, Leon Romanovsky wrote:

> Can you give me a suggestion where and how should I use include_directories
> command to be able to write "#include <utils/math.h>"?

You were close, you did 'include_directories(util/)' which causes
something like 'gcc -I../util' - but that already stripped the util/
prefix, you'd need something like 'include_directories(.)'

However, let us continue to use the publish_headers scheme, so I
recommend you squish this into your series:

>From 406b60c89bd928d54b98e097e914f2131794ea62 Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Date: Mon, 26 Sep 2016 11:15:09 -0600
Subject: [PATCH] Use the publish_headers scheme for internal utils headers too

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 CMakeLists.txt                    |  7 +------
 buildlib/publish_headers.cmake    | 18 ++++++++++++++----
 ibacm/linux/osd.h                 |  2 +-
 libibverbs/examples/rc_pingpong.c |  2 +-
 libmlx5/src/mlx5.h                |  4 ++--
 libocrdma/src/ocrdma_main.c       |  2 +-
 libocrdma/src/ocrdma_main.h       |  2 +-
 libocrdma/src/ocrdma_verbs.c      |  2 +-
 librdmacm/src/cma.h               |  2 +-
 utils/CMakeLists.txt              |  4 ++++
 10 files changed, 27 insertions(+), 18 deletions(-)
 create mode 100644 utils/CMakeLists.txt

diff --git a/CMakeLists.txt b/CMakeLists.txt
index c997c6d90f3c..b28977550c39 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -38,8 +38,6 @@ set(BUILD_INCLUDE ${CMAKE_BINARY_DIR}/include)
 set(BUILD_BIN ${CMAKE_BINARY_DIR}/bin)
 # Libraries
 set(BUILD_LIB ${CMAKE_BINARY_DIR}/lib)
-# Common code
-set(COMMON_CODE utils)
 
 # Location to place provider .driver files
 set(CONFIG_DIR "${CMAKE_INSTALL_FULL_SYSCONFDIR}/libibverbs.d")
@@ -141,10 +139,6 @@ RDMA_AddOptCFlag(CMAKE_SHARED_LINKER_FLAGS SUPPORTS_NO_UNDEFINED "-Wl,--no-undef
 find_package(LDSymVer REQUIRED)
 
 #-------------------------
-# Common code
-include_directories(${COMMON_CODE})
-
-#-------------------------
 # Find libraries
 # pthread
 FIND_PACKAGE (Threads REQUIRED)
@@ -228,6 +222,7 @@ configure_file("${BUILDLIB}/config.h.in" "${BUILD_INCLUDE}/config.h" ESCAPE_QUOT
 
 #-------------------------
 # Sub-directories
+add_subdirectory(utils)
 # Libraries
 add_subdirectory(libibumad/src)
 add_subdirectory(libibumad/man)
diff --git a/buildlib/publish_headers.cmake b/buildlib/publish_headers.cmake
index ccd22960c141..c1909d677586 100644
--- a/buildlib/publish_headers.cmake
+++ b/buildlib/publish_headers.cmake
@@ -1,10 +1,9 @@
 # COPYRIGHT (c) 2016 Obsidian Research Corporation. See COPYING file
 
-# Copy headers from the source directory to the proper place in the
-# build/include directory
-function(PUBLISH_HEADERS DEST)
+# Same as publish_headers but does not install them during the install phase
+function(publish_internal_headers DEST)
   if(NOT ARGN)
-    message(SEND_ERROR "Error: PUBLISH_HEADERS called without any files")
+    message(SEND_ERROR "Error: publish_internal_headers called without any files")
     return()
   endif()
 
@@ -15,6 +14,17 @@ function(PUBLISH_HEADERS DEST)
     get_filename_component(FIL ${SFIL} NAME)
     execute_process(COMMAND "ln" "-Tsf"
       "${CMAKE_CURRENT_SOURCE_DIR}/${SFIL}" "${DDIR}/${FIL}")
+  endforeach()
+endfunction()
+
+# Copy headers from the source directory to the proper place in the
+# build/include directory. This also installs them into /usr/include/xx during
+# the install phase
+function(publish_headers DEST)
+  publish_internal_headers("${DEST}" ${ARGN})
+
+  foreach(SFIL ${ARGN})
+    get_filename_component(FIL ${SFIL} NAME)
     install(FILES "${SFIL}" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/${DEST}/" RENAME "${FIL}")
   endforeach()
 endfunction()
diff --git a/ibacm/linux/osd.h b/ibacm/linux/osd.h
index 1a738e0bfc6b..2094e859e7eb 100644
--- a/ibacm/linux/osd.h
+++ b/ibacm/linux/osd.h
@@ -46,7 +46,7 @@
 #include <sys/time.h>
 #include <netinet/in.h>
 
-#include "../../utils/math.h"
+#include <utils/math.h>
 
 #define ACM_CONF_DIR  IBACM_CONFIG_PATH
 #define ACM_ADDR_FILE "ibacm_addr.cfg"
diff --git a/libibverbs/examples/rc_pingpong.c b/libibverbs/examples/rc_pingpong.c
index cc45741be152..59f369c2e1fc 100644
--- a/libibverbs/examples/rc_pingpong.c
+++ b/libibverbs/examples/rc_pingpong.c
@@ -49,7 +49,7 @@
 
 #include "pingpong.h"
 
-#include "../../utils/math.h"
+#include <utils/math.h>
 
 enum {
 	PINGPONG_RECV_WRID = 1,
diff --git a/libmlx5/src/mlx5.h b/libmlx5/src/mlx5.h
index 739249941650..aa270288e840 100644
--- a/libmlx5/src/mlx5.h
+++ b/libmlx5/src/mlx5.h
@@ -41,8 +41,8 @@
 #include "mlx5-abi.h"
 #include "bitmap.h"
 
-#include "../../utils/math.h"
-#include "../../utils/list.h"
+#include <utils/math.h>
+#include <utils/list.h>
 
 #ifdef __GNUC__
 #define likely(x)	__builtin_expect((x), 1)
diff --git a/libocrdma/src/ocrdma_main.c b/libocrdma/src/ocrdma_main.c
index 6bd892c1f799..d86b2d779dc1 100644
--- a/libocrdma/src/ocrdma_main.c
+++ b/libocrdma/src/ocrdma_main.c
@@ -46,7 +46,7 @@
 
 #include "ocrdma_main.h"
 #include "ocrdma_abi.h"
-#include "../../utils/list.h"
+#include <utils/list.h>
 
 #include <sys/types.h>
 #include <sys/stat.h>
diff --git a/libocrdma/src/ocrdma_main.h b/libocrdma/src/ocrdma_main.h
index c7359c8ffaef..dc96cd214aff 100644
--- a/libocrdma/src/ocrdma_main.h
+++ b/libocrdma/src/ocrdma_main.h
@@ -42,7 +42,7 @@
 #include <infiniband/driver.h>
 #include <infiniband/arch.h>
 
-#include "../../utils/list.h"
+#include <utils/list.h>
 
 #define ocrdma_err(format, arg...) printf(format, ##arg)
 
diff --git a/libocrdma/src/ocrdma_verbs.c b/libocrdma/src/ocrdma_verbs.c
index 3efc8bbdb623..0507e7c8252a 100644
--- a/libocrdma/src/ocrdma_verbs.c
+++ b/libocrdma/src/ocrdma_verbs.c
@@ -51,7 +51,7 @@
 
 #include "ocrdma_main.h"
 #include "ocrdma_abi.h"
-#include "../../utils/list.h"
+#include <utils/list.h>
 
 static void ocrdma_ring_cq_db(struct ocrdma_cq *cq, uint32_t armed,
 			      int solicited, uint32_t num_cqe);
diff --git a/librdmacm/src/cma.h b/librdmacm/src/cma.h
index 83d98b0d0a84..c01c3c0cc35c 100644
--- a/librdmacm/src/cma.h
+++ b/librdmacm/src/cma.h
@@ -47,7 +47,7 @@
 #include <rdma/rdma_cma.h>
 #include <infiniband/ib.h>
 
-#include "../../utils/math.h"
+#include <utils/math.h>
 
 #ifdef INCLUDE_VALGRIND
 #   include <valgrind/memcheck.h>
diff --git a/utils/CMakeLists.txt b/utils/CMakeLists.txt
new file mode 100644
index 000000000000..2ad8d90d5a75
--- /dev/null
+++ b/utils/CMakeLists.txt
@@ -0,0 +1,4 @@
+publish_internal_headers(utils
+  list.h
+  math.h
+  )
-- 
2.1.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

* Re: [PATCH rdma-core 2/5] utils: Create utils directory to put all common code and move min/max into it
From: Jason Gunthorpe @ 2016-09-26 17:20 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, yishaih-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <1474786207-2149-3-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On Sun, Sep 25, 2016 at 09:50:04AM +0300, Leon Romanovsky wrote:

> -#ifndef min
> -#define min(a, b) \
> -	({ typeof(a) _a = (a); \
> -	   typeof(b) _b = (b); \
> -	   _a < _b ? _a : _b; })
> -#endif
> -
> -#ifndef max
> -#define max(a, b) \
> -	({ typeof(a) _a = (a); \
> -	   typeof(b) _b = (b); \
> -	   _a > _b ? _a : _b; })
> -#endif

I'm not excited to replace the safer min above with the less safe min
below, that could actually break something.

> +/*
> + * This include contains all common mathematical functions
> + */
> +#ifndef min
> +#define min(a, b)	(((a) < (b)) ? (a) : (b))
> +#endif
> +
> +#ifndef max
> +#define max(a, b) 	(((a) > (b)) ? (a) : (b))
> +#endif
> +#endif /* _RDMA_MATH_H_ */

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 rdma-core 3/5] utils: Remove container_of and offset local declarations
From: Jason Gunthorpe @ 2016-09-26 17:27 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, yishaih-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <1474786207-2149-4-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

missed one in libibcm/src/cm.c..

Elaborate the description a bit:

Remove container_of and offsetof local declarations

This tree uses the version of container_of in infinband/verbs.h,
and requries stddef.h to declare offsetof

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 v5 02/16] IB/pvrdma: Add user-level shared functions
From: Adit Ranadive @ 2016-09-26 17:33 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	pv-drivers-pghWNbHTmq7QT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-pci-u79uwXL29TY76Z2rM5mHXA, jhansen-pghWNbHTmq7QT0dZR+AlfA,
	asarwade-pghWNbHTmq7QT0dZR+AlfA,
	georgezhang-pghWNbHTmq7QT0dZR+AlfA,
	bryantan-pghWNbHTmq7QT0dZR+AlfA
In-Reply-To: <20160926061338.GH4088-2ukJVAZIZ/Y@public.gmane.org>

On Sun, Sep 25, 2016 at 23:13:38PM -0700, Leon Romanovsky wrote:
> On Sun, Sep 25, 2016 at 09:22:11PM -0700, Adit Ranadive wrote:
>> On Sun, Sep 25 2016 at 10:26:24AM +0300, Leon Romanovsky wrote:
> > > > On Sat, Sep 24, 2016 at 04:21:26PM -0700, Adit Ranadive wrote:
> > > > We share some common structures with the user-level driver. This patch adds
> > > > those structures and shared functions to traverse the QP/CQ rings.
> >
> > <...>
> >
> > > > +
> > > > +#include <linux/types.h>
> > > > +
> > > > +#define PVRDMA_UVERBS_ABI_VERSION	3
> > > > +#define PVRDMA_BOARD_ID			1
> > > > +#define PVRDMA_REV_ID			1
> > > >
> > > > Please don't add defines which you are not using in the library and the
> > > > two above are not in use.
> > > >
> >
> > I'll move these to the pvrdma.h file.
> >
> > <...>
> >
> > > > diff --git a/include/uapi/rdma/pvrdma-uapi.h b/include/uapi/rdma/pvrdma-uapi.h
> > > > new file mode 100644
> > > > index 0000000..430d8a5
> >
> > <...>
> >
> > > > +
> > > > +#ifndef __PVRDMA_UAPI_H__
> > > > +#define __PVRDMA_UAPI_H__
> > > > +
> > > > +#include <linux/types.h>
> > > > +
> > > > +#define PVRDMA_VERSION 17
> > > >
> > > > What do you plan to do with this VERSION?
> > > > How is it related to ABI?
> > > >
> >
> > Not related. This is only for the driver to know which APIs to support.
> > For example, an older driver would still be able to work with a newer
> > device. I can move this to pvrdma.h as well.
> >
> > To be honest, I thought I can move this file into the uapi folder since
> > the structures here are shared with the user-level library. Based on
> > your comments in this thread and the other ones, I think it makes sense
> > to move this file back to the pvrdma driver folder and rename it
> > (pvrdma_wqe.h?) to avoid confusion. There might still be some duplicate
> > code (especially the UAR offsets and WQE structs) here and in our
> > user-level library.
> >
> > Let me know if that makes sense.
> >
> > > > +
> > > > +#define PVRDMA_UAR_HANDLE_MASK	0x00FFFFFF	/* Bottom 24 bits. */
> > > > +#define PVRDMA_UAR_QP_OFFSET	0		/* Offset of QP doorbell. */
> > > > +#define PVRDMA_UAR_QP_SEND	BIT(30)		/* Send bit. */
> > > > +#define PVRDMA_UAR_QP_RECV	BIT(31)		/* Recv bit. */
> > > > +#define PVRDMA_UAR_CQ_OFFSET	4		/* Offset of CQ doorbell. */
> > > > +#define PVRDMA_UAR_CQ_ARM_SOL	BIT(29)		/* Arm solicited bit. */
> > > > +#define PVRDMA_UAR_CQ_ARM	BIT(30)		/* Arm bit. */
> > > > +#define PVRDMA_UAR_CQ_POLL	BIT(31)		/* Poll bit. */
> > > > +#define PVRDMA_INVALID_IDX	-1		/* Invalid index. */
> > > >
> > > > +
> > > > +/* PVRDMA atomic compare and swap */
> > > > +struct pvrdma_exp_cmp_swap {
> > > >
> > > > _EXP_ looks very similar to MLNX_OFED naming convention.
> > > >
> >
> > Yes, the operation was based on that. Any concerns?
> > I can rename this and the one below.
> 
> Yes, please.
> The common practice in IB subsystem is to use _ex_ notation for such
> extended structures.
> 

Ok.

> >
> > > > +	__u64 swap_val;
> > > > +	__u64 compare_val;
> > > > +	__u64 swap_mask;
> > > > +	__u64 compare_mask;
> > > > +};
> > > > +
> > > > +/* PVRDMA atomic fetch and add */
> > > > +struct pvrdma_exp_fetch_add {
> > > >
> > > > The same as above.
> > > >
> > > > +	__u64 add_val;
> > > > +	__u64 field_boundary;
> > > > +};
> > > > +
> > > > +/* PVRDMA address vector. */
> > > > +struct pvrdma_av {
> > > > +	__u32 port_pd;
> > > > +	__u32 sl_tclass_flowlabel;
> > > > +	__u8 dgid[16];
> > > > +	__u8 src_path_bits;
> > > > +	__u8 gid_index;
> > > > +	__u8 stat_rate;
> > > > +	__u8 hop_limit;
> > > > +	__u8 dmac[6];
> > > > +	__u8 reserved[6];
> > > > +};
> > > > +
> > > > +/* PVRDMA scatter/gather entry */
> > > > +struct pvrdma_sge {
> > > > +	__u64   addr;
> > > > +	__u32   length;
> > > > +	__u32   lkey;
> > > > +};
> > > > +
> > > > +/* PVRDMA receive queue work request */
> > > > +struct pvrdma_rq_wqe_hdr {
> > > > +	__u64 wr_id;		/* wr id */
> > > > +	__u32 num_sge;		/* size of s/g array */
> > > > +	__u32 total_len;	/* reserved */
> > > > +};
> > > > +/* Use pvrdma_sge (ib_sge) for receive queue s/g array elements. */
> > > > +
> > > > +/* PVRDMA send queue work request */
> > > > +struct pvrdma_sq_wqe_hdr {
> > > > +	__u64 wr_id;		/* wr id */
> > > > +	__u32 num_sge;		/* size of s/g array */
> > > > +	__u32 total_len;	/* reserved */
> > > > +	__u32 opcode;		/* operation type */
> > > > +	__u32 send_flags;	/* wr flags */
> > > > +	union {
> > > > +		__u32 imm_data;
> > > > +		__u32 invalidate_rkey;
> > > > +	} ex;
> > > > +	__u32 reserved;
> > > > +	union {
> > > > +		struct {
> > > > +			__u64 remote_addr;
> > > > +			__u32 rkey;
> > > > +			__u8 reserved[4];
> > > > +		} rdma;
> > > > +		struct {
> > > > +			__u64 remote_addr;
> > > > +			__u64 compare_add;
> > > > +			__u64 swap;
> > > > +			__u32 rkey;
> > > > +			__u32 reserved;
> > > > +		} atomic;
> > > > +		struct {
> > > > +			__u64 remote_addr;
> > > > +			__u32 log_arg_sz;
> > > > +			__u32 rkey;
> > > > +			union {
> > > > +				struct pvrdma_exp_cmp_swap  cmp_swap;
> > > > +				struct pvrdma_exp_fetch_add fetch_add;
> > > > +			} wr_data;
> > > > +		} masked_atomics;
> > > > +		struct {
> > > > +			__u64 iova_start;
> > > > +			__u64 pl_pdir_dma;
> > > > +			__u32 page_shift;
> > > > +			__u32 page_list_len;
> > > > +			__u32 length;
> > > > +			__u32 access_flags;
> > > > +			__u32 rkey;
> > > > +		} fast_reg;
> > > > +		struct {
> > > > +			__u32 remote_qpn;
> > > > +			__u32 remote_qkey;
> > > > +			struct pvrdma_av av;
> > > > +		} ud;
> > > > +	} wr;
> > > > +};
> > > >
> > > > No, I have half-baked patch series which refactors this structure in kernel.
> > > > There is no need to put this structure in UAPI.
> > > >
> >
> > This is specific to our device.. We do need to enqueue the WQE in this format
> > for the device to recognize it. This is the same format that the user-level
> > library will put the WQE in. As I said above, we can move this to the main
> > pvrdma driver directory if you prefer.
> 
> This is different implementations between kernel and user space.
> We don't want to bring user space limitations to kernel.
> Take a look here:
> http://lxr.free-electrons.com/source/include/rdma/ib_verbs.h#L1192
>

We anyway convert the WR structs defined there to our own device-specific
format. Similarly, in the user-level library we would convert from the
user-space WR structure to a device-specific structure. The struct above
defines the device-specific format.

There might be some overlap in these structures with userspace and kernel,
since some of the same opcodes would be supported between the two.
As long as OFED hides those limitations shouldnt it be okay to have similar
structures between the two?
--
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 4/5] libocrdma: Move ocrdma's list implementation into common directory
From: Jason Gunthorpe @ 2016-09-26 17:40 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Leon Romanovsky, dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, yishaih-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <20160925162203.GA32434-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>

On Sun, Sep 25, 2016 at 09:22:03AM -0700, Christoph Hellwig wrote:

> But given that I don't know who owns every little bit of the Linux
> lists.h I'd simply avoid anything looking like it and use something
> like the BSD queue.h instead (which glibc also provides, but in a
> horribly outdated version).

Copyright isn't a patent, assuming freebsd didn't copy any code and
just implemented the same API independently (eg it is an Independent
Creation) they should be OK from a copyright perspective. As should be
Rusty's version in CCAN.

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 1/4] IB/hfi1: Restore EPROM read ability
From: Jason Gunthorpe @ 2016-09-26 17:42 UTC (permalink / raw)
  To: Luick, Dean
  Cc: Dalessandro, Dennis,
	dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Hariharan, Easwar
In-Reply-To: <4AF12E8016D2BF46BCDFCE8FAA77A3580BE0838F-AtyAts71sc9Qxe9IK+vIArfspsVTdybXVpNB7YpNyf8@public.gmane.org>

> > If it is not being used by userpsace then do not call copy_to_user...
> 
> The first patch in the series is a straight (partial) restore of the
> old code.  This is what you are commenting on.  The second patch
> does a "fixup" of the restored code and removes this call.  Should
> we squash patches 1 and 2 so that this never shows up?  It is a
> question of how much "show your work" is desired.

Yes, squish them - do not make patches that are wrong and then fix
them up later in the series without a really good reason...

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 05/13] Have cmake run man pages through text substitution
From: Jason Gunthorpe @ 2016-09-26 17:47 UTC (permalink / raw)
  To: Hefty, Sean
  Cc: Weiny, Ira, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1828884A29C6694DAF28B7E6B8A82373AB08EFE3-P5GAC/sN6hkd3b2yrw5b5LfspsVTdybXVpNB7YpNyf8@public.gmane.org>

On Mon, Sep 26, 2016 at 03:19:53PM +0000, Hefty, Sean wrote:
> > Infiniband-diags uses rst through rst2man.  But if I had to do it over
> > I would probably do it different.  rst2man is not always installed and
> > has caused build issues for some people.  Whatever tool we use I would
> > recommend it be something common.  I'm not too familiar with markdown.
> > Is this really common now?
> 
> Only by those who want to provide actual documentation.

Markdown is very popular on github and the integrated processing of
the web UI is why I picked it for the docs I started. But rst started
the trend of structued ascii..

IMHO, rst is more widely available in distros today than pandoc.

rst2man is incredibly widely distributed: it comes with
python-docutils which is widely included in every distribution. I have
no idea why people would complain to you on that point. If something
comes in-box in all the distros then it is fair game to depend upon,
IMHO.

> Libfabric uses a daemon that checks for updates to the markdown
> pages.  If it finds any, it generates the man page and commits those
> changes to the source tree.  So the tar file always has the actual
> man page.  Our tool selection was based on what worked well with
> github and web page generation.

I generally frown on checking in built files, but yeah, that is an
option.

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 v5 08/16] IB/pvrdma: Add device command support
From: Adit Ranadive @ 2016-09-26 18:06 UTC (permalink / raw)
  To: Yuval Shaia
  Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	pv-drivers-pghWNbHTmq7QT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-pci-u79uwXL29TY76Z2rM5mHXA, jhansen-pghWNbHTmq7QT0dZR+AlfA,
	asarwade-pghWNbHTmq7QT0dZR+AlfA,
	georgezhang-pghWNbHTmq7QT0dZR+AlfA,
	bryantan-pghWNbHTmq7QT0dZR+AlfA
In-Reply-To: <20160926071601.GA6352-Hxa29pjIrERMGUUWBy6pNA@public.gmane.org>

On Mon, Sep 26, 2016 at 00:16:01AM -0700, Yuval Shaia wrote:
> Minor question/suggestion inline.
> (sorry for missing it till now).
> 
> Yuval

<...>

> > +int
> > +pvrdma_cmd_post(struct pvrdma_dev *dev, union pvrdma_cmd_req *req,
> > +		union pvrdma_cmd_resp *resp, unsigned resp_code)
> > +{
> > +	int err;
> > +
> > +	dev_dbg(&dev->pdev->dev, "post request to device\n");
> > +
> > +	/* Serializiation */
> > +	down(&dev->cmd_sema);
> > +
> > +	BUILD_BUG_ON(sizeof(union pvrdma_cmd_req) !=
> > +		     sizeof(struct pvrdma_cmd_modify_qp));
> > +
> > +	spin_lock(&dev->cmd_lock);
> > +	memcpy(dev->cmd_slot, req, sizeof(*req));
> > +	spin_unlock(&dev->cmd_lock);
> > +
> > +	init_completion(&dev->cmd_done);
> > +	pvrdma_write_reg(dev, PVRDMA_REG_REQUEST, 0);
> > +
> > +	/* Make sure the request is written before reading status. */
> > +	mb();
> > +
> > +	err = pvrdma_read_reg(dev, PVRDMA_REG_ERR);
> > +	if (err == 0) {
> > +		if (resp != NULL)
> > +			err = pvrdma_cmd_recv(dev, resp, resp_code);
> > +	} else {
> > +		dev_warn(&dev->pdev->dev, "failed to write request %d\n", err);
> > +	}
> 
> Please note that callers checks if err<0  while here err!=0 considered as
> error.
> Looking at similar code in pvrdma_pci_probe i can see that function
> "translates" this error to -EINVAL. Suggesting to do the same here
> (although -EINVAL is not a good choice, if device can be more friendly with
> the return code then we can propagate it up or just print a suitable error
> i.e. EAGAIN or ENOMEM otherwise EFAULT should be sufficient).
>

Thanks for pointing this out.
I dont think we return a positive error from the error register. So the
(err == 0) should be (err >= 0). Also, I'll set the err to -EFAULT if
read_reg fails (based on your feedback on the other patch).
--
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 13/13] Remove ibsupport-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org from MAINTAINERS
From: Jason Gunthorpe @ 2016-09-26 18:12 UTC (permalink / raw)
  To: Dalessandro, Dennis; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1474744719.6625.3.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

On Sat, Sep 24, 2016 at 07:18:42PM +0000, Dalessandro, Dennis wrote:
> Actually on second thought, maybe better to use:
> 
> infinipath <infinipath-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

This Ok?

https://github.com/jgunthorpe/rdma-plumbing/commit/b8436b9b21bc9f8432bba6b49573ea31db945101

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 v5 13/16] IB/pvrdma: Add the main driver module for PVRDMA
From: Adit Ranadive @ 2016-09-26 18:15 UTC (permalink / raw)
  To: Yuval Shaia
  Cc: dledford, linux-rdma, pv-drivers, netdev, linux-pci, jhansen,
	asarwade, georgezhang, bryantan
In-Reply-To: <20160926072739.GB6352@yuval-lap.Home>

On Mon, Sep 26, 2016 at 00:27:40AM -0700, Yuval Shaia wrote:
> On Sat, Sep 24, 2016 at 04:21:37PM -0700, Adit Ranadive wrote:
> > +
> > +	/* Currently, the driver only supports RoCE mode. */
> > +	if (dev->dsr->caps.mode != PVRDMA_DEVICE_MODE_ROCE) {
> > +		dev_err(&pdev->dev, "unsupported transport %d\n",
> > +			dev->dsr->caps.mode);
> > +		ret = -EINVAL;
> 
> This is some fatal error with the device, not that something wrong with the
> function's argument.
> Suggesting to replace with -EFAULT.
> 

Thanks, will fix this one and the others here.

^ permalink raw reply

* Re: [PATCH v3 0/9] SELinux support for Infiniband RDMA
From: Jason Gunthorpe @ 2016-09-26 18:17 UTC (permalink / raw)
  To: Paul Moore
  Cc: Daniel Jurgens, Leon Romanovsky, chrisw@sous-sol.org,
	Stephen Smalley, Eric Paris, dledford@redhat.com,
	sean.hefty@intel.com, hal.rosenstock@gmail.com,
	selinux@tycho.nsa.gov, linux-security-module@vger.kernel.org,
	linux-rdma@vger.kernel.org, Yevgeny Petrilin
In-Reply-To: <CAHC9VhRXDMiK5V0rOQTvtZB0Q8jTX7XecGmAfRhm8oqF9aBaiw@mail.gmail.com>

On Tue, Sep 20, 2016 at 07:43:34PM -0400, Paul Moore wrote:

> So far the discussion has been around providing access controls at the
> transport layer, are there any RDMA entities that are transport
> agnostic that might be better suited for what we are trying to do?  Or
> is it simply that the RDMA layer is tied so tightly to the underlying
> transport that we can't separate the two and have to consider them as
> one?

The generic RDMA layer is called 'rdmacm' and it is the layer
Mellanox's already applied patches for RDMA namespace enablement
worked at.

Jason

^ permalink raw reply

* RE: [PATCH 05/13] Have cmake run man pages through text substitution
From: Weiny, Ira @ 2016-09-26 18:17 UTC (permalink / raw)
  To: Jason Gunthorpe, Hefty, Sean
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20160926174713.GF22965-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

> On Mon, Sep 26, 2016 at 03:19:53PM +0000, Hefty, Sean wrote:
> > > Infiniband-diags uses rst through rst2man.  But if I had to do it
> > > over I would probably do it different.  rst2man is not always
> > > installed and has caused build issues for some people.  Whatever
> > > tool we use I would recommend it be something common.  I'm not too
> familiar with markdown.
> > > Is this really common now?
> >
> > Only by those who want to provide actual documentation.
> 
> Markdown is very popular on github and the integrated processing of the
> web UI is why I picked it for the docs I started. But rst started the trend of
> structued ascii..
> 
> IMHO, rst is more widely available in distros today than pandoc.
> 
> rst2man is incredibly widely distributed: it comes with python-docutils which
> is widely included in every distribution. I have no idea why people would
> complain to you on that point. If something comes in-box in all the distros
> then it is fair game to depend upon, IMHO.

I thought so to.  But python-docutils is in the "optional" section of RH.

https://ask.openstack.org/en/question/67176/python-docutils-openstack-installation/

> 
> > Libfabric uses a daemon that checks for updates to the markdown pages.
> > If it finds any, it generates the man page and commits those changes
> > to the source tree.  So the tar file always has the actual man page.
> > Our tool selection was based on what worked well with github and web
> > page generation.
> 
> I generally frown on checking in built files, but yeah, that is an option.

Yep I agree.  But I resorted to this as well.

:-(

Ira

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 05/13] Have cmake run man pages through text substitution
From: Jason Gunthorpe @ 2016-09-26 18:21 UTC (permalink / raw)
  To: Weiny, Ira
  Cc: Hefty, Sean, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <2807E5FD2F6FDA4886F6618EAC48510E24EEC6E9-8k97q/ur5Z2krb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org>

On Mon, Sep 26, 2016 at 06:17:03PM +0000, Weiny, Ira wrote:

> > rst2man is incredibly widely distributed: it comes with python-docutils which
> > is widely included in every distribution. I have no idea why people would
> > complain to you on that point. If something comes in-box in all the distros
> > then it is fair game to depend upon, IMHO.
> 
> I thought so to.  But python-docutils is in the "optional" section of RH.

I mean it is available via 'yum' with no special repositories. I do
not expect software to build out of the box on any arbitary distro
install. I expect builders to install the required set of
packages. This is why the exect package set is documented in the
README.md

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

* [PATCH 0/9] Introduce blk_quiesce_queue() and blk_resume_queue()
From: Bart Van Assche @ 2016-09-26 18:25 UTC (permalink / raw)
  To: 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

Hello Jens,

Multiple block drivers need the functionality to stop a request queue 
and to wait until all ongoing request_fn() / queue_rq() calls have 
finished without waiting until all outstanding requests have finished. 
Hence this patch series that introduces the blk_quiesce_queue() and 
blk_resume_queue() functions. The dm-mq, SRP and nvme patches in this 
patch series are three examples of where these functions are useful. 
These patches apply on top of the September 21 version of your 
for-4.9/block branch. The individual patches in this series are:

0001-blk-mq-Introduce-blk_mq_queue_stopped.patch
0002-dm-Fix-a-race-condition-related-to-stopping-and-star.patch
0003-RFC-nvme-Use-BLK_MQ_S_STOPPED-instead-of-QUEUE_FLAG_.patch
0004-block-Move-blk_freeze_queue-and-blk_unfreeze_queue-c.patch
0005-block-Extend-blk_freeze_queue_start-to-the-non-blk-m.patch
0006-block-Rename-mq_freeze_wq-and-mq_freeze_depth.patch
0007-blk-mq-Introduce-blk_quiesce_queue-and-blk_resume_qu.patch
0008-SRP-transport-Port-srp_wait_for_queuecommand-to-scsi.patch
0009-RFC-nvme-Fix-a-race-condition.patch

Thanks,

Bart.

^ permalink raw reply

* [PATCH 1/9] blk-mq: Introduce blk_mq_queue_stopped()
From: Bart Van Assche @ 2016-09-26 18:26 UTC (permalink / raw)
  To: Jens Axboe
  Cc: 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: <7948dbb8-6333-dc62-2673-4da35b4dfdbc-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>

The function blk_queue_stopped() allows to test whether or not a
traditional request queue has been stopped. Introduce a helper
function that allows block drivers to query easily whether or not
one or more hardware contexts of a blk-mq queue have been stopped.

Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
Cc: Jens Axboe <axboe-b10kYP2dOMg@public.gmane.org>
Cc: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
 block/blk-mq.c         | 20 ++++++++++++++++++++
 include/linux/blk-mq.h |  1 +
 2 files changed, 21 insertions(+)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index e9ebe98..98d4812 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -938,6 +938,26 @@ void blk_mq_run_hw_queues(struct request_queue *q, bool async)
 }
 EXPORT_SYMBOL(blk_mq_run_hw_queues);
 
+/**
+ * blk_mq_queue_stopped() - check whether one or more hctxs have been stopped
+ * @q: request queue.
+ *
+ * The caller is responsible for serializing this function against
+ * blk_mq_{start,stop}_hw_queue().
+ */
+bool blk_mq_queue_stopped(struct request_queue *q)
+{
+	struct blk_mq_hw_ctx *hctx;
+	int i;
+
+	queue_for_each_hw_ctx(q, hctx, i)
+		if (test_bit(BLK_MQ_S_STOPPED, &hctx->state))
+			return true;
+
+	return false;
+}
+EXPORT_SYMBOL(blk_mq_queue_stopped);
+
 void blk_mq_stop_hw_queue(struct blk_mq_hw_ctx *hctx)
 {
 	cancel_work(&hctx->run_work);
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 5daa0ef..368c460d 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -233,6 +233,7 @@ void blk_mq_delay_kick_requeue_list(struct request_queue *q, unsigned long msecs
 void blk_mq_abort_requeue_list(struct request_queue *q);
 void blk_mq_complete_request(struct request *rq, int error);
 
+bool blk_mq_queue_stopped(struct request_queue *q);
 void blk_mq_stop_hw_queue(struct blk_mq_hw_ctx *hctx);
 void blk_mq_start_hw_queue(struct blk_mq_hw_ctx *hctx);
 void blk_mq_stop_hw_queues(struct request_queue *q);
-- 
2.10.0

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


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