[PATCH rdma-next V2 0/6] Refactor RDMA IOCTL declarations

[PATCH rdma-next V2 0/6] Refactor RDMA IOCTL declarations

[Leon Romanovsky]

Hi Doug,

This patch set refactors RDMA IOCTL declarations, unifies their
location, moves all current IOCTL exporters to one place and renames
them to fix the differences in coding style.

IOCTLs commands need to export the size of their data for their transfer
which is usually declared as a struct defined in relevant submodule.
These structures should be visible at the linkage stage.

There are number of possible solutions to overcome this limitations.

One of the options is to declare anonymous variable which will be visible
during the linkage. As a downside, such option won't allow direct use of
this header by other parts of code without their specific includes of
other logically unrelated submodules.

Another possible option will be to move declarations of that structures
to that common header file. Such move will cause to bloat this file
with different submodules declarations.

Our decision was to choose direct include of other exported files with
declarations of such structures to simplify future usage of this file.
There is no change for legacy applications and libraries.

Our decision was to choose direct include of other exported files with
declarations of such structures to simplify future usage of this file.
There is no change for legacy applications and libraries.

This is the second version of patch series and the
changes are in commit messages of patch number 3 and 6.

Available in the "topic/ioctl-header" topic branch of this git repo:
git://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git

Or for browsing:
https://git.kernel.org/cgit/linux/kernel/git/leon/linux-rdma.git/log/?h=topic/ioctl-header

Thanks

Leon Romanovsky (6):
  RDMA/core: Commonize RDMA IOCTL declarations location
  RDMA/core: Move legacy MAD IOCTL declarations to common file
  RDMA/hfi1: Avoid redeclaration error
  RDMA/core: Move HFI1 IOCTL declarations to common file
  RDMA/core: Rename RDMA magic number
  RDMA/core: Unify style of IOCTL commands

 include/uapi/rdma/Kbuild            |   1 +
 include/uapi/rdma/hfi/Kbuild        |   1 +
 include/uapi/rdma/hfi/hfi1_ioctl.h  | 173 +++++++++++++++++++++++++++++++++++
 include/uapi/rdma/hfi/hfi1_user.h   | 175 +-----------------------------------
 include/uapi/rdma/ib_user_mad.h     |  14 +--
 include/uapi/rdma/rdma_user_ioctl.h | 127 ++++++++++++++++++++++++++
 6 files changed, 304 insertions(+), 187 deletions(-)
 create mode 100644 include/uapi/rdma/hfi/hfi1_ioctl.h
 create mode 100644 include/uapi/rdma/rdma_user_ioctl.h

--
2.7.4

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

[PATCH rdma-next V2 1/6] RDMA/core: Commonize RDMA IOCTL declarations location

[Leon Romanovsky]

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

This patch provides one common file (rdma_user_ioctl.h)
for all RDMA UAPI IOCTLs.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 include/uapi/rdma/Kbuild            |  1 +
 include/uapi/rdma/hfi/hfi1_user.h   |  2 +-
 include/uapi/rdma/ib_user_mad.h     |  4 +---
 include/uapi/rdma/rdma_user_ioctl.h | 43 +++++++++++++++++++++++++++++++++++++
 4 files changed, 46 insertions(+), 4 deletions(-)
 create mode 100644 include/uapi/rdma/rdma_user_ioctl.h

diff --git a/include/uapi/rdma/Kbuild b/include/uapi/rdma/Kbuild
index 4edb0f2..ac62908 100644
--- a/include/uapi/rdma/Kbuild
+++ b/include/uapi/rdma/Kbuild
@@ -1,5 +1,6 @@
 # UAPI Header export list
 header-y += ib_user_cm.h
+header-y += rdma_user_ioctl.h
 header-y += ib_user_mad.h
 header-y += ib_user_sa.h
 header-y += ib_user_verbs.h
diff --git a/include/uapi/rdma/hfi/hfi1_user.h b/include/uapi/rdma/hfi/hfi1_user.h
index d15e728..a404919 100644
--- a/include/uapi/rdma/hfi/hfi1_user.h
+++ b/include/uapi/rdma/hfi/hfi1_user.h
@@ -57,6 +57,7 @@
 #define _LINUX__HFI1_USER_H
 
 #include <linux/types.h>
+#include <rdma/rdma_user_ioctl.h>
 
 /*
  * This version number is given to the driver by the user code during
@@ -132,7 +133,6 @@
  * User IOCTLs can not go above 128 if they do then see common.h and change the
  * base for the snoop ioctl
  */
-#define IB_IOCTL_MAGIC 0x1b /* See Documentation/ioctl/ioctl-number.txt */
 
 /*
  * Make the ioctls occupy the last 0xf0-0xff portion of the IB range
diff --git a/include/uapi/rdma/ib_user_mad.h b/include/uapi/rdma/ib_user_mad.h
index 09f809f..9de7a2b 100644
--- a/include/uapi/rdma/ib_user_mad.h
+++ b/include/uapi/rdma/ib_user_mad.h
@@ -35,7 +35,7 @@
 #define IB_USER_MAD_H
 
 #include <linux/types.h>
-#include <linux/ioctl.h>
+#include <rdma/rdma_user_ioctl.h>
 
 /*
  * Increment this value if any changes that break userspace ABI
@@ -230,8 +230,6 @@ struct ib_user_mad_reg_req2 {
 	__u8	reserved[3];
 };
 
-#define IB_IOCTL_MAGIC		0x1b
-
 #define IB_USER_MAD_REGISTER_AGENT	_IOWR(IB_IOCTL_MAGIC, 1, \
 					      struct ib_user_mad_reg_req)
 
diff --git a/include/uapi/rdma/rdma_user_ioctl.h b/include/uapi/rdma/rdma_user_ioctl.h
new file mode 100644
index 0000000..ba1bcdd
--- /dev/null
+++ b/include/uapi/rdma/rdma_user_ioctl.h
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2016 Mellanox Technologies, LTD. 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 RDMA_USER_IOCTL_H
+#define RDMA_USER_IOCTL_H
+
+#include <linux/types.h>
+#include <linux/ioctl.h>
+
+/* Documentation/ioctl/ioctl-number.txt */
+#define RDMA_IOCTL_MAGIC		0x1b
+#define IB_IOCTL_MAGIC			RDMA_IOCTL_MAGIC
+
+#endif /* RDMA_USER_IOCTL_H */
-- 
2.7.4

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

[PATCH rdma-next V2 2/6] RDMA/core: Move legacy MAD IOCTL declarations to common file

[Leon Romanovsky]

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

Move legacy MAD IOCTL declarations to rdma_user_ioctl.h file.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 include/uapi/rdma/ib_user_mad.h     | 10 ----------
 include/uapi/rdma/rdma_user_ioctl.h | 11 +++++++++++
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/include/uapi/rdma/ib_user_mad.h b/include/uapi/rdma/ib_user_mad.h
index 9de7a2b..5c7abd8 100644
--- a/include/uapi/rdma/ib_user_mad.h
+++ b/include/uapi/rdma/ib_user_mad.h
@@ -230,14 +230,4 @@ struct ib_user_mad_reg_req2 {
 	__u8	reserved[3];
 };
 
-#define IB_USER_MAD_REGISTER_AGENT	_IOWR(IB_IOCTL_MAGIC, 1, \
-					      struct ib_user_mad_reg_req)
-
-#define IB_USER_MAD_UNREGISTER_AGENT	_IOW(IB_IOCTL_MAGIC, 2, __u32)
-
-#define IB_USER_MAD_ENABLE_PKEY		_IO(IB_IOCTL_MAGIC, 3)
-
-#define IB_USER_MAD_REGISTER_AGENT2     _IOWR(IB_IOCTL_MAGIC, 4, \
-					      struct ib_user_mad_reg_req2)
-
 #endif /* IB_USER_MAD_H */
diff --git a/include/uapi/rdma/rdma_user_ioctl.h b/include/uapi/rdma/rdma_user_ioctl.h
index ba1bcdd..820bf34 100644
--- a/include/uapi/rdma/rdma_user_ioctl.h
+++ b/include/uapi/rdma/rdma_user_ioctl.h
@@ -35,9 +35,20 @@
 
 #include <linux/types.h>
 #include <linux/ioctl.h>
+#include <rdma/ib_user_mad.h>
 
 /* Documentation/ioctl/ioctl-number.txt */
 #define RDMA_IOCTL_MAGIC		0x1b
 #define IB_IOCTL_MAGIC			RDMA_IOCTL_MAGIC
 
+#define IB_USER_MAD_REGISTER_AGENT	_IOWR(IB_IOCTL_MAGIC, 1, \
+					      struct ib_user_mad_reg_req)
+
+#define IB_USER_MAD_UNREGISTER_AGENT	_IOW(IB_IOCTL_MAGIC, 2, __u32)
+
+#define IB_USER_MAD_ENABLE_PKEY		_IO(IB_IOCTL_MAGIC, 3)
+
+#define IB_USER_MAD_REGISTER_AGENT2     _IOWR(IB_IOCTL_MAGIC, 4, \
+					      struct ib_user_mad_reg_req2)
+
 #endif /* RDMA_USER_IOCTL_H */
-- 
2.7.4

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

[PATCH rdma-next V2 3/6] RDMA/hfi1: Avoid redeclaration error

[Leon Romanovsky]

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

Move hfi1 ioctl definitions to a new header which can be included by
both the hfi1 and qib drivers to avoid a duplicate enum definition
as shown in this build error for qib:

  CC [M] drivers/infiniband/hw/qib/qib_sysfs.o
In file included from ./include/uapi/rdma/rdma_user_ioctl.h:39:0,
		 from include/uapi/rdma/ib_user_mad.h:38,
		 from include/rdma/ib_mad.h:43,
		 from include/rdma/ib_pma.h:38,
		 from drivers/infiniband/hw/qib/qib_mad.h:37,
		 from drivers/infiniband/hw/qib/qib_init.c:49:
./include/uapi/rdma/hfi/hfi1_user.h:370:2: error: redeclaration of
enumerator ‘ur_rcvhdrtail’
  ur_rcvhdrtail = 0,

Move hfi1 structures to separate file to avoid this failure.

The actual move of the ioctl definitions comes in a follow on patch.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 include/uapi/rdma/hfi/Kbuild       |   1 +
 include/uapi/rdma/hfi/hfi1_ioctl.h | 173 +++++++++++++++++++++++++++++++++++++
 include/uapi/rdma/hfi/hfi1_user.h  | 120 +------------------------
 3 files changed, 175 insertions(+), 119 deletions(-)
 create mode 100644 include/uapi/rdma/hfi/hfi1_ioctl.h

