* [RFC ABI V4 1/7] RDMA/core: Export RDMA IOCTL declarations
From: Matan Barak @ 2016-09-28 15:10 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: Doug Ledford, Jason Gunthorpe, Sean Hefty, Liran Liss,
Haggai Eran, Matan Barak, Majd Dibbiny, Christoph Lameter,
Leon Romanovsky
In-Reply-To: <1475075417-30189-1-git-send-email-matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Place all external RDMA IOCTL declarations into one UAPI exported
header file and move all legacy MAD commands to that 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>
---
drivers/infiniband/core/user_mad.c | 2 +-
include/rdma/rdma_ioctl.h | 38 ++++++++++++++++++++++++
include/uapi/rdma/Kbuild | 1 +
include/uapi/rdma/ib_user_mad.h | 12 --------
include/uapi/rdma/rdma_user_ioctl.h | 59 +++++++++++++++++++++++++++++++++++++
5 files changed, 99 insertions(+), 13 deletions(-)
create mode 100644 include/rdma/rdma_ioctl.h
create mode 100644 include/uapi/rdma/rdma_user_ioctl.h
diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
index 415a318..70e974e 100644
--- a/drivers/infiniband/core/user_mad.c
+++ b/drivers/infiniband/core/user_mad.c
@@ -52,8 +52,8 @@
#include <asm/uaccess.h>
+#include <rdma/rdma_ioctl.h>
#include <rdma/ib_mad.h>
-#include <rdma/ib_user_mad.h>
MODULE_AUTHOR("Roland Dreier");
MODULE_DESCRIPTION("InfiniBand userspace MAD packet access");
diff --git a/include/rdma/rdma_ioctl.h b/include/rdma/rdma_ioctl.h
new file mode 100644
index 0000000..f62a359
--- /dev/null
+++ b/include/rdma/rdma_ioctl.h
@@ -0,0 +1,38 @@
+/*
+ * 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_IOCTL_H
+#define RDMA_IOCTL_H
+
+#include <rdma/rdma_user_ioctl.h>
+
+#endif /* RDMA_IOCTL_H */
diff --git a/include/uapi/rdma/Kbuild b/include/uapi/rdma/Kbuild
index 231901b..537e40e 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/ib_user_mad.h b/include/uapi/rdma/ib_user_mad.h
index 09f809f..7e722e7 100644
--- a/include/uapi/rdma/ib_user_mad.h
+++ b/include/uapi/rdma/ib_user_mad.h
@@ -230,16 +230,4 @@ 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)
-
-#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
new file mode 100644
index 0000000..5c1117a
--- /dev/null
+++ b/include/uapi/rdma/rdma_user_ioctl.h
@@ -0,0 +1,59 @@
+/*
+ * 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>
+
+#define RDMA_IOCTL_MAGIC 0x1b
+/*
+ * For hfi1 driver
+ */
+#define IB_IOCTL_MAGIC RDMA_IOCTL_MAGIC
+
+/* Legacy part
+ * !!!! NOTE: It uses the same command index as VERBS
+ */
+#include <rdma/ib_user_mad.h>
+#define IB_USER_MAD_REGISTER_AGENT _IOWR(RDMA_IOCTL_MAGIC, 1, \
+ struct ib_user_mad_reg_req)
+
+#define IB_USER_MAD_UNREGISTER_AGENT _IOW(RDMA_IOCTL_MAGIC, 2, __u32)
+
+#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)
+
+#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
^ permalink raw reply related
* [RFC ABI V4 0/7] SG-based RDMA ABI Proposal
From: Matan Barak @ 2016-09-28 15:10 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: Doug Ledford, Jason Gunthorpe, Sean Hefty, Liran Liss,
Haggai Eran, Matan Barak, Majd Dibbiny, Christoph Lameter,
Leon Romanovsky
The following patch set comes to enrich security model as a follow up
to commit e6bd18f57aad ('IB/security: Restrict use of the write() interface').
DISCLAIMER:
These patches are far from being completed. They present working init_ucontext
and query_device (both regular and extended version). In addition, they are
given as a basis of discussions.
NOT ALL COMMENTS GIVEN ON PREVIOUS VERSIONS ARE HANDLED IN THIS SERIES,
SOME OF THEM WILL BE HANDLED IN THE FUTURE.
The ideas presented here are based on our V1/V2/V3 series in addition to some
ideas presented in OFVWG and Sean's series.
This patch series add ioctl() interface to the existing write() interface and
provide an easy route to backport this change to legacy supported systems.
Analyzing the current uverbs role in dispatching and parsing commands, we find
that:
(a) uverbs validates the basic properties of the command
(b) uverbs is responsible of doing all the IDR and uobject management and
locking. It's also responsible of handling completion FDs.
(c) uverbs transforms the user<-->kernel ABI to kernel API.
(a) and (b) are valid for every kABI. Although the nature of commands could
change, they still have to be validated and transform to kernel pointers.
In order to avoid duplications between the various drivers, we would like to
keep (a) and (b) as shared code.
In addition, this is a good time to expand the ABI to be more scalable, so we
added a few goals:
(1) Command's attributes shall be extensible in an easy one. Either by allowing
drivers to have their own extensible set of attributes or core code
extensible attributes. Moreover, driver's specific attributes could some
day become core's standard attributes. We would like to still support
old user-space while avoid duplicating the code in kernel.
(2) Each driver may have specific type system (i.e QP, CQ, ....). It may
or may not even implement the standard type system. It could extend this
type system in the future. Try to avoid duplicating existing types or
actions.
(3) Do not change or recompile driver libraries and don't copy their data.
(4) Efficient dispatching.
Thus, in order to allow this flexibility, we decide giving (a) and (b) as a
common infrastructure, but use per-driver guidelines in order to do that
parsing and uobject management. Handlers are also set by the drivers
themselves (though they can point to either shared common code) or
driver specific code.
Since types are no longer enforced by the common infrastructure, there is no
point of pre-allocating common IDR types in the common code. Instead, we
provide an API for driver to add new types. We use one IDR per driver
for all its types. The driver declared all its supported types, their
free function and release order. After that, all uboject, exclusive access
and types are handled automatically for the driver by the infrastructure.
Scatter gather was chosen in order to allow us not to recompile user space
drivers. By using pointers to driver specific data, we could just use it
without introduce copying data and without changing the user-space driver at
all.
We chose to go with non blocking lock user objects. When exclusive
(WRITE or DESTROY) access is required, we dispatch the action if and only if
no other action needs this object as well. Otherwise, -EBUSY is returned to
the user-space. Device removal is synced with SRCU as of today.
If we were using locks, we would have need to sort the given user-space handles.
Otherwise, a user-space application may result in causing a deadlock.
Moving to a non blocking lock based behaviour, the dispatching in kernel
becomes more efficient.
Further uverbs related subsystem (such as RDMA-CM) may use other fds or use
other ioctl codes.
Note, we might switch to submitting one task (i.e - change locking schema) once
the concepts are more mature.
Regards,
Liran, Haggai, Leon and Matan
TODO:
1. Check other models for implementing FDs (as suggested in OFVWG).
2. Currently, this code only works with the new ioctl based libibverbs.
Make this compatible with the old version.
3. Rebase over latest kernel bits.
Changes from V3:
1. Add create_cq and create_comp_channel.
2. Add FD as ib_uobject into the type system
Changes from V2:
1. Use types declerations in order to declare release order and free function
2. Allow the driver to extend and use existing building blocks in any level:
a. Add more types
b. Add actions to exsiting types
c. Add attributes to existing actions (existed in V2)
Such a driver will only duplicate structs which it actually changed.
3. Fixed bugs in ucontext teardown and type allocation/locking.
4. Add reg_mr and init_pd
Changes from V1:
1. Refined locking system
a. try_read_lock and write lock to sync exclusive access
b. SRCU to sync device removal from commands execution
c. Future rwsem to sync close context from commands execution
2. Added temporary udata usage for vendor's data
3. Add query_device and init_ucontext command with mlx5 implementation
4. Fixed bugs in ioctl dispatching
5. Change callbacks to get ib_uverbs_file instead of ucontext
6. Add general types initialization and cleanups
Leon Romanovsky (2):
RDMA/core: Export RDMA IOCTL declarations
RDMA/core: Refactor IDR to be per-device
Matan Barak (5):
RDMA/core: Add support for custom types
RDMA/core: Add new ioctl interface
RDMA/core: Add initialize and cleanup of common types
RDMA/core: Add uverbs types, actions, handlers and attributes
IB/mlx5: Implement common uverb objects
drivers/infiniband/core/Makefile | 3 +-
drivers/infiniband/core/device.c | 18 +
drivers/infiniband/core/rdma_core.c | 505 +++++++++++++++++++
drivers/infiniband/core/rdma_core.h | 77 +++
drivers/infiniband/core/user_mad.c | 2 +-
drivers/infiniband/core/uverbs.h | 30 +-
drivers/infiniband/core/uverbs_cmd.c | 157 +++---
drivers/infiniband/core/uverbs_ioctl.c | 306 ++++++++++++
drivers/infiniband/core/uverbs_ioctl_cmd.c | 757 +++++++++++++++++++++++++++++
drivers/infiniband/core/uverbs_main.c | 165 ++-----
drivers/infiniband/hw/mlx5/main.c | 3 +
include/rdma/ib_verbs.h | 33 +-
include/rdma/rdma_ioctl.h | 38 ++
include/rdma/uverbs_ioctl.h | 342 +++++++++++++
include/rdma/uverbs_ioctl_cmd.h | 254 ++++++++++
include/uapi/rdma/Kbuild | 1 +
include/uapi/rdma/ib_user_mad.h | 12 -
include/uapi/rdma/ib_user_verbs.h | 13 +
include/uapi/rdma/rdma_user_ioctl.h | 82 ++++
19 files changed, 2571 insertions(+), 227 deletions(-)
create mode 100644 drivers/infiniband/core/rdma_core.c
create mode 100644 drivers/infiniband/core/rdma_core.h
create mode 100644 drivers/infiniband/core/uverbs_ioctl.c
create mode 100644 drivers/infiniband/core/uverbs_ioctl_cmd.c
create mode 100644 include/rdma/rdma_ioctl.h
create mode 100644 include/rdma/uverbs_ioctl.h
create mode 100644 include/rdma/uverbs_ioctl_cmd.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
^ permalink raw reply
* Re: [PATCH for-next 0/8] Bug Fixes and Code Improvement in HNS driver
From: Doug Ledford @ 2016-09-28 14:57 UTC (permalink / raw)
To: Salil Mehta
Cc: xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org,
oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org,
yisen.zhuang-hv44wF8Li93QT0dZR+AlfA@public.gmane.org,
mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linuxarm-hv44wF8Li93QT0dZR+AlfA@public.gmane.org
In-Reply-To: <20160910040930.25988-1-salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 3155 bytes --]
On 9/10/16 12:09 AM, Salil Mehta wrote:
> This patch-set introduces fix to some Bugs, potential problems
> and code improvements identified during internal review and
> testing of Hisilicon Network Subsystem driver.
>
> Hi Doug,
> These are few HNS patches which are not related to HNS RoCE driver but
> are being sent through your repository hns-roce (as requested) to mitigate
> any conflicts with HNS RoCE driver for the 4.9 merge window. In future
> i.e. after 4.9 we will go by more formal git-pull request method once
> HNS RoCE base driver becomes part of 4.9.
Hi Salil,
I've chatted with Dave Miller about this series. Here's where we stand.
First, the review feedback from Dave:
---
In patch #7, their comments are mis-formatted and these
hns guys do this a lot.
/*fix hardware broadcast/multicast packets queue loopback */
They seem to have a hard time putting an initial space in the comment,
and properly capitalizing and punctuating their sentences.
Also in that new function, they need to order local variables from
longest to shortest line (reverse christmas tree format).
Patch #6 has the local variable ordering issue as well as does patch
#3.
---
Please address those issues and resubmit.
As for the method of resubmission, please send the set to netdev@ and
also linux-rdma@, make note that, as a one off thing, these will need to
go through my tree to avoid conflicts with patches already in my tree,
and that future submissions will follow the Mellanox model of submitting
shared code to both mine and David's tree that is needed to avoid
conflicts for that merge window's submissions. After the netdev people
have reviewed it, and Dave acks it, either he can pick it up and sort
the conflicts out, or I can pick it up.
>
> Daode Huang (6):
> net: hns: bug fix about setting coalsecs-usecs to 0
> net: hns: add fini_process for v2 napi process
> net: hns: delete repeat read fbd num after while
> net: hns: fix the bug of forwarding table
> net: hns: bug fix about broadcast/multicast packets
> net: hns: delete redundant broadcast packet filter process
>
> Kejian Yan (1):
> net: hns: fix port not available after testing loopback
>
> lipeng (1):
> net: hns: fix port unavailable after hnae_reserve_buffer_map fail
>
> drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c | 11 ++-
> drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c | 13 ++-
> drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h | 2 +
> drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 10 --
> drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.h | 1 -
> drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c | 16 ++++
> drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h | 4 +
> drivers/net/ethernet/hisilicon/hns/hns_enet.c | 106 ++++++++++++++-------
> drivers/net/ethernet/hisilicon/hns/hns_ethtool.c | 7 ++
> 9 files changed, 117 insertions(+), 53 deletions(-)
>
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> GPG Key ID: 0E572FDD
Red Hat, Inc.
100 E. Davie St
Raleigh, NC 27601 USA
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 907 bytes --]
^ permalink raw reply
* RE: [PATCH 9/9] [RFC] nvme: Fix a race condition
From: Steve Wise @ 2016-09-28 14:23 UTC (permalink / raw)
To: 'Bart Van Assche', 'James Bottomley',
'Jens Axboe'
Cc: linux-block-u79uwXL29TY76Z2rM5mHXA, 'Martin K. Petersen',
'Mike Snitzer', linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
'Keith Busch', 'Doug Ledford',
linux-scsi-u79uwXL29TY76Z2rM5mHXA, 'Christoph Hellwig'
In-Reply-To: <3fb86a78-de3a-b764-a493-cb5bc5b6d8b6-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>
> Hello James and Steve,
>
> I will add a comment.
>
> Please note that the above patch does not change the behavior of
> nvme_stop_queues() except that it causes nvme_stop_queues() to wait
> until any ongoing nvme_queue_rq() calls have finished.
> blk_resume_queue() does not affect the value of the BLK_MQ_S_STOPPED bit
> that has been set by blk_mq_stop_hw_queues(). All it does is to resume
> pending blk_queue_enter() calls and to ensure that future
> blk_queue_enter() calls do not block. Even after blk_resume_queue() has
> been called if a new request is queued queue_rq() won't be invoked
> because the BLK_MQ_S_STOPPED bit is still set. Patch "dm: Fix a race
> condition related to stopping and starting queues" realizes a similar
> change in the dm driver and that change has been tested extensively.
>
Thanks for the detailed explanation! I think your code, then, is correct as-is. And this series doesn't fix the issue I'm hitting, so I'll keep digging. :)
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 0/2] cxgb4 FR_NSMR_TPTE_WR support
From: Steve Wise @ 2016-09-27 22:03 UTC (permalink / raw)
To: 'Doug Ledford'
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
'David Miller'
In-Reply-To: <20160919.102920.1126387243122900012.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
>
> >
> >>
> >> From: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
> >> Date: Fri, 16 Sep 2016 07:54:55 -0700
> >>
> >> > This series enables a new work request to optimize small REG_MR
> >> > operations. This is intended for 4.9. If everyone agrees, I suggest
> >> > Doug take both the cxgb4 and iw_cxgb4 patches through his tree.
> >>
> >> I'm assuming this mean that I do _not_ apply these to my tree.
> >
> > Yes, if you're ok with that.
>
> I am.
Doug, please include this for 4.9 if it looks good to you. Let me know.
Thanks,
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 v5 13/16] IB/pvrdma: Add the main driver module for PVRDMA
From: Adit Ranadive @ 2016-09-27 18:50 UTC (permalink / raw)
To: David Laight, Yuval Shaia
Cc: dledford@redhat.com, linux-rdma@vger.kernel.org,
pv-drivers@vmware.com, netdev@vger.kernel.org,
linux-pci@vger.kernel.org, jhansen@vmware.com,
asarwade@vmware.com, georgezhang@vmware.com, bryantan@vmware.com
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB010A6EA@AcuExch.aculab.com>
On Tue, Sep 27, 2016 at 09:21:27AM +0000, David Laight wrote:
> From: Adit Ranadive
> > Sent: 26 September 2016 19:15
> > On Mon, Sep 26, 2016 at 00:27:40AM -0700, Yuval Shaia wrote:
> > > On Sat, Sep 24, 2016 at 04:21:37PM -0700, Adit Ranadive wrote:
> > > > +
> > > > + /* Currently, the driver only supports RoCE mode. */
> > > > + if (dev->dsr->caps.mode != PVRDMA_DEVICE_MODE_ROCE) {
> > > > + dev_err(&pdev->dev, "unsupported transport %d\n",
> > > > + dev->dsr->caps.mode);
> > > > + ret = -EINVAL;
> > >
> > > This is some fatal error with the device, not that something wrong with the
> > > function's argument.
> > > Suggesting to replace with -EFAULT.
> > >
> >
> > Thanks, will fix this one and the others here.
>
> Won't EFAULT generate SIGSEGV ?
Since this is called at module load time, wouldn't the module load fail with
this error rather than generate a SIGSEGV?
I'm slightly unclear about what would if it is compiled into the kernel though
I think it should fail with the error.
The only other error value to return here that could make sense is EIO.
^ permalink raw reply
* Re: [PATCH rdma-core 4/5] libocrdma: Move ocrdma's list implementation into common directory
From: Jason Gunthorpe @ 2016-09-27 18:03 UTC (permalink / raw)
To: Yishai Hadas
Cc: Christoph Hellwig, Leon Romanovsky,
dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, yishaih-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <ff764b75-19af-340b-7228-328462c524ae-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
On Tue, Sep 27, 2016 at 07:14:09PM +0300, Yishai Hadas wrote:
> Leon is out of office for a week, I can take it from here and come to the
> list in coming days with a new candidate series based on previous notes.
Right, I forgot..
> Few notes to sync on:
> 1) Looking at ocrdma_list.h it extends the list default functionality to use
> an internal mutex, see list_lock & list_unlock calls. This is not a standard
> usage of a list. It may require some logic outside list.h to replace the
> usage without introducing the mutex in the shared new H file.
I noticed that. It doesn't look too bad, there are only two call sites
to list_lock, so I'd just move the mutex from the list head to the
ocrdma_dev_list.
> 2) Taking list.h from the 'CCAN' URL requires taking as well few other H
> files that it uses internally (e.g. build_assert.h). In few cases I don't
> see any reason to take the full file into rdma-core (e.g. ccan/str/str.h for
> stringify) will take only the needed functionality into list.h.
Hum. We have various other places using stringify and other
macros. Nothing in str.h looks bad at my first glance, so I'd just
take the whole thing.
It will be easier to work with ccan going forward if you minimize the
changes made to their stuff.
I suggest putting into a top level ccan/ (or util/ccan?) directory,
flatten the directory structure and we will compile the C component of
it into libccan.a and libccan_pic.a and link everything to it.
> 3) Need to clean up the 'CCAN' files from its include to "config.h", in
> addition, need to consider the functionality that need to be taken when
> there are some #if HAVE_XXX internally. (see #if HAVE_TYPEOF in list.h).
If you can get the code ported with a hardwired config.h (just add
stuff to buildlib/config.h.in) and whatever else, send it to me and
I'll get everything sorted out for cmake.
Generally speaking though, as code that targets only Linux, and only
Linux distros of a certain age, we can safely make a lot of
assumptions.
> 4) Re the licensing disclaimer in each H file, what do you suggest to put ?
> for example minmax.h uses licenses/CC0 see
> https://github.com/rustyrussell/ccan/blob/master/ccan/minmax/LICENSE which
> is different comparing list.h which is ../../licenses/BSD-MIT.
If we go with the ccan/ top level then add the the files as
ccan/COPYING.CCO and ccan/COPYING.MIT, etc and update the comments
accordingly.
So far all the options we've found require accepting single-licened
code. I view this as OK since those two licenses are widely regarded
to be GPLv2 compatible, and specifically OK'd by the FSF. I'd prefer
this situation to the current situation of having potentially
GPLv2-only code...
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 9/9] [RFC] nvme: Fix a race condition
From: Bart Van Assche @ 2016-09-27 17:09 UTC (permalink / raw)
To: James Bottomley, Steve Wise, 'Jens Axboe'
Cc: linux-block-u79uwXL29TY76Z2rM5mHXA, 'Martin K. Petersen',
'Mike Snitzer', linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
'Keith Busch', 'Doug Ledford',
linux-scsi-u79uwXL29TY76Z2rM5mHXA, 'Christoph Hellwig'
In-Reply-To: <1474995360.2716.19.camel-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
On 09/27/2016 09:56 AM, James Bottomley wrote:
> On Tue, 2016-09-27 at 09:43 -0700, Bart Van Assche wrote:
>> On 09/27/2016 09:31 AM, Steve Wise wrote:
>>>> @@ -2079,11 +2075,15 @@ EXPORT_SYMBOL_GPL(nvme_kill_queues);
>>>> void nvme_stop_queues(struct nvme_ctrl *ctrl)
>>>> {
>>>> struct nvme_ns *ns;
>>>> + struct request_queue *q;
>>>>
>>>> mutex_lock(&ctrl->namespaces_mutex);
>>>> list_for_each_entry(ns, &ctrl->namespaces, list) {
>>>> - blk_mq_cancel_requeue_work(ns->queue);
>>>> - blk_mq_stop_hw_queues(ns->queue);
>>>> + q = ns->queue;
>>>> + blk_quiesce_queue(q);
>>>> + blk_mq_cancel_requeue_work(q);
>>>> + blk_mq_stop_hw_queues(q);
>>>> + blk_resume_queue(q);
>>>> }
>>>> mutex_unlock(&ctrl->namespaces_mutex);
>>>
>>> Hey Bart, should nvme_stop_queues() really be resuming the blk
>>> queue?
>>
>> Hello Steve,
>>
>> Would you perhaps prefer that blk_resume_queue(q) is called from
>> nvme_start_queues()? I think that would make the NVMe code harder to
>> review. The above code won't cause any unexpected side effects if an
>> NVMe namespace is removed after nvme_stop_queues() has been called
>> and before nvme_start_queues() is called. Moving the
>> blk_resume_queue(q) call into nvme_start_queues() will only work as
>> expected if no namespaces are added nor removed between the
>> nvme_stop_queues() and nvme_start_queues() calls. I'm not familiar
>> enough with the NVMe code to know whether or not this change is safe
>> ...
>
> It's something that looks obviously wrong, so explain why you need to
> do it, preferably in a comment above the function.
Hello James and Steve,
I will add a comment.
Please note that the above patch does not change the behavior of
nvme_stop_queues() except that it causes nvme_stop_queues() to wait
until any ongoing nvme_queue_rq() calls have finished.
blk_resume_queue() does not affect the value of the BLK_MQ_S_STOPPED bit
that has been set by blk_mq_stop_hw_queues(). All it does is to resume
pending blk_queue_enter() calls and to ensure that future
blk_queue_enter() calls do not block. Even after blk_resume_queue() has
been called if a new request is queued queue_rq() won't be invoked
because the BLK_MQ_S_STOPPED bit is still set. Patch "dm: Fix a race
condition related to stopping and starting queues" realizes a similar
change in the dm driver and that change has been tested extensively.
Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH 9/9] [RFC] nvme: Fix a race condition
From: Steve Wise @ 2016-09-27 16:56 UTC (permalink / raw)
To: 'Bart Van Assche', 'Jens Axboe'
Cc: linux-block, 'James Bottomley',
'Martin K. Petersen', 'Mike Snitzer', linux-rdma,
linux-nvme, 'Keith Busch', 'Doug Ledford',
linux-scsi, 'Christoph Hellwig'
In-Reply-To: <9d8e0f32-6703-cf23-d424-bcecb65c2a26@sandisk.com>
> On 09/27/2016 09:31 AM, Steve Wise wrote:
> >> @@ -2079,11 +2075,15 @@ EXPORT_SYMBOL_GPL(nvme_kill_queues);
> >> void nvme_stop_queues(struct nvme_ctrl *ctrl)
> >> {
> >> struct nvme_ns *ns;
> >> + struct request_queue *q;
> >>
> >> mutex_lock(&ctrl->namespaces_mutex);
> >> list_for_each_entry(ns, &ctrl->namespaces, list) {
> >> - blk_mq_cancel_requeue_work(ns->queue);
> >> - blk_mq_stop_hw_queues(ns->queue);
> >> + q = ns->queue;
> >> + blk_quiesce_queue(q);
> >> + blk_mq_cancel_requeue_work(q);
> >> + blk_mq_stop_hw_queues(q);
> >> + blk_resume_queue(q);
> >> }
> >> mutex_unlock(&ctrl->namespaces_mutex);
> >
> > Hey Bart, should nvme_stop_queues() really be resuming the blk queue?
>
> Hello Steve,
>
> Would you perhaps prefer that blk_resume_queue(q) is called from
> nvme_start_queues()? I think that would make the NVMe code harder to
> review.
I'm still learning the blk code (and nvme code :)), but I would think
blk_resume_queue() would cause requests to start being submit on the NVME
queues, which I believe shouldn't happen when they are stopped. I'm currently
debugging a problem where requests are submitted to the nvme-rdma driver while
it has supposedly stopped all the nvme and blk mqs. I tried your series at
Christoph's request to see if it resolved my problem, but it didn't.
> The above code won't cause any unexpected side effects if an
> NVMe namespace is removed after nvme_stop_queues() has been called and
> before nvme_start_queues() is called. Moving the blk_resume_queue(q)
> call into nvme_start_queues() will only work as expected if no
> namespaces are added nor removed between the nvme_stop_queues() and
> nvme_start_queues() calls. I'm not familiar enough with the NVMe code to
> know whether or not this change is safe ...
>
I'll have to look and see if new namespaces can be added/deleted while a nvme
controller is in the RECONNECTING state. In the meantime, I'm going to move
the blk_resume_queue() to nvme_start_queues() and see if it helps my problem.
Christoph: Thoughts?
Steve.
^ permalink raw reply
* Re: [PATCH 9/9] [RFC] nvme: Fix a race condition
From: James Bottomley @ 2016-09-27 16:56 UTC (permalink / raw)
To: Bart Van Assche, Steve Wise, 'Jens Axboe'
Cc: linux-block, 'Martin K. Petersen', 'Mike Snitzer',
linux-rdma, linux-nvme, 'Keith Busch',
'Doug Ledford', linux-scsi, 'Christoph Hellwig'
In-Reply-To: <9d8e0f32-6703-cf23-d424-bcecb65c2a26@sandisk.com>
On Tue, 2016-09-27 at 09:43 -0700, Bart Van Assche wrote:
> On 09/27/2016 09:31 AM, Steve Wise wrote:
> > > @@ -2079,11 +2075,15 @@ EXPORT_SYMBOL_GPL(nvme_kill_queues);
> > > void nvme_stop_queues(struct nvme_ctrl *ctrl)
> > > {
> > > struct nvme_ns *ns;
> > > + struct request_queue *q;
> > >
> > > mutex_lock(&ctrl->namespaces_mutex);
> > > list_for_each_entry(ns, &ctrl->namespaces, list) {
> > > - blk_mq_cancel_requeue_work(ns->queue);
> > > - blk_mq_stop_hw_queues(ns->queue);
> > > + q = ns->queue;
> > > + blk_quiesce_queue(q);
> > > + blk_mq_cancel_requeue_work(q);
> > > + blk_mq_stop_hw_queues(q);
> > > + blk_resume_queue(q);
> > > }
> > > mutex_unlock(&ctrl->namespaces_mutex);
> >
> > Hey Bart, should nvme_stop_queues() really be resuming the blk
> > queue?
>
> Hello Steve,
>
> Would you perhaps prefer that blk_resume_queue(q) is called from
> nvme_start_queues()? I think that would make the NVMe code harder to
> review. The above code won't cause any unexpected side effects if an
> NVMe namespace is removed after nvme_stop_queues() has been called
> and before nvme_start_queues() is called. Moving the
> blk_resume_queue(q) call into nvme_start_queues() will only work as
> expected if no namespaces are added nor removed between the
> nvme_stop_queues() and nvme_start_queues() calls. I'm not familiar
> enough with the NVMe code to know whether or not this change is safe
> ...
It's something that looks obviously wrong, so explain why you need to
do it, preferably in a comment above the function.
James
^ permalink raw reply
* Re: [PATCH 9/9] [RFC] nvme: Fix a race condition
From: Bart Van Assche @ 2016-09-27 16:43 UTC (permalink / raw)
To: Steve Wise, 'Jens Axboe'
Cc: linux-block, 'James Bottomley',
'Martin K. Petersen', 'Mike Snitzer', linux-rdma,
linux-nvme, 'Keith Busch', 'Doug Ledford',
linux-scsi, 'Christoph Hellwig'
In-Reply-To: <013c01d218dc$8a5406c0$9efc1440$@opengridcomputing.com>
On 09/27/2016 09:31 AM, Steve Wise wrote:
>> @@ -2079,11 +2075,15 @@ EXPORT_SYMBOL_GPL(nvme_kill_queues);
>> void nvme_stop_queues(struct nvme_ctrl *ctrl)
>> {
>> struct nvme_ns *ns;
>> + struct request_queue *q;
>>
>> mutex_lock(&ctrl->namespaces_mutex);
>> list_for_each_entry(ns, &ctrl->namespaces, list) {
>> - blk_mq_cancel_requeue_work(ns->queue);
>> - blk_mq_stop_hw_queues(ns->queue);
>> + q = ns->queue;
>> + blk_quiesce_queue(q);
>> + blk_mq_cancel_requeue_work(q);
>> + blk_mq_stop_hw_queues(q);
>> + blk_resume_queue(q);
>> }
>> mutex_unlock(&ctrl->namespaces_mutex);
>
> Hey Bart, should nvme_stop_queues() really be resuming the blk queue?
Hello Steve,
Would you perhaps prefer that blk_resume_queue(q) is called from
nvme_start_queues()? I think that would make the NVMe code harder to
review. The above code won't cause any unexpected side effects if an
NVMe namespace is removed after nvme_stop_queues() has been called and
before nvme_start_queues() is called. Moving the blk_resume_queue(q)
call into nvme_start_queues() will only work as expected if no
namespaces are added nor removed between the nvme_stop_queues() and
nvme_start_queues() calls. I'm not familiar enough with the NVMe code to
know whether or not this change is safe ...
Bart.
^ permalink raw reply
* RE: [PATCH 9/9] [RFC] nvme: Fix a race condition
From: Steve Wise @ 2016-09-27 16:31 UTC (permalink / raw)
To: 'Bart Van Assche', 'Jens Axboe'
Cc: linux-block, 'James Bottomley',
'Martin K. Petersen', 'Mike Snitzer', linux-rdma,
linux-nvme, 'Keith Busch', 'Doug Ledford',
linux-scsi, 'Christoph Hellwig'
In-Reply-To: <9c372b04-a194-58c4-a64f-b155b52a5244@sandisk.com>
> @@ -2079,11 +2075,15 @@ EXPORT_SYMBOL_GPL(nvme_kill_queues);
> void nvme_stop_queues(struct nvme_ctrl *ctrl)
> {
> struct nvme_ns *ns;
> + struct request_queue *q;
>
> mutex_lock(&ctrl->namespaces_mutex);
> list_for_each_entry(ns, &ctrl->namespaces, list) {
> - blk_mq_cancel_requeue_work(ns->queue);
> - blk_mq_stop_hw_queues(ns->queue);
> + q = ns->queue;
> + blk_quiesce_queue(q);
> + blk_mq_cancel_requeue_work(q);
> + blk_mq_stop_hw_queues(q);
> + blk_resume_queue(q);
> }
> mutex_unlock(&ctrl->namespaces_mutex);
Hey Bart, should nvme_stop_queues() really be resuming the blk queue?
^ permalink raw reply
* Re: [PATCH rdma-core 4/5] libocrdma: Move ocrdma's list implementation into common directory
From: Yishai Hadas @ 2016-09-27 16:14 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Christoph Hellwig, Leon Romanovsky,
dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, yishaih-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <20160926222319.GA2358-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
On 9/27/2016 1:23 AM, Jason Gunthorpe wrote:
> On Mon, Sep 26, 2016 at 03:14:40PM -0700, Christoph Hellwig wrote:
>> On Mon, Sep 26, 2016 at 11:40:57AM -0600, Jason Gunthorpe wrote:
>>> Copyright isn't a patent,
>>
>> I know very well, thanks..
>
> Right, mainly for others..
>
>>> assuming freebsd didn't copy any code and
>>> just implemented the same API independently (eg it is an Independent
>>> Creation) they should be OK from a copyright perspective.
>>
>> That's a big IF, as I'm honestly not sure it is. I don't want to blame
>> anywone because I really don't know but if I had to decided to use it
>> or not I would error on the safe side.
>
> Sounds reasonable.
>
>>> As should be
>>> Rusty's version in CCAN.
>>
>> That's a much safer choice. It's a lightly different API, though.
>
> Leon? Does this seem doable to you?
Hi Jason,
Leon is out of office for a week, I can take it from here and come to
the list in coming days with a new candidate series based on previous notes.
Few notes to sync on:
1) Looking at ocrdma_list.h it extends the list default functionality to
use an internal mutex, see list_lock & list_unlock calls. This is not a
standard usage of a list. It may require some logic outside list.h to
replace the usage without introducing the mutex in the shared new H file.
2) Taking list.h from the 'CCAN' URL requires taking as well few other H
files that it uses internally (e.g. build_assert.h). In few cases I
don't see any reason to take the full file into rdma-core (e.g.
ccan/str/str.h for stringify) will take only the needed functionality
into list.h.
3) Need to clean up the 'CCAN' files from its include to "config.h", in
addition, need to consider the functionality that need to be taken when
there are some #if HAVE_XXX internally. (see #if HAVE_TYPEOF in list.h).
4) Re the licensing disclaimer in each H file, what do you suggest to
put ? for example minmax.h uses licenses/CC0 see
https://github.com/rustyrussell/ccan/blob/master/ccan/minmax/LICENSE
which is different comparing list.h which is ../../licenses/BSD-MIT.
Yishai
--
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 5/9] block: Extend blk_freeze_queue_start() to the non-blk-mq path
From: Bart Van Assche @ 2016-09-27 15:55 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <a8e8796e-7266-4c8a-a4eb-31cedf73ec8a-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
On 09/27/2016 07:42 AM, Bart Van Assche wrote:
> Jens, regarding non-blk-mq mode and q_usage_counter: do you prefer that
> I rework patch 8/9 such that blk_quiesce_queue() and blk_resume_queue()
> are only used in blk-mq mode or are you OK with adding a
> blk_queue_enter() call in get_request() and a blk_queue_exit() call to
> __blk_put_request()?
(replying to my own e-mail)
Although it is easy to make q_usage_counter count non-blk-mq requests,
extending the blk_quiesce_queue() waiting mechanism to the non-blk-mq
path is non-trivial. To limit the number of changes in this patch series
I will drop the non-blk-mq changes from this patch series.
Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 5/9] block: Extend blk_freeze_queue_start() to the non-blk-mq path
From: Bart Van Assche @ 2016-09-27 14:42 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <CACVXFVO=GFp=ArC_FT+xPSmL84m01fX6bzReBQ8DeLQ4fUD5cQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 09/27/16 06:22, Ming Lei wrote:
> On Tue, Sep 27, 2016 at 2:27 AM, Bart Van Assche
> <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> wrote:
>> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
>> ---
>> block/blk-core.c | 15 ++++++---------
>> 1 file changed, 6 insertions(+), 9 deletions(-)
>>
>> diff --git a/block/blk-core.c b/block/blk-core.c
>> index 8cc8006..5ecc7ab 100644
>> --- a/block/blk-core.c
>> +++ b/block/blk-core.c
>> @@ -689,7 +689,10 @@ void blk_freeze_queue_start(struct request_queue *q)
>> freeze_depth = atomic_inc_return(&q->mq_freeze_depth);
>> if (freeze_depth == 1) {
>> percpu_ref_kill(&q->q_usage_counter);
>> - blk_mq_run_hw_queues(q, false);
>> + if (q->mq_ops)
>> + blk_mq_run_hw_queues(q, false);
>> + else if (q->request_fn)
>> + blk_run_queue(q);
>
> Just wondering if you have a non-blk-mq drivers which need this change,
> cause we only hold .q_usage_counter for sync bio.
Hello Ming Lei,
Patch 8/9 calls blk_quiesce_queue() and blk_resume_queue() from a code
path that is used in both blk-mq and non-blk-mq mode. Although it
wouldn't be hard to modify that patch such that it only uses these two
functions in blk-mq mode, that wouldn't be very elegant.
Jens, regarding non-blk-mq mode and q_usage_counter: do you prefer that
I rework patch 8/9 such that blk_quiesce_queue() and blk_resume_queue()
are only used in blk-mq mode or are you OK with adding a
blk_queue_enter() call in get_request() and a blk_queue_exit() call to
__blk_put_request()?
Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 5/9] block: Extend blk_freeze_queue_start() to the non-blk-mq path
From: Ming Lei @ 2016-09-27 13:22 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org
In-Reply-To: <7c8af8d2-2ac9-2767-5e5f-8de72eba3814@sandisk.com>
On Tue, Sep 27, 2016 at 2:27 AM, Bart Van Assche
<bart.vanassche@sandisk.com> wrote:
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> ---
> block/blk-core.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 8cc8006..5ecc7ab 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -689,7 +689,10 @@ void blk_freeze_queue_start(struct request_queue *q)
> freeze_depth = atomic_inc_return(&q->mq_freeze_depth);
> if (freeze_depth == 1) {
> percpu_ref_kill(&q->q_usage_counter);
> - blk_mq_run_hw_queues(q, false);
> + if (q->mq_ops)
> + blk_mq_run_hw_queues(q, false);
> + else if (q->request_fn)
> + blk_run_queue(q);
Just wondering if you have a non-blk-mq drivers which need this change,
cause we only hold .q_usage_counter for sync bio.
> }
> }
>
> @@ -700,17 +703,11 @@ void blk_freeze_queue_wait(struct request_queue *q)
>
> /*
> * Guarantee no request is in use, so we can change any data structure of
> - * the queue afterward.
> + * the queue afterward. Increases q->mq_freeze_depth and waits until
> + * q->q_usage_counter drops to zero.
> */
> void blk_freeze_queue(struct request_queue *q)
> {
> - /*
> - * In the !blk_mq case we are only calling this to kill the
> - * q_usage_counter, otherwise this increases the freeze depth
> - * and waits for it to return to zero. For this reason there is
> - * no blk_unfreeze_queue(), and blk_freeze_queue() is not
> - * exported to drivers as the only user for unfreeze is blk_mq.
> - */
> blk_freeze_queue_start(q);
> blk_freeze_queue_wait(q);
> }
> --
> 2.10.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Ming Lei
^ permalink raw reply
* RE: [PATCH v5 13/16] IB/pvrdma: Add the main driver module for PVRDMA
From: David Laight @ 2016-09-27 9:21 UTC (permalink / raw)
To: 'Adit Ranadive', Yuval Shaia
Cc: dledford@redhat.com, linux-rdma@vger.kernel.org,
pv-drivers@vmware.com, netdev@vger.kernel.org,
linux-pci@vger.kernel.org, jhansen@vmware.com,
asarwade@vmware.com, georgezhang@vmware.com, bryantan@vmware.com
In-Reply-To: <651b18ee-55ca-0c72-af9c-c55c42b57fd1@vmware.com>
From: Adit Ranadive
> Sent: 26 September 2016 19:15
> On Mon, Sep 26, 2016 at 00:27:40AM -0700, Yuval Shaia wrote:
> > On Sat, Sep 24, 2016 at 04:21:37PM -0700, Adit Ranadive wrote:
> > > +
> > > + /* Currently, the driver only supports RoCE mode. */
> > > + if (dev->dsr->caps.mode != PVRDMA_DEVICE_MODE_ROCE) {
> > > + dev_err(&pdev->dev, "unsupported transport %d\n",
> > > + dev->dsr->caps.mode);
> > > + ret = -EINVAL;
> >
> > This is some fatal error with the device, not that something wrong with the
> > function's argument.
> > Suggesting to replace with -EFAULT.
> >
>
> Thanks, will fix this one and the others here.
Won't EFAULT generate SIGSEGV ?
David
^ permalink raw reply
* Re: [PATCH 4/9] block: Move blk_freeze_queue() and blk_unfreeze_queue() code
From: Johannes Thumshirn @ 2016-09-27 7:52 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Bart Van Assche, Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <ffad7b3c-e7f5-56d7-0a75-574e65ac5b31-l3A5Bk7waGM@public.gmane.org>
On Tue, Sep 27, 2016 at 08:26:19AM +0200, Hannes Reinecke wrote:
> On 09/26/2016 08:27 PM, Bart Van Assche wrote:
> > Move the blk_freeze_queue() and blk_unfreeze_queue() implementations
> > from block/blk-mq.c to block/blk-core.c. Drop "_mq" from the name of
> > the functions that have been moved.
> >
> > Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> > ---
> > block/blk-core.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> > block/blk-mq.c | 41 +++--------------------------------------
> > block/blk.h | 3 +++
> > 3 files changed, 51 insertions(+), 38 deletions(-)
> >
> > diff --git a/block/blk-core.c b/block/blk-core.c
> > index b75d688..8cc8006 100644
> > --- a/block/blk-core.c
> > +++ b/block/blk-core.c
> > @@ -682,6 +682,51 @@ static void blk_queue_usage_counter_release(struct percpu_ref *ref)
> > wake_up_all(&q->mq_freeze_wq);
> > }
> >
> > +void blk_freeze_queue_start(struct request_queue *q)
> > +{
> > + int freeze_depth;
> > +
> > + freeze_depth = atomic_inc_return(&q->mq_freeze_depth);
> > + if (freeze_depth == 1) {
> > + percpu_ref_kill(&q->q_usage_counter);
> > + blk_mq_run_hw_queues(q, false);
> > + }
> > +}
> > +
> As you dropped the 'mq_' prefix, maybe you should rename the counter to
> 'freeze_depth', to?
See PATCH 6/9
--
Johannes Thumshirn Storage
jthumshirn-l3A5Bk7waGM@public.gmane.org +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
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 6/9] block: Rename mq_freeze_wq and mq_freeze_depth
From: Johannes Thumshirn @ 2016-09-27 7:51 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org
In-Reply-To: <129b2a52-1ff7-7048-dfa3-89416ea42dd2@sandisk.com>
On Mon, Sep 26, 2016 at 11:28:08AM -0700, Bart Van Assche wrote:
> Since these two structure members are now used in blk-mq and !blk-mq
> paths, remove the mq_prefix. This patch does not change any
> functionality.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> ---
Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply
* Re: [PATCH 5/9] block: Extend blk_freeze_queue_start() to the non-blk-mq path
From: Johannes Thumshirn @ 2016-09-27 7:50 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org
In-Reply-To: <7c8af8d2-2ac9-2767-5e5f-8de72eba3814@sandisk.com>
On Mon, Sep 26, 2016 at 11:27:49AM -0700, Bart Van Assche wrote:
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> ---
Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply
* Re: [PATCH 2/9] dm: Fix a race condition related to stopping and starting queues
From: Johannes Thumshirn @ 2016-09-27 7:47 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org
In-Reply-To: <ba8ddc16-b709-e7fb-1120-b6e1bd393540@sandisk.com>
On Mon, Sep 26, 2016 at 11:26:50AM -0700, Bart Van Assche wrote:
> Ensure that all ongoing dm_mq_queue_rq() and dm_mq_requeue_request()
> calls have stopped before setting the "queue stopped" flag. This
> allows to remove the "queue stopped" test from dm_mq_queue_rq() and
> dm_mq_requeue_request(). This patch fixes a race condition because
> dm_mq_queue_rq() is called without holding the queue lock and hence
> BLK_MQ_S_STOPPED can be set at any time while dm_mq_queue_rq() is
> in progress.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Mike Snitzer <snitzer@redhat.com>
> ---
Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply
* Re: [PATCH 1/9] blk-mq: Introduce blk_mq_queue_stopped()
From: Johannes Thumshirn @ 2016-09-27 7:38 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Keith Busch,
linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
linux-rdma@vger.kernel.org, linux-nvme@lists.infradead.org
In-Reply-To: <b2210943-62b2-959a-569a-a404da03bcae@sandisk.com>
On Mon, Sep 26, 2016 at 11:26:26AM -0700, Bart Van Assche wrote:
> The function blk_queue_stopped() allows to test whether or not a
> traditional request queue has been stopped. Introduce a helper
> function that allows block drivers to query easily whether or not
> one or more hardware contexts of a blk-mq queue have been stopped.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Jens Axboe <axboe@fb.com>
> Cc: Christoph Hellwig <hch@lst.de>
> ---
Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply
* RQ overflow seen running isert traffic
From: Potnuri Bharat Teja @ 2016-09-27 7:01 UTC (permalink / raw)
To: sagi; +Cc: swise, target-devel, nab, linux-rdma
Hi Sagi,
I've been trying to understand the isert functionality with respect to
RDMA Receive Queue sizing and Queue full handling. Here is the problem
is see with iw_cxgb4:
After running few minutes of iSER traffic with iw_cxgb4, I am seeing
post receive failures due to receive queue full returning -ENOMEM.
In case of iw_cxgb4 the RQ size is 130 with qp attribute max_recv_wr = 129,
passed down by isert to iw_cxgb4.isert decides on max_recv_wr as 129 based
on (ISERT_QP_MAX_RECV_DTOS = ISCSI_DEF_XMIT_CMDS_MAX = 128) + 1.
My debug suggests that at some point isert tries to post more than
129 receive WRs into the RQ and fails as the queue is full already. From
the code most of the recv wr are posted only after a recieve completion,
but few datain operations(isert_put_datain()) are done independent of
receive completions. In fact the last WR failed to post in to RQ is from
isert_put_datain() through target_complete_ok_work(). CQ stats at the
time of failure shows the cq polled to empty.
How could I check for the possible conditions leading to this issue?
I am happy to provide you any details further.
Thanks for your time.
Bharat.
^ permalink raw reply
* Re: [PATCH 4/9] block: Move blk_freeze_queue() and blk_unfreeze_queue() code
From: Hannes Reinecke @ 2016-09-27 6:26 UTC (permalink / raw)
To: Bart Van Assche, Jens Axboe
Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
Mike Snitzer, Doug Ledford, Keith Busch,
linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <f990694e-76ea-f6b1-10a4-c23b9a738b39-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
On 09/26/2016 08:27 PM, Bart Van Assche wrote:
> Move the blk_freeze_queue() and blk_unfreeze_queue() implementations
> from block/blk-mq.c to block/blk-core.c. Drop "_mq" from the name of
> the functions that have been moved.
>
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> ---
> block/blk-core.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> block/blk-mq.c | 41 +++--------------------------------------
> block/blk.h | 3 +++
> 3 files changed, 51 insertions(+), 38 deletions(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index b75d688..8cc8006 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -682,6 +682,51 @@ static void blk_queue_usage_counter_release(struct percpu_ref *ref)
> wake_up_all(&q->mq_freeze_wq);
> }
>
> +void blk_freeze_queue_start(struct request_queue *q)
> +{
> + int freeze_depth;
> +
> + freeze_depth = atomic_inc_return(&q->mq_freeze_depth);
> + if (freeze_depth == 1) {
> + percpu_ref_kill(&q->q_usage_counter);
> + blk_mq_run_hw_queues(q, false);
> + }
> +}
> +
As you dropped the 'mq_' prefix, maybe you should rename the counter to
'freeze_depth', to?
And are you sure this works in the !mq case, too?
> +void blk_freeze_queue_wait(struct request_queue *q)
> +{
> + wait_event(q->mq_freeze_wq, percpu_ref_is_zero(&q->q_usage_counter));
> +}
> +
Same here, rename ->mq_freeze_wq to ->freeze_wq.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare-l3A5Bk7waGM@public.gmane.org +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
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 2/9] dm: Fix a race condition related to stopping and starting queues
From: Hannes Reinecke @ 2016-09-27 6:21 UTC (permalink / raw)
To: Bart Van Assche, Jens Axboe
Cc: Christoph Hellwig, James Bottomley, Martin K. Petersen,
Mike Snitzer, Doug Ledford, Keith Busch,
linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <ba8ddc16-b709-e7fb-1120-b6e1bd393540-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
On 09/26/2016 08:26 PM, Bart Van Assche wrote:
> Ensure that all ongoing dm_mq_queue_rq() and dm_mq_requeue_request()
> calls have stopped before setting the "queue stopped" flag. This
> allows to remove the "queue stopped" test from dm_mq_queue_rq() and
> dm_mq_requeue_request(). This patch fixes a race condition because
> dm_mq_queue_rq() is called without holding the queue lock and hence
> BLK_MQ_S_STOPPED can be set at any time while dm_mq_queue_rq() is
> in progress.
>
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> Cc: Mike Snitzer <snitzer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> drivers/md/dm-rq.c | 14 +++-----------
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare-IBi9RG/b67k@public.gmane.org>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare-l3A5Bk7waGM@public.gmane.org +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox