Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* Re: [PATCH rdma-core] libibumad/sysfs.h: Add copyright
From: Hal Rosenstock @ 2016-10-18 18:53 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Leon Romanovsky,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20161018184352.GA15413-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>

On 10/18/2016 2:43 PM, Christoph Hellwig wrote:
> That seems like an odd copyright statement for a file that I created..
> 
> While the delcarations came from another file the whitespaces and
> oinclude headers take up just as little lines and have just as little
> substance..

What's the best thing to do ?

--
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] libibumad/sysfs.h: Add copyright
From: Christoph Hellwig @ 2016-10-18 18:43 UTC (permalink / raw)
  To: Hal Rosenstock
  Cc: Leon Romanovsky,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <dfe69226-fb23-0bd5-c371-ad55c1356340-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>

That seems like an odd copyright statement for a file that I created..

While the delcarations came from another file the whitespaces and
oinclude headers take up just as little lines and have just as little
substance..
--
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] qedr: return -EINVAL if pd is null and avoid null ptr dereference
From: Colin King @ 2016-10-18 18:39 UTC (permalink / raw)
  To: Doug Ledford, Sean Hefty, Hal Rosenstock, Rajesh Borundia,
	Ram Amrani, linux-rdma-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA

From: Colin Ian King <colin.king-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

Currently, if pd is null then we hit a null pointer derference
on accessing pd->pd_id.  Instead of just printing an error message
we should also return -EINVAL immediately.

Signed-off-by: Colin Ian King <colin.king-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 drivers/infiniband/hw/qedr/verbs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
index a615142..b2a0eb8 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -511,8 +511,10 @@ 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)
+	if (!pd) {
 		pr_err("Invalid PD received in dealloc_pd\n");
+		return -EINVAL;
+	}
 
 	DP_DEBUG(dev, QEDR_MSG_INIT, "Deallocating PD %d\n", pd->pd_id);
 	dev->ops->rdma_dealloc_pd(dev->rdma_ctx, pd->pd_id);
-- 
2.9.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: [PATCH rdma-core] libibumad/sysfs.h: Add copyright
From: Leon Romanovsky @ 2016-10-18 17:30 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <dfe69226-fb23-0bd5-c371-ad55c1356340-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>

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

On Tue, Oct 18, 2016 at 01:20:18PM -0400, Hal Rosenstock wrote:
>
> Signed-off-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Thanks, applied.
https://github.com/linux-rdma/rdma-core/pull/22

> ---
> diff --git a/libibumad/sysfs.h b/libibumad/sysfs.h
> index 7f8cbfe..489f24b 100644
> --- a/libibumad/sysfs.h
> +++ b/libibumad/sysfs.h
> @@ -1,3 +1,35 @@
> +/*
> + * Copyright (c) 2008 Voltaire Inc.  All rights reserved.
> + *
> + * This software is available to you under a choice of one of two
> + * licenses.  You may choose to be licensed under the terms of the GNU
> + * General Public License (GPL) Version 2, available from the file
> + * COPYING in the main directory of this source tree, or the
> + * OpenIB.org BSD license below:
> + *
> + *     Redistribution and use in source and binary forms, with or
> + *     without modification, are permitted provided that the following
> + *     conditions are met:
> + *
> + *      - Redistributions of source code must retain the above
> + *        copyright notice, this list of conditions and the following
> + *        disclaimer.
> + *
> + *      - Redistributions in binary form must reproduce the above
> + *        copyright notice, this list of conditions and the following
> + *        disclaimer in the documentation and/or other materials
> + *        provided with the distribution.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + *
> + */
>  #ifndef _UMAD_SYSFS_H
>  #define _UMAD_SYSFS_H
>
> --
> 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

* [PATCH] IB/mad: Eliminate redundant SM class version defines for OPA
From: Hal Rosenstock @ 2016-10-18 17:20 UTC (permalink / raw)
  To: Doug Ledford
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Weiny, Ira


and rename class version define to indicate SM rather than SMP or SMI