diff --git a/include/uapi/rdma/hfi/Kbuild b/include/uapi/rdma/hfi/Kbuild
index ef23c29..b65b0b3 100644
--- a/include/uapi/rdma/hfi/Kbuild
+++ b/include/uapi/rdma/hfi/Kbuild
@@ -1,2 +1,3 @@
 # UAPI Header export list
 header-y += hfi1_user.h
+header-y += hfi1_ioctl.h
diff --git a/include/uapi/rdma/hfi/hfi1_ioctl.h b/include/uapi/rdma/hfi/hfi1_ioctl.h
new file mode 100644
index 0000000..4791cc8
--- /dev/null
+++ b/include/uapi/rdma/hfi/hfi1_ioctl.h
@@ -0,0 +1,173 @@
+/*
+ *
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright(c) 2015 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * BSD LICENSE
+ *
+ * Copyright(c) 2015 Intel Corporation.
+ *
+ * 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.
+ *  - Neither the name of Intel Corporation nor the names of its
+ *    contributors may be used to endorse or promote products derived
+ *    from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#ifndef _LINUX__HFI1_IOCTL_H
+#define _LINUX__HFI1_IOCTL_H
+#include <linux/types.h>
+
+/*
+ * This structure is passed to the driver to tell it where
+ * user code buffers are, sizes, etc.   The offsets and sizes of the
+ * fields must remain unchanged, for binary compatibility.  It can
+ * be extended, if userversion is changed so user code can tell, if needed
+ */
+struct hfi1_user_info {
+	/*
+	 * version of user software, to detect compatibility issues.
+	 * Should be set to HFI1_USER_SWVERSION.
+	 */
+	__u32 userversion;
+	__u32 pad;
+	/*
+	 * If two or more processes wish to share a context, each process
+	 * must set the subcontext_cnt and subcontext_id to the same
+	 * values.  The only restriction on the subcontext_id is that
+	 * it be unique for a given node.
+	 */
+	__u16 subctxt_cnt;
+	__u16 subctxt_id;
+	/* 128bit UUID passed in by PSM. */
+	__u8 uuid[16];
+};
+
+struct hfi1_ctxt_info {
+	__u64 runtime_flags;    /* chip/drv runtime flags (HFI1_CAP_*) */
+	__u32 rcvegr_size;      /* size of each eager buffer */
+	__u16 num_active;       /* number of active units */
+	__u16 unit;             /* unit (chip) assigned to caller */
+	__u16 ctxt;             /* ctxt on unit assigned to caller */
+	__u16 subctxt;          /* subctxt on unit assigned to caller */
+	__u16 rcvtids;          /* number of Rcv TIDs for this context */
+	__u16 credits;          /* number of PIO credits for this context */
+	__u16 numa_node;        /* NUMA node of the assigned device */
+	__u16 rec_cpu;          /* cpu # for affinity (0xffff if none) */
+	__u16 send_ctxt;        /* send context in use by this user context */
+	__u16 egrtids;          /* number of RcvArray entries for Eager Rcvs */
+	__u16 rcvhdrq_cnt;      /* number of RcvHdrQ entries */
+	__u16 rcvhdrq_entsize;  /* size (in bytes) for each RcvHdrQ entry */
+	__u16 sdma_ring_size;   /* number of entries in SDMA request ring */
+};
+
+struct hfi1_tid_info {
+	/* virtual address of first page in transfer */
+	__u64 vaddr;
+	/* pointer to tid array. this array is big enough */
+	__u64 tidlist;
+	/* number of tids programmed by this request */
+	__u32 tidcnt;
+	/* length of transfer buffer programmed by this request */
+	__u32 length;
+};
+
+/*
+ * This structure is returned by the driver immediately after
+ * open to get implementation-specific info, and info specific to this
+ * instance.
+ *
+ * This struct must have explicit pad fields where type sizes
+ * may result in different alignments between 32 and 64 bit
+ * programs, since the 64 bit * bit kernel requires the user code
+ * to have matching offsets
+ */
+struct hfi1_base_info {
+	/* version of hardware, for feature checking. */
+	__u32 hw_version;
+	/* version of software, for feature checking. */
+	__u32 sw_version;
+	/* Job key */
+	__u16 jkey;
+	__u16 padding1;
+	/*
+	 * The special QP (queue pair) value that identifies PSM
+	 * protocol packet from standard IB packets.
+	 */
+	__u32 bthqp;
+	/* PIO credit return address, */
+	__u64 sc_credits_addr;
+	/*
+	 * Base address of write-only pio buffers for this process.
+	 * Each buffer has sendpio_credits*64 bytes.
+	 */
+	__u64 pio_bufbase_sop;
+	/*
+	 * Base address of write-only pio buffers for this process.
+	 * Each buffer has sendpio_credits*64 bytes.
+	 */
+	__u64 pio_bufbase;
+	/* address where receive buffer queue is mapped into */
+	__u64 rcvhdr_bufbase;
+	/* base address of Eager receive buffers. */
+	__u64 rcvegr_bufbase;
+	/* base address of SDMA completion ring */
+	__u64 sdma_comp_bufbase;
+	/*
+	 * User register base for init code, not to be used directly by
+	 * protocol or applications.  Always maps real chip register space.
+	 * the register addresses are:
+	 * ur_rcvhdrhead, ur_rcvhdrtail, ur_rcvegrhead, ur_rcvegrtail,
+	 * ur_rcvtidflow
+	 */
+	__u64 user_regbase;
+	/* notification events */
+	__u64 events_bufbase;
+	/* status page */
+	__u64 status_bufbase;
+	/* rcvhdrtail update */
+	__u64 rcvhdrtail_base;
+	/*
+	 * shared memory pages for subctxts if ctxt is shared; these cover
+	 * all the processes in the group sharing a single context.
+	 * all have enough space for the num_subcontexts value on this job.
+	 */
+	__u64 subctxt_uregbase;
+	__u64 subctxt_rcvegrbuf;
+	__u64 subctxt_rcvhdrbuf;
+};
+#endif /* _LINIUX__HFI1_IOCTL_H */
diff --git a/include/uapi/rdma/hfi/hfi1_user.h b/include/uapi/rdma/hfi/hfi1_user.h
index a404919..8aa3867 100644
--- a/include/uapi/rdma/hfi/hfi1_user.h
+++ b/include/uapi/rdma/hfi/hfi1_user.h
@@ -58,6 +58,7 @@
 
 #include <linux/types.h>
 #include <rdma/rdma_user_ioctl.h>
