* Re: [PATCH rdma-rc 08/12] IB/mlx5: Wait for all async command completions to complete
From: Or Gerlitz @ 2016-10-28 13:04 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1477575407-20562-9-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On Thu, Oct 27, 2016 at 4:36 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> +static void wait_for_async_commands(struct mlx5_ib_dev *dev)
> +{
> + struct mlx5_mr_cache *cache = &dev->cache;
> + struct mlx5_cache_ent *ent;
> + int total = 0;
> + int i;
> + int j;
> +
> + for (i = 0; i < MAX_MR_CACHE_ENTRIES; i++) {
> + ent = &cache->ent[i];
> + for (j = 0 ; j < 1000; j++) {
> + if (!ent->pending)
> + break;
> + msleep(50);
> + }
you had another patch on this series which change a hard coded
constant into a define, why this patch add two new hard coded
constants, so all to all, we're not making progress on that
no-hard-coded-constants front... better decide where you want to go
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH rdma-rc 01/12] IB/mlx5: Replace numerical constant with predefined MACRO
From: Leon Romanovsky @ 2016-10-28 14:55 UTC (permalink / raw)
To: Or Gerlitz
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Max Gurtovoy
In-Reply-To: <CAJ3xEMiNynmpbBt0M=k5qywrvnfAq+ogZLD2PUbHBvYVJ4pLyQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1092 bytes --]
On Fri, Oct 28, 2016 at 03:54:25PM +0300, Or Gerlitz wrote:
> On Thu, Oct 27, 2016 at 4:36 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > From: Max Gurtovoy <maxg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > Replace the pre-defined macro signifying inline umr instead
> > of the numerical constant.
>
> Leon,
>
> By all means (or no means, choose), this is a fix. If you are telling
> vmware and Broadcom people here for months how to write their code,
> let's stop for a minute and look in the mirror, drop this patch.
Thanks Or,
I appreciate your time invested in the review and providing so much
valuable feedback.
Since change of numeric value to the same value defined by macro is not
a fix but improvement and provided here to simplify maintainer's work (Doug),
I have no plans to change the patch or/and drop it.
>
> Or.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* RE: [PATCH rdma-core 2/4] Move rdma_netlink compat into CMake
From: Steve Wise @ 2016-10-28 14:59 UTC (permalink / raw)
To: 'Jason Gunthorpe', 'Doug Ledford',
linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: 'Tatyana Nikolova', 'Latif, Faisal'
In-Reply-To: <1477609570-8087-3-git-send-email-jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
+Tatyana
+Faisal
Intel, please have a look.
>
> Steve,
>
> Can you check if the changes to iwpmd/iwarp_pm_server.c make sense?
I was confused at first, but I see that the iwarp_clients array still has
clients 2 and 3, so it should work fine and still preserve compatibility with
older drivers that embedded the iwarp port mapping in iw_cxgb4 and iw_nes. And
the RDMA_NL_* enum is consistent with the local header and the kernel uapi
header. So it looks fine. I should test this though to make sure...
>
> Should we do something to fix the kernel header?
>
What is wrong with the kernel header exactly?
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: [RFC ABI V5 07/10] IB/core: Support getting IOCTL header/SGEs from kernel space
From: Leon Romanovsky @ 2016-10-28 15:16 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Matan Barak, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford,
Jason Gunthorpe, Sean Hefty, Christoph Lameter, Liran Liss,
Haggai Eran, Majd Dibbiny, Tal Alon
In-Reply-To: <20161028065943.GA10418-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 931 bytes --]
On Thu, Oct 27, 2016 at 11:59:43PM -0700, Christoph Hellwig wrote:
> > + mm_segment_t currentfs = get_fs();
> >
> > if (!ib_dev)
> > return -EIO;
> > @@ -240,8 +242,10 @@ static long ib_uverbs_cmd_verbs(struct ib_device *ib_dev,
> > goto out;
> > }
> >
> > + set_fs(oldfs);
> > err = uverbs_handle_action(buf, ctx->uattrs, hdr->num_attrs, ib_dev,
> > file, action, ctx->uverbs_attr_array);
> > + set_fs(currentfs);
>
> Adding this magic in new code is not acceptable. Any given API
> must take either a kernel or a user pointer.
And it is indeed happen for new code. This magic is needed to allow legacy
write interface to be converted to new ioctl interface internally in
kernel.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [RFC ABI V5 07/10] IB/core: Support getting IOCTL header/SGEs from kernel space
From: Christoph Hellwig @ 2016-10-28 15:21 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Christoph Hellwig, Matan Barak, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
Doug Ledford, Jason Gunthorpe, Sean Hefty, Christoph Lameter,
Liran Liss, Haggai Eran, Majd Dibbiny, Tal Alon
In-Reply-To: <20161028151606.GN3617-2ukJVAZIZ/Y@public.gmane.org>
On Fri, Oct 28, 2016 at 06:16:06PM +0300, Leon Romanovsky wrote:
> And it is indeed happen for new code. This magic is needed to allow legacy
> write interface to be converted to new ioctl interface internally in
> kernel.
You just added these statements which are by defintion new code.
Don't do that - create a clean kernel internal interface instead.
The canonical one would be to only pass kernel pointers in the internal
interface.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC ABI V5 07/10] IB/core: Support getting IOCTL header/SGEs from kernel space
From: Leon Romanovsky @ 2016-10-28 15:33 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Matan Barak, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford,
Jason Gunthorpe, Sean Hefty, Christoph Lameter, Liran Liss,
Haggai Eran, Majd Dibbiny, Tal Alon
In-Reply-To: <20161028152138.GA16421-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1147 bytes --]
On Fri, Oct 28, 2016 at 08:21:38AM -0700, Christoph Hellwig wrote:
> On Fri, Oct 28, 2016 at 06:16:06PM +0300, Leon Romanovsky wrote:
> > And it is indeed happen for new code. This magic is needed to allow legacy
> > write interface to be converted to new ioctl interface internally in
> > kernel.
>
> You just added these statements which are by defintion new code.
> Don't do that - create a clean kernel internal interface instead.
>
> The canonical one would be to only pass kernel pointers in the internal
> interface.
Just to summarize, to be sure that I understood you correctly.
--------- --------------------
| write | -> | conversion logic | ---
--------- -------------------- | ----------------------
-----> | internal interface |
--------- | ----------------------
| ioctl | ---------------------------
---------
Am I right?
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [RFC ABI V5 07/10] IB/core: Support getting IOCTL header/SGEs from kernel space
From: Christoph Hellwig @ 2016-10-28 15:37 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Christoph Hellwig, Matan Barak, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
Doug Ledford, Jason Gunthorpe, Sean Hefty, Christoph Lameter,
Liran Liss, Haggai Eran, Majd Dibbiny, Tal Alon
In-Reply-To: <20161028153306.GO3617-2ukJVAZIZ/Y@public.gmane.org>
On Fri, Oct 28, 2016 at 06:33:06PM +0300, Leon Romanovsky wrote:
> Just to summarize, to be sure that I understood you correctly.
>
> --------- --------------------
> | write | -> | conversion logic | ---
> --------- -------------------- | ----------------------
> -----> | internal interface |
> --------- | ----------------------
> | ioctl | ---------------------------
> ---------
>
> Am I right?
Yes, as long as the write and ioctl boxes do the copy_{from,to}_user.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH rdma-core 3/4] verbs: Replace infiniband/sa-kern-abi.h with the kernel's uapi/rdma/ib_user_sa.h
From: Jason Gunthorpe @ 2016-10-28 15:42 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161028065344.GA28303-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
On Thu, Oct 27, 2016 at 11:53:44PM -0700, Christoph Hellwig wrote:
> I can't see how this is supposed to work, there is no copy at all
> of ib_user_sa.h in the tree.
It uses the one that comes with the distro. Which is new enough on all
supported distros..
In this specific case because ib_user_sa.h is included by a public
header we really don't have much choice, it has to work with the
distro-version.
Fortunately ib_user_sa.h has not changed in a long time, so this is
not a problem and I didn't include an in-tree copy. Several of the
other headers are like that too.
rxe and netlink are counter examples, where we need really new headers
and they are not used in public headers so they have in-tree copies.
> E.g. by default use headers from rdma-core/kernel/headers, but
> optionally allow the build systems to use those from a kernel tree
> explicitly specified.
I've done both of these things, with the twist that the build tests
the distro-header first and uses it before the in-tree copy if it is
new enough.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH rdma-core 2/4] Move rdma_netlink compat into CMake
From: Jason Gunthorpe @ 2016-10-28 15:44 UTC (permalink / raw)
To: Steve Wise
Cc: 'Doug Ledford', linux-rdma-u79uwXL29TY76Z2rM5mHXA,
'Tatyana Nikolova', 'Latif, Faisal'
In-Reply-To: <002501d2312b$da377cf0$8ea676d0$@opengridcomputing.com>
On Fri, Oct 28, 2016 at 09:59:16AM -0500, Steve Wise wrote:
> +Tatyana
> +Faisal
>
> Intel, please have a look.
>
> >
> > Steve,
> >
> > Can you check if the changes to iwpmd/iwarp_pm_server.c make sense?
>
> I was confused at first, but I see that the iwarp_clients array still has
> clients 2 and 3, so it should work fine and still preserve compatibility with
> older drivers that embedded the iwarp port mapping in iw_cxgb4 and iw_nes. And
> the RDMA_NL_* enum is consistent with the local header and the kernel uapi
> header. So it looks fine. I should test this though to make sure...
>
> >
> > Should we do something to fix the kernel header?
>
> What is wrong with the kernel header exactly?
RDMA_NL_RSVD, should perhaps be called RDMA_NL_IWCM_COMPAT since user
space still need to use that value for compat with old kernels.
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: [RFC ABI V5 07/10] IB/core: Support getting IOCTL header/SGEs from kernel space
From: Leon Romanovsky @ 2016-10-28 15:46 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Matan Barak, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford,
Jason Gunthorpe, Sean Hefty, Christoph Lameter, Liran Liss,
Haggai Eran, Majd Dibbiny, Tal Alon
In-Reply-To: <20161028153725.GA14166-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 880 bytes --]
On Fri, Oct 28, 2016 at 08:37:25AM -0700, Christoph Hellwig wrote:
> On Fri, Oct 28, 2016 at 06:33:06PM +0300, Leon Romanovsky wrote:
> > Just to summarize, to be sure that I understood you correctly.
> >
> > --------- --------------------
> > | write | -> | conversion logic | ---
> > --------- -------------------- | ----------------------
> > -----> | internal interface |
> > --------- | ----------------------
> > | ioctl | ---------------------------
> > ---------
> >
> > Am I right?
>
> Yes, as long as the write and ioctl boxes do the copy_{from,to}_user.
Thanks
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* RE: [PATCH rdma-core 2/4] Move rdma_netlink compat into CMake
From: Steve Wise @ 2016-10-28 15:46 UTC (permalink / raw)
To: 'Jason Gunthorpe'
Cc: 'Doug Ledford', linux-rdma-u79uwXL29TY76Z2rM5mHXA,
'Tatyana Nikolova', 'Latif, Faisal'
In-Reply-To: <20161028154455.GB10441-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> > > Can you check if the changes to iwpmd/iwarp_pm_server.c make sense?
> >
> > I was confused at first, but I see that the iwarp_clients array still has
> > clients 2 and 3, so it should work fine and still preserve compatibility
with
> > older drivers that embedded the iwarp port mapping in iw_cxgb4 and iw_nes.
> And
> > the RDMA_NL_* enum is consistent with the local header and the kernel uapi
> > header. So it looks fine. I should test this though to make sure...
> >
> > >
> > > Should we do something to fix the kernel header?
> >
> > What is wrong with the kernel header exactly?
>
> RDMA_NL_RSVD, should perhaps be called RDMA_NL_IWCM_COMPAT since user
> space still need to use that value for compat with old kernels.
Sounds reasonable.
For this patch:
Acked-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 11/12] nvme: Use BLK_MQ_S_STOPPED instead of QUEUE_FLAG_STOPPED in blk-mq code
From: Keith Busch @ 2016-10-28 16:01 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Ming Lei,
Laurence Oberman, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-nvme@lists.infradead.org
In-Reply-To: <805e1911-cd10-0563-c76b-256d76054b08@sandisk.com>
On Wed, Oct 26, 2016 at 03:56:04PM -0700, Bart Van Assche wrote:
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 7bb73ba..b662416 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -205,7 +205,7 @@ void nvme_requeue_req(struct request *req)
>
> blk_mq_requeue_request(req, false);
> spin_lock_irqsave(req->q->queue_lock, flags);
> - if (!blk_queue_stopped(req->q))
> + if (!blk_mq_queue_stopped(req->q))
> blk_mq_kick_requeue_list(req->q);
> spin_unlock_irqrestore(req->q->queue_lock, flags);
> }
> @@ -2079,10 +2079,6 @@ void nvme_stop_queues(struct nvme_ctrl *ctrl)
>
> mutex_lock(&ctrl->namespaces_mutex);
> list_for_each_entry(ns, &ctrl->namespaces, list) {
> - spin_lock_irq(ns->queue->queue_lock);
> - queue_flag_set(QUEUE_FLAG_STOPPED, ns->queue);
> - spin_unlock_irq(ns->queue->queue_lock);
> -
> blk_mq_cancel_requeue_work(ns->queue);
> blk_mq_stop_hw_queues(ns->queue);
There's actually a reason the queue stoppage is using a different flag
than blk_mq_queue_stopped: kicking the queue starts stopped queues.
The driver has to ensure the requeue work can't be kicked prior to
cancelling the current requeue work. Once we know requeue work isn't
running and can't restart again, then we're safe to stop the hw queues.
It's a pretty obscure condition, requiring the controller post an
error completion at the same time the driver decides to reset the
controller. Here's the sequence with the wrong outcome:
CPU A CPU B
----- -----
nvme_stop_queues nvme_requeue_req
blk_mq_stop_hw_queues if (blk_mq_queue_stopped) <- returns false
blk_mq_stop_hw_queue blk_mq_kick_requeue_list
blk_mq_requeue_work
blk_mq_start_hw_queues
^ permalink raw reply
* [PATCH] qedr: Fix missing unlock on error in qedr_post_send()
From: Wei Yongjun @ 2016-10-28 16:33 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Hal Rosenstock, Ram Amrani,
Rajesh Borundia
Cc: Wei Yongjun, linux-rdma-u79uwXL29TY76Z2rM5mHXA
From: Wei Yongjun <weiyongjun1-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Add the missing unlock before return from function qedr_post_send()
in the error handling case.
Signed-off-by: Wei Yongjun <weiyongjun1-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
drivers/infiniband/hw/qedr/verbs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
index a615142..e7c7417 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -2983,7 +2983,8 @@ int qedr_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
if (!wr) {
DP_ERR(dev, "Got an empty post send.\n");
- return -EINVAL;
+ rc = -EINVAL;
+ goto out_unlock;
}
while (wr) {
@@ -3012,6 +3013,7 @@ int qedr_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
/* Make sure write sticks */
mmiowb();
+out_unlock:
spin_unlock_irqrestore(&qp->q_lock, flags);
return rc;
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH] qedr: Fix possible memory leak in qedr_create_qp()
From: Wei Yongjun @ 2016-10-28 16:33 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Hal Rosenstock, Ram Amrani,
Rajesh Borundia
Cc: Wei Yongjun, linux-rdma-u79uwXL29TY76Z2rM5mHXA
From: Wei Yongjun <weiyongjun1-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
'qp' is malloced in qedr_create_qp() and should be freed before leaving
from the error handling cases, otherwise it will cause memory leak.
Signed-off-by: Wei Yongjun <weiyongjun1-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
drivers/infiniband/hw/qedr/verbs.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
index a615142..b60f145 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -1477,6 +1477,7 @@ struct ib_qp *qedr_create_qp(struct ib_pd *ibpd,
struct qedr_ucontext *ctx = NULL;
struct qedr_create_qp_ureq ureq;
struct qedr_qp *qp;
+ struct ib_qp *ibqp;
int rc = 0;
DP_DEBUG(dev, QEDR_MSG_QP, "create qp: called from %s, pd=%p\n",
@@ -1486,13 +1487,13 @@ struct ib_qp *qedr_create_qp(struct ib_pd *ibpd,
if (rc)
return ERR_PTR(rc);
+ if (attrs->srq)
+ return ERR_PTR(-EINVAL);
+
qp = kzalloc(sizeof(*qp), GFP_KERNEL);
if (!qp)
return ERR_PTR(-ENOMEM);
- if (attrs->srq)
- return ERR_PTR(-EINVAL);
-
DP_DEBUG(dev, QEDR_MSG_QP,
"create qp: sq_cq=%p, sq_icid=%d, rq_cq=%p, rq_icid=%d\n",
get_qedr_cq(attrs->send_cq),
@@ -1508,7 +1509,10 @@ struct ib_qp *qedr_create_qp(struct ib_pd *ibpd,
"create qp: unexpected udata when creating GSI QP\n");
goto err0;
}
- return qedr_create_gsi_qp(dev, attrs, qp);
+ ibqp = qedr_create_gsi_qp(dev, attrs, qp);
+ if (IS_ERR(ibqp))
+ kfree(qp);
+ return ibqp;
}
memset(&in_params, 0, sizeof(in_params));
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH rdma-core 1/7] libhns: Add initial main frame
From: Jason Gunthorpe @ 2016-10-28 16:40 UTC (permalink / raw)
To: oulijun
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linuxarm-hv44wF8Li93QT0dZR+AlfA
In-Reply-To: <58130573.4010902-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
On Fri, Oct 28, 2016 at 03:59:47PM +0800, oulijun wrote:
> total 0
> lrwxrwxrwx 1 root root 0 Oct 27 11:07 driver -> ../../../../bus/platform/drivers/hns_roce
>
> but I think it is the standard approach. because my device(hip06) is
> only platform device and the other device(hip07/hip0x0 will be pcie
> device, it will be distinguished separately. Hence, we adpot the
> origin approach.
You have to parse out 'hns_roce' at the end of the readlink result and
compare against that, drop the 'bus/platform/drivers' stuff
Your PCI and Platform device should both have the same driver name.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH rdma-rc 01/12] IB/mlx5: Replace numerical constant with predefined MACRO
From: Or Gerlitz @ 2016-10-28 16:48 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Max Gurtovoy
In-Reply-To: <20161028145534.GM3617-2ukJVAZIZ/Y@public.gmane.org>
On Fri, Oct 28, 2016 at 5:55 PM, Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> On Fri, Oct 28, 2016 at 03:54:25PM +0300, Or Gerlitz wrote:
>> By all means (or no means, choose), this is a fix. If you are telling
> Since change of numeric value to the same value defined by macro is not
> a fix but improvement [..] I have no plans to change the patch or/and drop it.
RU listening? it's a -next patch, not rc
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH rdma-rc 12/12] IB/mlx5: Limit mkey page size to 2GB
From: Or Gerlitz @ 2016-10-28 16:53 UTC (permalink / raw)
To: Majd Dibbiny
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Maor Gottlieb
In-Reply-To: <31841A29-5CE1-45A9-999A-1FAAA3CDFD4F-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
On Fri, Oct 28, 2016 at 4:21 PM, Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> On Oct 28, 2016, at 4:02 PM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> In case we are working with 1GB hugepage, it is possible to find more than
> two continuous pages, and end up with more than 2GB..
got it
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH rdma-core v2 4/4] redhat/spec: build split rpm packages
From: Jarod Wilson @ 2016-10-28 17:11 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161027211059.GA7224-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
On Thu, Oct 27, 2016 at 03:10:59PM -0600, Jason Gunthorpe wrote:
> On Thu, Oct 20, 2016 at 11:33:57AM -0400, Jarod Wilson wrote:
> > @@ -7,10 +7,11 @@ Summary: RDMA core userspace libraries and daemons
> > # providers/ipathverbs/ Dual licensed using a BSD license with an extra patent clause
> > # providers/rxe/ Incorporates code from ipathverbs and contains the patent clause
> > # providers/hfi1verbs Uses the 3 Clause BSD license
> > -License: (GPLv2 or BSD) and (GPLv2 or PathScale-BSD)
> > +License: GPLv2 or BSD
>
> Is this Ok? The Fedora guidelines I read suggested the PathScale
> license would need to be assigned a short tag, and I'd be surprised if
> 'BSD' is the right tag due to the patent stuff..
Our standalone libipathverbs has just "GPLv2 or BSD", and I didn't see
anything specific about PathScale, but I may not have been looking hard
enough or in the right place in the Fedora packaging guidelines. Where did
you see that?
> > Url: http://openfabrics.org/
>
> I guess we should change this url to
> https://github.com/linux-rdma/rdma-core ?
Either one works for me.
> > Source: rdma-core-%{version}.tgz
> > -BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root
> > +# https://github.com/linux-rdma/rdma-core
> > +BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
>
> I always wondered why there was so much variability in spec files
> here.. I followed the Fedora guidelines, should we copy the above into
> the other spec file?
I believe the current Fedora guidelines actually say "just omit
BuildRoot", because rpm will figure out a sane default by itself. The one
with mktemp was introduced by the security-conscious/paranoid, I just
copied it over from another of the specs I was merging together here, not
sure what the "best" route is here now.
> > @@ -19,20 +20,15 @@ BuildRequires: pkgconfig
> > BuildRequires: pkgconfig(libnl-3.0)
> > BuildRequires: pkgconfig(libnl-route-3.0)
> > BuildRequires: valgrind-devel
> > +BuildRequires: libnl3-devel
>
> ?
>
> Isn't pkgconfig(libnl-3.0) the same thing?
Oops, probably. Another copy-paste addition from another spec.
> >%define systemd_dep systemd-units
> >%if 0%{?fedora} >= 18
> >%define systemd_dep systemd
> >%endif
>
> The source package probably doesn't even build on FC 18.. can probably
> remove this
Works for me. FC18 is 3-4 years old now, and nobody sane should be running
it for anything useful that they want to deploy new software on.
> > +Summary: InfiniBand Communication Manager Assistant
> > +Requires(post): %{systemd_dep}
> > +Requires(preun): %{systemd_dep}
> > +Requires(postun): %{systemd_dep}
>
> I suppose we need these and related in the other spec file too?
> Looks like this spec file isn't going to work on C6, so you can
> probably drop the other systemd compat stuff:
Ah, yes, probably a good idea to add these there too. I'd meant to add all
the stuff that should go in both specs *before* the split/copy, but alas, I
failed. :)
> --- a/redhat/rdma-core.spec
> +++ b/redhat/rdma-core.spec
> @@ -202,13 +202,6 @@ discover and use SCSI devices via the SCSI RDMA Protocol over InfiniBand.
>
> %build
>
> -# Detect if systemd is supported on this system
> -%if 0%{?_unitdir:1}
> -%define my_unitdir %{_unitdir}
> -%else
> -%define my_unitdir /tmp/
> -%endif
> -
> # New RPM defines _rundir, usually as /run
> %if 0%{?_rundir:1}
> %else
> @@ -228,7 +221,7 @@ discover and use SCSI devices via the SCSI RDMA Protocol over InfiniBand.
> -DCMAKE_INSTALL_INFODIR:PATH=%{_infodir} \
> -DCMAKE_INSTALL_MANDIR:PATH=%{_mandir} \
> -DCMAKE_INSTALL_SYSCONFDIR:PATH=%{_sysconfdir} \
> - -DCMAKE_INSTALL_SYSTEMD_SERVICEDIR:PATH=%{my_unitdir} \
> + -DCMAKE_INSTALL_SYSTEMD_SERVICEDIR:PATH=%{_unitdir} \
> -DCMAKE_INSTALL_INITDDIR:PATH=%{_initrddir} \
> -DCMAKE_INSTALL_RUNDIR:PATH=%{_rundir} \
> -DCMAKE_INSTALL_DOCDIR:PATH=%{_docdir}/%{name}-%{version}
> @@ -276,8 +269,6 @@ install -D -m0644 redhat/srp_daemon.service %{buildroot}%{_unitdir}/
>
> %if 0%{?_unitdir:1}
> rm -rf %{buildroot}/%{_initrddir}/
> -%else
> -rm -rf %{buildroot}/%{my_unitdir}/
> %endif
>
> %post -p /sbin/ldconfig
Looks good to me. And yeah, we have no plans to update the el6 rdma stack
with rdma-core and driver refreshes at this stage in it's lifecycle, so
there's really no sense in pretending to maybe support it.
> > +%package -n librdmacm-utils
> > +Summary: Examples for the librdmacm library
> > +Requires: librdmacm%{?_isa} = %{version}-%{release}
>
> Why the requires? Shouldn't auto shlib dependencies take care of that?
Probably. I think this was another legacy bit copied over from a
stand-alone spec file.
> Anyhow, this all looks fine to me, I put a branch here, with one
> change to make the debian packaging work after the README.md change:
>
> https://github.com/jgunthorpe/rdma-plumbing/tree/redhat-packaging
>
> If you want to make any final adjustments let me know, otherwise I
> will send this on..
Nah, I think things look good, and we can always keep tweaking as needed,
this is a solid update to work on top of.
--
Jarod Wilson
jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RDMA developer gatherings around Kernel Summit and Linux Plumbers in Santa Fe
From: Christoph Lameter @ 2016-10-28 17:17 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: Doug Ledford, skc-YOWKrPYUwWM, ira.weiny-ral2JQCrhuEAvxtiuMwx3w,
Jason Gunthorpe, john.fleck-ral2JQCrhuEAvxtiuMwx3w,
leon-DgEjT+Ai2ygdnm+yROfE0A, liranl-VPRAkNaXOzVWk0Htik3J/w,
matanb-VPRAkNaXOzVWk0Htik3J/w, tzahio-VPRAkNaXOzVWk0Htik3J/w
We have two days dedicated to meetings about RDMA technology
- RDMA workshop on Tuesday, 1st of November
Meeting in Sweeney AB from 9am till 5pm.
See https://www.linuxplumbersconf.org/2016/ocw/events/LPC2016/schedule
This is part of the Kernel Summit and the Linux Plumbers
Conference. Only open to KS and LPC attendees with an invitation.
Sessions:
Implementing a new Linux RDMA provider Knut Omang
New IOCTL ABI Matan Barak
Consolidation of RDMA User-space code Jason Gunthorpe
Debuggability and Tracing Leon Romanovsky
Integration with other subsystems Leon Romanovsky
Lunch Break
Containers and RDMA Liran Liss
New Fabrics Ira Weiny
Future and Roadmap for RDMA Doug Ledford
User Mode Ethernet Verbs Tzahi Oved
Dual Licensing Susan Coulter
Note that there are some speakers who have not filled out the speakers profile
and some abstract needs to be filled out. Those list me as a speaker until
the time that we can put the information about the real speaker in there.
- RDMA Summit on Saturday the 5th of November
This event will be held at
Hilton Santa Fe
100 Sandoval St.
Santa Fe, NM 87501
Phone: (505)988-2811
Meeting in the Canyon Ballroom 9am till 4pm.
This event is sponsored by the Open Fabrics Alliance and will be
open to all
The workshop will be focusing more on the technical topics (some
of which could be carried over to Saturday if necessary because we have
more time then. We will summarize what happened on Tuesday for those who
were not able to participate earlier.
Saturday sessions 9am till 4pm. 12-1pm Lunchtime
9am Refine TODO list for consolidated library - Jason Gunthorpe
10am Submission process for multi subsystem drivers - Doug Ledford
11am Multicast features and gaps - Christoph Lameter
1pm Licensing carryover - Susan/Christoph
2pm Standard network tools, integrating to the regular network stack - Christoph
3pm Open Discussion/Reserve Session - TBD
4pm Closing Session - TBD
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH rdma-core v2 4/4] redhat/spec: build split rpm packages
From: Jason Gunthorpe @ 2016-10-28 17:25 UTC (permalink / raw)
To: Jarod Wilson; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161028171147.GJ42084-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Fri, Oct 28, 2016 at 01:11:47PM -0400, Jarod Wilson wrote:
> On Thu, Oct 27, 2016 at 03:10:59PM -0600, Jason Gunthorpe wrote:
> > On Thu, Oct 20, 2016 at 11:33:57AM -0400, Jarod Wilson wrote:
> > > @@ -7,10 +7,11 @@ Summary: RDMA core userspace libraries and daemons
> > > # providers/ipathverbs/ Dual licensed using a BSD license with an extra patent clause
> > > # providers/rxe/ Incorporates code from ipathverbs and contains the patent clause
> > > # providers/hfi1verbs Uses the 3 Clause BSD license
> > > -License: (GPLv2 or BSD) and (GPLv2 or PathScale-BSD)
> > > +License: GPLv2 or BSD
> >
> > Is this Ok? The Fedora guidelines I read suggested the PathScale
> > license would need to be assigned a short tag, and I'd be surprised if
> > 'BSD' is the right tag due to the patent stuff..
>
> Our standalone libipathverbs has just "GPLv2 or BSD",
I suspect that was a mistake, the difference in the pathscale license
is subtle and several other people assumed it was the cannonical 'BSD'
text...
> and I didn't see anything specific about PathScale, but I may not
> have been looking hard enough or in the right place in the Fedora
> packaging guidelines. Where did you see that?
https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing
'All software in Fedora must be under licenses in the Fedora licensing list.'
The Pathscale license is not in the licensing list. The patent clause
seems legally significant enough to at least ask what tag is OK.
Also, we now have CC0 and MIT licensed files, so I guess a 'and CC0
and MIT' is appropriate too?
> Nah, I think things look good, and we can always keep tweaking as needed,
> this is a solid update to work on top of.
Okay, I'll patch in the changes from this discussion and send the
pull.
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 for-next 00/14][PULL request] Mellanox mlx5 core driver updates 2016-10-25
From: David Miller @ 2016-10-28 17:53 UTC (permalink / raw)
To: saeedm-VPRAkNaXOzVWk0Htik3J/w
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
ogerlitz-VPRAkNaXOzVWk0Htik3J/w, leonro-VPRAkNaXOzVWk0Htik3J/w,
talal-VPRAkNaXOzVWk0Htik3J/w, matanb-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <1477407617-20745-1-git-send-email-saeedm-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
I really disalike pull requests of this form.
You add lots of datastructures and helper functions but no actual
users of these facilities to the driver.
Do this instead:
1) Add TSAR infrastructure
2) Add use of TSAR facilities to the driver
That's one pull request.
I don't care if this is hard, or if there are entanglements with
Infiniband or whatever, you must submit changes in this manner.
I will not accept additions to a driver that don't even get really
used.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH rdma-core 2/4] Move rdma_netlink compat into CMake
From: Nikolova, Tatyana E @ 2016-10-28 18:00 UTC (permalink / raw)
To: Jason Gunthorpe, Doug Ledford,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Steve Wise
In-Reply-To: <1477609570-8087-3-git-send-email-jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Hi Jason,
The patch looks good.
I don't think that RDMA_NL_I40IW enum bellow is necessary.
+enum {
+ RDMA_NL_RDMA_CM = 1,
+ RDMA_NL_IWCM,
+ RDMA_NL_RSVD,
+ RDMA_NL_LS, /* RDMA Local Services */
+ RDMA_NL_I40IW,
+ RDMA_NL_NUM_CLIENTS
+};
Thank you,
Tatyana
-----Original Message-----
From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Jason Gunthorpe
Sent: Thursday, October 27, 2016 6:06 PM
To: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
Subject: [PATCH rdma-core 2/4] Move rdma_netlink compat into CMake
Detect if the distro's rdma_netlink.h is new enough, if not replace it with the built in copy, and eliminate the two loose copies of the header.
The built in copy is from v4.8
Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
| 2 +-
buildlib/fixup-include/rdma-rdma_netlink.h | 225 +++++++++++++++++++++++++++++
ibacm/src/acm.c | 3 -
ibacm/src/acm_netlink.h | 128 ----------------
iwpmd/iwarp_pm.h | 2 +-
iwpmd/iwarp_pm_common.c | 5 -
iwpmd/iwarp_pm_server.c | 4 +-
iwpmd/iwpm_netlink.h | 214 ---------------------------
8 files changed, 229 insertions(+), 354 deletions(-) create mode 100644 buildlib/fixup-include/rdma-rdma_netlink.h
delete mode 100644 ibacm/src/acm_netlink.h delete mode 100644 iwpmd/iwpm_netlink.h
Steve,
Can you check if the changes to iwpmd/iwarp_pm_server.c make sense?
Should we do something to fix the kernel header?
--git a/buildlib/RDMA_LinuxHeaders.cmake b/buildlib/RDMA_LinuxHeaders.cmake
index bd16d8deca72..c67b0a6113d2 100644
--- a/buildlib/RDMA_LinuxHeaders.cmake
+++ b/buildlib/RDMA_LinuxHeaders.cmake
@@ -80,6 +80,6 @@ rdma_check_kheader("rdma/ib_user_verbs.h" "${DEFAULT_TEST}") rdma_check_kheader("rdma/ib_user_sa.h" "${DEFAULT_TEST}") rdma_check_kheader("rdma/ib_user_cm.h" "${DEFAULT_TEST}") rdma_check_kheader("rdma/ib_user_mad.h" "${DEFAULT_TEST}") -rdma_check_kheader("rdma/rdma_netlink.h" "${DEFAULT_TEST}")
+rdma_check_kheader("rdma/rdma_netlink.h" "int main(int argc,const char
+*argv[]) { return RDMA_NL_IWPM_REMOTE_INFO && RDMA_NL_IWCM; }")
rdma_check_kheader("rdma/rdma_user_cm.h" "${DEFAULT_TEST}") rdma_check_kheader("rdma/rdma_user_rxe.h" "${DEFAULT_TEST}") diff --git a/buildlib/fixup-include/rdma-rdma_netlink.h b/buildlib/fixup-include/rdma-rdma_netlink.h
new file mode 100644
index 000000000000..02fe8390c18f
--- /dev/null
+++ b/buildlib/fixup-include/rdma-rdma_netlink.h
@@ -0,0 +1,225 @@
+#ifndef _UAPI_RDMA_NETLINK_H
+#define _UAPI_RDMA_NETLINK_H
+
+#include <linux/types.h>
+
+enum {
+ RDMA_NL_RDMA_CM = 1,
+ RDMA_NL_IWCM,
+ RDMA_NL_RSVD,
+ RDMA_NL_LS, /* RDMA Local Services */
+ RDMA_NL_I40IW,
+ RDMA_NL_NUM_CLIENTS
+};
+
+enum {
+ RDMA_NL_GROUP_CM = 1,
+ RDMA_NL_GROUP_IWPM,
+ RDMA_NL_GROUP_LS,
+ RDMA_NL_NUM_GROUPS
+};
+
+#define RDMA_NL_GET_CLIENT(type) ((type & (((1 << 6) - 1) << 10)) >>
+10) #define RDMA_NL_GET_OP(type) (type & ((1 << 10) - 1)) #define
+RDMA_NL_GET_TYPE(client, op) ((client << 10) + op)
+
+enum {
+ RDMA_NL_RDMA_CM_ID_STATS = 0,
+ RDMA_NL_RDMA_CM_NUM_OPS
+};
+
+enum {
+ RDMA_NL_RDMA_CM_ATTR_SRC_ADDR = 1,
+ RDMA_NL_RDMA_CM_ATTR_DST_ADDR,
+ RDMA_NL_RDMA_CM_NUM_ATTR,
+};
+
+/* iwarp port mapper op-codes */
+enum {
+ RDMA_NL_IWPM_REG_PID = 0,
+ RDMA_NL_IWPM_ADD_MAPPING,
+ RDMA_NL_IWPM_QUERY_MAPPING,
+ RDMA_NL_IWPM_REMOVE_MAPPING,
+ RDMA_NL_IWPM_REMOTE_INFO,
+ RDMA_NL_IWPM_HANDLE_ERR,
+ RDMA_NL_IWPM_MAPINFO,
+ RDMA_NL_IWPM_MAPINFO_NUM,
+ RDMA_NL_IWPM_NUM_OPS
+};
+
+struct rdma_cm_id_stats {
+ __u32 qp_num;
+ __u32 bound_dev_if;
+ __u32 port_space;
+ __s32 pid;
+ __u8 cm_state;
+ __u8 node_type;
+ __u8 port_num;
+ __u8 qp_type;
+};
+
+enum {
+ IWPM_NLA_REG_PID_UNSPEC = 0,
+ IWPM_NLA_REG_PID_SEQ,
+ IWPM_NLA_REG_IF_NAME,
+ IWPM_NLA_REG_IBDEV_NAME,
+ IWPM_NLA_REG_ULIB_NAME,
+ IWPM_NLA_REG_PID_MAX
+};
+
+enum {
+ IWPM_NLA_RREG_PID_UNSPEC = 0,
+ IWPM_NLA_RREG_PID_SEQ,
+ IWPM_NLA_RREG_IBDEV_NAME,
+ IWPM_NLA_RREG_ULIB_NAME,
+ IWPM_NLA_RREG_ULIB_VER,
+ IWPM_NLA_RREG_PID_ERR,
+ IWPM_NLA_RREG_PID_MAX
+
+};
+
+enum {
+ IWPM_NLA_MANAGE_MAPPING_UNSPEC = 0,
+ IWPM_NLA_MANAGE_MAPPING_SEQ,
+ IWPM_NLA_MANAGE_ADDR,
+ IWPM_NLA_MANAGE_MAPPED_LOC_ADDR,
+ IWPM_NLA_RMANAGE_MAPPING_ERR,
+ IWPM_NLA_RMANAGE_MAPPING_MAX
+};
+
+#define IWPM_NLA_MANAGE_MAPPING_MAX 3
+#define IWPM_NLA_QUERY_MAPPING_MAX 4
+#define IWPM_NLA_MAPINFO_SEND_MAX 3
+
+enum {
+ IWPM_NLA_QUERY_MAPPING_UNSPEC = 0,
+ IWPM_NLA_QUERY_MAPPING_SEQ,
+ IWPM_NLA_QUERY_LOCAL_ADDR,
+ IWPM_NLA_QUERY_REMOTE_ADDR,
+ IWPM_NLA_RQUERY_MAPPED_LOC_ADDR,
+ IWPM_NLA_RQUERY_MAPPED_REM_ADDR,
+ IWPM_NLA_RQUERY_MAPPING_ERR,
+ IWPM_NLA_RQUERY_MAPPING_MAX
+};
+
+enum {
+ IWPM_NLA_MAPINFO_REQ_UNSPEC = 0,
+ IWPM_NLA_MAPINFO_ULIB_NAME,
+ IWPM_NLA_MAPINFO_ULIB_VER,
+ IWPM_NLA_MAPINFO_REQ_MAX
+};
+
+enum {
+ IWPM_NLA_MAPINFO_UNSPEC = 0,
+ IWPM_NLA_MAPINFO_LOCAL_ADDR,
+ IWPM_NLA_MAPINFO_MAPPED_ADDR,
+ IWPM_NLA_MAPINFO_MAX
+};
+
+enum {
+ IWPM_NLA_MAPINFO_NUM_UNSPEC = 0,
+ IWPM_NLA_MAPINFO_SEQ,
+ IWPM_NLA_MAPINFO_SEND_NUM,
+ IWPM_NLA_MAPINFO_ACK_NUM,
+ IWPM_NLA_MAPINFO_NUM_MAX
+};
+
+enum {
+ IWPM_NLA_ERR_UNSPEC = 0,
+ IWPM_NLA_ERR_SEQ,
+ IWPM_NLA_ERR_CODE,
+ IWPM_NLA_ERR_MAX
+};
+
+/*
+ * Local service operations:
+ * RESOLVE - The client requests the local service to resolve a path.
+ * SET_TIMEOUT - The local service requests the client to set the timeout.
+ * IP_RESOLVE - The client requests the local service to resolve an IP to GID.
+ */
+enum {
+ RDMA_NL_LS_OP_RESOLVE = 0,
+ RDMA_NL_LS_OP_SET_TIMEOUT,
+ RDMA_NL_LS_OP_IP_RESOLVE,
+ RDMA_NL_LS_NUM_OPS
+};
+
+/* Local service netlink message flags */
+#define RDMA_NL_LS_F_ERR 0x0100 /* Failed response */
+
+/*
+ * Local service resolve operation family header.
+ * The layout for the resolve operation:
+ * nlmsg header
+ * family header
+ * attributes
+ */
+
+/*
+ * Local service path use:
+ * Specify how the path(s) will be used.
+ * ALL - For connected CM operation (6 pathrecords)
+ * UNIDIRECTIONAL - For unidirectional UD (1 pathrecord)
+ * GMP - For miscellaneous GMP like operation (at least 1 reversible
+ * pathrecord)
+ */
+enum {
+ LS_RESOLVE_PATH_USE_ALL = 0,
+ LS_RESOLVE_PATH_USE_UNIDIRECTIONAL,
+ LS_RESOLVE_PATH_USE_GMP,
+ LS_RESOLVE_PATH_USE_MAX
+};
+
+#define LS_DEVICE_NAME_MAX 64
+
+struct rdma_ls_resolve_header {
+ __u8 device_name[LS_DEVICE_NAME_MAX];
+ __u8 port_num;
+ __u8 path_use;
+};
+
+struct rdma_ls_ip_resolve_header {
+ __u32 ifindex;
+};
+
+/* Local service attribute type */
+#define RDMA_NLA_F_MANDATORY (1 << 13)
+#define RDMA_NLA_TYPE_MASK (~(NLA_F_NESTED | NLA_F_NET_BYTEORDER | \
+ RDMA_NLA_F_MANDATORY))
+
+/*
+ * Local service attributes:
+ * Attr Name Size Byte order
+ * -----------------------------------------------------
+ * PATH_RECORD struct ib_path_rec_data
+ * TIMEOUT u32 cpu
+ * SERVICE_ID u64 cpu
+ * DGID u8[16] BE
+ * SGID u8[16] BE
+ * TCLASS u8
+ * PKEY u16 cpu
+ * QOS_CLASS u16 cpu
+ * IPV4 u32 BE
+ * IPV6 u8[16] BE
+ */
+enum {
+ LS_NLA_TYPE_UNSPEC = 0,
+ LS_NLA_TYPE_PATH_RECORD,
+ LS_NLA_TYPE_TIMEOUT,
+ LS_NLA_TYPE_SERVICE_ID,
+ LS_NLA_TYPE_DGID,
+ LS_NLA_TYPE_SGID,
+ LS_NLA_TYPE_TCLASS,
+ LS_NLA_TYPE_PKEY,
+ LS_NLA_TYPE_QOS_CLASS,
+ LS_NLA_TYPE_IPV4,
+ LS_NLA_TYPE_IPV6,
+ LS_NLA_TYPE_MAX
+};
+
+/* Local service DGID/SGID attribute: big endian */ struct
+rdma_nla_ls_gid {
+ __u8 gid[16];
+};
+
+#endif /* _UAPI_RDMA_NETLINK_H */
diff --git a/ibacm/src/acm.c b/ibacm/src/acm.c index cc7dd065f69c..5f4068f619b4 100644
--- a/ibacm/src/acm.c
+++ b/ibacm/src/acm.c
@@ -61,9 +61,6 @@
#include <ccan/list.h>
#include "acm_mad.h"
#include "acm_util.h"
-#if !defined(RDMA_NL_LS_F_ERR)
- #include "acm_netlink.h"
-#endif
#define src_out data[0]
#define src_index data[1]
diff --git a/ibacm/src/acm_netlink.h b/ibacm/src/acm_netlink.h deleted file mode 100644 index 867ae8c838fc..000000000000 diff --git a/iwpmd/iwarp_pm.h b/iwpmd/iwarp_pm.h index b5a5a457a423..fc09e4fd752a 100644
--- a/iwpmd/iwarp_pm.h
+++ b/iwpmd/iwarp_pm.h
@@ -53,7 +53,7 @@
#include <syslog.h>
#include <netlink/msg.h>
#include <ccan/list.h>
-#include "iwpm_netlink.h"
+#include <rdma/rdma_netlink.h>
#define IWARP_PM_PORT 3935
#define IWARP_PM_VER_SHIFT 6
diff --git a/iwpmd/iwarp_pm_common.c b/iwpmd/iwarp_pm_common.c index 58b1089a1998..941e0406ade7 100644
--- a/iwpmd/iwarp_pm_common.c
+++ b/iwpmd/iwarp_pm_common.c
@@ -33,11 +33,6 @@
#include "iwarp_pm.h"
-/* Necessary only for SLES11 */
-#if !defined (NETLINK_RDMA)
- #define NETLINK_RDMA 20
-#endif
-
/* iwpm config params */
static const char * iwpm_param_names[IWPM_PARAM_NUM] =
{ "nl_sock_rbuf_size" };
diff --git a/iwpmd/iwarp_pm_server.c b/iwpmd/iwarp_pm_server.c index ab90c6c4b077..ef541c8175ed 100644
--- a/iwpmd/iwarp_pm_server.c
+++ b/iwpmd/iwarp_pm_server.c
@@ -1214,8 +1214,8 @@ static int init_iwpm_clients(__u32 iwarp_clients[]) {
int client_num = 2;
- iwarp_clients[0] = RDMA_NL_NES;
- iwarp_clients[1] = RDMA_NL_C4IW;
+ iwarp_clients[0] = RDMA_NL_IWCM;
+ iwarp_clients[1] = RDMA_NL_IWCM+1; /* Legacy RDMA_NL_C4IW for old
+kernels */
return client_num;
}
diff --git a/iwpmd/iwpm_netlink.h b/iwpmd/iwpm_netlink.h deleted file mode 100644 index 0edcb620de99..000000000000
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH 11/12] nvme: Use BLK_MQ_S_STOPPED instead of QUEUE_FLAG_STOPPED in blk-mq code
From: Bart Van Assche @ 2016-10-28 18:51 UTC (permalink / raw)
To: Keith Busch
Cc: Jens Axboe, Christoph Hellwig, James Bottomley,
Martin K. Petersen, Mike Snitzer, Doug Ledford, Ming Lei,
Laurence Oberman, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-nvme@lists.infradead.org
In-Reply-To: <20161028160145.GC5621@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 2248 bytes --]
On 10/28/2016 08:51 AM, Keith Busch wrote:
> On Wed, Oct 26, 2016 at 03:56:04PM -0700, Bart Van Assche wrote:
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index 7bb73ba..b662416 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -205,7 +205,7 @@ void nvme_requeue_req(struct request *req)
>>
>> blk_mq_requeue_request(req, false);
>> spin_lock_irqsave(req->q->queue_lock, flags);
>> - if (!blk_queue_stopped(req->q))
>> + if (!blk_mq_queue_stopped(req->q))
>> blk_mq_kick_requeue_list(req->q);
>> spin_unlock_irqrestore(req->q->queue_lock, flags);
>> }
>> @@ -2079,10 +2079,6 @@ void nvme_stop_queues(struct nvme_ctrl *ctrl)
>>
>> mutex_lock(&ctrl->namespaces_mutex);
>> list_for_each_entry(ns, &ctrl->namespaces, list) {
>> - spin_lock_irq(ns->queue->queue_lock);
>> - queue_flag_set(QUEUE_FLAG_STOPPED, ns->queue);
>> - spin_unlock_irq(ns->queue->queue_lock);
>> -
>> blk_mq_cancel_requeue_work(ns->queue);
>> blk_mq_stop_hw_queues(ns->queue);
>
> There's actually a reason the queue stoppage is using a different flag
> than blk_mq_queue_stopped: kicking the queue starts stopped queues.
> The driver has to ensure the requeue work can't be kicked prior to
> cancelling the current requeue work. Once we know requeue work isn't
> running and can't restart again, then we're safe to stop the hw queues.
>
> It's a pretty obscure condition, requiring the controller post an
> error completion at the same time the driver decides to reset the
> controller. Here's the sequence with the wrong outcome:
>
> CPU A CPU B
> ----- -----
> nvme_stop_queues nvme_requeue_req
> blk_mq_stop_hw_queues if (blk_mq_queue_stopped) <- returns false
> blk_mq_stop_hw_queue blk_mq_kick_requeue_list
> blk_mq_requeue_work
> blk_mq_start_hw_queues
Hello Keith,
I think it is wrong that kicking the requeue list starts stopped queues
because this makes it impossible to stop request processing without
setting an additional flag next to BLK_MQ_S_STOPPED. Can you have a look
at the attached two patches? These patches survive my dm-multipath and
SCSI tests.
Thanks,
Bart.
[-- Attachment #2: 0001-block-Avoid-that-requeueing-starts-stopped-queues.patch --]
[-- Type: text/x-patch, Size: 3538 bytes --]
>From e93799f726485a3eeee98837c992c5c0068d7180 Mon Sep 17 00:00:00 2001
From: Bart Van Assche <bart.vanassche@sandisk.com>
Date: Fri, 28 Oct 2016 10:48:58 -0700
Subject: [PATCH 1/2] block: Avoid that requeueing starts stopped queues
Since blk_mq_requeue_work() starts stopped queues and since
execution of this function can be scheduled after a queue has
been stopped it is not possible to stop queues without using
an additional state variable to track whether or not the queue
has been stopped. Hence modify blk_mq_requeue_work() such that it
does not start stopped queues. My conclusion after a review of
the blk_mq_stop_hw_queues() and blk_mq_{delay_,}kick_requeue_list()
callers is as follows:
* In the dm driver starting and stopping queues should only happen
if __dm_suspend() or __dm_resume() is called and not if the
requeue list is processed.
* In the SCSI core queue stopping and starting should only be
performed by the scsi_internal_device_block() and
scsi_internal_device_unblock() functions but not by any other
function.
* In the NVMe core only the functions that call
blk_mq_start_stopped_hw_queues() explicitly should start stopped
queues.
* A blk_mq_start_stopped_hwqueues() call must be added in the
xen-blkfront driver in its blkif_recover() function.
---
block/blk-mq.c | 6 +-----
drivers/block/xen-blkfront.c | 1 +
drivers/md/dm-rq.c | 7 +------
drivers/scsi/scsi_lib.c | 1 -
4 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index a49b8af..24dfd0d 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -528,11 +528,7 @@ static void blk_mq_requeue_work(struct work_struct *work)
blk_mq_insert_request(rq, false, false, false);
}
- /*
- * Use the start variant of queue running here, so that running
- * the requeue work will kick stopped queues.
- */
- blk_mq_start_hw_queues(q);
+ blk_mq_run_hw_queues(q, false);
}
void blk_mq_add_to_requeue_list(struct request *rq, bool at_head,
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 1ca702d..a3e1727 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -2045,6 +2045,7 @@ static int blkif_recover(struct blkfront_info *info)
BUG_ON(req->nr_phys_segments > segs);
blk_mq_requeue_request(req, false);
}
+ blk_mq_start_stopped_hwqueues(info->rq);
blk_mq_kick_requeue_list(info->rq);
while ((bio = bio_list_pop(&info->bio_list)) != NULL) {
diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index 107ed19..b951ae83 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -326,12 +326,7 @@ static void dm_old_requeue_request(struct request *rq)
static void __dm_mq_kick_requeue_list(struct request_queue *q, unsigned long msecs)
{
- unsigned long flags;
-
- spin_lock_irqsave(q->queue_lock, flags);
- if (!blk_mq_queue_stopped(q))
- blk_mq_delay_kick_requeue_list(q, msecs);
- spin_unlock_irqrestore(q->queue_lock, flags);
+ blk_mq_delay_kick_requeue_list(q, msecs);
}
void dm_mq_kick_requeue_list(struct mapped_device *md)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 4cddaff..94f54ac 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1939,7 +1939,6 @@ static int scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
out:
switch (ret) {
case BLK_MQ_RQ_QUEUE_BUSY:
- blk_mq_stop_hw_queue(hctx);
if (atomic_read(&sdev->device_busy) == 0 &&
!scsi_device_blocked(sdev))
blk_mq_delay_queue(hctx, SCSI_QUEUE_DELAY);
--
2.10.1
[-- Attachment #3: 0002-blk-mq-Remove-blk_mq_cancel_requeue_work.patch --]
[-- Type: text/x-patch, Size: 2638 bytes --]
>From 47eec3bdcf4b673e3ab606543cb3acdf7f4de593 Mon Sep 17 00:00:00 2001
From: Bart Van Assche <bart.vanassche@sandisk.com>
Date: Fri, 28 Oct 2016 10:50:04 -0700
Subject: [PATCH 2/2] blk-mq: Remove blk_mq_cancel_requeue_work()
Since blk_mq_requeue_work() no longer restarts stopped queues
canceling requeue work is no longer needed to prevent that a
stopped queue would be restarted. Hence remove this function.
---
block/blk-mq.c | 6 ------
drivers/md/dm-rq.c | 2 --
drivers/nvme/host/core.c | 1 -
include/linux/blk-mq.h | 1 -
4 files changed, 10 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 24dfd0d..1aa79e5 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -557,12 +557,6 @@ void blk_mq_add_to_requeue_list(struct request *rq, bool at_head,
}
EXPORT_SYMBOL(blk_mq_add_to_requeue_list);
-void blk_mq_cancel_requeue_work(struct request_queue *q)
-{
- cancel_delayed_work_sync(&q->requeue_work);
-}
-EXPORT_SYMBOL_GPL(blk_mq_cancel_requeue_work);
-
void blk_mq_kick_requeue_list(struct request_queue *q)
{
kblockd_schedule_delayed_work(&q->requeue_work, 0);
diff --git a/drivers/md/dm-rq.c b/drivers/md/dm-rq.c
index b951ae83..7f426ab 100644
--- a/drivers/md/dm-rq.c
+++ b/drivers/md/dm-rq.c
@@ -102,8 +102,6 @@ static void dm_mq_stop_queue(struct request_queue *q)
if (blk_mq_queue_stopped(q))
return;
- /* Avoid that requeuing could restart the queue. */
- blk_mq_cancel_requeue_work(q);
blk_mq_stop_hw_queues(q);
/* Wait until dm_mq_queue_rq() has finished. */
blk_mq_quiesce_queue(q);
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index d6ab9a0..a67e815 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2075,7 +2075,6 @@ void nvme_stop_queues(struct nvme_ctrl *ctrl)
list_for_each_entry(ns, &ctrl->namespaces, list) {
struct request_queue *q = ns->queue;
- blk_mq_cancel_requeue_work(q);
blk_mq_stop_hw_queues(q);
blk_mq_quiesce_queue(q);
}
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 76f6319..35a0af5 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -221,7 +221,6 @@ void __blk_mq_end_request(struct request *rq, int error);
void blk_mq_requeue_request(struct request *rq, bool kick_requeue_list);
void blk_mq_add_to_requeue_list(struct request *rq, bool at_head,
bool kick_requeue_list);
-void blk_mq_cancel_requeue_work(struct request_queue *q);
void blk_mq_kick_requeue_list(struct request_queue *q);
void blk_mq_delay_kick_requeue_list(struct request_queue *q, unsigned long msecs);
void blk_mq_abort_requeue_list(struct request_queue *q);
--
2.10.1
^ permalink raw reply related
* Re: [PATCH 2/3] iopmem : Add a block device driver for PCIe attached IO memory.
From: Logan Gunthorpe @ 2016-10-28 19:22 UTC (permalink / raw)
To: Christoph Hellwig, Stephen Bates
Cc: jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
sbates-Rgftl6RXld5BDgjK7y7TUQ, haggaie-VPRAkNaXOzVWk0Htik3J/w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-nvdimm-y27Ovi1pjclAfugRpC6u6w, corbet-T1hC0tSOHrs,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jim.macdonald-FgSLVYC75IpWk0Htik3J/w,
linux-block-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, axboe-b10kYP2dOMg
In-Reply-To: <20161028064556.GA3231-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Hi Christoph,
Thanks so much for the detailed review of the code! Even though by the
sounds of things we will be moving to device dax and most of this is
moot. Still, it's great to get some feedback and learn a few things.
I've given some responses below.
On 28/10/16 12:45 AM, Christoph Hellwig wrote:
>> + * This driver is heavily based on drivers/block/pmem.c.
>> + * Copyright (c) 2014, Intel Corporation.
>> + * Copyright (C) 2007 Nick Piggin
>> + * Copyright (C) 2007 Novell Inc.
>
> Is there anything left of it actually? I didn't spot anything
> obvious. Nevermind that we don't have a file with that name anymore :)
Yes, actually there's still a lot of similarities with the current
pmem.c. Though, yes, the path was on oversight. Some of this code is
getting pretty old (it started from an out-of-tree version of pmem.c)
and we've tried our best to track as many of the changes to the pmem.c
as possible. This proved to be difficult. Note: this is now the nvdimm
pmem and not the dax pmem (drivers/nvdimm/pmem.c)
>> + /*
>> + * We can only access the iopmem device with full 32-bit word
>> + * accesses which cannot be gaurantee'd by the regular memcpy
>> + */
>
> Odd comment formatting.
Oops. I'm surprised check_patch didn't pick up on that.
>
>> +static void memcpy_from_iopmem(void *dst, const void *src, size_t sz)
>> +{
>> + u64 *wdst = dst;
>> + const u64 *wsrc = src;
>> + u64 tmp;
>> +
>> + while (sz >= sizeof(*wdst)) {
>> + *wdst++ = *wsrc++;
>> + sz -= sizeof(*wdst);
>> + }
>> +
>> + if (!sz)
>> + return;
>> +
>> + tmp = *wsrc;
>> + memcpy(wdst, &tmp, sz);
>> +}
>
> And then we dod a memcpy here anyway. And no volatile whatsover, so
> the compiler could do anything to it. I defintively feel a bit uneasy
> about having this in the driver as well. Can we define the exact
> semantics for this and define it by the system, possibly in an arch
> specific way?
Yeah, you're right. We should have reviewed this function a bit more.
Anyway, I'd be interested in learning a better approach to forcing a
copy from a mapped BAR with larger widths.
>> +static void iopmem_do_bvec(struct iopmem_device *iopmem, struct page *page,
>> + unsigned int len, unsigned int off, bool is_write,
>> + sector_t sector)
>> +{
>> + phys_addr_t iopmem_off = sector * 512;
>> + void *iopmem_addr = iopmem->virt_addr + iopmem_off;
>> +
>> + if (!is_write) {
>> + read_iopmem(page, off, iopmem_addr, len);
>> + flush_dcache_page(page);
>> + } else {
>> + flush_dcache_page(page);
>> + write_iopmem(iopmem_addr, page, off, len);
>> + }
>
> How about moving the address and offset calculation as well as the
> cache flushing into read_iopmem/write_iopmem and removing this function?
Could do. This was copied from the existing pmem.c and once the bad_pmem
stuff was stripped out this function became relatively simple.
>
>> +static blk_qc_t iopmem_make_request(struct request_queue *q, struct bio *bio)
>> +{
>> + struct iopmem_device *iopmem = q->queuedata;
>> + struct bio_vec bvec;
>> + struct bvec_iter iter;
>> +
>> + bio_for_each_segment(bvec, bio, iter) {
>> + iopmem_do_bvec(iopmem, bvec.bv_page, bvec.bv_len,
>> + bvec.bv_offset, op_is_write(bio_op(bio)),
>> + iter.bi_sector);
>
> op_is_write just checks the data direction. I'd feel much more
> comfortable with a switch on the op, e.g.
That makes sense. This was also copied from pmem.c, so this same change
may make sense there too.
>> +static long iopmem_direct_access(struct block_device *bdev, sector_t sector,
>> + void **kaddr, pfn_t *pfn, long size)
>> +{
>> + struct iopmem_device *iopmem = bdev->bd_queue->queuedata;
>> + resource_size_t offset = sector * 512;
>> +
>> + if (!iopmem)
>> + return -ENODEV;
>
> I don't think this can ever happen, can it?
Yes, I think now that's the case. This is probably a holdover from a
previous version.
> Just use ida_simple_get/ida_simple_remove instead to take care
> of the locking and preloading, and get rid of these two functions.
Thanks, noted. That would be much better. I never found a simple example
of that when I was looking, though I expected there should have been.
>
>> +static int iopmem_attach_disk(struct iopmem_device *iopmem)
>> +{
>> + struct gendisk *disk;
>> + int nid = dev_to_node(iopmem->dev);
>> + struct request_queue *q = iopmem->queue;
>> +
>> + blk_queue_write_cache(q, true, true);
>
> You don't handle flush commands or the fua bit in make_request, so
> this setting seems wrong.
Yup, ok. I'm afraid this is a case of copying without complete
comprehension.
>
>> + int err = 0;
>> + int nid = dev_to_node(&pdev->dev);
>> +
>> + if (pci_enable_device_mem(pdev) < 0) {
>
> propagate the actual error code, please.
Hmm, yup. Not sure why that was missed.
Thanks,
Logan
^ permalink raw reply
* Re: [PATCH rdma-core v2 4/4] redhat/spec: build split rpm packages
From: Jason Gunthorpe @ 2016-10-28 20:57 UTC (permalink / raw)
To: Jarod Wilson; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161028172503.GA28451-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
On Fri, Oct 28, 2016 at 11:25:03AM -0600, Jason Gunthorpe wrote:
> > Nah, I think things look good, and we can always keep tweaking as needed,
> > this is a solid update to work on top of.
>
> Okay, I'll patch in the changes from this discussion and send the
> pull.
I added two commits:
https://github.com/linux-rdma/rdma-core/pull/30
This bit seems important:
--- a/redhat/rdma-core.spec
+++ b/redhat/rdma-core.spec
@@ -267,7 +267,7 @@ install -D -m0644 redhat/rdma.fixup-mtrr.awk %{buildroot}%{_libexecdir}/rdma-fix
install -D -m0755 redhat/rdma.mlx4-setup.sh %{buildroot}%{_libexecdir}/mlx4-setup.sh
# ibacm
-%{buildroot}/%{_bindir}/ib_acme -D . -O
+bin/ib_acme -D . -O
install -D -m0644 ibacm_opts.cfg %{buildroot}%{_sysconfdir}/rdma/
install -D -m0644 redhat/ibacm.service %{buildroot}%{_unitdir}/
--
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