Signed-off-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
index 40cbd6b..bc2a9c2 100644
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -769,7 +769,7 @@ static int handle_outgoing_dr_smp(struct ib_mad_agent_private *mad_agent_priv,
 	 * If we are at the start of the LID routed part, don't update the
 	 * hop_ptr or hop_cnt.  See section 14.2.2, Vol 1 IB spec.
 	 */
-	if (opa && smp->class_version == OPA_SMP_CLASS_VERSION) {
+	if (opa && smp->class_version == OPA_SM_CLASS_VERSION) {
 		u32 opa_drslid;
 
 		if ((opa_get_smp_direction(opa_smp)
@@ -2167,7 +2167,7 @@ static bool generate_unmatched_resp(const struct ib_mad_private *recv,
 	struct ib_mad_hdr *mad_hdr = (struct ib_mad_hdr *)recv->mad;
 
 	if (opa && mad_hdr->base_version == OPA_MGMT_BASE_VERSION &&
-	    mad_hdr->class_version == OPA_SMI_CLASS_VERSION)
+	    mad_hdr->class_version == OPA_SM_CLASS_VERSION)
 		return handle_opa_smi(port_priv, qp_info, wc, port_num, recv,
 				      response);
 
diff --git a/drivers/infiniband/hw/hfi1/mad.c b/drivers/infiniband/hw/hfi1/mad.c
index 9487c9b..320e4da 100644
--- a/drivers/infiniband/hw/hfi1/mad.c
+++ b/drivers/infiniband/hw/hfi1/mad.c
@@ -128,7 +128,7 @@ static void send_trap(struct hfi1_ibport *ibp, void *data, unsigned len)
 	smp = send_buf->mad;
 	smp->base_version = OPA_MGMT_BASE_VERSION;
 	smp->mgmt_class = IB_MGMT_CLASS_SUBN_LID_ROUTED;
-	smp->class_version = OPA_SMI_CLASS_VERSION;
+	smp->class_version = OPA_SM_CLASS_VERSION;
 	smp->method = IB_MGMT_METHOD_TRAP;
 	ibp->rvp.tid++;
 	smp->tid = cpu_to_be64(ibp->rvp.tid);
@@ -343,7 +343,7 @@ static int __subn_get_opa_nodeinfo(struct opa_smp *smp, u32 am, u8 *data,
 
 	ni->port_guid = cpu_to_be64(dd->pport[pidx].guid);
 	ni->base_version = OPA_MGMT_BASE_VERSION;
-	ni->class_version = OPA_SMI_CLASS_VERSION;
+	ni->class_version = OPA_SM_CLASS_VERSION;
 	ni->node_type = 1;     /* channel adapter */
 	ni->num_ports = ibdev->phys_port_cnt;
 	/* This is already in network order */
@@ -379,7 +379,7 @@ static int subn_get_nodeinfo(struct ib_smp *smp, struct ib_device *ibdev,
 		nip->port_guid = cpu_to_be64(dd->pport[pidx].guid);
 
 	nip->base_version = OPA_MGMT_BASE_VERSION;
-	nip->class_version = OPA_SMI_CLASS_VERSION;
+	nip->class_version = OPA_SM_CLASS_VERSION;
 	nip->node_type = 1;     /* channel adapter */
 	nip->num_ports = ibdev->phys_port_cnt;
 	/* This is already in network order */
@@ -2302,7 +2302,7 @@ static int pma_get_opa_classportinfo(struct opa_pma_mad *pmp,
 		pmp->mad_hdr.status |= IB_SMP_INVALID_FIELD;
 
 	p->base_version = OPA_MGMT_BASE_VERSION;
-	p->class_version = OPA_SMI_CLASS_VERSION;
+	p->class_version = OPA_SM_CLASS_VERSION;
 	/*
 	 * Expected response time is 4.096 usec. * 2^18 == 1.073741824 sec.
 	 */
@@ -4022,7 +4022,7 @@ static int process_subn_opa(struct ib_device *ibdev, int mad_flags,
 
 	am = be32_to_cpu(smp->attr_mod);
 	attr_id = smp->attr_id;
-	if (smp->class_version != OPA_SMI_CLASS_VERSION) {
+	if (smp->class_version != OPA_SM_CLASS_VERSION) {
 		smp->status |= IB_SMP_UNSUP_VERSION;
 		ret = reply((struct ib_mad_hdr *)smp);
 		return ret;
@@ -4232,7 +4232,7 @@ static int process_perf_opa(struct ib_device *ibdev, u8 port,
 
 	*out_mad = *in_mad;
 
-	if (pmp->mad_hdr.class_version != OPA_SMI_CLASS_VERSION) {
+	if (pmp->mad_hdr.class_version != OPA_SM_CLASS_VERSION) {
 		pmp->mad_hdr.status |= IB_SMP_UNSUP_VERSION;
 		return reply((struct ib_mad_hdr *)pmp);
 	}
diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h
index c8a773f..981214b 100644
--- a/include/rdma/ib_mad.h
+++ b/include/rdma/ib_mad.h
@@ -46,7 +46,7 @@
 #define IB_MGMT_BASE_VERSION			1
 #define OPA_MGMT_BASE_VERSION			0x80
 
-#define OPA_SMP_CLASS_VERSION			0x80
+#define OPA_SM_CLASS_VERSION			0x80
 
 /* Management classes */
 #define IB_MGMT_CLASS_SUBN_LID_ROUTED		0x01
diff --git a/include/rdma/opa_smi.h b/include/rdma/opa_smi.h
index 4a529ef..f789611 100644
--- a/include/rdma/opa_smi.h
+++ b/include/rdma/opa_smi.h
@@ -44,8 +44,6 @@
 #define OPA_MAX_SLS				32
 #define OPA_MAX_SCS				32
 
-#define OPA_SMI_CLASS_VERSION			0x80
-
 #define OPA_LID_PERMISSIVE			cpu_to_be32(0xFFFFFFFF)
 
 struct opa_smp {
--
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

* [PATCH rdma-core] libibumad/sysfs.h: Add copyright
From: Hal Rosenstock @ 2016-10-18 17:20 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org


Signed-off-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
diff --git a/libibumad/sysfs.h b/libibumad/sysfs.h
index 7f8cbfe..489f24b 100644
--- a/libibumad/sysfs.h
+++ b/libibumad/sysfs.h
@@ -1,3 +1,35 @@
+/*
+ * Copyright (c) 2008 Voltaire Inc.  All rights reserved.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses.  You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ *     Redistribution and use in source and binary forms, with or
+ *     without modification, are permitted provided that the following
+ *     conditions are met:
+ *
+ *      - Redistributions of source code must retain the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer.
+ *
+ *      - Redistributions in binary form must reproduce the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer in the documentation and/or other materials
+ *        provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ */
 #ifndef _UMAD_SYSFS_H
 #define _UMAD_SYSFS_H
 
--
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: building rdma-core from travis
From: Jason Gunthorpe @ 2016-10-18 17:00 UTC (permalink / raw)
  To: Hefty, Sean
  Cc: Leon Romanovsky,
	linux-rdma (linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org)
In-Reply-To: <1828884A29C6694DAF28B7E6B8A82373AB096CC3-P5GAC/sN6hkd3b2yrw5b5LfspsVTdybXVpNB7YpNyf8@public.gmane.org>

On Tue, Oct 18, 2016 at 04:31:23PM +0000, Hefty, Sean wrote:
> > Right. rdma-core does not support precise, at all. Aside from cmake,
> > the original source packages had some non-functional 'compat' for such
> > an old glibc (from 2012!) that I stripped out as well.
> 
> To people who use vi to write C code with text-based email for their
> code review process, 2012 is spaceship futuristic.

Well, those people will suffer then *shrug*

> > I can't recommend that anybody working on a c-language project should
> > be using the precise images on travis for anything <shrug>
> 
> We use containers, which are supposed to be faster for processing.

I seem to recall the GCE images have faster CPUs but spend more time
booting the VM.

For rdma-core I counter balanced the VM spin up time by avoiding the
matrix feature. rdma-core has a very fast compile at 19s, I can run
all four compiler passes in less time than it takes to spin up and
configure the VM, container or not :\.

For something like libfabric you might get a win if you do the same
and run fabtests on one of the compiler's output instead of for both.

Plus, if you use trusty you don't need to download and compile
rdma-core, just install libibverbs-dev and librdmacm-dev from Ubuntu.

But at the end of the day, it depend what you expect out of
travis. rdma-core has travis setup to try and show that the code is
clean with bleeding edge rolling release distros (eg forward
looking).

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: building rdma-core from travis
From: Hefty, Sean @ 2016-10-18 16:31 UTC (permalink / raw)
  To: Jason Gunthorpe, Leon Romanovsky
  Cc: linux-rdma (linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org)
In-Reply-To: <20161018160716.GB24189-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

> Right. rdma-core does not support precise, at all. Aside from cmake,
> the original source packages had some non-functional 'compat' for such
> an old glibc (from 2012!) that I stripped out as well.

To people who use vi to write C code with text-based email for their code review process, 2012 is spaceship futuristic.

> I can't recommend that anybody working on a c-language project should
> be using the precise images on travis for anything <shrug>

We use containers, which are supposed to be faster for processing.
--
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 TRIVIAL infiniband-diags] ibaddr.c: Fix LID printfs to be unsigned rather than signed
From: ira. weiny @ 2016-10-18 16:27 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <b0f11628-e2ef-15d9-5a9e-68edbc81ca77-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>

On Mon, Sep 26, 2016 at 07:38:46AM -0400, Hal Rosenstock wrote:
> 
> %u is what's used in other places for unicast LIDs
> 
> Signed-off-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Thanks applied,
Ira

> ---
> diff --git a/src/ibaddr.c b/src/ibaddr.c
> index 5845b4e..1a721d0 100644
> --- a/src/ibaddr.c
> +++ b/src/ibaddr.c
> @@ -83,7 +83,7 @@ static int ib_resolve_addr(ib_portid_t * portid, int portnum, int show_lid,
>  		printf("LID start 0x%x end 0x%x", portid->lid,
>  		       portid->lid + (1 << lmc) - 1);
>  	else if (show_lid < 0)
> -		printf("LID start %d end %d", portid->lid,
> +		printf("LID start %u end %u", portid->lid,
>  		       portid->lid + (1 << lmc) - 1);
>  	printf("\n");
>  	return 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

* Re: [PATCH TRIVIAL infiniband-diags] ibportstate.c: When not switch, port can be router port
From: ira.weiny @ 2016-10-18 16:25 UTC (permalink / raw)
  To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <f44f2a4f-15f7-6ae5-d1fb-7af0a77aeaee-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>

On Fri, Sep 23, 2016 at 09:17:10AM -0400, Hal Rosenstock wrote:
> 
> Indicate PortInfo is on CA/RT port rather than just CA port when not
> on Switch port. RT is what's used by ibnetdiscover for router nodes.
> 
> Signed-off-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>

Thanks applied,
Ira

> ---
> diff --git a/src/ibportstate.c b/src/ibportstate.c
> index cb47aa9..cfb8be7 100644
> --- a/src/ibportstate.c
> +++ b/src/ibportstate.c
> @@ -522,9 +522,9 @@ int main(int argc, char **argv)
>  		IBEXIT("Can't set M_Key fields on switch port != 0");
>  
>  	if (port_op != QUERY || changed)
> -		printf("Initial %s PortInfo:\n", is_switch ? "Switch" : "CA");
> +		printf("Initial %s PortInfo:\n", is_switch ? "Switch" : "CA/RT");
>  	else
> -		printf("%s PortInfo:\n", is_switch ? "Switch" : "CA");
> +		printf("%s PortInfo:\n", is_switch ? "Switch" : "CA/RT");
>  	espeed_cap = get_port_info(&portid, data, portnum, is_switch);
>  	show_port_info(&portid, data, portnum, espeed_cap, is_switch);
>  	if (is_mlnx_ext_port_info_supported(devid)) {
--
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: ibacm cleanups
From: Leon Romanovsky @ 2016-10-18 16:25 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476731482-26491-1-git-send-email-hch-jcswGhMUV9g@public.gmane.org>

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

On Mon, Oct 17, 2016 at 09:11:09PM +0200, Christoph Hellwig wrote:
> This series fixed various sparse issues in ibacm, makes it use
> the common list helpers and removes various cruft.

Thanks, applied
https://github.com/linux-rdma/rdma-core/pull/21

>
> --
> 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: SQ overflow seen running isert traffic
From: Jason Gunthorpe @ 2016-10-18 16:13 UTC (permalink / raw)
  To: Steve Wise
  Cc: 'Sagi Grimberg', 'Potnuri Bharat Teja',
	target-devel, nab, linux-rdma
In-Reply-To: <005101d2294c$be5bb460$3b131d20$@opengridcomputing.com>

On Tue, Oct 18, 2016 at 09:34:33AM -0500, Steve Wise wrote:

> 1) we believe the iSER + RW API correctly sizes the SQ, yet we're seeing SQ
> overflows.  So the SQ sizing needs more investigation.

NFS had this sort of problem - in that case it was because the code
was assuming that a RQ completion implied SQ space - that is not
legal, only direct completions from SQ WCs can guide available space
in the SQ..

Jason

^ permalink raw reply

* Re: building rdma-core from travis
From: Jason Gunthorpe @ 2016-10-18 16:07 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Hefty, Sean,
	linux-rdma (linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org)
In-Reply-To: <20161018050834.GF25013-2ukJVAZIZ/Y@public.gmane.org>

On Tue, Oct 18, 2016 at 08:08:34AM +0300, Leon Romanovsky wrote:
> > > I can see that travis runs successfully against rdma-core.  Was there a
> > > magic setting used to work-around the travis version?
> >
> > It looks like rdma-core requires the 'trusty' build on travis...
> 
> Yes, you are right.
> "# We need at least cmake 2.12, this means we need to use trusty."
> http://marc.info/?l=linux-rdma&m=147587977913428&w=2

Right. rdma-core does not support precise, at all. Aside from cmake,
the original source packages had some non-functional 'compat' for such
an old glibc (from 2012!) that I stripped out as well.

I can't recommend that anybody working on a c-language project should
be using the precise images on travis for anything <shrug>

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 v3 net-next 00/11] net: Fix netdev adjacency tracking
From: David Ahern @ 2016-10-18 15:58 UTC (permalink / raw)
  To: David Miller
  Cc: jiri, netdev, dledford, sean.hefty, hal.rosenstock, linux-rdma,
	j.vosburgh, vfalico, andy, jeffrey.t.kirsher, intel-wired-lan
In-Reply-To: <20161018.114631.1954199143257806104.davem@davemloft.net>

On 10/18/16 9:46 AM, David Miller wrote:
> Series applied, but the recursion is disappointing.
> 
> If we run into problems due to kernel stack depth because of this with
> some configurations (reasonable or not, if we allow it then it can't
> crash the kernel), we will either need to find a way to make this walk
> iterative or revert these changes.

understood.

Since 4.9 is tagged as the next LTS I would like to see the series applied to it at some point - assuming no problems show up with wider exposure in net-next.

^ permalink raw reply

* Re: ???For help??? configure crossbar build tool in CMakelist.txt
From: Jason Gunthorpe @ 2016-10-18 15:50 UTC (permalink / raw)
  To: oulijun; +Cc: linux-rdma, Linuxarm
In-Reply-To: <5805FF8C.1060902-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>

On Tue, Oct 18, 2016 at 06:55:08PM +0800, oulijun wrote:

>   if I use crossbar build tool aarch64-linux-gnu-gcc for building
>   the directory(provider/hns), what i should do it ?

You cannot cross compile only a part of the project, you must cross
compile everything.

$ mkdir build; cd build
$ CC=aarch64-linux-gnu-gcc cmake -GNinja
$ ninja

I haven't extensively tested cross compiling, you may run into
problems, in particular I know pkgconfig for libnl doesn't work
reliably when cross compiling.

You may need to build with -DENABLE_RESOLVE_NEIGH=0 if you don't have
a cross compiled libnl3 available.

> My modification currently according to the others as fllows:
> 
> in the file : provider/hns/CMakelist.txt
> 
> set(CMAKE_C_COMPILER /opt/gcc-linaro-aarch64-linux-gnu-4.9-2014.09_linux/bin/aarch64-linux-gnu-gcc)

This is not the right approach, the hns provider CMakelist should
detect the compiler is not ARM64 and just do nothing.

Perhaps something like this:

CHECK_C_SOURCE_COMPILES("
#ifndef __ARM64__
#error Failed
#endif
 int main(int argc,const char *argv[]) { return 1; }"
 HAVE_ARCH_ARM64)

if (HAVE_ARCH_ARM64)
 [..]
endif()
 
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 v3 net-next 00/11] net: Fix netdev adjacency tracking
From: David Miller @ 2016-10-18 15:46 UTC (permalink / raw)
  To: dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR
  Cc: jiri-VPRAkNaXOzVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
	dledford-H+wXaHxf7aLQT0dZR+AlfA,
	sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
	hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	j.vosburgh-Re5JQEeQqe8AvxtiuMwx3w, vfalico-Re5JQEeQqe8AvxtiuMwx3w,
	andy-QlMahl40kYEqcZcGjlUOXw,
	jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
	intel-wired-lan-qjLDD68F18P21nG7glBr7A
In-Reply-To: <1476756953-30923-1-git-send-email-dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>

From: David Ahern <dsa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
Date: Mon, 17 Oct 2016 19:15:42 -0700

> The netdev adjacency tracking is failing to create proper dependencies
> for some topologies. For example this topology
 ...
> hits 1 of 2 problems depending on the order of enslavement. The base set of
> commands for both cases:
 ...
> Case 1 enslave macvlan to the vrf before enslaving the bond to the bridge:
 ...
> Attempts to delete the VRF:
>     ip link delete myvrf
> 
> trigger the BUG in __netdev_adjacent_dev_remove:
 ...
> When the BUG is converted to a WARN_ON it shows 4 missing adjacencies:
>   eth3 - myvrf, mvrf - eth3, bond1 - myvrf and myvrf - bond1
> 
> All of those are because the __netdev_upper_dev_link function does not
> properly link macvlan lower devices to myvrf when it is enslaved.
 ...
> Rather than try to maintain a linked list of all upper and lower devices
> per netdevice, only track the direct neighbors. The remaining stack can
> be determined by recursively walking the neighbors.
> 
> The existing netdev_for_each_all_upper_dev_rcu,
> netdev_for_each_all_lower_dev and netdev_for_each_all_lower_dev_rcu macros
> are replaced with APIs that walk the upper and lower device lists. The
> new APIs take a callback function and a data arg that is passed to the
> callback for each device in the list. Drivers using the old macros are
> converted in separate patches to make it easier on reviewers. It is an
> API conversion only; no functional change is intended.

Series applied, but the recursion is disappointing.

If we run into problems due to kernel stack depth because of this with
some configurations (reasonable or not, if we allow it then it can't
crash the kernel), we will either need to find a way to make this walk
iterative or revert these changes.

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

^ permalink raw reply

* Re: [PATCH 00/10] mm: adjust get_user_pages* functions to explicitly pass FOLL_* flags
From: Michal Hocko @ 2016-10-18 15:30 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: linux-mm, Linus Torvalds, Jan Kara, Hugh Dickins, Dave Hansen,
	Rik van Riel, Mel Gorman, Andrew Morton, adi-buildroot-devel,
	ceph-devel, dri-devel, intel-gfx, kvm, linux-alpha,
	linux-arm-kernel, linux-cris-kernel, linux-fbdev, linux-fsdevel,
	linux-ia64, linux-kernel, linux-media, linux-mips, linux-rdma,
	linux-s390, linux-samsung-soc, linux-scsi, linux-security-module,
	linux-sh@
In-Reply-To: <20161013002020.3062-1-lstoakes@gmail.com>

On Thu 13-10-16 01:20:10, Lorenzo Stoakes wrote:
> This patch series adjusts functions in the get_user_pages* family such that
> desired FOLL_* flags are passed as an argument rather than implied by flags.
> 
> The purpose of this change is to make the use of FOLL_FORCE explicit so it is
> easier to grep for and clearer to callers that this flag is being used. The use
> of FOLL_FORCE is an issue as it overrides missing VM_READ/VM_WRITE flags for the
> VMA whose pages we are reading from/writing to, which can result in surprising
> behaviour.
> 
> The patch series came out of the discussion around commit 38e0885, which
> addressed a BUG_ON() being triggered when a page was faulted in with PROT_NONE
> set but having been overridden by FOLL_FORCE. do_numa_page() was run on the
> assumption the page _must_ be one marked for NUMA node migration as an actual
> PROT_NONE page would have been dealt with prior to this code path, however
> FOLL_FORCE introduced a situation where this assumption did not hold.
> 
> See https://marc.info/?l=linux-mm&m=147585445805166 for the patch proposal.

I like this cleanup. Tracking FOLL_FORCE users was always a nightmare
and the flag behavior is really subtle so we should better be explicit
about it. I haven't gone through each patch separately but rather
applied the whole series and checked the resulting diff. This all seems
OK to me and feel free to add
Acked-by: Michal Hocko <mhocko@suse.com>

I am wondering whether we can go further. E.g. it is not really clear to
me whether we need an explicit FOLL_REMOTE when we can in fact check
mm != current->mm and imply that. Maybe there are some contexts which
wouldn't work, I haven't checked.

Then I am also wondering about FOLL_TOUCH behavior.
__get_user_pages_unlocked has only few callers which used to be
get_user_pages_unlocked before 1e9877902dc7e ("mm/gup: Introduce
get_user_pages_remote()"). To me a dropped FOLL_TOUCH seems
unintentional. Now that get_user_pages_unlocked has gup_flags argument I
guess we might want to get rid of the __g-u-p-u version altogether, no?

__get_user_pages is quite low level and imho shouldn't be exported. It's
only user - kvm - should rather pull those two functions to gup instead
and export them. There is nothing really KVM specific in them.

I also cannot say I would be entirely thrilled about get_user_pages_locked,
we only have one user which can simply do lock g-u-p unlock AFAICS.

I guess there is more work in that area and I do not want to impose all
that work on you, but I couldn't resist once I saw you playing in that
area ;) Definitely a good start!
-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH rdma-core 2/4] glue/redhat: add udev/systemd/etc infrastructure bits
From: Jarod Wilson @ 2016-10-18 14:51 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Weiny, Ira, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Doug Ledford, Hefty, Sean
In-Reply-To: <20161017201309.GA24291-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

On Mon, Oct 17, 2016 at 02:13:09PM -0600, Jason Gunthorpe wrote:
> On Mon, Oct 17, 2016 at 07:10:46PM +0000, Weiny, Ira wrote:
> 
> > What I have been worried about is conflicts between infiniband-diags
> > and the new rdma-core.  RH made a separate package out of rdma-ndd
> > so that would be easy but I don't think other distros have.
> 
> > So how do you obsolete "part" of a package?
> 
> The distros know how to do this, they just conflict with the old
> version of infiniband-diags.

Yeah, basically, you spin up a new infiniband-diags package that doesn't
have rdma-ndd and a new rdma-core package that includes it, along with (in
rpm-ese) 'Conflicts: infiniband-diags < <first version w/o rdma-ndd>', and
that should pretty much cover it.

-- 
Jarod Wilson
jarod-H+wXaHxf7aLQT0dZR+AlfA@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 rdma-core 4/4] glue/redhat/spec: build split rpm packages
From: Jarod Wilson @ 2016-10-18 14:44 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	dledford-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <20161017190705.GB8122-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

On Mon, Oct 17, 2016 at 01:07:05PM -0600, Jason Gunthorpe wrote:
> On Mon, Oct 17, 2016 at 02:45:06PM -0400, Jarod Wilson wrote:
> 
> > > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libcxgb3-11-1.el7.x86_64.rpm
> > > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libcxgb4-11-1.el7.x86_64.rpm
> > > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libhfi1-11-1.el7.x86_64.rpm
> > > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libi40iw-11-1.el7.x86_64.rpm
> > > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libipathverbs-11-1.el7.x86_64.rpm
> > > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libmlx4-11-1.el7.x86_64.rpm
> > > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libmlx5-11-1.el7.x86_64.rpm
> > > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libmthca-11-1.el7.x86_64.rpm
> > > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libnes-11-1.el7.x86_64.rpm
> > > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libocrdma-11-1.el7.x86_64.rpm
> > > > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/librxe-11-1.el7.x86_64.rpm
> > > 
> > > Does this mean RH is not going to do a single package for the
> > > providers? You know another 5 or so are coming..
> > 
> > Putting all the providers in a single sub-package is certainly something
> > we could do. With rpm virtual Provides and Obsoletes, we could make the
> > upgrade path transparent to at least yum/dnf/rpm, but it might still
> > confuse people who go looking for the same package they've always had
> > installed, so it could be something we do in Fedora and the next RHEL
> > major release, rather than in a current RHEL minor update.
> 
> I would strongly encoruage this - the current scheme is silly, you
> need to know to look for the right provider to get things working
> (which is very much unlike the way the kernel modules work).
> 
> Suggest to dump the providers in libiverbs or add a ibverbs-providers
> package..

I'm playing with a setup where libibverbs includes all the providers now.
I think I like it. It'd require some documentation to explain where libfoo
went, but it doesn't look too bad:

$ sudo rpm -Uvh libibverbs-11-1.el7.x86_64.rpm libibverbs-utils-11-1.el7.x86_64.rpm
Preparing...                          ################################# [100%]
Updating / installing...
   1:libibverbs-11-1.el7              ################################# [ 14%]
   2:libibverbs-utils-11-1.el7        ################################# [ 29%]
Cleaning up / removing...
   3:libibverbs-utils-1.2.1-1.el7     ################################# [ 43%]
   4:libi40iw-0.5.227-2.el7           ################################# [ 57%]
   5:libocrdma-1.0.8-1.el7            ################################# [ 71%]
   6:libmlx5-1.2.1-8.el7              ################################# [ 86%]
   7:libibverbs-1.2.1-1.el7           ################################# [100%]

Seems I have the Provides/Obsoletes correct at least. This also eliminates
the somewhat gross libibverbs-driver Provides from each driver and the
matching Requires in libibverbs-utils, which I like...

Doug, any thoughts from you on this front?

> > > > +install -D -m0644 Documentation/{ibacm,ibsrpdm,libibcm,libibverbs,librdmacm,rxe}.md %{buildroot}/%{_docdir}/%{name}-%{version}/
> > > > +install -D -m0644 README.md %{buildroot}/%{_docdir}/%{name}-%{version}/
> > > 
> > > I guess README should go in that patch I sent you..
> > 
> > D'oh, yeah, I'll mix that in here locally.
> 
> I think you can just add ../README.md to the
> Documentation/CMakeFile.txt

First cut, I added a Files line to the root CMakeLists.txt. Would the
above be preferred over that? Both seem to work.

> > > > +%{buildroot}/%{_bindir}/ib_acme -D . -O
> > > > +# Fixup a multilib conflict in ibacm_opts.cfg:
> > > > +sed -i -e '/^# Specifies the directory of the provider libraries$/ a\
> > > > +# Use /usr/lib64/ibacm on 64bit, /usr/lib/libacm on 32bit.
> > > > +' -e 's%^\(# provider_lib_path
> > > > /usr/\)lib\(64\)\?/ibacm$%\1lib64/ibacm%' ibacm_opts.cfg
> > > 
> > > Hum? I'm pretty sure this is basically fixed in the code now, was done here
> > > 5eebdb9baaaae420a4bb16e586a96807823916a0
> > > 
> > > Adjusting the comment like that doesn't really make sense, the acm
> > > daemon has a fixed endianness and looks in a single place to load the
> > > plugin. If someone wants to use 32 bit plugins they have to install
> > > the 32 bit acm daemon, which would have the 32 bit path in the sample
> > > conf file...
> > 
> > This was just copy and paste from our existing libacm spec, might well be
> > something that was fixed and I just missed it.
> 
> It looks OK as-is to me, I'd drop it now.

Dropped, things do look fine w/o that.

> > They weren't getting installed, so I threw that hack in, meant to
> > actually say something about that, got lost in the shuffle. I'll
> > double-check if they're still not getting installed... Okay, they're
> > getting installed now. May have just been a transient error with an
> > earlier tarball, or something else I did wrong. I'll drop that bit.
> 
> How strange.. FWIW, the tarball should be built with git-archive ..

There was a ton of churn from what I was doing, I blame me. ;) All good
now, and yes, creating tarballs with git archive:

$ git archive --prefix=rdma-core-11/ -o ~/rpmbuild/SOURCES/rdma-core-11.tgz --format tar.gz HEAD

-- 
Jarod Wilson
jarod-H+wXaHxf7aLQT0dZR+AlfA@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: SQ overflow seen running isert traffic
From: Steve Wise @ 2016-10-18 14:34 UTC (permalink / raw)
  To: 'Sagi Grimberg', 'Potnuri Bharat Teja'
  Cc: target-devel-u79uwXL29TY76Z2rM5mHXA, nab-IzHhD5pYlfBP7FQvKIMDCQ,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <ed7ebb39-be81-00b3-ef23-3f4c0e3afbb1-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org>

> 
> > I tried out this change and it works fine with iwarp. I dont see SQ
> > overflow. Apparently we have increased the sq too big to overflow. I am
going
> > to let it run with higher workloads for longer time, to see if it holds
good.
> 
> Actually on second thought, this patch is an overkill. Effectively we
> now set:
> 
> MAX_CMD=266
> and max_rdma_ctx=128 so together we take 394 which seems to too much.
> 
> If we go by the scheme of 1 rdma + 1 send for each IO we need:
> - 128 sends
> - 128 rdmas
> - 10 miscs
> 
> so this gives 266.
> 
> Perhaps this is due to the fact that iWARP needs to register memory for
> rdma reads as well? (and also rdma writes > 128k for chelsio HW right?)
>

iWARP definitely needs to register memory for the target of reads, due to
REMOTE_WRITE requirement for the protocol.  The source of a write doesn't need
to register memory, but the SGE depth can cause multiple WRITE WRs to be
required to service the IO.  And in theory there should be some threshold where
it might be better performance-wise to do a memory register + 1 WRITE vs X
WRITEs.    

As you mentioned, the RW API should account for this, but perhaps it is still
off some.  Bharat, have a look into the RDMA-RW API and let us see if we can
figure out if the additional SQ depth it adds is sufficient.
 
> What is the workload you are running? with immediatedata enabled you
> should issue reg+rdma_read+send only for writes > 8k.
> 
> Does this happen when you run only reads for example?
> 
> I guess its time to get the sq accounting into shape...

So to sum up - 2 issues:

1) we believe the iSER + RW API correctly sizes the SQ, yet we're seeing SQ
overflows.  So the SQ sizing needs more investigation.

2) if the SQ is full, then the iSER/target code is supposed to resubmit.  And
apparently that isn't working.

Steve.

--
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 04/10] mm: replace get_user_pages_locked() write/force parameters with gup_flags
From: Lorenzo Stoakes @ 2016-10-18 13:56 UTC (permalink / raw)
  To: Jan Kara
  Cc: linux-mm, Linus Torvalds, Hugh Dickins, Dave Hansen, Rik van Riel,
	Mel Gorman, Andrew Morton, adi-buildroot-devel, ceph-devel,
	dri-devel, intel-gfx, kvm, linux-alpha, linux-arm-kernel,
	linux-cris-kernel, linux-fbdev, linux-fsdevel, linux-ia64,
	linux-kernel, linux-media, linux-mips, linux-rdma, linux-s390,
	linux-samsung-soc
In-Reply-To: <20161018125425.GD29967@quack2.suse.cz>

On Tue, Oct 18, 2016 at 02:54:25PM +0200, Jan Kara wrote:
> > @@ -1282,7 +1282,7 @@ long get_user_pages(unsigned long start, unsigned long nr_pages,
> >  			    int write, int force, struct page **pages,
> >  			    struct vm_area_struct **vmas);
> >  long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
> > -		    int write, int force, struct page **pages, int *locked);
> > +		    unsigned int gup_flags, struct page **pages, int *locked);
>
> Hum, the prototype is inconsistent with e.g. __get_user_pages_unlocked()
> where gup_flags come after **pages argument. Actually it makes more sense
> to have it before **pages so that input arguments come first and output
> arguments second but I don't care that much. But it definitely should be
> consistent...

It was difficult to decide quite how to arrange parameters as there was
inconsitency with regards to parameter ordering already - for example
__get_user_pages() places its flags argument before pages whereas, as you note,
__get_user_pages_unlocked() puts them afterwards.

I ended up compromising by trying to match the existing ordering of the function
as much as I could by replacing write, force pairs with gup_flags in the same
location (with the exception of get_user_pages_unlocked() which I felt should
match __get_user_pages_unlocked() in signature) or if there was already a
gup_flags parameter as in the case of __get_user_pages_unlocked() I simply
removed the write, force pair and left the flags as the last parameter.

I am happy to rearrange parameters as needed, however I am not sure if it'd be
worthwhile for me to do so (I am keen to try to avoid adding too much noise here
:)

If we were to rearrange parameters for consistency I'd suggest adjusting
__get_user_pages_unlocked() to put gup_flags before pages and do the same with
get_user_pages_unlocked(), let me know what you think.

^ permalink raw reply

* Re: SQ overflow seen running isert traffic
From: Sagi Grimberg @ 2016-10-18 13:17 UTC (permalink / raw)
  To: Potnuri Bharat Teja; +Cc: Steve Wise, target-devel, nab, linux-rdma
In-Reply-To: <20161018112801.GA3117@chelsio.com>


> I tried out this change and it works fine with iwarp. I dont see SQ
> overflow. Apparently we have increased the sq too big to overflow. I am going
> to let it run with higher workloads for longer time, to see if it holds good.

Actually on second thought, this patch is an overkill. Effectively we
now set:

MAX_CMD=266
and max_rdma_ctx=128 so together we take 394 which seems to too much.

If we go by the scheme of 1 rdma + 1 send for each IO we need:
- 128 sends
- 128 rdmas
- 10 miscs

so this gives 266.

Perhaps this is due to the fact that iWARP needs to register memory for
rdma reads as well? (and also rdma writes > 128k for chelsio HW right?)

What is the workload you are running? with immediatedata enabled you
should issue reg+rdma_read+send only for writes > 8k.

Does this happen when you run only reads for example?

I guess its time to get the sq accounting into shape...

^ permalink raw reply

* Re: [PATCH 04/10] mm: replace get_user_pages_locked() write/force parameters with gup_flags
From: Jan Kara @ 2016-10-18 12:54 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: linux-mips, linux-fbdev, Jan Kara, kvm, linux-sh, Dave Hansen,
	dri-devel, linux-mm, netdev, sparclinux, linux-ia64, linux-s390,
	linux-samsung-soc, linux-scsi, linux-rdma, x86, Hugh Dickins,
	linux-media, Rik van Riel, intel-gfx, adi-buildroot-devel,
	ceph-devel, linux-arm-kernel, linux-cris-kernel, Linus Torvalds,
	linuxppc-dev, linux-kernel, linux-security-module, linux-alpha
In-Reply-To: <20161013002020.3062-5-lstoakes@gmail.com>

On Thu 13-10-16 01:20:14, Lorenzo Stoakes wrote:
> This patch removes the write and force parameters from get_user_pages_locked()
> and replaces them with a gup_flags parameter to make the use of FOLL_FORCE
> explicit in callers as use of this flag can result in surprising behaviour (and
> hence bugs) within the mm subsystem.
> 
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
> ---
>  include/linux/mm.h |  2 +-
>  mm/frame_vector.c  |  8 +++++++-
>  mm/gup.c           | 12 +++---------
>  mm/nommu.c         |  5 ++++-
>  4 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 6adc4bc..27ab538 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1282,7 +1282,7 @@ long get_user_pages(unsigned long start, unsigned long nr_pages,
>  			    int write, int force, struct page **pages,
>  			    struct vm_area_struct **vmas);
>  long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
> -		    int write, int force, struct page **pages, int *locked);
> +		    unsigned int gup_flags, struct page **pages, int *locked);

Hum, the prototype is inconsistent with e.g. __get_user_pages_unlocked()
where gup_flags come after **pages argument. Actually it makes more sense
to have it before **pages so that input arguments come first and output
arguments second but I don't care that much. But it definitely should be
consistent...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [PATCH 03/10] mm: replace get_user_pages_unlocked() write/force parameters with gup_flags
From: Jan Kara @ 2016-10-18 12:50 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: linux-mips, linux-fbdev, Jan Kara, kvm, linux-sh, Dave Hansen,
	dri-devel, linux-mm, netdev, sparclinux, linux-ia64, linux-s390,
	linux-samsung-soc, linux-scsi, linux-rdma, x86, Hugh Dickins,
	linux-media, Rik van Riel, intel-gfx, adi-buildroot-devel,
	ceph-devel, linux-arm-kernel, linux-cris-kernel, Linus Torvalds,
	linuxppc-dev, linux-kernel, linux-security-module, linux-alpha
In-Reply-To: <20161013002020.3062-4-lstoakes@gmail.com>

On Thu 13-10-16 01:20:13, Lorenzo Stoakes wrote:
> This patch removes the write and force parameters from get_user_pages_unlocked()
> and replaces them with a gup_flags parameter to make the use of FOLL_FORCE
> explicit in callers as use of this flag can result in surprising behaviour (and
> hence bugs) within the mm subsystem.
> 
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>

Looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [PATCH 02/10] mm: remove write/force parameters from __get_user_pages_unlocked()
From: Jan Kara @ 2016-10-18 12:46 UTC (permalink / raw)
  To: Lorenzo Stoakes
  Cc: linux-mips, linux-fbdev, Jan Kara, kvm, linux-sh, Dave Hansen,
	dri-devel, linux-mm, netdev, sparclinux, linux-ia64, linux-s390,
	linux-samsung-soc, linux-scsi, linux-rdma, x86, Hugh Dickins,
	linux-media, Rik van Riel, intel-gfx, adi-buildroot-devel,
	ceph-devel, linux-arm-kernel, linux-cris-kernel, Linus Torvalds,
	linuxppc-dev, linux-kernel, linux-security-module, linux-alpha
In-Reply-To: <20161013002020.3062-3-lstoakes@gmail.com>

On Thu 13-10-16 01:20:12, Lorenzo Stoakes wrote:
> This patch removes the write and force parameters from
> __get_user_pages_unlocked() to make the use of FOLL_FORCE explicit in callers as
> use of this flag can result in surprising behaviour (and hence bugs) within the
> mm subsystem.
> 
> Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>

The patch looks good. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ 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