+#include <rdma/hfi/hfi1_ioctl.h>
 
 /*
  * This version number is given to the driver by the user code during
@@ -211,60 +212,6 @@ struct hfi1_cmd;
 #define HFI1_POLL_TYPE_ANYRCV     0x0
 #define HFI1_POLL_TYPE_URGENT     0x1
 
-/*
- * This structure is passed to the driver to tell it where
- * user code buffers are, sizes, etc.   The offsets and sizes of the
- * fields must remain unchanged, for binary compatibility.  It can
- * be extended, if userversion is changed so user code can tell, if needed
- */
-struct hfi1_user_info {
-	/*
-	 * version of user software, to detect compatibility issues.
-	 * Should be set to HFI1_USER_SWVERSION.
-	 */
-	__u32 userversion;
-	__u32 pad;
-	/*
-	 * If two or more processes wish to share a context, each process
-	 * must set the subcontext_cnt and subcontext_id to the same
-	 * values.  The only restriction on the subcontext_id is that
-	 * it be unique for a given node.
-	 */
-	__u16 subctxt_cnt;
-	__u16 subctxt_id;
-	/* 128bit UUID passed in by PSM. */
-	__u8 uuid[16];
-};
-
-struct hfi1_ctxt_info {
-	__u64 runtime_flags;    /* chip/drv runtime flags (HFI1_CAP_*) */
-	__u32 rcvegr_size;      /* size of each eager buffer */
-	__u16 num_active;       /* number of active units */
-	__u16 unit;             /* unit (chip) assigned to caller */
-	__u16 ctxt;             /* ctxt on unit assigned to caller */
-	__u16 subctxt;          /* subctxt on unit assigned to caller */
-	__u16 rcvtids;          /* number of Rcv TIDs for this context */
-	__u16 credits;          /* number of PIO credits for this context */
-	__u16 numa_node;        /* NUMA node of the assigned device */
-	__u16 rec_cpu;          /* cpu # for affinity (0xffff if none) */
-	__u16 send_ctxt;        /* send context in use by this user context */
-	__u16 egrtids;          /* number of RcvArray entries for Eager Rcvs */
-	__u16 rcvhdrq_cnt;      /* number of RcvHdrQ entries */
-	__u16 rcvhdrq_entsize;  /* size (in bytes) for each RcvHdrQ entry */
-	__u16 sdma_ring_size;   /* number of entries in SDMA request ring */
-};
-
-struct hfi1_tid_info {
-	/* virtual address of first page in transfer */
-	__u64 vaddr;
-	/* pointer to tid array. this array is big enough */
-	__u64 tidlist;
-	/* number of tids programmed by this request */
-	__u32 tidcnt;
-	/* length of transfer buffer programmed by this request */
-	__u32 length;
-};
-
 enum hfi1_sdma_comp_state {
 	FREE = 0,
 	QUEUED,
@@ -289,71 +236,6 @@ struct hfi1_status {
 	char freezemsg[0];
 };
 
-/*
- * This structure is returned by the driver immediately after
- * open to get implementation-specific info, and info specific to this
- * instance.
- *
- * This struct must have explicit pad fields where type sizes
- * may result in different alignments between 32 and 64 bit
- * programs, since the 64 bit * bit kernel requires the user code
- * to have matching offsets
- */
-struct hfi1_base_info {
-	/* version of hardware, for feature checking. */
-	__u32 hw_version;
-	/* version of software, for feature checking. */
-	__u32 sw_version;
-	/* Job key */
-	__u16 jkey;
-	__u16 padding1;
-	/*
-	 * The special QP (queue pair) value that identifies PSM
-	 * protocol packet from standard IB packets.
-	 */
-	__u32 bthqp;
-	/* PIO credit return address, */
-	__u64 sc_credits_addr;
-	/*
-	 * Base address of write-only pio buffers for this process.
-	 * Each buffer has sendpio_credits*64 bytes.
-	 */
-	__u64 pio_bufbase_sop;
-	/*
-	 * Base address of write-only pio buffers for this process.
-	 * Each buffer has sendpio_credits*64 bytes.
-	 */
-	__u64 pio_bufbase;
-	/* address where receive buffer queue is mapped into */
-	__u64 rcvhdr_bufbase;
-	/* base address of Eager receive buffers. */
-	__u64 rcvegr_bufbase;
-	/* base address of SDMA completion ring */
-	__u64 sdma_comp_bufbase;
-	/*
-	 * User register base for init code, not to be used directly by
-	 * protocol or applications.  Always maps real chip register space.
-	 * the register addresses are:
-	 * ur_rcvhdrhead, ur_rcvhdrtail, ur_rcvegrhead, ur_rcvegrtail,
-	 * ur_rcvtidflow
-	 */
-	__u64 user_regbase;
-	/* notification events */
-	__u64 events_bufbase;
-	/* status page */
-	__u64 status_bufbase;
-	/* rcvhdrtail update */
-	__u64 rcvhdrtail_base;
-	/*
-	 * shared memory pages for subctxts if ctxt is shared; these cover
-	 * all the processes in the group sharing a single context.
-	 * all have enough space for the num_subcontexts value on this job.
-	 */
-	__u64 subctxt_uregbase;
-	__u64 subctxt_rcvegrbuf;
-	__u64 subctxt_rcvhdrbuf;
-};
-
 enum sdma_req_opcode {
 	EXPECTED = 0,
 	EAGER
-- 
2.7.4

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

[PATCH rdma-next V2 4/6] RDMA/core: Move HFI1 IOCTL declarations to common file

[Leon Romanovsky]

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

Move HFI1 IOCTL declarations to rdma_user_ioctl.h file.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 include/uapi/rdma/hfi/hfi1_user.h   | 55 -------------------------------------
 include/uapi/rdma/rdma_user_ioctl.h | 54 ++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 55 deletions(-)

diff --git a/include/uapi/rdma/hfi/hfi1_user.h b/include/uapi/rdma/hfi/hfi1_user.h
index 8aa3867..8807f06 100644
--- a/include/uapi/rdma/hfi/hfi1_user.h
+++ b/include/uapi/rdma/hfi/hfi1_user.h
@@ -58,7 +58,6 @@
 
 #include <linux/types.h>
 #include <rdma/rdma_user_ioctl.h>
-#include <rdma/hfi/hfi1_ioctl.h>
 
 /*
  * This version number is given to the driver by the user code during
@@ -114,60 +113,6 @@
 #define HFI1_RCVHDR_ENTSIZE_16   (1UL << 1)
 #define HFI1_RCVDHR_ENTSIZE_32   (1UL << 2)
 
-/* User commands. */
-#define HFI1_CMD_ASSIGN_CTXT     1	/* allocate HFI and context */
-#define HFI1_CMD_CTXT_INFO       2	/* find out what resources we got */
-#define HFI1_CMD_USER_INFO       3	/* set up userspace */
-#define HFI1_CMD_TID_UPDATE      4	/* update expected TID entries */
-#define HFI1_CMD_TID_FREE        5	/* free expected TID entries */
-#define HFI1_CMD_CREDIT_UPD      6	/* force an update of PIO credit */
-
-#define HFI1_CMD_RECV_CTRL       8	/* control receipt of packets */
-#define HFI1_CMD_POLL_TYPE       9	/* set the kind of polling we want */
-#define HFI1_CMD_ACK_EVENT       10	/* ack & clear user status bits */
-#define HFI1_CMD_SET_PKEY        11     /* set context's pkey */
-#define HFI1_CMD_CTXT_RESET      12     /* reset context's HW send context */
-#define HFI1_CMD_TID_INVAL_READ  13     /* read TID cache invalidations */
-#define HFI1_CMD_GET_VERS	 14	/* get the version of the user cdev */
-
-/*
- * User IOCTLs can not go above 128 if they do then see common.h and change the
- * base for the snoop ioctl
- */
-
-/*
- * Make the ioctls occupy the last 0xf0-0xff portion of the IB range
- */
-#define __NUM(cmd) (HFI1_CMD_##cmd + 0xe0)
-
-struct hfi1_cmd;
-#define HFI1_IOCTL_ASSIGN_CTXT \
-	_IOWR(IB_IOCTL_MAGIC, __NUM(ASSIGN_CTXT), struct hfi1_user_info)
-#define HFI1_IOCTL_CTXT_INFO \
-	_IOW(IB_IOCTL_MAGIC, __NUM(CTXT_INFO), struct hfi1_ctxt_info)
-#define HFI1_IOCTL_USER_INFO \
-	_IOW(IB_IOCTL_MAGIC, __NUM(USER_INFO), struct hfi1_base_info)
-#define HFI1_IOCTL_TID_UPDATE \
-	_IOWR(IB_IOCTL_MAGIC, __NUM(TID_UPDATE), struct hfi1_tid_info)
-#define HFI1_IOCTL_TID_FREE \
-	_IOWR(IB_IOCTL_MAGIC, __NUM(TID_FREE), struct hfi1_tid_info)
-#define HFI1_IOCTL_CREDIT_UPD \
-	_IO(IB_IOCTL_MAGIC, __NUM(CREDIT_UPD))
-#define HFI1_IOCTL_RECV_CTRL \
-	_IOW(IB_IOCTL_MAGIC, __NUM(RECV_CTRL), int)
-#define HFI1_IOCTL_POLL_TYPE \
-	_IOW(IB_IOCTL_MAGIC, __NUM(POLL_TYPE), int)
-#define HFI1_IOCTL_ACK_EVENT \
-	_IOW(IB_IOCTL_MAGIC, __NUM(ACK_EVENT), unsigned long)
-#define HFI1_IOCTL_SET_PKEY \
-	_IOW(IB_IOCTL_MAGIC, __NUM(SET_PKEY), __u16)
-#define HFI1_IOCTL_CTXT_RESET \
-	_IO(IB_IOCTL_MAGIC, __NUM(CTXT_RESET))
-#define HFI1_IOCTL_TID_INVAL_READ \
-	_IOWR(IB_IOCTL_MAGIC, __NUM(TID_INVAL_READ), struct hfi1_tid_info)
-#define HFI1_IOCTL_GET_VERS \
-	_IOR(IB_IOCTL_MAGIC, __NUM(GET_VERS), int)
-
 #define _HFI1_EVENT_FROZEN_BIT         0
 #define _HFI1_EVENT_LINKDOWN_BIT       1
 #define _HFI1_EVENT_LID_CHANGE_BIT     2
diff --git a/include/uapi/rdma/rdma_user_ioctl.h b/include/uapi/rdma/rdma_user_ioctl.h
index 820bf34..e9a69f0 100644
--- a/include/uapi/rdma/rdma_user_ioctl.h
+++ b/include/uapi/rdma/rdma_user_ioctl.h
@@ -36,6 +36,7 @@
 #include <linux/types.h>
 #include <linux/ioctl.h>
 #include <rdma/ib_user_mad.h>
+#include <rdma/hfi/hfi1_ioctl.h>
 
 /* Documentation/ioctl/ioctl-number.txt */
 #define RDMA_IOCTL_MAGIC		0x1b
@@ -51,4 +52,57 @@
 #define IB_USER_MAD_REGISTER_AGENT2     _IOWR(IB_IOCTL_MAGIC, 4, \
 					      struct ib_user_mad_reg_req2)
 
+/* User commands. */
+#define HFI1_CMD_ASSIGN_CTXT     1	/* allocate HFI and context */
+#define HFI1_CMD_CTXT_INFO       2	/* find out what resources we got */
+#define HFI1_CMD_USER_INFO       3	/* set up userspace */
+#define HFI1_CMD_TID_UPDATE      4	/* update expected TID entries */
+#define HFI1_CMD_TID_FREE        5	/* free expected TID entries */
+#define HFI1_CMD_CREDIT_UPD      6	/* force an update of PIO credit */
+
+#define HFI1_CMD_RECV_CTRL       8	/* control receipt of packets */
+#define HFI1_CMD_POLL_TYPE       9	/* set the kind of polling we want */
+#define HFI1_CMD_ACK_EVENT       10	/* ack & clear user status bits */
+#define HFI1_CMD_SET_PKEY        11     /* set context's pkey */
+#define HFI1_CMD_CTXT_RESET      12     /* reset context's HW send context */
+#define HFI1_CMD_TID_INVAL_READ  13     /* read TID cache invalidations */
+#define HFI1_CMD_GET_VERS	 14	/* get the version of the user cdev */
+
+/*
+ * User IOCTLs can not go above 128 if they do then see common.h and change the
+ * base for the snoop ioctl
+ */
+
+/*
+ * Make the ioctls occupy the last 0xf0-0xff portion of the IB range
+ */
+#define __NUM(cmd) (HFI1_CMD_##cmd + 0xe0)
+
+#define HFI1_IOCTL_ASSIGN_CTXT \
+	_IOWR(IB_IOCTL_MAGIC, __NUM(ASSIGN_CTXT), struct hfi1_user_info)
+#define HFI1_IOCTL_CTXT_INFO \
+	_IOW(IB_IOCTL_MAGIC, __NUM(CTXT_INFO), struct hfi1_ctxt_info)
+#define HFI1_IOCTL_USER_INFO \
+	_IOW(IB_IOCTL_MAGIC, __NUM(USER_INFO), struct hfi1_base_info)
+#define HFI1_IOCTL_TID_UPDATE \
+	_IOWR(IB_IOCTL_MAGIC, __NUM(TID_UPDATE), struct hfi1_tid_info)
+#define HFI1_IOCTL_TID_FREE \
+	_IOWR(IB_IOCTL_MAGIC, __NUM(TID_FREE), struct hfi1_tid_info)
+#define HFI1_IOCTL_CREDIT_UPD \
+	_IO(IB_IOCTL_MAGIC, __NUM(CREDIT_UPD))
+#define HFI1_IOCTL_RECV_CTRL \
+	_IOW(IB_IOCTL_MAGIC, __NUM(RECV_CTRL), int)
+#define HFI1_IOCTL_POLL_TYPE \
+	_IOW(IB_IOCTL_MAGIC, __NUM(POLL_TYPE), int)
+#define HFI1_IOCTL_ACK_EVENT \
+	_IOW(IB_IOCTL_MAGIC, __NUM(ACK_EVENT), unsigned long)
+#define HFI1_IOCTL_SET_PKEY \
+	_IOW(IB_IOCTL_MAGIC, __NUM(SET_PKEY), __u16)
+#define HFI1_IOCTL_CTXT_RESET \
+	_IO(IB_IOCTL_MAGIC, __NUM(CTXT_RESET))
+#define HFI1_IOCTL_TID_INVAL_READ \
+	_IOWR(IB_IOCTL_MAGIC, __NUM(TID_INVAL_READ), struct hfi1_tid_info)
+#define HFI1_IOCTL_GET_VERS \
+	_IOR(IB_IOCTL_MAGIC, __NUM(GET_VERS), int)
+
 #endif /* RDMA_USER_IOCTL_H */
-- 
2.7.4

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

[PATCH rdma-next V2 5/6] RDMA/core: Rename RDMA magic number

[Leon Romanovsky]

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

Rename RDMA magic number to better describe IOCTLs.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 include/uapi/rdma/rdma_user_ioctl.h | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/include/uapi/rdma/rdma_user_ioctl.h b/include/uapi/rdma/rdma_user_ioctl.h
index e9a69f0..7ecf8cd 100644
--- a/include/uapi/rdma/rdma_user_ioctl.h
+++ b/include/uapi/rdma/rdma_user_ioctl.h
@@ -40,16 +40,17 @@
 
 /* Documentation/ioctl/ioctl-number.txt */
 #define RDMA_IOCTL_MAGIC		0x1b
+/* Legacy name, for user space application which already use it */
 #define IB_IOCTL_MAGIC			RDMA_IOCTL_MAGIC
 
-#define IB_USER_MAD_REGISTER_AGENT	_IOWR(IB_IOCTL_MAGIC, 1, \
+#define IB_USER_MAD_REGISTER_AGENT	_IOWR(RDMA_IOCTL_MAGIC, 1, \
 					      struct ib_user_mad_reg_req)
 
-#define IB_USER_MAD_UNREGISTER_AGENT	_IOW(IB_IOCTL_MAGIC, 2, __u32)
+#define IB_USER_MAD_UNREGISTER_AGENT	_IOW(RDMA_IOCTL_MAGIC, 2, __u32)
 
-#define IB_USER_MAD_ENABLE_PKEY		_IO(IB_IOCTL_MAGIC, 3)
+#define IB_USER_MAD_ENABLE_PKEY		_IO(RDMA_IOCTL_MAGIC, 3)
 
-#define IB_USER_MAD_REGISTER_AGENT2     _IOWR(IB_IOCTL_MAGIC, 4, \
+#define IB_USER_MAD_REGISTER_AGENT2     _IOWR(RDMA_IOCTL_MAGIC, 4, \
 					      struct ib_user_mad_reg_req2)
 
 /* User commands. */
@@ -79,30 +80,30 @@
 #define __NUM(cmd) (HFI1_CMD_##cmd + 0xe0)
 
 #define HFI1_IOCTL_ASSIGN_CTXT \
-	_IOWR(IB_IOCTL_MAGIC, __NUM(ASSIGN_CTXT), struct hfi1_user_info)
+	_IOWR(RDMA_IOCTL_MAGIC, __NUM(ASSIGN_CTXT), struct hfi1_user_info)
 #define HFI1_IOCTL_CTXT_INFO \
-	_IOW(IB_IOCTL_MAGIC, __NUM(CTXT_INFO), struct hfi1_ctxt_info)
+	_IOW(RDMA_IOCTL_MAGIC, __NUM(CTXT_INFO), struct hfi1_ctxt_info)
 #define HFI1_IOCTL_USER_INFO \
-	_IOW(IB_IOCTL_MAGIC, __NUM(USER_INFO), struct hfi1_base_info)
+	_IOW(RDMA_IOCTL_MAGIC, __NUM(USER_INFO), struct hfi1_base_info)
 #define HFI1_IOCTL_TID_UPDATE \
-	_IOWR(IB_IOCTL_MAGIC, __NUM(TID_UPDATE), struct hfi1_tid_info)
+	_IOWR(RDMA_IOCTL_MAGIC, __NUM(TID_UPDATE), struct hfi1_tid_info)
 #define HFI1_IOCTL_TID_FREE \
-	_IOWR(IB_IOCTL_MAGIC, __NUM(TID_FREE), struct hfi1_tid_info)
+	_IOWR(RDMA_IOCTL_MAGIC, __NUM(TID_FREE), struct hfi1_tid_info)
 #define HFI1_IOCTL_CREDIT_UPD \
-	_IO(IB_IOCTL_MAGIC, __NUM(CREDIT_UPD))
+	_IO(RDMA_IOCTL_MAGIC, __NUM(CREDIT_UPD))
 #define HFI1_IOCTL_RECV_CTRL \
-	_IOW(IB_IOCTL_MAGIC, __NUM(RECV_CTRL), int)
+	_IOW(RDMA_IOCTL_MAGIC, __NUM(RECV_CTRL), int)
 #define HFI1_IOCTL_POLL_TYPE \
-	_IOW(IB_IOCTL_MAGIC, __NUM(POLL_TYPE), int)
+	_IOW(RDMA_IOCTL_MAGIC, __NUM(POLL_TYPE), int)
 #define HFI1_IOCTL_ACK_EVENT \
-	_IOW(IB_IOCTL_MAGIC, __NUM(ACK_EVENT), unsigned long)
+	_IOW(RDMA_IOCTL_MAGIC, __NUM(ACK_EVENT), unsigned long)
 #define HFI1_IOCTL_SET_PKEY \
-	_IOW(IB_IOCTL_MAGIC, __NUM(SET_PKEY), __u16)
+	_IOW(RDMA_IOCTL_MAGIC, __NUM(SET_PKEY), __u16)
 #define HFI1_IOCTL_CTXT_RESET \
-	_IO(IB_IOCTL_MAGIC, __NUM(CTXT_RESET))
+	_IO(RDMA_IOCTL_MAGIC, __NUM(CTXT_RESET))
 #define HFI1_IOCTL_TID_INVAL_READ \
-	_IOWR(IB_IOCTL_MAGIC, __NUM(TID_INVAL_READ), struct hfi1_tid_info)
+	_IOWR(RDMA_IOCTL_MAGIC, __NUM(TID_INVAL_READ), struct hfi1_tid_info)
 #define HFI1_IOCTL_GET_VERS \
-	_IOR(IB_IOCTL_MAGIC, __NUM(GET_VERS), int)
+	_IOR(RDMA_IOCTL_MAGIC, __NUM(GET_VERS), int)
 
 #endif /* RDMA_USER_IOCTL_H */
-- 
2.7.4

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

[PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[Leon Romanovsky]

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

MAD and HFI1 have different naming convention, this patch
simplifies and unifies their defines and names.

As part of cleanup, the HFI1 _NUM() macro was removed.
It has a potential to break applications which use this macro
directly.

Signed-off-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 include/uapi/rdma/rdma_user_ioctl.h | 98 ++++++++++++++++++++++---------------
 1 file changed, 58 insertions(+), 40 deletions(-)

diff --git a/include/uapi/rdma/rdma_user_ioctl.h b/include/uapi/rdma/rdma_user_ioctl.h
index 7ecf8cd..9abd6d1 100644
--- a/include/uapi/rdma/rdma_user_ioctl.h
+++ b/include/uapi/rdma/rdma_user_ioctl.h
@@ -39,71 +39,89 @@
 #include <rdma/hfi/hfi1_ioctl.h>
 
 /* Documentation/ioctl/ioctl-number.txt */
-#define RDMA_IOCTL_MAGIC		0x1b
+#define RDMA_IOCTL_MAGIC	0x1b
 /* Legacy name, for user space application which already use it */
-#define IB_IOCTL_MAGIC			RDMA_IOCTL_MAGIC
+#define IB_IOCTL_MAGIC		RDMA_IOCTL_MAGIC
 
-#define IB_USER_MAD_REGISTER_AGENT	_IOWR(RDMA_IOCTL_MAGIC, 1, \
-					      struct ib_user_mad_reg_req)
+/* General blocks assignments */
+#define MAD_CMD_BASE		0x00
+#define HFI1_CMD_BASE		0xE0
 
-#define IB_USER_MAD_UNREGISTER_AGENT	_IOW(RDMA_IOCTL_MAGIC, 2, __u32)
+/* MAD specific section */
+#define MAD_CMD_REG_AGENT	(MAD_CMD_BASE + 0x01)
+#define MAD_CMD_UNREG_AGENT	(MAD_CMD_BASE + 0x02)
+#define MAD_CMD_ENABLE_PKEY	(MAD_CMD_BASE + 0x03)
+#define MAD_CMD_REG_AGENT2	(MAD_CMD_BASE + 0x04)
 
-#define IB_USER_MAD_ENABLE_PKEY		_IO(RDMA_IOCTL_MAGIC, 3)
-
-#define IB_USER_MAD_REGISTER_AGENT2     _IOWR(RDMA_IOCTL_MAGIC, 4, \
-					      struct ib_user_mad_reg_req2)
+#define IB_USER_MAD_REGISTER_AGENT \
+	_IOWR(RDMA_IOCTL_MAGIC, MAD_CMD_REG_AGENT, struct ib_user_mad_reg_req)
+#define IB_USER_MAD_UNREGISTER_AGENT \
+	_IOW(RDMA_IOCTL_MAGIC, MAD_CMD_UNREG_AGENT, __u32)
+#define IB_USER_MAD_ENABLE_PKEY \
+	_IO(RDMA_IOCTL_MAGIC, MAD_CMD_ENABLE_PKEY)
+#define IB_USER_MAD_REGISTER_AGENT2 \
+	_IOWR(RDMA_IOCTL_MAGIC, MAD_CMD_REG_AGENT2, struct ib_user_mad_reg_req2)
 
+/* HFI specific section */
 /* User commands. */
-#define HFI1_CMD_ASSIGN_CTXT     1	/* allocate HFI and context */
-#define HFI1_CMD_CTXT_INFO       2	/* find out what resources we got */
-#define HFI1_CMD_USER_INFO       3	/* set up userspace */
-#define HFI1_CMD_TID_UPDATE      4	/* update expected TID entries */
-#define HFI1_CMD_TID_FREE        5	/* free expected TID entries */
-#define HFI1_CMD_CREDIT_UPD      6	/* force an update of PIO credit */
+/* allocate HFI and context */
+#define HFI1_CMD_ASSIGN_CTXT     (HFI1_CMD_BASE + 0x01)
+/* find out what resources we got */
+#define HFI1_CMD_CTXT_INFO       (HFI1_CMD_BASE + 0x02)
+/* set up userspace */
+#define HFI1_CMD_USER_INFO       (HFI1_CMD_BASE + 0x03)
+/* update expected TID entries */
+#define HFI1_CMD_TID_UPDATE      (HFI1_CMD_BASE + 0x04)
+/* free expected TID entries */
+#define HFI1_CMD_TID_FREE        (HFI1_CMD_BASE + 0x05)
+/* force an update of PIO credit */
+#define HFI1_CMD_CREDIT_UPD      (HFI1_CMD_BASE + 0x06)
 
-#define HFI1_CMD_RECV_CTRL       8	/* control receipt of packets */
-#define HFI1_CMD_POLL_TYPE       9	/* set the kind of polling we want */
-#define HFI1_CMD_ACK_EVENT       10	/* ack & clear user status bits */
-#define HFI1_CMD_SET_PKEY        11     /* set context's pkey */
-#define HFI1_CMD_CTXT_RESET      12     /* reset context's HW send context */
-#define HFI1_CMD_TID_INVAL_READ  13     /* read TID cache invalidations */
-#define HFI1_CMD_GET_VERS	 14	/* get the version of the user cdev */
+/* control receipt of packets */
+#define HFI1_CMD_RECV_CTRL       (HFI1_CMD_BASE + 0x08)
+/* set the kind of polling we want */
+#define HFI1_CMD_POLL_TYPE       (HFI1_CMD_BASE + 0x09)
+/* ack & clear user status bits */
+#define HFI1_CMD_ACK_EVENT       (HFI1_CMD_BASE + 0x0A)
+/* set context's pkey */
+#define HFI1_CMD_SET_PKEY        (HFI1_CMD_BASE + 0x0B)
+/* reset context's HW send context */
+#define HFI1_CMD_CTXT_RESET      (HFI1_CMD_BASE + 0x0C)
+/* read TID cache invalidations */
+#define HFI1_CMD_TID_INVAL_READ  (HFI1_CMD_BASE + 0x0D)
+/* get the version of the user cdev */
+#define HFI1_CMD_GET_VERS	 (HFI1_CMD_BASE + 0x0E)
 
 /*
  * User IOCTLs can not go above 128 if they do then see common.h and change the
  * base for the snoop ioctl
  */
 
-/*
- * Make the ioctls occupy the last 0xf0-0xff portion of the IB range
- */
-#define __NUM(cmd) (HFI1_CMD_##cmd + 0xe0)
-
 #define HFI1_IOCTL_ASSIGN_CTXT \
-	_IOWR(RDMA_IOCTL_MAGIC, __NUM(ASSIGN_CTXT), struct hfi1_user_info)
+	_IOWR(RDMA_IOCTL_MAGIC, HFI1_CMD_ASSIGN_CTXT, struct hfi1_user_info)
 #define HFI1_IOCTL_CTXT_INFO \
-	_IOW(RDMA_IOCTL_MAGIC, __NUM(CTXT_INFO), struct hfi1_ctxt_info)
+	_IOW(RDMA_IOCTL_MAGIC, HFI1_CMD_CTXT_INFO, struct hfi1_ctxt_info)
 #define HFI1_IOCTL_USER_INFO \
-	_IOW(RDMA_IOCTL_MAGIC, __NUM(USER_INFO), struct hfi1_base_info)
+	_IOW(RDMA_IOCTL_MAGIC, HFI1_CMD_USER_INFO, struct hfi1_base_info)
 #define HFI1_IOCTL_TID_UPDATE \
-	_IOWR(RDMA_IOCTL_MAGIC, __NUM(TID_UPDATE), struct hfi1_tid_info)
+	_IOWR(RDMA_IOCTL_MAGIC, HFI1_CMD_TID_UPDATE, struct hfi1_tid_info)
 #define HFI1_IOCTL_TID_FREE \
-	_IOWR(RDMA_IOCTL_MAGIC, __NUM(TID_FREE), struct hfi1_tid_info)
+	_IOWR(RDMA_IOCTL_MAGIC, HFI1_CMD_TID_FREE, struct hfi1_tid_info)
 #define HFI1_IOCTL_CREDIT_UPD \
-	_IO(RDMA_IOCTL_MAGIC, __NUM(CREDIT_UPD))
+	_IO(RDMA_IOCTL_MAGIC, HFI1_CMD_CREDIT_UPD)
 #define HFI1_IOCTL_RECV_CTRL \
-	_IOW(RDMA_IOCTL_MAGIC, __NUM(RECV_CTRL), int)
+	_IOW(RDMA_IOCTL_MAGIC, HFI1_CMD_RECV_CTRL, int)
 #define HFI1_IOCTL_POLL_TYPE \
-	_IOW(RDMA_IOCTL_MAGIC, __NUM(POLL_TYPE), int)
+	_IOW(RDMA_IOCTL_MAGIC, HFI1_CMD_POLL_TYPE, int)
 #define HFI1_IOCTL_ACK_EVENT \
-	_IOW(RDMA_IOCTL_MAGIC, __NUM(ACK_EVENT), unsigned long)
+	_IOW(RDMA_IOCTL_MAGIC, HFI1_CMD_ACK_EVENT, unsigned long)
 #define HFI1_IOCTL_SET_PKEY \
-	_IOW(RDMA_IOCTL_MAGIC, __NUM(SET_PKEY), __u16)
+	_IOW(RDMA_IOCTL_MAGIC, HFI1_CMD_SET_PKEY, __u16)
 #define HFI1_IOCTL_CTXT_RESET \
-	_IO(RDMA_IOCTL_MAGIC, __NUM(CTXT_RESET))
+	_IO(RDMA_IOCTL_MAGIC, HFI1_CMD_CTXT_RESET)
 #define HFI1_IOCTL_TID_INVAL_READ \
-	_IOWR(RDMA_IOCTL_MAGIC, __NUM(TID_INVAL_READ), struct hfi1_tid_info)
+	_IOWR(RDMA_IOCTL_MAGIC, HFI1_CMD_TID_INVAL_READ, struct hfi1_tid_info)
 #define HFI1_IOCTL_GET_VERS \
-	_IOR(RDMA_IOCTL_MAGIC, __NUM(GET_VERS), int)
+	_IOR(RDMA_IOCTL_MAGIC, HFI1_CMD_GET_VERS, int)
 
 #endif /* RDMA_USER_IOCTL_H */
-- 
2.7.4

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

Re: [PATCH rdma-next V2 0/6] Refactor RDMA IOCTL declarations

[Jason Gunthorpe]

On Wed, Aug 24, 2016 at 12:31:41AM +0300, Leon Romanovsky wrote:
> This patch set refactors RDMA IOCTL declarations, unifies their
> location, moves all current IOCTL exporters to one place and renames
> them to fix the differences in coding style.

The end result looks reasonable enough to me.

For the series

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

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

Re: [PATCH rdma-next V2 0/6] Refactor RDMA IOCTL declarations

[Leon Romanovsky]

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

On Tue, Aug 23, 2016 at 03:47:59PM -0600, Jason Gunthorpe wrote:
> On Wed, Aug 24, 2016 at 12:31:41AM +0300, Leon Romanovsky wrote:
> > This patch set refactors RDMA IOCTL declarations, unifies their
> > location, moves all current IOCTL exporters to one place and renames
> > them to fix the differences in coding style.
>
> The end result looks reasonable enough to me.
>
> For the series
>
> Reviewed-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

Thank you

>
> Jason

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

Re: [PATCH rdma-next V2 0/6] Refactor RDMA IOCTL declarations

[Leon Romanovsky]

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

On Tue, Aug 23, 2016 at 03:47:59PM -0600, Jason Gunthorpe wrote:
> On Wed, Aug 24, 2016 at 12:31:41AM +0300, Leon Romanovsky wrote:
> > This patch set refactors RDMA IOCTL declarations, unifies their
> > location, moves all current IOCTL exporters to one place and renames
> > them to fix the differences in coding style.
>
> The end result looks reasonable enough to me.
>
> For the series
>
> Reviewed-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

Hi Ira and Dennis,
Can I get your tested-by/reviewed-by too?

I want to be sure that it is done deal.

Thanks.

>
> Jason

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

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[Dalessandro, Dennis]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1798 bytes --]

On Wed, 2016-08-24 at 00:31 +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
> 
> -#define HFI1_CMD_ASSIGN_CTXT     1	/* allocate HFI and
> context */
> -#define HFI1_CMD_CTXT_INFO       2	/* find out what resources
> we got */
> -#define HFI1_CMD_USER_INFO       3	/* set up userspace */
> -#define HFI1_CMD_TID_UPDATE      4	/* update expected TID
> entries */
> -#define HFI1_CMD_TID_FREE        5	/* free expected TID
> entries */
> -#define HFI1_CMD_CREDIT_UPD      6	/* force an update of PIO
> credit */
> +/* allocate HFI and context */
> +#define HFI1_CMD_ASSIGN_CTXT     (HFI1_CMD_BASE + 0x01)
> +/* find out what resources we got */
> +#define HFI1_CMD_CTXT_INFO       (HFI1_CMD_BASE + 0x02)
> +/* set up userspace */
> +#define HFI1_CMD_USER_INFO       (HFI1_CMD_BASE + 0x03)
> +/* update expected TID entries */
> +#define HFI1_CMD_TID_UPDATE      (HFI1_CMD_BASE + 0x04)
> +/* free expected TID entries */
> +#define HFI1_CMD_TID_FREE        (HFI1_CMD_BASE + 0x05)
> +/* force an update of PIO credit */
> +#define HFI1_CMD_CREDIT_UPD      (HFI1_CMD_BASE + 0x06)

This is a minor issue, but the problem here is when we build PSM
against this kernel it will no longer work for older kernels because
the value of HFI1_CMD_ASSIGN_CTXT has changed where as it used to be 1.
Right now PSM is backwards compatible, this breaks that compatibility.

So while no one uses the __NUM() macro directly it lets us not change
the PSM command values. Can we put that part back and keep the command
values unchanged?

-Denny
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[Leon Romanovsky]

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

On Thu, Sep 01, 2016 at 02:05:44PM +0000, Dalessandro, Dennis wrote:
> On Wed, 2016-08-24 at 00:31 +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > 
> > -#define HFI1_CMD_ASSIGN_CTXT     1	/* allocate HFI and
> > context */
> > -#define HFI1_CMD_CTXT_INFO       2	/* find out what resources
> > we got */
> > -#define HFI1_CMD_USER_INFO       3	/* set up userspace */
> > -#define HFI1_CMD_TID_UPDATE      4	/* update expected TID
> > entries */
> > -#define HFI1_CMD_TID_FREE        5	/* free expected TID
> > entries */
> > -#define HFI1_CMD_CREDIT_UPD      6	/* force an update of PIO
> > credit */
> > +/* allocate HFI and context */
> > +#define HFI1_CMD_ASSIGN_CTXT     (HFI1_CMD_BASE + 0x01)
> > +/* find out what resources we got */
> > +#define HFI1_CMD_CTXT_INFO       (HFI1_CMD_BASE + 0x02)
> > +/* set up userspace */
> > +#define HFI1_CMD_USER_INFO       (HFI1_CMD_BASE + 0x03)
> > +/* update expected TID entries */
> > +#define HFI1_CMD_TID_UPDATE      (HFI1_CMD_BASE + 0x04)
> > +/* free expected TID entries */
> > +#define HFI1_CMD_TID_FREE        (HFI1_CMD_BASE + 0x05)
> > +/* force an update of PIO credit */
> > +#define HFI1_CMD_CREDIT_UPD      (HFI1_CMD_BASE + 0x06)
>
> This is a minor issue, but the problem here is when we build PSM
> against this kernel it will no longer work for older kernels because
> the value of HFI1_CMD_ASSIGN_CTXT has changed where as it used to be 1.
> Right now PSM is backwards compatible, this breaks that compatibility.
>
> So while no one uses the __NUM() macro directly it lets us not change
> the PSM command values. Can we put that part back and keep the command
> values unchanged?

Sure, I'll post new version.
Just to be sure, you are not using __NUM() and I can redefine that
macro, am I right?

Thanks.

>
> -Denny

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

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[Jason Gunthorpe]

On Thu, Sep 01, 2016 at 02:05:44PM +0000, Dalessandro, Dennis wrote:

> > +/* allocate HFI and context */
> > +#define HFI1_CMD_ASSIGN_CTXT     (HFI1_CMD_BASE + 0x01)
> 
> This is a minor issue, but the problem here is when we build PSM
> against this kernel it will no longer work for older kernels because
> the value of HFI1_CMD_ASSIGN_CTXT has changed where as it used to be 1.
> Right now PSM is backwards compatible, this breaks that compatibility.

What? Someone renumberd an ioctl? When? Why? How?

Was this ioctl ever in a mainline non-staging kernel? If not, too bad,
deal with it in your user space..

If yes, can we revert the renumbering? Why was that even done??

> So while no one uses the __NUM() macro directly it lets us not change
> the PSM command values. Can we put that part back and keep the command
> values unchanged?

Please no, do not do crazy subtle things like this. If the ioctl has
two valid numbers then you need two entries in the ioctl file.

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

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[Dalessandro, Dennis]

On Thu, 2016-09-01 at 19:20 +0300, Leon Romanovsky wrote:
> On Thu, Sep 01, 2016 at 02:05:44PM +0000, Dalessandro, Dennis wrote:
> > On Wed, 2016-08-24 at 00:31 +0300, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@mellanox.com>
> > >  
> > > -#define HFI1_CMD_ASSIGN_CTXT     1	/* allocate HFI and
> > > context */
> > > -#define HFI1_CMD_CTXT_INFO       2	/* find out what
> > > resources
> > > we got */
> > > -#define HFI1_CMD_USER_INFO       3	/* set up userspace */
> > > -#define HFI1_CMD_TID_UPDATE      4	/* update expected TID
> > > entries */
> > > -#define HFI1_CMD_TID_FREE        5	/* free expected TID
> > > entries */
> > > -#define HFI1_CMD_CREDIT_UPD      6	/* force an update of
> > > PIO
> > > credit */
> > > +/* allocate HFI and context */
> > > +#define HFI1_CMD_ASSIGN_CTXT     (HFI1_CMD_BASE + 0x01)
> > > +/* find out what resources we got */
> > > +#define HFI1_CMD_CTXT_INFO       (HFI1_CMD_BASE + 0x02)
> > > +/* set up userspace */
> > > +#define HFI1_CMD_USER_INFO       (HFI1_CMD_BASE + 0x03)
> > > +/* update expected TID entries */
> > > +#define HFI1_CMD_TID_UPDATE      (HFI1_CMD_BASE + 0x04)
> > > +/* free expected TID entries */
> > > +#define HFI1_CMD_TID_FREE        (HFI1_CMD_BASE + 0x05)
> > > +/* force an update of PIO credit */
> > > +#define HFI1_CMD_CREDIT_UPD      (HFI1_CMD_BASE + 0x06)
> > 
> > This is a minor issue, but the problem here is when we build PSM
> > against this kernel it will no longer work for older kernels
> > because
> > the value of HFI1_CMD_ASSIGN_CTXT has changed where as it used to
> > be 1.
> > Right now PSM is backwards compatible, this breaks that
> > compatibility.
> > 
> > So while no one uses the __NUM() macro directly it lets us not
> > change
> > the PSM command values. Can we put that part back and keep the
> > command
> > values unchanged?
> 
> Sure, I'll post new version.
> Just to be sure, you are not using __NUM() and I can redefine that
> macro, am I right?
> 

Yeah you can call it something else, or do it in a different way. I
verified that the patch works with the current PSM reverting the CMD
numbers. I'll grab your new patch and re-test it then.

Thanks

-Denny

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[Leon Romanovsky]

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

On Thu, Sep 01, 2016 at 10:46:24AM -0600, Jason Gunthorpe wrote:
> On Thu, Sep 01, 2016 at 02:05:44PM +0000, Dalessandro, Dennis wrote:
>
> > > +/* allocate HFI and context */
> > > +#define HFI1_CMD_ASSIGN_CTXT     (HFI1_CMD_BASE + 0x01)
> >
> > This is a minor issue, but the problem here is when we build PSM
> > against this kernel it will no longer work for older kernels because
> > the value of HFI1_CMD_ASSIGN_CTXT has changed where as it used to be 1.
> > Right now PSM is backwards compatible, this breaks that compatibility.
>
> What? Someone renumberd an ioctl? When? Why? How?
>
> Was this ioctl ever in a mainline non-staging kernel? If not, too bad,
> deal with it in your user space..
>
> If yes, can we revert the renumbering? Why was that even done??
>
> > So while no one uses the __NUM() macro directly it lets us not change
> > the PSM command values. Can we put that part back and keep the command
> > values unchanged?
>
> Please no, do not do crazy subtle things like this. If the ioctl has
> two valid numbers then you need two entries in the ioctl file.

I didn't renumbered ioctls, but renumbered one of the internals number
which is not used in kernel, but for any reasons used in their
user-space.

➜  linux-rdma git:(master) grep -r HFI1_CMD_ASSIGN_CTXT drivers/infiniband/hw/hfi1/* include/*
include/uapi/rdma/hfi/hfi1_user.h:#define HFI1_CMD_ASSIGN_CTXT     1 /* allocate HFI and context */

So what should I do? respin or not?

>
> Jason

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

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[Dalessandro, Dennis]

On Thu, 2016-09-01 at 10:46 -0600, Jason Gunthorpe wrote:
> On Thu, Sep 01, 2016 at 02:05:44PM +0000, Dalessandro, Dennis wrote:
> 
> > > +/* allocate HFI and context */
> > > +#define HFI1_CMD_ASSIGN_CTXT     (HFI1_CMD_BASE + 0x01)
> > 
> > This is a minor issue, but the problem here is when we build PSM
> > against this kernel it will no longer work for older kernels
> > because
> > the value of HFI1_CMD_ASSIGN_CTXT has changed where as it used to
> > be 1.
> > Right now PSM is backwards compatible, this breaks that
> > compatibility.
> 
> What? Someone renumberd an ioctl? When? Why? How?

No ioctl number was still the same. It's the thing that is used to
define the ioctl number.

The ioctl was originally defined as:
_IOWR(RDMA_IOCTL_MAGIC, __NUM(ASSIGN_CTXT), struct hfi1_user_info)

Where __NUM(ASSIGN_CTXT) = HFI1_CMD_ASSIGN_CTXT + 0xE0 = 1 + 0xE0

It was changed to be:

_IOWR(RDMA_IOCTL_MAGIC, HFI1_CMD_ASSIGN_CTXT, struct hfi1_user_info)

Where HFI1_CMD_ASSIGN_CTXT = 0xE1.

So the ioctl number doens't change. The value of HFI1_CMD_ASSIGN_CTXT
does, it was 1 now it is 0xE1. HFI1_CMD_ASSIGN_CTXT is used by
userspace.

-Denny

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[Dalessandro, Dennis]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 811 bytes --]

On Thu, 2016-09-01 at 19:55 +0300, Leon Romanovsky wrote:
> 
> I didn't renumbered ioctls, but renumbered one of the internals
> number
> which is not used in kernel, but for any reasons used in their
> user-space.
> 
> ➜  linux-rdma git:(master) grep -r HFI1_CMD_ASSIGN_CTXT
> drivers/infiniband/hw/hfi1/* include/*
> include/uapi/rdma/hfi/hfi1_user.h:#define HFI1_CMD_ASSIGN_CTXT     1
> /* allocate HFI and context */
> 
> So what should I do? respin or not?
> 

I'd rather not have to go retool PSM for something that shouldn't
really matter to the kernel. So we'd appreciate keeping the
HFI1_CMD_ASSIGN_CTXT and friends values the same.

-DennyN‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[Jason Gunthorpe]

On Thu, Sep 01, 2016 at 07:55:52PM +0300, Leon Romanovsky wrote:
> On Thu, Sep 01, 2016 at 10:46:24AM -0600, Jason Gunthorpe wrote:
> > On Thu, Sep 01, 2016 at 02:05:44PM +0000, Dalessandro, Dennis wrote:
> >
> > > > +/* allocate HFI and context */
> > > > +#define HFI1_CMD_ASSIGN_CTXT??????????(HFI1_CMD_BASE + 0x01)
> > >
> > > This is a minor issue, but the problem here is when we build PSM
> > > against this kernel it will no longer work for older kernels because
> > > the value of HFI1_CMD_ASSIGN_CTXT has changed where as it used to be 1.
> > > Right now PSM is backwards compatible, this breaks that compatibility.
> >
> > What? Someone renumberd an ioctl? When? Why? How?
> >
> > Was this ioctl ever in a mainline non-staging kernel? If not, too bad,
> > deal with it in your user space..
> >
> > If yes, can we revert the renumbering? Why was that even done??
> >
> > > So while no one uses the __NUM() macro directly it lets us not change
> > > the PSM command values. Can we put that part back and keep the command
> > > values unchanged?
> >
> > Please no, do not do crazy subtle things like this. If the ioctl has
> > two valid numbers then you need two entries in the ioctl file.
> 
> I didn't renumbered ioctls, but renumbered one of the internals number
> which is not used in kernel, but for any reasons used in their
> user-space.

If it is not used in the kernel why is it in this header?

Ah, I see, this was part of the staging clean up, this is part of the
old non-ioctl UAPI that got trashed ??

> ???  linux-rdma git:(master) grep -r HFI1_CMD_ASSIGN_CTXT drivers/infiniband/hw/hfi1/* include/*
> include/uapi/rdma/hfi/hfi1_user.h:#define HFI1_CMD_ASSIGN_CTXT     1 /* allocate HFI and context */
> 
> So what should I do? respin or not?

Drop the cruft. These header file definitions should have been deleted
when the API was dropped during the staging review.

We do not promise anything for drivers during their time in staging,
and we do not support ABIs that only existed during staging.

Dennis should use an internal definition in PSM if he wishes to
continue to support the staging kernel ABI.

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

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[Dalessandro, Dennis]

On Thu, 2016-09-01 at 11:07 -0600, Jason Gunthorpe wrote:
> On Thu, Sep 01, 2016 at 07:55:52PM +0300, Leon Romanovsky wrote:
> > On Thu, Sep 01, 2016 at 10:46:24AM -0600, Jason Gunthorpe wrote:
> > > On Thu, Sep 01, 2016 at 02:05:44PM +0000, Dalessandro, Dennis
> > > wrote:
> > > 
> > > > > +/* allocate HFI and context */
> > > > > +#define HFI1_CMD_ASSIGN_CTXT??????????(HFI1_CMD_BASE + 0x01)
> > > > 
> > > > This is a minor issue, but the problem here is when we build
> > > > PSM
> > > > against this kernel it will no longer work for older kernels
> > > > because
> > > > the value of HFI1_CMD_ASSIGN_CTXT has changed where as it used
> > > > to be 1.
> > > > Right now PSM is backwards compatible, this breaks that
> > > > compatibility.
> > > 
> > > What? Someone renumberd an ioctl? When? Why? How?
> > > 
> > > Was this ioctl ever in a mainline non-staging kernel? If not, too
> > > bad,
> > > deal with it in your user space..
> > > 
> > > If yes, can we revert the renumbering? Why was that even done??
> > > 
> > > > So while no one uses the __NUM() macro directly it lets us not
> > > > change
> > > > the PSM command values. Can we put that part back and keep the
> > > > command
> > > > values unchanged?
> > > 
> > > Please no, do not do crazy subtle things like this. If the ioctl
> > > has
> > > two valid numbers then you need two entries in the ioctl file.
> > 
> > I didn't renumbered ioctls, but renumbered one of the internals
> > number
> > which is not used in kernel, but for any reasons used in their
> > user-space.
> 
> If it is not used in the kernel why is it in this header?
> 
> Ah, I see, this was part of the staging clean up, this is part of the
> old non-ioctl UAPI that got trashed ??
> 
> > ???  linux-rdma git:(master) grep -r HFI1_CMD_ASSIGN_CTXT
> > drivers/infiniband/hw/hfi1/* include/*
> > include/uapi/rdma/hfi/hfi1_user.h:#define
> > HFI1_CMD_ASSIGN_CTXT     1 /* allocate HFI and context */
> > 
> > So what should I do? respin or not?
> 
> Drop the cruft. These header file definitions should have been
> deleted
> when the API was dropped during the staging review.
> 
> We do not promise anything for drivers during their time in staging,
> and we do not support ABIs that only existed during staging.
> 
> Dennis should use an internal definition in PSM if he wishes to
> continue to support the staging kernel ABI.

It's not just the backward compatibility. PSM uses these command
definitions.So this breaks current support with current driver.

-Denny

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[Jason Gunthorpe]

On Thu, Sep 01, 2016 at 05:09:31PM +0000, Dalessandro, Dennis wrote:

> > Dennis should use an internal definition in PSM if he wishes to
> > continue to support the staging kernel ABI.
> 
> It's not just the backward compatibility. PSM uses these command
> definitions.So this breaks current support with current driver.

How exactly? The HFI1_CMD_ASSIGN_CTXT constant is never used in the kernel?

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

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[Dalessandro, Dennis]

On Thu, 2016-09-01 at 11:11 -0600, Jason Gunthorpe wrote:
> On Thu, Sep 01, 2016 at 05:09:31PM +0000, Dalessandro, Dennis wrote:
> 
> > > Dennis should use an internal definition in PSM if he wishes to
> > > continue to support the staging kernel ABI.
> > 
> > It's not just the backward compatibility. PSM uses these command
> > definitions.So this breaks current support with current driver.
> 
> How exactly? The HFI1_CMD_ASSIGN_CTXT constant is never used in the
> kernel?

This is used in PSM library. Agree it's not used in the kernel, so I
can see the argument to get rid of it, or not care. However
removing/changing it breaks PSM and requires changes. They are not hard
changes to make, but as it stands now PSM will work with 4.7 but not
with 4.8.

Reverting this part of the patch doesn't change or detract from what
the series does in the first place. Patch still accomplishes its job.

-Denny

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[Jason Gunthorpe]

On Thu, Sep 01, 2016 at 05:17:26PM +0000, Dalessandro, Dennis wrote:
> On Thu, 2016-09-01 at 11:11 -0600, Jason Gunthorpe wrote:
> > On Thu, Sep 01, 2016 at 05:09:31PM +0000, Dalessandro, Dennis wrote:
> > 
> > > > Dennis should use an internal definition in PSM if he wishes to
> > > > continue to support the staging kernel ABI.
> > > 
> > > It's not just the backward compatibility. PSM uses these command
> > > definitions.So this breaks current support with current driver.
> > 
> > How exactly? The HFI1_CMD_ASSIGN_CTXT constant is never used in the
> > kernel?
> 
> This is used in PSM library. Agree it's not used in the kernel, so I
> can see the argument to get rid of it, or not care.

Lets get rid of all the #defines. Leon you should just inline the
ioctl numbers into the ioctl definition like normal and get rid of
this extra layer of macros.

This way Dennis will get a build failure when PSM is build with these
headers instead of subtle runtime breakage. He can import the required
definitions to support the staging compat ABI into PSM, where they
belong, and make a new release to build with new kernel headers. Lots
of time to do that before these headers hit the distros.

Upstream is not the place to carry that stuff, and keeping strange
subtleness with __NUM is just going to risk future breakage.

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

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[Leon Romanovsky]

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

On Thu, Sep 01, 2016 at 11:33:20AM -0600, Jason Gunthorpe wrote:
> On Thu, Sep 01, 2016 at 05:17:26PM +0000, Dalessandro, Dennis wrote:
> > On Thu, 2016-09-01 at 11:11 -0600, Jason Gunthorpe wrote:
> > > On Thu, Sep 01, 2016 at 05:09:31PM +0000, Dalessandro, Dennis wrote:
> > >
> > > > > Dennis should use an internal definition in PSM if he wishes to
> > > > > continue to support the staging kernel ABI.
> > > >
> > > > It's not just the backward compatibility. PSM uses these command
> > > > definitions.So this breaks current support with current driver.
> > >
> > > How exactly? The HFI1_CMD_ASSIGN_CTXT constant is never used in the
> > > kernel?
> >
> > This is used in PSM library. Agree it's not used in the kernel, so I
> > can see the argument to get rid of it, or not care.
>
> Lets get rid of all the #defines. Leon you should just inline the
> ioctl numbers into the ioctl definition like normal and get rid of
> this extra layer of macros.

Ohh, it looks like you returned to us filled with positive energy :)

I tried to follow DRM subsystem [1] while converted these names with
minimal impact on the current implementation.


> This way Dennis will get a build failure when PSM is build with these
> headers instead of subtle runtime breakage. He can import the required
> definitions to support the staging compat ABI into PSM, where they
> belong, and make a new release to build with new kernel headers. Lots
> of time to do that before these headers hit the distros.
>
> Upstream is not the place to carry that stuff, and keeping strange
> subtleness with __NUM is just going to risk future breakage.

I would love to go in this direction, to get rid of unused defines
(especially in UAPIs).

Let's wait till Sunday, maybe we will see more discussions on the topic.

[1] http://lxr.free-electrons.com/source/include/uapi/drm/i915_drm.h

>
> Jason

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

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[ira.weiny]

On Thu, Sep 01, 2016 at 08:52:22PM +0300, Leon Romanovsky wrote:
> On Thu, Sep 01, 2016 at 11:33:20AM -0600, Jason Gunthorpe wrote:
> > On Thu, Sep 01, 2016 at 05:17:26PM +0000, Dalessandro, Dennis wrote:
> > > On Thu, 2016-09-01 at 11:11 -0600, Jason Gunthorpe wrote:
> > > > On Thu, Sep 01, 2016 at 05:09:31PM +0000, Dalessandro, Dennis wrote:
> > > >
> > > > > > Dennis should use an internal definition in PSM if he wishes to
> > > > > > continue to support the staging kernel ABI.
> > > > >
> > > > > It's not just the backward compatibility. PSM uses these command
> > > > > definitions.So this breaks current support with current driver.
> > > >
> > > > How exactly? The HFI1_CMD_ASSIGN_CTXT constant is never used in the
> > > > kernel?
> > >
> > > This is used in PSM library. Agree it's not used in the kernel, so I
> > > can see the argument to get rid of it, or not care.
> >
> > Lets get rid of all the #defines. Leon you should just inline the
> > ioctl numbers into the ioctl definition like normal and get rid of
> > this extra layer of macros.
> 
> Ohh, it looks like you returned to us filled with positive energy :)
> 
> I tried to follow DRM subsystem [1] while converted these names with
> minimal impact on the current implementation.

I like the idea of borrowing from the DRM subsystem but that is not quite what
I see here.

That subsystem defines a constant command base and a common Command base
(defined as an "end" for vendor specific):


/**
 * Device specific ioctls should only be in their respective headers
 * The device specific ioctl range is from 0x40 to 0x9f.
 * Generic IOCTLS restart at 0xA0.
 *
 * \sa drmCommandNone(), drmCommandRead(), drmCommandWrite(), and
 * drmCommandReadWrite().
 **/
#define DRM_COMMAND_BASE                0x40
#define DRM_COMMAND_END                 0xA0


And then all the vendor specific commands are defined like:

#define DRM_AMDGPU_GEM_CREATE           0x00
...

#define DRM_I810_INIT           0x00
...

#define DRM_I915_INIT           0x00
...


These defines are _not_ used anywhere in the kernel but rather define a
device specific command _offset_ within the drm vendor ioctl range.


When we agreed that HFI1 would use an 0x80 offset I thought that was the
idea.[*]  That IB would have an upper range which was device specific and HFI1
would be the first users of that range.

Maybe it would be better to use:

#define RDMA_VENDOR_BASE 0x80

Rather than HFI1_CMD_BASE?


What I think is wrong is that your patch changes the HFI1_CMD_ASSIGN_CTXT
define from being an offset from the base (0x01) to a hard coded RDMA define
0x81.

This deviates from the idea that these defines are offsets from the common
base.  I can see the argument Jason presents.  But I think we should leave the
current defines for the following reasons:

	1) that was not the agreed upon interface when hfi1 came out of staging.
	2) This prevents other devices from utilizing this vendor range.
	3) This allows us to use this range in the future for other devices
	4) There is little reason to break userspace unnecessarily.



All that said:

I know that we are not heading down the path of vendor devices having the same
IOCTLs but I don't think it is an idea we should abandon.

The patch set from Matan has the following in ib_uverbs_ioctl:


	if (cmd == RDMA_DIRECT_IOCTL) {
                /* TODO? */
                err = -ENOSYS;
                goto out;
        } else {


Given that we already have a vendor ioctl range concept I think we should just
keep this range for potential use instead of the above code.  And keep the
defines the way they are: as an HFI1 specific offset into the vendor ioctl
range.


Finally, I have to disagree with Jason that any define in a uapi file is
required to be used in the kernel.  Looking through the drm code that is
clearly not the case.


Ira


[*] I can't find the email reference at this time...  :-(

> 
> 
> > This way Dennis will get a build failure when PSM is build with these
> > headers instead of subtle runtime breakage. He can import the required
> > definitions to support the staging compat ABI into PSM, where they
> > belong, and make a new release to build with new kernel headers. Lots
> > of time to do that before these headers hit the distros.
> >
> > Upstream is not the place to carry that stuff, and keeping strange
> > subtleness with __NUM is just going to risk future breakage.
> 
> I would love to go in this direction, to get rid of unused defines
> (especially in UAPIs).
> 
> Let's wait till Sunday, maybe we will see more discussions on the topic.
> 
> [1] http://lxr.free-electrons.com/source/include/uapi/drm/i915_drm.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

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[Jason Gunthorpe]

On Tue, Sep 06, 2016 at 05:03:14PM -0400, ira.weiny wrote:

> These defines are _not_ used anywhere in the kernel but rather define a
> device specific command _offset_ within the drm vendor ioctl range.

That is not completely true, the defines are used when setting up the
kernel ioctl #. I have no idea why they did this, to me it is
polluting a uapi header with unneeded defines which is a big no-no.

Userspace should certainly *NEVER* use these constants, removing them
is the best way to achieve that.

> When we agreed that HFI1 would use an 0x80 offset I thought that was the
> idea.[*]  That IB would have an upper range which was device specific and HFI1
> would be the first users of that range.

I'd rather not have non-unique ioctls if we can avoid it......

But even if we do, removing these indirection constants does nothing to
change that - other drivers can still alias 0x80.

That is doable, particularly if the struct is a different size, then
we still have unique ioctl numbers. With some care other vendors can
probably manage...

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

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[Leon Romanovsky]

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

On Tue, Sep 06, 2016 at 05:03:14PM -0400, ira.weiny wrote:
> On Thu, Sep 01, 2016 at 08:52:22PM +0300, Leon Romanovsky wrote:
> > On Thu, Sep 01, 2016 at 11:33:20AM -0600, Jason Gunthorpe wrote:
> > > On Thu, Sep 01, 2016 at 05:17:26PM +0000, Dalessandro, Dennis wrote:
> > > > On Thu, 2016-09-01 at 11:11 -0600, Jason Gunthorpe wrote:
> > > > > On Thu, Sep 01, 2016 at 05:09:31PM +0000, Dalessandro, Dennis wrote:
> > > > >
> > > > > > > Dennis should use an internal definition in PSM if he wishes to
> > > > > > > continue to support the staging kernel ABI.
> > > > > >
> > > > > > It's not just the backward compatibility. PSM uses these command
> > > > > > definitions.So this breaks current support with current driver.
> > > > >
> > > > > How exactly? The HFI1_CMD_ASSIGN_CTXT constant is never used in the
> > > > > kernel?
> > > >
> > > > This is used in PSM library. Agree it's not used in the kernel, so I
> > > > can see the argument to get rid of it, or not care.
> > >
> > > Lets get rid of all the #defines. Leon you should just inline the
> > > ioctl numbers into the ioctl definition like normal and get rid of
> > > this extra layer of macros.
> >
> > Ohh, it looks like you returned to us filled with positive energy :)
> >
> > I tried to follow DRM subsystem [1] while converted these names with
> > minimal impact on the current implementation.
>
> I like the idea of borrowing from the DRM subsystem but that is not quite what
> I see here.

"followed" != ("borrowed" || "copy-pasted")

To make long story short.

1. You took 0xE0 as a base for HFI while claimed to take 0xF0. There is
nothing about 0x80 as a base. See commit 8d970cf991a6c38a5566572979487b906d643740

+/*
+ * Make the ioctls occupy the last 0xf0-0xff portion of the IB range
+ */
+#define __NUM(cmd) (HFI1_CMD_##cmd + 0xe0)

2. In new ABI, you will define your special objects, mark them as a
vendor specific and the driver will be called immediately. IMHO, there
is no need in special ioctl.

3. The Jason's point do not pollute code with defines/code which not in
use has very solid background. It is right time to stop misuse of UAPI
which you did. I'm pretty sure that you was supposed to update your PSM
library when you moved from staging and converted from write to ioctl.
I didn't hear complains about it.

Thanks.

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

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[ira.weiny]

On Tue, Sep 06, 2016 at 03:19:59PM -0600, Jason Gunthorpe wrote:
> On Tue, Sep 06, 2016 at 05:03:14PM -0400, ira.weiny wrote:
> 
> > These defines are _not_ used anywhere in the kernel but rather define a
> > device specific command _offset_ within the drm vendor ioctl range.
> 
> That is not completely true, the defines are used when setting up the
> kernel ioctl #. I have no idea why they did this, to me it is
> polluting a uapi header with unneeded defines which is a big no-no.

Well...  That is the same for the HFI1 defines as the __NUM macro expanded to
those defines...

#define __NUM(cmd) (HFI1_CMD_##cmd + 0xe0)

But really aren't we just nit picking now?

> 
> Userspace should certainly *NEVER* use these constants, removing them
> is the best way to achieve that.

I assume the DRM user space uses them to help distinguish between 

#define DRM_AMDGPU_GEM_CREATE           0x00

And

#define DRM_I810_INIT           0x00


I'm not at all familiar with the drm user space code but I could see some sort
of "provider" architecture where the providers pass these constants and some
"core" layer adds the vendor offset automatically???

> 
> > When we agreed that HFI1 would use an 0x80 offset I thought that was the
> > idea.[*]  That IB would have an upper range which was device specific and HFI1
> > would be the first users of that range.
> 
> I'd rather not have non-unique ioctls if we can avoid it......

I'm still mulling over the plan for this.

> 
> But even if we do, removing these indirection constants does nothing to
> change that - other drivers can still alias 0x80.

Yes but having walked through the drm headers it is nice in how explicit they
are.

> 
> That is doable, particularly if the struct is a different size, then
> we still have unique ioctl numbers. With some care other vendors can
> probably manage...

Yes, I am sure we can manage.  There are always different ways to do things.
In the end if Doug accepts this series we will manage in PSM.  However, I
really don't think what was done was "wrong".

Ira


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

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[ira.weiny]

On Wed, Sep 07, 2016 at 08:55:19AM +0300, Leon Romanovsky wrote:
> On Tue, Sep 06, 2016 at 05:03:14PM -0400, ira.weiny wrote:
> > On Thu, Sep 01, 2016 at 08:52:22PM +0300, Leon Romanovsky wrote:
> > > On Thu, Sep 01, 2016 at 11:33:20AM -0600, Jason Gunthorpe wrote:
> > > > On Thu, Sep 01, 2016 at 05:17:26PM +0000, Dalessandro, Dennis wrote:
> > > > > On Thu, 2016-09-01 at 11:11 -0600, Jason Gunthorpe wrote:
> > > > > > On Thu, Sep 01, 2016 at 05:09:31PM +0000, Dalessandro, Dennis wrote:
> > > > > >
> > > > > > > > Dennis should use an internal definition in PSM if he wishes to
> > > > > > > > continue to support the staging kernel ABI.
> > > > > > >
> > > > > > > It's not just the backward compatibility. PSM uses these command
> > > > > > > definitions.So this breaks current support with current driver.
> > > > > >
> > > > > > How exactly? The HFI1_CMD_ASSIGN_CTXT constant is never used in the
> > > > > > kernel?
> > > > >
> > > > > This is used in PSM library. Agree it's not used in the kernel, so I
> > > > > can see the argument to get rid of it, or not care.
> > > >
> > > > Lets get rid of all the #defines. Leon you should just inline the
> > > > ioctl numbers into the ioctl definition like normal and get rid of
> > > > this extra layer of macros.
> > >
> > > Ohh, it looks like you returned to us filled with positive energy :)
> > >
> > > I tried to follow DRM subsystem [1] while converted these names with
> > > minimal impact on the current implementation.
> >
> > I like the idea of borrowing from the DRM subsystem but that is not quite what
> > I see here.
> 
> "followed" != ("borrowed" || "copy-pasted")

Understood.

> 
> To make long story short.
> 
> 1. You took 0xE0 as a base for HFI while claimed to take 0xF0. There is
> nothing about 0x80 as a base. See commit 8d970cf991a6c38a5566572979487b906d643740
> 
> +/*
> + * Make the ioctls occupy the last 0xf0-0xff portion of the IB range
> + */
> +#define __NUM(cmd) (HFI1_CMD_##cmd + 0xe0)

Ok the comment was wrong.  Sorry.

> 
> 2. In new ABI, you will define your special objects, mark them as a
> vendor specific and the driver will be called immediately. IMHO, there
> is no need in special ioctl.

Well I'm still trying to get time to comment on that series.  One thing which
we are a bit worried about are the number of objects which may be tracked by
for a device and the performance to get to a special driver object (like a PSM
context).  Right now PSM opens a FD and associates a single context with that
FD so the access of that context is O(1).  The addition of special objects and
looking them up adds overhead which _MAY_ be ok but until we get a chance to
evaluate better I can't commit that what you say here is true.

> 
> 3. The Jason's point do not pollute code with defines/code which not in
> use has very solid background. It is right time to stop misuse of UAPI
> which you did. I'm pretty sure that you was supposed to update your PSM
> library when you moved from staging and converted from write to ioctl.
> I didn't hear complains about it.

We did update PSM to move from staging to the ioctls.  The fact that we are
using these defines is because they were exported (just like the drm exports
similar defines.)  Perhaps drm does not use their defines I don't know.

Perhaps these are "polluting" the header but generally I just can't agree that
it is "wrong" to have defines like this.  I think it is just a different style.

As I said to Jason, we can adapt and I will not lose any sleep over this.

Ira

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

Re: [PATCH rdma-next V2 6/6] RDMA/core: Unify style of IOCTL commands

[Leon Romanovsky]

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

On Wed, Sep 07, 2016 at 07:55:09PM -0400, ira.weiny wrote:
> >
> > 2. In new ABI, you will define your special objects, mark them as a
> > vendor specific and the driver will be called immediately. IMHO, there
> > is no need in special ioctl.
>
> Well I'm still trying to get time to comment on that series.  One thing which
> we are a bit worried about are the number of objects which may be tracked by
> for a device and the performance to get to a special driver object (like a PSM
> context).  Right now PSM opens a FD and associates a single context with that
> FD so the access of that context is O(1).  The addition of special objects and
> looking them up adds overhead which _MAY_ be ok but until we get a chance to
> evaluate better I can't commit that what you say here is true.

Good to know.

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