* Re: [PATCH infiniband-diags] ibportstate: Fixed peer port probing when using DR routing
From: Dag Moxnes @ 2016-09-26 8:12 UTC (permalink / raw)
To: ira.weiny, Hal Rosenstock
Cc: Knut Omang, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Line Holen
In-Reply-To: <20160926053442.GE29780-W4f6Xiosr+yv7QzWx2u06xL4W9x8LtSr@public.gmane.org>
On 09/26/2016 07:34 AM, ira.weiny wrote:
> On Fri, Sep 23, 2016 at 09:06:45AM -0400, Hal Rosenstock wrote:
>> On 9/23/2016 2:42 AM, Knut Omang wrote:
>>> From: Dag Moxnes <dag.moxnes-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>> Title might be better as:
>> ibportstate: Fixed switch peer port probing when using DR routing
> Agreed.
Thanks for the suggestion. I will change this.
>
>
>>> When querying a remote port
>> on a switch
I will change this.
>>
>>> using ibportportstate,
>> Typo: ibportstate
I will change this.
>>
>>> queries to the peer port
>>> would result in timeouts. The reason for this is that the DR path (or partial
>>> DR path was not correctly constructed.
>> It looks like there are 2 cases fixed below and it's best to describe
>> them. My take on it is:
>> 1. local LID (for switch port 0) is not yet configured
>> 2. SMI requester port (I think this is what you term remote port above)
>> is on local rather than remote switch
>> Is that accurate ?
It is correct that this patch does cover 2 cases.
The two cases are:
1. The user has run ibportstate with a direct route, e.g.
"ibportstate -D 0,1,15 6"
In this case, the current implementation will use a direct route of
"0,6" when probing the peer node. Instead, the portnumber it should
be using the
direct route "0,1,15,6".
2. The other case is actually a workaround for a problem with partial DR
routing on
some of our switches. As this is a workaround for a problem
elsewhere, I release
that it should not be part of the ibportstate utility. I will
remove this part of the
commit.
>>
>>> Signed-off-by: Dag Moxnes <dag.moxnes-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>>> Reviewed-by: Line Holen <line.holen-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>>> Signed-off-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>>> ---
>>> src/ibportstate.c | 36 +++++++++++++++++++++++++++---------
>>> 1 file changed, 27 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/src/ibportstate.c b/src/ibportstate.c
>>> index cb47aa9..cd0f680 100644
>>> --- a/src/ibportstate.c
>>> +++ b/src/ibportstate.c
>>> @@ -1,6 +1,8 @@
>>> /*
>>> * Copyright (c) 2004-2009 Voltaire Inc. All rights reserved.
>>> * Copyright (c) 2010,2011 Mellanox Technologies LTD. All rights reserved.
>>> + * Copyright (c) 2011, 2015, Oracle and/or its affiliates. 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
>>> @@ -655,15 +657,31 @@ int main(int argc, char **argv)
>>>
>>> /* Setup portid for peer port */
>>> memcpy(&peerportid, &portid, sizeof(peerportid));
>>> - peerportid.drpath.cnt = 1;
>>> - peerportid.drpath.p[1] = (uint8_t) portnum;
>>> -
>>> - /* Set DrSLID to local lid */
>>> - if (resolve_self(ibd_ca, ibd_ca_port, &selfportid,
>>> - &selfport, 0) < 0)
>>> - IBEXIT("could not resolve self");
>>> - peerportid.drpath.drslid = (uint16_t) selfportid.lid;
>>> - peerportid.drpath.drdlid = 0xffff;
>>> + if (portid.lid == 0) {
>>> + peerportid.drpath.cnt = portid.drpath.cnt + 1;
> Nit: peerportid.drpath.cnt++;
Thanks I will change that.
>
> Ira
>
>>> + if (peerportid.drpath.cnt == IB_SUBNET_PATH_HOPS_MAX) {
>>> + exit(0);
>> Nit: IBEXIT("Too many hops") rather than exit(0)
Thanks, I will correct that.
>>
>>> + }
>>> + peerportid.drpath.p[peerportid.drpath.cnt] = (uint8_t) portnum;
>>> + } else {
>>> + /* Set DrSLID to local lid */
>>> + if (resolve_portid_str(ibd_ca, ibd_ca_port, &portid, argv[0],
>>> + ibd_dest_type, ibd_sm_id, srcport) < 0)
>>> + IBEXIT("could not resolve self");
>>> +
>>> + peerportid.drpath.cnt = 1;
>>> + peerportid.drpath.p[1] = (uint8_t) portnum;
>>> +
>>> +
>> Nit: Eliminate extra blank line
Thanks, I will correct that.
>>
>>> + /* Use partial LID routing if remote switch */
>>> + if ((portid.lid == selfportid.lid)) {
>>> + peerportid.drpath.drslid = 0xffff;
>>> + peerportid.drpath.drdlid = 0xffff;
>>> + } else {
>>> + peerportid.drpath.drslid = selfportid.lid;
>>> + peerportid.drpath.drdlid = 0xffff;
>>> + }
>> The following can be factored out of the above if/else:
>> peerportid.drpath.drdlid = 0xffff;
Thanks, I will do that (I believe this will removed from the commit as
explained above).
>>
>>> + }
>>>
>>> /* Get peer port NodeInfo to obtain peer port number */
>>> is_peer_switch = get_node_info(&peerportid, data);
>>>
>>> base-commit: 2937cf99350a2e423b705e8b8dd10499796a7b41
>>>
Thanks for the review,
Dag
--
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: Yuval Shaia @ 2016-09-26 7:27 UTC (permalink / raw)
To: Adit Ranadive
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
pv-drivers-pghWNbHTmq7QT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, jhansen-pghWNbHTmq7QT0dZR+AlfA,
asarwade-pghWNbHTmq7QT0dZR+AlfA,
georgezhang-pghWNbHTmq7QT0dZR+AlfA,
bryantan-pghWNbHTmq7QT0dZR+AlfA
In-Reply-To: <8db982c37a0a96b37df02826564474a216922f25.1474759181.git.aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
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.
> + goto err_free_cq_ring;
> + }
> +
> + /* Currently, the driver only supports RoCE V1. */
> + if (!(dev->dsr->caps.gid_types & PVRDMA_GID_TYPE_FLAG_ROCE_V1)) {
> + dev_err(&pdev->dev, "driver needs RoCE v1 support\n");
> + ret = -EINVAL;
Ditto.
> + goto err_free_cq_ring;
> + }
> +
> + /* Paired vmxnet3 will have same bus, slot. But func will be 0 */
> + pdev_net = pci_get_slot(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 0));
> + if (!pdev_net) {
> + dev_err(&pdev->dev, "failed to find paired net device\n");
> + ret = -ENODEV;
> + goto err_free_cq_ring;
> + }
> +
> + if (pdev_net->vendor != PCI_VENDOR_ID_VMWARE ||
> + pdev_net->device != PCI_DEVICE_ID_VMWARE_VMXNET3) {
> + dev_err(&pdev->dev, "failed to find paired vmxnet3 device\n");
> + pci_dev_put(pdev_net);
> + ret = -ENODEV;
> + goto err_free_cq_ring;
> + }
> +
> + dev->netdev = pci_get_drvdata(pdev_net);
> + pci_dev_put(pdev_net);
> + if (!dev->netdev) {
> + dev_err(&pdev->dev, "failed to get vmxnet3 device\n");
> + ret = -ENODEV;
> + goto err_free_cq_ring;
> + }
> +
> + dev_info(&pdev->dev, "paired device to %s\n", dev->netdev->name);
> +
> + /* Interrupt setup */
> + ret = pvrdma_alloc_intrs(dev);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to allocate interrupts\n");
> + ret = -ENOMEM;
> + goto err_netdevice;
> + }
> +
> + /* Allocate UAR table. */
> + ret = pvrdma_uar_table_init(dev);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to allocate UAR table\n");
> + ret = -ENOMEM;
> + goto err_free_intrs;
> + }
> +
> + /* Allocate GID table */
> + dev->sgid_tbl = kcalloc(dev->dsr->caps.gid_tbl_len,
> + sizeof(union ib_gid), GFP_KERNEL);
> + if (!dev->sgid_tbl) {
> + ret = -ENOMEM;
> + goto err_free_uar_table;
> + }
> + dev_dbg(&pdev->dev, "gid table len %d\n", dev->dsr->caps.gid_tbl_len);
> +
> + pvrdma_enable_intrs(dev);
> +
> + /* Activate pvrdma device */
> + pvrdma_write_reg(dev, PVRDMA_REG_CTL, PVRDMA_DEVICE_CTL_ACTIVATE);
> +
> + /* Make sure the write is complete before reading status. */
> + mb();
> +
> + /* Check if device was successfully activated */
> + ret = pvrdma_read_reg(dev, PVRDMA_REG_ERR);
> + if (ret != 0) {
> + dev_err(&pdev->dev, "failed to activate device\n");
> + ret = -EINVAL;
Suggesting -EFAULT.
> + goto err_disable_intr;
> + }
> +
> + /* Register IB device */
> + ret = pvrdma_register_device(dev);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register IB device\n");
> + goto err_disable_intr;
> + }
> +
> + dev->nb_netdev.notifier_call = pvrdma_netdevice_event;
> + ret = register_netdevice_notifier(&dev->nb_netdev);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register netdevice events\n");
> + goto err_unreg_ibdev;
> + }
> +
> + dev_info(&pdev->dev, "attached to device\n");
> + return 0;
> +
> +err_unreg_ibdev:
> + ib_unregister_device(&dev->ib_dev);
> +err_disable_intr:
> + pvrdma_disable_intrs(dev);
> + kfree(dev->sgid_tbl);
> +err_free_uar_table:
> + pvrdma_uar_table_cleanup(dev);
> +err_free_intrs:
> + pvrdma_free_irq(dev);
> + pvrdma_disable_msi_all(dev);
> +err_netdevice:
> + unregister_netdevice_notifier(&dev->nb_netdev);
> +err_free_cq_ring:
> + pvrdma_page_dir_cleanup(dev, &dev->cq_pdir);
> +err_free_async_ring:
> + pvrdma_page_dir_cleanup(dev, &dev->async_pdir);
> +err_free_slots:
> + pvrdma_free_slots(dev);
> +err_free_dsr:
> + dma_free_coherent(&pdev->dev, sizeof(*dev->dsr), dev->dsr,
> + dev->dsrbase);
> +err_uar_unmap:
> + iounmap(dev->driver_uar.map);
> +err_unmap_regs:
> + iounmap(dev->regs);
> +err_free_resource:
> + pci_release_regions(pdev);
> +err_disable_pdev:
> + pci_disable_device(pdev);
> + pci_set_drvdata(pdev, NULL);
> +err_free_device:
> + mutex_lock(&pvrdma_device_list_lock);
> + list_del(&dev->device_link);
> + mutex_unlock(&pvrdma_device_list_lock);
> + ib_dealloc_device(&dev->ib_dev);
> + return ret;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 08/16] IB/pvrdma: Add device command support
From: Yuval Shaia @ 2016-09-26 7:16 UTC (permalink / raw)
To: Adit Ranadive
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
pv-drivers-pghWNbHTmq7QT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, jhansen-pghWNbHTmq7QT0dZR+AlfA,
asarwade-pghWNbHTmq7QT0dZR+AlfA,
georgezhang-pghWNbHTmq7QT0dZR+AlfA,
bryantan-pghWNbHTmq7QT0dZR+AlfA
In-Reply-To: <47e16986707ddbe0ffa15795ef9ae60d15bdd728.1474759181.git.aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
Minor question/suggestion inline.
(sorry for missing it till now).
Yuval
On Sat, Sep 24, 2016 at 04:21:32PM -0700, Adit Ranadive wrote:
> This patch enables posting Verb requests and receiving responses to/from
> the backend PVRDMA emulation layer.
>
> Reviewed-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Jorgen Hansen <jhansen-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Reviewed-by: George Zhang <georgezhang-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Bryan Tan <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> ---
> Changes v4->v5:
> - Moved the timeout to pvrdma_cmd_recv.
> - Added additional response code parameter to pvrdma_cmd_post.
>
> Changes v3->v4:
> - Removed the min check and added a BUILD_BUG_ON check for size.
>
> Changes v2->v3:
> - Converted pvrdma_cmd_recv to inline.
> - Added a min check in the memcpy to cmd_slot.
> - Removed the boolean from pvrdma_cmd_post.
> ---
> drivers/infiniband/hw/pvrdma/pvrdma_cmd.c | 117 ++++++++++++++++++++++++++++++
> 1 file changed, 117 insertions(+)
> create mode 100644 drivers/infiniband/hw/pvrdma/pvrdma_cmd.c
>
> diff --git a/drivers/infiniband/hw/pvrdma/pvrdma_cmd.c b/drivers/infiniband/hw/pvrdma/pvrdma_cmd.c
> new file mode 100644
> index 0000000..21f1af8
> --- /dev/null
> +++ b/drivers/infiniband/hw/pvrdma/pvrdma_cmd.c
> @@ -0,0 +1,117 @@
> +/*
> + * Copyright (c) 2012-2016 VMware, Inc. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of EITHER the GNU General Public License
> + * version 2 as published by the Free Software Foundation or the BSD
> + * 2-Clause License. This program is distributed in the hope that it
> + * will be useful, but WITHOUT ANY WARRANTY; WITHOUT EVEN THE IMPLIED
> + * WARRANTY OF MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE.
> + * See the GNU General Public License version 2 for more details at
> + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program available in the file COPYING in the main
> + * directory of this source tree.
> + *
> + * The BSD 2-Clause License
> + *
> + * 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.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> + * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
> + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
> + * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
> + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
> + * OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include <linux/list.h>
> +
> +#include "pvrdma.h"
> +
> +#define PVRDMA_CMD_TIMEOUT 10000 /* ms */
> +
> +static inline int pvrdma_cmd_recv(struct pvrdma_dev *dev,
> + union pvrdma_cmd_resp *resp,
> + unsigned resp_code)
> +{
> + int err;
> +
> + dev_dbg(&dev->pdev->dev, "receive response from device\n");
> +
> + err = wait_for_completion_interruptible_timeout(&dev->cmd_done,
> + msecs_to_jiffies(PVRDMA_CMD_TIMEOUT));
> + if (err == 0 || err == -ERESTARTSYS) {
> + dev_warn(&dev->pdev->dev,
> + "completion timeout or interrupted\n");
> + return -ETIMEDOUT;
> + }
> +
> + spin_lock(&dev->cmd_lock);
> + memcpy(resp, dev->resp_slot, sizeof(*resp));
> + spin_unlock(&dev->cmd_lock);
> +
> + if (resp->hdr.ack != resp_code) {
> + dev_warn(&dev->pdev->dev,
> + "unknown response %#x expected %#x\n",
> + resp->hdr.ack, resp_code);
> + return -EFAULT;
> + }
> +
> + return 0;
> +}
> +
> +int
> +pvrdma_cmd_post(struct pvrdma_dev *dev, union pvrdma_cmd_req *req,
> + union pvrdma_cmd_resp *resp, unsigned resp_code)
> +{
> + int err;
> +
> + dev_dbg(&dev->pdev->dev, "post request to device\n");
> +
> + /* Serializiation */
> + down(&dev->cmd_sema);
> +
> + BUILD_BUG_ON(sizeof(union pvrdma_cmd_req) !=
> + sizeof(struct pvrdma_cmd_modify_qp));
> +
> + spin_lock(&dev->cmd_lock);
> + memcpy(dev->cmd_slot, req, sizeof(*req));
> + spin_unlock(&dev->cmd_lock);
> +
> + init_completion(&dev->cmd_done);
> + pvrdma_write_reg(dev, PVRDMA_REG_REQUEST, 0);
> +
> + /* Make sure the request is written before reading status. */
> + mb();
> +
> + err = pvrdma_read_reg(dev, PVRDMA_REG_ERR);
> + if (err == 0) {
> + if (resp != NULL)
> + err = pvrdma_cmd_recv(dev, resp, resp_code);
> + } else {
> + dev_warn(&dev->pdev->dev, "failed to write request %d\n", err);
> + }
Please note that callers checks if err<0 while here err!=0 considered as
error.
Looking at similar code in pvrdma_pci_probe i can see that function
"translates" this error to -EINVAL. Suggesting to do the same here
(although -EINVAL is not a good choice, if device can be more friendly with
the return code then we can propagate it up or just print a suitable error
i.e. EAGAIN or ENOMEM otherwise EFAULT should be sufficient).
> +
> + up(&dev->cmd_sema);
> +
> + return err;
> +}
> --
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH rdma-core 2/5] utils: Create utils directory to put all common code and move min/max into it
From: Leon Romanovsky @ 2016-09-26 7:05 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
Cc: jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
yishaih-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <1474786207-2149-3-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 758 bytes --]
On Sun, Sep 25, 2016 at 09:50:04AM +0300, Leon Romanovsky wrote:
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
> CMakeLists.txt | 6 ++++++
> ibacm/linux/osd.h | 5 ++---
> libibverbs/examples/rc_pingpong.c | 8 +------
> libmlx5/src/mlx5.h | 16 ++------------
> librdmacm/src/cma.h | 5 ++---
> utils/math.h | 44 +++++++++++++++++++++++++++++++++++++++
> 6 files changed, 57 insertions(+), 27 deletions(-)
> create mode 100644 utils/math.h
>
>
> +#include "../../utils/math.h"
Jason,
Can you give me a suggestion where and how should I use include_directories
command to be able to write "#include <utils/math.h>"?
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 05/13] Have cmake run man pages through text substitution
From: Leon Romanovsky @ 2016-09-26 7:01 UTC (permalink / raw)
To: Weiny, Ira
Cc: Jason Gunthorpe, Hefty, Sean,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <2807E5FD2F6FDA4886F6618EAC48510E24EEBF1A-8k97q/ur5Z2krb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1862 bytes --]
On Mon, Sep 26, 2016 at 06:30:21AM +0000, Weiny, Ira wrote:
> >
> > On Fri, Sep 23, 2016 at 09:23:42PM +0000, Hefty, Sean wrote:
> > > > rsocket.7 had an errant text substitution that never worked, it is a
> > > > good idea to have the man pages use the correct paths, so let us
> > > > have cmake run them through.
> > > >
> > > > Any man page ending in '.in' will be substituted automatically.
> > > >
> > > > Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> > >
> > > Acked-by: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > >
> > > FWIW, I'm in favor of replacing the man pages with markdown and then
> > > generating the man pages from that. I volunteer someone else to do
> > > that work.
> >
> > I concur, and someone else should do the work ..
> >
> > We could begin by requiring all new man pages to be in markdown format.
> > Do you do this in libfabric? What conversion tool do you like?
> >
> > I've used rst for this in the past with reasonable results..
>
> Infiniband-diags uses rst through rst2man. But if I had to do it over I would probably do it different. rst2man is not always installed and has caused build issues for some people. Whatever tool we use I would recommend it be something common. I'm not too familiar with markdown. Is this really common now?
Among hipsters - yes :)
>
> Ira
>
> >
> > Jason
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the
> > body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at
> > http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* RE: [PATCH 05/13] Have cmake run man pages through text substitution
From: Weiny, Ira @ 2016-09-26 6:30 UTC (permalink / raw)
To: Jason Gunthorpe, Hefty, Sean
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20160923213819.GC8490-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
>
> On Fri, Sep 23, 2016 at 09:23:42PM +0000, Hefty, Sean wrote:
> > > rsocket.7 had an errant text substitution that never worked, it is a
> > > good idea to have the man pages use the correct paths, so let us
> > > have cmake run them through.
> > >
> > > Any man page ending in '.in' will be substituted automatically.
> > >
> > > Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> >
> > Acked-by: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >
> > FWIW, I'm in favor of replacing the man pages with markdown and then
> > generating the man pages from that. I volunteer someone else to do
> > that work.
>
> I concur, and someone else should do the work ..
>
> We could begin by requiring all new man pages to be in markdown format.
> Do you do this in libfabric? What conversion tool do you like?
>
> I've used rst for this in the past with reasonable results..
Infiniband-diags uses rst through rst2man. But if I had to do it over I would probably do it different. rst2man is not always installed and has caused build issues for some people. Whatever tool we use I would recommend it be something common. I'm not too familiar with markdown. Is this really common now?
Ira
>
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the
> body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 2/2] libibmad: configure.ac: Update configure.in to configure.ac
From: ira.weiny-ral2JQCrhuEAvxtiuMwx3w @ 2016-09-26 6:20 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: Ira Weiny
In-Reply-To: <1474870855-21158-1-git-send-email-ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
From: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
To avoid this warning:
automake: warning: autoconf input should be named 'configure.ac', not 'configure.in'
Signed-off-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
configure.ac | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
configure.in | 66 ------------------------------------------------------------
2 files changed, 66 insertions(+), 66 deletions(-)
create mode 100644 configure.ac
delete mode 100644 configure.in
diff --git a/configure.ac b/configure.ac
new file mode 100644
index 000000000000..843dbfd0d6b9
--- /dev/null
+++ b/configure.ac
@@ -0,0 +1,66 @@
+dnl Process this file with autoconf to produce a configure script.
+
+AC_PREREQ(2.57)
+AC_INIT(libibmad, 1.3.12, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org)
+AC_CONFIG_SRCDIR([src/sa.c])
+AC_CONFIG_AUX_DIR(config)
+AC_CONFIG_MACRO_DIR(config)
+AM_CONFIG_HEADER(config.h)
+AM_INIT_AUTOMAKE
+
+AC_SUBST(RELEASE, ${RELEASE:-unknown})
+AC_SUBST(TARBALL, ${TARBALL:-${PACKAGE}-${VERSION}.tar.gz})
+
+dnl the library version info is available in the file: libibmad.ver
+ibmad_api_version=`grep LIBVERSION $srcdir/libibmad.ver | sed 's/LIBVERSION=//'`
+if test -z $ibmad_api_version; then
+ ibmad_api_version=1:0:0
+fi
+AC_SUBST(ibmad_api_version)
+
+AC_ARG_ENABLE(libcheck, [ --disable-libcheck do not test for presence of ib libraries],
+[ if test x$enableval = xno ; then
+ disable_libcheck=yes
+ fi
+])
+
+AM_PROG_LIBTOOL
+
+dnl Checks for programs
+AC_PROG_CC
+
+dnl Checks for libraries
+if test "$disable_libcheck" != "yes"
+then
+AC_CHECK_LIB(ibumad, umad_init, [],
+ AC_MSG_ERROR([umad_init() not found. libibmad requires libibumad.]))
+fi
+
+dnl Checks for header files.
+AC_HEADER_STDC
+AC_CHECK_HEADERS([netinet/in.h stdlib.h string.h sys/time.h unistd.h])
+if test "$disable_libcheck" != "yes"
+then
+AC_CHECK_HEADER(infiniband/umad.h, [],
+ AC_MSG_ERROR([<infiniband/umad.h> not found. libibmad requires libibumad.])
+)
+fi
+
+dnl Checks for library functions
+AC_CHECK_FUNCS([memset strrchr strtol])
+
+dnl Checks for typedefs, structures, and compiler characteristics.
+AC_C_CONST
+AC_C_INLINE
+
+AC_CACHE_CHECK(whether ld accepts --version-script, ac_cv_version_script,
+ if test -n "`$LD --help < /dev/null 2>/dev/null | grep version-script`"; then
+ ac_cv_version_script=yes
+ else
+ ac_cv_version_script=no
+ fi)
+
+AM_CONDITIONAL(HAVE_LD_VERSION_SCRIPT, test "$ac_cv_version_script" = "yes")
+
+AC_CONFIG_FILES([Makefile libibmad.spec])
+AC_OUTPUT
diff --git a/configure.in b/configure.in
deleted file mode 100644
index 843dbfd0d6b9..000000000000
--- a/configure.in
+++ /dev/null
@@ -1,66 +0,0 @@
-dnl Process this file with autoconf to produce a configure script.
-
-AC_PREREQ(2.57)
-AC_INIT(libibmad, 1.3.12, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org)
-AC_CONFIG_SRCDIR([src/sa.c])
-AC_CONFIG_AUX_DIR(config)
-AC_CONFIG_MACRO_DIR(config)
-AM_CONFIG_HEADER(config.h)
-AM_INIT_AUTOMAKE
-
-AC_SUBST(RELEASE, ${RELEASE:-unknown})
-AC_SUBST(TARBALL, ${TARBALL:-${PACKAGE}-${VERSION}.tar.gz})
-
-dnl the library version info is available in the file: libibmad.ver
-ibmad_api_version=`grep LIBVERSION $srcdir/libibmad.ver | sed 's/LIBVERSION=//'`
-if test -z $ibmad_api_version; then
- ibmad_api_version=1:0:0
-fi
-AC_SUBST(ibmad_api_version)
-
-AC_ARG_ENABLE(libcheck, [ --disable-libcheck do not test for presence of ib libraries],
-[ if test x$enableval = xno ; then
- disable_libcheck=yes
- fi
-])
-
-AM_PROG_LIBTOOL
-
-dnl Checks for programs
-AC_PROG_CC
-
-dnl Checks for libraries
-if test "$disable_libcheck" != "yes"
-then
-AC_CHECK_LIB(ibumad, umad_init, [],
- AC_MSG_ERROR([umad_init() not found. libibmad requires libibumad.]))
-fi
-
-dnl Checks for header files.
-AC_HEADER_STDC
-AC_CHECK_HEADERS([netinet/in.h stdlib.h string.h sys/time.h unistd.h])
-if test "$disable_libcheck" != "yes"
-then
-AC_CHECK_HEADER(infiniband/umad.h, [],
- AC_MSG_ERROR([<infiniband/umad.h> not found. libibmad requires libibumad.])
-)
-fi
-
-dnl Checks for library functions
-AC_CHECK_FUNCS([memset strrchr strtol])
-
-dnl Checks for typedefs, structures, and compiler characteristics.
-AC_C_CONST
-AC_C_INLINE
-
-AC_CACHE_CHECK(whether ld accepts --version-script, ac_cv_version_script,
- if test -n "`$LD --help < /dev/null 2>/dev/null | grep version-script`"; then
- ac_cv_version_script=yes
- else
- ac_cv_version_script=no
- fi)
-
-AM_CONDITIONAL(HAVE_LD_VERSION_SCRIPT, test "$ac_cv_version_script" = "yes")
-
-AC_CONFIG_FILES([Makefile libibmad.spec])
-AC_OUTPUT
--
1.8.2
--
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 1/2] libibmad: Makefile.am: Fix INCLUDES warning
From: ira.weiny-ral2JQCrhuEAvxtiuMwx3w @ 2016-09-26 6:20 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: Ira Weiny
In-Reply-To: <1474870855-21158-1-git-send-email-ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
From: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Change INCLUDES to AM_CPPFLAGS to avoid the following warning
Makefile.am:4: warning: 'INCLUDES' is the old name for 'AM_CPPFLAGS' (or '*_CPPFLAGS')
Signed-off-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
Makefile.am | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile.am b/Makefile.am
index b96657eb8d8a..acd0ae219a82 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,7 +1,7 @@
ACLOCAL_AMFLAGS = -I config
SUBDIRS = .
-INCLUDES = -I$(srcdir)/include -I$(includedir)
+AM_CPPFLAGS = -I$(srcdir)/include -I$(includedir)
lib_LTLIBRARIES = libibmad.la
--
1.8.2
--
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 0/2] libibmad: Modernize auto files.
From: ira.weiny-ral2JQCrhuEAvxtiuMwx3w @ 2016-09-26 6:20 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: Ira Weiny
From: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Update the Makefile and configure.in files to avoid autogen warnings.
Ira Weiny (2):
libibmad: Makefile.am: Fix INCLUDES warning
configure.ac: Update configure.in to configure.ac
Makefile.am | 2 +-
configure.ac | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
configure.in | 66 ------------------------------------------------------------
3 files changed, 67 insertions(+), 67 deletions(-)
create mode 100644 configure.ac
delete mode 100644 configure.in
--
1.8.2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 02/16] IB/pvrdma: Add user-level shared functions
From: Leon Romanovsky @ 2016-09-26 6:13 UTC (permalink / raw)
To: Adit Ranadive
Cc: dledford, linux-rdma, pv-drivers, netdev, linux-pci, jhansen,
asarwade, georgezhang, bryantan
In-Reply-To: <9c9f3668-ff2b-f421-2270-3193c0f62cc9@vmware.com>
[-- Attachment #1: Type: text/plain, Size: 9529 bytes --]
On Sun, Sep 25, 2016 at 09:22:11PM -0700, Adit Ranadive wrote:
> On Sun, Sep 25 2016 at 10:26:24AM +0300, Leon Romanovsky wrote:
> > > On Sat, Sep 24, 2016 at 04:21:26PM -0700, Adit Ranadive wrote:
> > > We share some common structures with the user-level driver. This patch adds
> > > those structures and shared functions to traverse the QP/CQ rings.
>
> <...>
>
> > > +
> > > +#include <linux/types.h>
> > > +
> > > +#define PVRDMA_UVERBS_ABI_VERSION 3
> > > +#define PVRDMA_BOARD_ID 1
> > > +#define PVRDMA_REV_ID 1
> > >
> > > Please don't add defines which you are not using in the library and the
> > > two above are not in use.
> > >
>
> I'll move these to the pvrdma.h file.
>
> <...>
>
> > > diff --git a/include/uapi/rdma/pvrdma-uapi.h b/include/uapi/rdma/pvrdma-uapi.h
> > > new file mode 100644
> > > index 0000000..430d8a5
>
> <...>
>
> > > +
> > > +#ifndef __PVRDMA_UAPI_H__
> > > +#define __PVRDMA_UAPI_H__
> > > +
> > > +#include <linux/types.h>
> > > +
> > > +#define PVRDMA_VERSION 17
> > >
> > > What do you plan to do with this VERSION?
> > > How is it related to ABI?
> > >
>
> Not related. This is only for the driver to know which APIs to support.
> For example, an older driver would still be able to work with a newer
> device. I can move this to pvrdma.h as well.
>
> To be honest, I thought I can move this file into the uapi folder since
> the structures here are shared with the user-level library. Based on
> your comments in this thread and the other ones, I think it makes sense
> to move this file back to the pvrdma driver folder and rename it
> (pvrdma_wqe.h?) to avoid confusion. There might still be some duplicate
> code (especially the UAR offsets and WQE structs) here and in our
> user-level library.
>
> Let me know if that makes sense.
>
> > > +
> > > +#define PVRDMA_UAR_HANDLE_MASK 0x00FFFFFF /* Bottom 24 bits. */
> > > +#define PVRDMA_UAR_QP_OFFSET 0 /* Offset of QP doorbell. */
> > > +#define PVRDMA_UAR_QP_SEND BIT(30) /* Send bit. */
> > > +#define PVRDMA_UAR_QP_RECV BIT(31) /* Recv bit. */
> > > +#define PVRDMA_UAR_CQ_OFFSET 4 /* Offset of CQ doorbell. */
> > > +#define PVRDMA_UAR_CQ_ARM_SOL BIT(29) /* Arm solicited bit. */
> > > +#define PVRDMA_UAR_CQ_ARM BIT(30) /* Arm bit. */
> > > +#define PVRDMA_UAR_CQ_POLL BIT(31) /* Poll bit. */
> > > +#define PVRDMA_INVALID_IDX -1 /* Invalid index. */
> > >
> > > +
> > > +/* PVRDMA atomic compare and swap */
> > > +struct pvrdma_exp_cmp_swap {
> > >
> > > _EXP_ looks very similar to MLNX_OFED naming convention.
> > >
>
> Yes, the operation was based on that. Any concerns?
> I can rename this and the one below.
Yes, please.
The common practice in IB subsystem is to use _ex_ notation for such
extended structures.
>
> > > + __u64 swap_val;
> > > + __u64 compare_val;
> > > + __u64 swap_mask;
> > > + __u64 compare_mask;
> > > +};
> > > +
> > > +/* PVRDMA atomic fetch and add */
> > > +struct pvrdma_exp_fetch_add {
> > >
> > > The same as above.
> > >
> > > + __u64 add_val;
> > > + __u64 field_boundary;
> > > +};
> > > +
> > > +/* PVRDMA address vector. */
> > > +struct pvrdma_av {
> > > + __u32 port_pd;
> > > + __u32 sl_tclass_flowlabel;
> > > + __u8 dgid[16];
> > > + __u8 src_path_bits;
> > > + __u8 gid_index;
> > > + __u8 stat_rate;
> > > + __u8 hop_limit;
> > > + __u8 dmac[6];
> > > + __u8 reserved[6];
> > > +};
> > > +
> > > +/* PVRDMA scatter/gather entry */
> > > +struct pvrdma_sge {
> > > + __u64 addr;
> > > + __u32 length;
> > > + __u32 lkey;
> > > +};
> > > +
> > > +/* PVRDMA receive queue work request */
> > > +struct pvrdma_rq_wqe_hdr {
> > > + __u64 wr_id; /* wr id */
> > > + __u32 num_sge; /* size of s/g array */
> > > + __u32 total_len; /* reserved */
> > > +};
> > > +/* Use pvrdma_sge (ib_sge) for receive queue s/g array elements. */
> > > +
> > > +/* PVRDMA send queue work request */
> > > +struct pvrdma_sq_wqe_hdr {
> > > + __u64 wr_id; /* wr id */
> > > + __u32 num_sge; /* size of s/g array */
> > > + __u32 total_len; /* reserved */
> > > + __u32 opcode; /* operation type */
> > > + __u32 send_flags; /* wr flags */
> > > + union {
> > > + __u32 imm_data;
> > > + __u32 invalidate_rkey;
> > > + } ex;
> > > + __u32 reserved;
> > > + union {
> > > + struct {
> > > + __u64 remote_addr;
> > > + __u32 rkey;
> > > + __u8 reserved[4];
> > > + } rdma;
> > > + struct {
> > > + __u64 remote_addr;
> > > + __u64 compare_add;
> > > + __u64 swap;
> > > + __u32 rkey;
> > > + __u32 reserved;
> > > + } atomic;
> > > + struct {
> > > + __u64 remote_addr;
> > > + __u32 log_arg_sz;
> > > + __u32 rkey;
> > > + union {
> > > + struct pvrdma_exp_cmp_swap cmp_swap;
> > > + struct pvrdma_exp_fetch_add fetch_add;
> > > + } wr_data;
> > > + } masked_atomics;
> > > + struct {
> > > + __u64 iova_start;
> > > + __u64 pl_pdir_dma;
> > > + __u32 page_shift;
> > > + __u32 page_list_len;
> > > + __u32 length;
> > > + __u32 access_flags;
> > > + __u32 rkey;
> > > + } fast_reg;
> > > + struct {
> > > + __u32 remote_qpn;
> > > + __u32 remote_qkey;
> > > + struct pvrdma_av av;
> > > + } ud;
> > > + } wr;
> > > +};
> > >
> > > No, I have half-baked patch series which refactors this structure in kernel.
> > > There is no need to put this structure in UAPI.
> > >
>
> This is specific to our device.. We do need to enqueue the WQE in this format
> for the device to recognize it. This is the same format that the user-level
> library will put the WQE in. As I said above, we can move this to the main
> pvrdma driver directory if you prefer.
This is different implementations between kernel and user space.
We don't want to bring user space limitations to kernel.
Take a look here:
http://lxr.free-electrons.com/source/include/rdma/ib_verbs.h#L1192
>
> > > +/* Use pvrdma_sge (ib_sge) for send queue s/g array elements. */
> > > +
> > > +/* Completion queue element. */
> > > +struct pvrdma_cqe {
> > > + __u64 wr_id;
> > > + __u64 qp;
> > > + __u32 opcode;
> > > + __u32 status;
> > > + __u32 byte_len;
> > > + __u32 imm_data;
> > > + __u32 src_qp;
> > > + __u32 wc_flags;
> > > + __u32 vendor_err;
> > > + __u16 pkey_index;
> > > + __u16 slid;
> > > + __u8 sl;
> > > + __u8 dlid_path_bits;
> > > + __u8 port_num;
> > > + __u8 smac[6];
> > > + __u8 reserved2[7]; /* Pad to next power of 2 (64). */
> > > +};
> > > +
> > > +struct pvrdma_ring {
> > > + atomic_t prod_tail; /* Producer tail. */
> > > + atomic_t cons_head; /* Consumer head. */
> > > +};
> > > +
> > > +struct pvrdma_ring_state {
> > > + struct pvrdma_ring tx; /* Tx ring. */
> > > + struct pvrdma_ring rx; /* Rx ring. */
> > > +};
> > > +
> > > +static inline int pvrdma_idx_valid(__u32 idx, __u32 max_elems)
> > > +{
> > > + /* Generates fewer instructions than a less-than. */
> > > + return (idx & ~((max_elems << 1) - 1)) == 0;
> > > +}
> > > +
> > > +static inline __s32 pvrdma_idx(atomic_t *var, __u32 max_elems)
> > > +{
> > > + const unsigned int idx = atomic_read(var);
> > > +
> > > + if (pvrdma_idx_valid(idx, max_elems))
> > > + return idx & (max_elems - 1);
> > > + return PVRDMA_INVALID_IDX;
> > > +}
> > > +
> > > +static inline void pvrdma_idx_ring_inc(atomic_t *var, __u32 max_elems)
> > > +{
> > > + __u32 idx = atomic_read(var) + 1; /* Increment. */
> > >
> > > It is definitely different atomic_read than you expect. From my grep
> > > searches on my machine, linux kernel doesn't export in standard headers
> > > the atomic_* functions and C has their implementation of that functions.
> > >
>
> This would probably change for the user-level library, so no need have this file
> in UAPI.
>
> > > +
> > > + idx &= (max_elems << 1) - 1; /* Modulo size, flip gen. */
> > > + atomic_set(var, idx);
> > > +}
> > > +
> > > +static inline __s32 pvrdma_idx_ring_has_space(const struct pvrdma_ring *r,
> > > + __u32 max_elems, __u32 *out_tail)
> > > +{
> > > + const __u32 tail = atomic_read(&r->prod_tail);
> > > + const __u32 head = atomic_read(&r->cons_head);
> > > +
> > > + if (pvrdma_idx_valid(tail, max_elems) &&
> > > + pvrdma_idx_valid(head, max_elems)) {
> > > + *out_tail = tail & (max_elems - 1);
> > > + return tail != (head ^ max_elems);
> > > + }
> > > + return PVRDMA_INVALID_IDX;
> > > +}
> > > +
> > > +static inline __s32 pvrdma_idx_ring_has_data(const struct pvrdma_ring *r,
> > > + __u32 max_elems, __u32 *out_head)
> > > +{
> > > + const __u32 tail = atomic_read(&r->prod_tail);
> > > + const __u32 head = atomic_read(&r->cons_head);
> > > +
> > > + if (pvrdma_idx_valid(tail, max_elems) &&
> > > + pvrdma_idx_valid(head, max_elems)) {
> > > + *out_head = head & (max_elems - 1);
> > > + return tail != head;
> > > + }
> > > + return PVRDMA_INVALID_IDX;
> > > +}
> > > +
> > > +static inline bool pvrdma_idx_ring_is_valid_idx(const struct pvrdma_ring *r,
> > > + __u32 max_elems, __u32 *idx)
> > > +{
> > > + const __u32 tail = atomic_read(&r->prod_tail);
> > > + const __u32 head = atomic_read(&r->cons_head);
> > > +
> > > + if (pvrdma_idx_valid(tail, max_elems) &&
> > > + pvrdma_idx_valid(head, max_elems) &&
> > > + pvrdma_idx_valid(*idx, max_elems)) {
> > > + if (tail > head && (*idx < tail && *idx >= head))
> > > + return true;
> > > + else if (head > tail && (*idx >= head || *idx < tail))
> > > + return true;
> > > + }
> > > + return false;
> > > +}
> > > +
> > > +#endif /* __PVRDMA_UAPI_H__ */
> > >
> > > I suggest completely remove this file from UAPI headers folder.
> > >
>
> I can move this back to the pvrdma driver folder.
Yes, please.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v5 13/16] IB/pvrdma: Add the main driver module for PVRDMA
From: Leon Romanovsky @ 2016-09-26 6:03 UTC (permalink / raw)
To: Adit Ranadive
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
pv-drivers-pghWNbHTmq7QT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, jhansen-pghWNbHTmq7QT0dZR+AlfA,
asarwade-pghWNbHTmq7QT0dZR+AlfA,
georgezhang-pghWNbHTmq7QT0dZR+AlfA,
bryantan-pghWNbHTmq7QT0dZR+AlfA
In-Reply-To: <da4a09fc-bef1-60ea-f9af-7c505a9af6d6-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3500 bytes --]
On Sun, Sep 25, 2016 at 10:10:43PM -0700, Adit Ranadive wrote:
> On sun, Sep 25 2016 at 10:57:03AM +0300, Leon Romanovsky wrote:
> > On Sat, Sep 24, 2016 at 04:21:37PM -0700, Adit Ranadive wrote:
> > > This patch adds the support to register a RDMA device with the kernel RDMA
> > > stack as well as a kernel module. This also initializes the underlying
> > > virtual PCI device.
> > >
> > > Reviewed-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > > Reviewed-by: Jorgen Hansen <jhansen-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > > Reviewed-by: George Zhang <georgezhang-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > > Reviewed-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > > Reviewed-by: Bryan Tan <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > > Signed-off-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
>
> <...>
>
> > > +
> > > +#include <linux/errno.h>
> > > +#include <linux/inetdevice.h>
> > > +#include <linux/init.h>
> > > +#include <linux/module.h>
> > > +#include <linux/slab.h>
> > > +#include <rdma/ib_addr.h>
> > > +#include <rdma/ib_smi.h>
> > > +#include <rdma/ib_user_verbs.h>
> > > +#include <net/addrconf.h>
> > > +#include <rdma/pvrdma-abi.h>
> > > +
> > > +#include "pvrdma.h"
> > > +
> > > +#define DRV_NAME "pvrdma"
> > > +#define DRV_VERSION "1.0"
> > > +#define DRV_RELDATE "January 1, 2013"
> > > +
> > > +static const char pvrdma_version[] =
> > > + DRV_NAME ": PVRDMA InfiniBand driver v"
> > > + DRV_VERSION " (" DRV_RELDATE ")\n";
> >
> > This is a good example why driver version and reldate are useless in
> > kernel. We are in 2016 and not in 2013. All these data are forgotten to
> > update right after the developer copied it from other pre-historic driver
> > (very common practice in netdev). It is worth to stop to use this totally
> > useless defines.
> >
>
> Sorry, in our case it was the Mellanox mlx4 driver :). Yeah, I can remove
> DRV_VERSION and DRV_RELDATE. I'll keep the MODULE_VERSION since we keep
> track of our virtual driver versions that way. So when there are any changes
> to our driver we typically bump up the version. Not sure if that's a netdev
> or misc device practice and if thats followed here.
They always promise to do it, but after year or two they are stopping.
The funny thing starts when they start debug customer issues in specific
distro with mix of backported fixes/features from old and new kernels.
>
> <...>
>
> > > +static void pvrdma_get_fw_ver_str(struct ib_device *device, char *str,
> > > + size_t str_len)
> > > +{
> > > + struct pvrdma_dev *dev =
> > > + container_of(device, struct pvrdma_dev, ib_dev);
> > > + snprintf(str, str_len, "%d.%d.%d\n",
> > > + (int) (dev->dsr->caps.fw_ver >> 32),
> > > + (int) (dev->dsr->caps.fw_ver >> 16) & 0xffff,
> > > + (int) dev->dsr->caps.fw_ver & 0xffff);
> > > +}
> >
> > Yuval already pointed it to you.
> > Thanks to Ira, we have general function in core to print FW version.
> > Please use them.
> >
>
> If you are referring to commit 5fa76c20458518ed6181adddef2e31c5afc0745c
> then isnt this is exactly what Ira did for the other providers?
> Here, the pvrdma_get_fw_ver_str function is registered as a callback for
> the get_dev_fw_str API. Please our pvrdma_register_device function where
> we register our callbacks with ib_core:
> https://patchwork.kernel.org/patch/9349357/
>
> > > + dev->ib_dev.get_dev_fw_str = pvrdma_get_fw_ver_str;
You are right,
Sorry for that.
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v5 00/16] Add Paravirtual RDMA Driver
From: Leon Romanovsky @ 2016-09-26 5:57 UTC (permalink / raw)
To: Adit Ranadive
Cc: dledford, linux-rdma, pv-drivers, netdev, linux-pci, jhansen,
asarwade, georgezhang, bryantan
In-Reply-To: <9f65ab5c-d8c2-e8e3-9334-5d1865a20dc9@vmware.com>
[-- Attachment #1: Type: text/plain, Size: 544 bytes --]
On Sun, Sep 25, 2016 at 10:25:12PM -0700, Adit Ranadive wrote:
> On Sun, Sep 25 2016 at 10:03:52AM +0300, Leon Romanovsky wrote:
> > On Sat, Sep 24, 2016 at 04:21:24PM -0700, Adit Ranadive wrote:
> >
> > <...>
> >
> > > include/uapi/rdma/pvrdma-abi.h | 99 ++
> > > include/uapi/rdma/pvrdma-uapi.h | 255 +++++
> >
> > As Jason said, you need a very good reason to split and create number of
> > files per-driver in UAPI folder.
>
> I can move the pvrdma-uapi.h back to the pvrdma driver folder.
Yes, please.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v5 16/16] MAINTAINERS: Update for PVRDMA driver
From: Leon Romanovsky @ 2016-09-26 5:56 UTC (permalink / raw)
To: Adit Ranadive
Cc: dledford, linux-rdma, pv-drivers, netdev, linux-pci, jhansen,
asarwade, georgezhang, bryantan
In-Reply-To: <7ddc5906-6ba8-335e-c0b8-eb60427a1be8@vmware.com>
[-- Attachment #1: Type: text/plain, Size: 1722 bytes --]
On Sun, Sep 25, 2016 at 10:22:02PM -0700, Adit Ranadive wrote:
> On Sun, Sep 25 2016 at 10:30:10AM +0300, Leon Romanovsky wrote:
> > On Sat, Sep 24, 2016 at 04:21:40PM -0700, Adit Ranadive wrote:
> > > Add maintainer info for the PVRDMA driver.
> > >
> > > Reviewed-by: Jorgen Hansen <jhansen@vmware.com>
> > > Reviewed-by: George Zhang <georgezhang@vmware.com>
> > > Reviewed-by: Aditya Sarwade <asarwade@vmware.com>
> > > Reviewed-by: Bryan Tan <bryantan@vmware.com>
> > > Signed-off-by: Adit Ranadive <aditr@vmware.com>
> > > ---
> > > Changes v4->v5:
> > > - Added pvrdma files to common UAPI folder.
> > > ---
> > > MAINTAINERS | 9 +++++++++
> > > 1 file changed, 9 insertions(+)
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 87e23cd..5023dc0 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -12615,6 +12615,15 @@ S: Maintained
> > > F: drivers/scsi/vmw_pvscsi.c
> > > F: drivers/scsi/vmw_pvscsi.h
> > >
> > > +VMWARE PVRDMA DRIVER
> > > +M: Adit Ranadive <aditr@vmware.com>
> > > +M: VMware PV-Drivers <pv-drivers@vmware.com>
> > > +L: linux-rdma@vger.kernel.org
> > > +S: Maintained
> > > +F: drivers/infiniband/hw/pvrdma/
> > > +F: include/uapi/rdma/pvrdma-abi.h
> > > +F: include/uapi/rdma/pvrdma-uapi.h
> >
> > Please remove the last two lines, these files will be maintained by
> > Doug.
> >
> > Thanks
> >
>
> Ok. Based on your recent patch series on export vendor specific ABIs,
> the ABI files were added to be maintained [1] by individual driver owners.
> Is that not the case now?
I based my answer on this response from Doug [1], maybe I'm wrong.
[1] http://marc.info/?l=linux-rdma&m=147464894811998&w=2
>
> [1] http://marc.info/?l=linux-rdma&m=147455473718235&w=2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH TRIVIAL infiniband-diags] ibportstate.c: When not switch, port can be router port
From: ira.weiny @ 2016-09-26 5:47 UTC (permalink / raw)
To: Hal Rosenstock; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <f44f2a4f-15f7-6ae5-d1fb-7af0a77aeaee-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
On Fri, Sep 23, 2016 at 09:17:10AM -0400, Hal Rosenstock wrote:
>
> Indicate PortInfo is on CA/RT port rather than just CA port when not
> on Switch port. RT is what's used by ibnetdiscover for router nodes.
>
> Signed-off-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Thanks Applied,
Ira
> ---
> diff --git a/src/ibportstate.c b/src/ibportstate.c
> index cb47aa9..cfb8be7 100644
> --- a/src/ibportstate.c
> +++ b/src/ibportstate.c
> @@ -522,9 +522,9 @@ int main(int argc, char **argv)
> IBEXIT("Can't set M_Key fields on switch port != 0");
>
> if (port_op != QUERY || changed)
> - printf("Initial %s PortInfo:\n", is_switch ? "Switch" : "CA");
> + printf("Initial %s PortInfo:\n", is_switch ? "Switch" : "CA/RT");
> else
> - printf("%s PortInfo:\n", is_switch ? "Switch" : "CA");
> + printf("%s PortInfo:\n", is_switch ? "Switch" : "CA/RT");
> espeed_cap = get_port_info(&portid, data, portnum, is_switch);
> show_port_info(&portid, data, portnum, espeed_cap, is_switch);
> if (is_mlnx_ext_port_info_supported(devid)) {
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH infiniband-diags] ibportstate: Fixed peer port probing when using DR routing
From: ira.weiny @ 2016-09-26 5:34 UTC (permalink / raw)
To: Hal Rosenstock
Cc: Knut Omang, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Line Holen,
Dag Moxnes
In-Reply-To: <01da2b4d-270a-f8c9-3d5f-ec7a4c0b7d22-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
On Fri, Sep 23, 2016 at 09:06:45AM -0400, Hal Rosenstock wrote:
> On 9/23/2016 2:42 AM, Knut Omang wrote:
> > From: Dag Moxnes <dag.moxnes-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>
> Title might be better as:
> ibportstate: Fixed switch peer port probing when using DR routing
Agreed.
>
> >
> > When querying a remote port
>
> on a switch
>
> > using ibportportstate,
>
> Typo: ibportstate
>
> > queries to the peer port
> > would result in timeouts. The reason for this is that the DR path (or partial
> > DR path was not correctly constructed.
>
> It looks like there are 2 cases fixed below and it's best to describe
> them. My take on it is:
> 1. local LID (for switch port 0) is not yet configured
> 2. SMI requester port (I think this is what you term remote port above)
> is on local rather than remote switch
> Is that accurate ?
>
> > Signed-off-by: Dag Moxnes <dag.moxnes-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > Reviewed-by: Line Holen <line.holen-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > Signed-off-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > ---
> > src/ibportstate.c | 36 +++++++++++++++++++++++++++---------
> > 1 file changed, 27 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/ibportstate.c b/src/ibportstate.c
> > index cb47aa9..cd0f680 100644
> > --- a/src/ibportstate.c
> > +++ b/src/ibportstate.c
> > @@ -1,6 +1,8 @@
> > /*
> > * Copyright (c) 2004-2009 Voltaire Inc. All rights reserved.
> > * Copyright (c) 2010,2011 Mellanox Technologies LTD. All rights reserved.
> > + * Copyright (c) 2011, 2015, Oracle and/or its affiliates. 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
> > @@ -655,15 +657,31 @@ int main(int argc, char **argv)
> >
> > /* Setup portid for peer port */
> > memcpy(&peerportid, &portid, sizeof(peerportid));
> > - peerportid.drpath.cnt = 1;
> > - peerportid.drpath.p[1] = (uint8_t) portnum;
> > -
> > - /* Set DrSLID to local lid */
> > - if (resolve_self(ibd_ca, ibd_ca_port, &selfportid,
> > - &selfport, 0) < 0)
> > - IBEXIT("could not resolve self");
> > - peerportid.drpath.drslid = (uint16_t) selfportid.lid;
> > - peerportid.drpath.drdlid = 0xffff;
> > + if (portid.lid == 0) {
> > + peerportid.drpath.cnt = portid.drpath.cnt + 1;
Nit: peerportid.drpath.cnt++;
Ira
> > + if (peerportid.drpath.cnt == IB_SUBNET_PATH_HOPS_MAX) {
> > + exit(0);
>
> Nit: IBEXIT("Too many hops") rather than exit(0)
>
> > + }
> > + peerportid.drpath.p[peerportid.drpath.cnt] = (uint8_t) portnum;
> > + } else {
> > + /* Set DrSLID to local lid */
> > + if (resolve_portid_str(ibd_ca, ibd_ca_port, &portid, argv[0],
> > + ibd_dest_type, ibd_sm_id, srcport) < 0)
> > + IBEXIT("could not resolve self");
> > +
> > + peerportid.drpath.cnt = 1;
> > + peerportid.drpath.p[1] = (uint8_t) portnum;
> > +
> > +
>
> Nit: Eliminate extra blank line
>
> > + /* Use partial LID routing if remote switch */
> > + if ((portid.lid == selfportid.lid)) {
> > + peerportid.drpath.drslid = 0xffff;
> > + peerportid.drpath.drdlid = 0xffff;
> > + } else {
> > + peerportid.drpath.drslid = selfportid.lid;
> > + peerportid.drpath.drdlid = 0xffff;
> > + }
>
> The following can be factored out of the above if/else:
> peerportid.drpath.drdlid = 0xffff;
>
> > + }
> >
> > /* Get peer port NodeInfo to obtain peer port number */
> > is_peer_switch = get_node_info(&peerportid, data);
> >
> > base-commit: 2937cf99350a2e423b705e8b8dd10499796a7b41
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 00/16] Add Paravirtual RDMA Driver
From: Adit Ranadive @ 2016-09-26 5:25 UTC (permalink / raw)
To: Leon Romanovsky
Cc: dledford, linux-rdma, pv-drivers, netdev, linux-pci, jhansen,
asarwade, georgezhang, bryantan
In-Reply-To: <20160925070352.GU4088@leon.nu>
On Sun, Sep 25 2016 at 10:03:52AM +0300, Leon Romanovsky wrote:
> On Sat, Sep 24, 2016 at 04:21:24PM -0700, Adit Ranadive wrote:
>
> <...>
>
> > include/uapi/rdma/pvrdma-abi.h | 99 ++
> > include/uapi/rdma/pvrdma-uapi.h | 255 +++++
>
> As Jason said, you need a very good reason to split and create number of
> files per-driver in UAPI folder.
I can move the pvrdma-uapi.h back to the pvrdma driver folder.
^ permalink raw reply
* Re: [PATCH v5 16/16] MAINTAINERS: Update for PVRDMA driver
From: Adit Ranadive @ 2016-09-26 5:22 UTC (permalink / raw)
To: Leon Romanovsky
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
pv-drivers-pghWNbHTmq7QT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, jhansen-pghWNbHTmq7QT0dZR+AlfA,
asarwade-pghWNbHTmq7QT0dZR+AlfA,
georgezhang-pghWNbHTmq7QT0dZR+AlfA,
bryantan-pghWNbHTmq7QT0dZR+AlfA
In-Reply-To: <20160925073010.GW4088-2ukJVAZIZ/Y@public.gmane.org>
On Sun, Sep 25 2016 at 10:30:10AM +0300, Leon Romanovsky wrote:
> On Sat, Sep 24, 2016 at 04:21:40PM -0700, Adit Ranadive wrote:
> > Add maintainer info for the PVRDMA driver.
> >
> > Reviewed-by: Jorgen Hansen <jhansen-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > Reviewed-by: George Zhang <georgezhang-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > Reviewed-by: Aditya Sarwade <asarwade-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > Reviewed-by: Bryan Tan <bryantan-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > Signed-off-by: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > ---
> > Changes v4->v5:
> > - Added pvrdma files to common UAPI folder.
> > ---
> > MAINTAINERS | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 87e23cd..5023dc0 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -12615,6 +12615,15 @@ S: Maintained
> > F: drivers/scsi/vmw_pvscsi.c
> > F: drivers/scsi/vmw_pvscsi.h
> >
> > +VMWARE PVRDMA DRIVER
> > +M: Adit Ranadive <aditr-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > +M: VMware PV-Drivers <pv-drivers-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org>
> > +L: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > +S: Maintained
> > +F: drivers/infiniband/hw/pvrdma/
> > +F: include/uapi/rdma/pvrdma-abi.h
> > +F: include/uapi/rdma/pvrdma-uapi.h
>
> Please remove the last two lines, these files will be maintained by
> Doug.
>
> Thanks
>
Ok. Based on your recent patch series on export vendor specific ABIs,
the ABI files were added to be maintained [1] by individual driver owners.
Is that not the case now?
[1] http://marc.info/?l=linux-rdma&m=147455473718235&w=2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 13/16] IB/pvrdma: Add the main driver module for PVRDMA
From: Adit Ranadive @ 2016-09-26 5:10 UTC (permalink / raw)
To: Leon Romanovsky
Cc: dledford, linux-rdma, pv-drivers, netdev, linux-pci, jhansen,
asarwade, georgezhang, bryantan
In-Reply-To: <20160925075703.GX4088@leon.nu>
On sun, Sep 25 2016 at 10:57:03AM +0300, Leon Romanovsky wrote:
> On Sat, Sep 24, 2016 at 04:21:37PM -0700, Adit Ranadive wrote:
> > This patch adds the support to register a RDMA device with the kernel RDMA
> > stack as well as a kernel module. This also initializes the underlying
> > virtual PCI device.
> >
> > Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com>
> > Reviewed-by: Jorgen Hansen <jhansen@vmware.com>
> > Reviewed-by: George Zhang <georgezhang@vmware.com>
> > Reviewed-by: Aditya Sarwade <asarwade@vmware.com>
> > Reviewed-by: Bryan Tan <bryantan@vmware.com>
> > Signed-off-by: Adit Ranadive <aditr@vmware.com>
<...>
> > +
> > +#include <linux/errno.h>
> > +#include <linux/inetdevice.h>
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <rdma/ib_addr.h>
> > +#include <rdma/ib_smi.h>
> > +#include <rdma/ib_user_verbs.h>
> > +#include <net/addrconf.h>
> > +#include <rdma/pvrdma-abi.h>
> > +
> > +#include "pvrdma.h"
> > +
> > +#define DRV_NAME "pvrdma"
> > +#define DRV_VERSION "1.0"
> > +#define DRV_RELDATE "January 1, 2013"
> > +
> > +static const char pvrdma_version[] =
> > + DRV_NAME ": PVRDMA InfiniBand driver v"
> > + DRV_VERSION " (" DRV_RELDATE ")\n";
>
> This is a good example why driver version and reldate are useless in
> kernel. We are in 2016 and not in 2013. All these data are forgotten to
> update right after the developer copied it from other pre-historic driver
> (very common practice in netdev). It is worth to stop to use this totally
> useless defines.
>
Sorry, in our case it was the Mellanox mlx4 driver :). Yeah, I can remove
DRV_VERSION and DRV_RELDATE. I'll keep the MODULE_VERSION since we keep
track of our virtual driver versions that way. So when there are any changes
to our driver we typically bump up the version. Not sure if that's a netdev
or misc device practice and if thats followed here.
<...>
> > +static void pvrdma_get_fw_ver_str(struct ib_device *device, char *str,
> > + size_t str_len)
> > +{
> > + struct pvrdma_dev *dev =
> > + container_of(device, struct pvrdma_dev, ib_dev);
> > + snprintf(str, str_len, "%d.%d.%d\n",
> > + (int) (dev->dsr->caps.fw_ver >> 32),
> > + (int) (dev->dsr->caps.fw_ver >> 16) & 0xffff,
> > + (int) dev->dsr->caps.fw_ver & 0xffff);
> > +}
>
> Yuval already pointed it to you.
> Thanks to Ira, we have general function in core to print FW version.
> Please use them.
>
If you are referring to commit 5fa76c20458518ed6181adddef2e31c5afc0745c
then isnt this is exactly what Ira did for the other providers?
Here, the pvrdma_get_fw_ver_str function is registered as a callback for
the get_dev_fw_str API. Please our pvrdma_register_device function where
we register our callbacks with ib_core:
https://patchwork.kernel.org/patch/9349357/
> > + dev->ib_dev.get_dev_fw_str = pvrdma_get_fw_ver_str;
^ permalink raw reply
* Re: [infiniband-diags PATCH 4/4] A few typo fixes in the manpages
From: ira.weiny @ 2016-09-26 4:56 UTC (permalink / raw)
To: Ana Guerrero López; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160905214910.GA9795-2v28OwtJ5VXkvb5ro2YpKQ@public.gmane.org>
On Mon, Sep 05, 2016 at 11:49:10PM +0200, Ana Guerrero López wrote:
> They have been detected by lintian, the Debian package checker
>
> Signed-off-by: Ana Guerrero López <ana-kLLpUjk7vcnYtjvyW6yDsg@public.gmane.org>
Thanks Applied,
Ira
> ---
> doc/rst/iblinkinfo.8.in.rst | 4 ++--
> doc/rst/ibqueryerrors.8.in.rst | 2 +-
> doc/rst/infiniband-diags.8.in.rst | 4 ++--
> doc/rst/rdma-ndd.8.in.rst | 4 ++--
> 4 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/doc/rst/iblinkinfo.8.in.rst b/doc/rst/iblinkinfo.8.in.rst
> index ee2aefb..5f87f37 100644
> --- a/doc/rst/iblinkinfo.8.in.rst
> +++ b/doc/rst/iblinkinfo.8.in.rst
> @@ -57,7 +57,7 @@ The node to start a partial scan can be specified with the following addresses.
>
> **--switch, -S <port_guid>** same as "-G". (provided only for backward compatibility)
>
> -How much of the scan to be printed can be controled with the following.
> +How much of the scan to be printed can be controlled with the following.
>
> **--all, -a**
> Print all nodes found in a partial fabric scan. Normally a
> @@ -81,7 +81,7 @@ Filter downports indicated in a ibnetdiscover cache. If a port was previously
> indicated as down in the specified cache, and is still down, do not output it in the
> resulting output. This option may be particularly useful for environments
> where switches are not fully populated, thus much of the default iblinkinfo
> -info is considered unuseful. See **ibnetdiscover** for information on caching
> +info is considered useless. See **ibnetdiscover** for information on caching
> ibnetdiscover output.
>
>
> diff --git a/doc/rst/ibqueryerrors.8.in.rst b/doc/rst/ibqueryerrors.8.in.rst
> index df2901c..0a7992b 100644
> --- a/doc/rst/ibqueryerrors.8.in.rst
> +++ b/doc/rst/ibqueryerrors.8.in.rst
> @@ -63,7 +63,7 @@ using a QoS aware routing engine as it can cause a credit deadlock.
>
> **CAUTION** clearing data or error counters will occur regardless of if they
> are printed or not. See **--counters** and **--data** for details on
> -controling which counters are printed.
> +controlling which counters are printed.
>
> **--details** include receive error and transmit discard details
>
> diff --git a/doc/rst/infiniband-diags.8.in.rst b/doc/rst/infiniband-diags.8.in.rst
> index 7776a30..be77ce3 100644
> --- a/doc/rst/infiniband-diags.8.in.rst
> +++ b/doc/rst/infiniband-diags.8.in.rst
> @@ -120,8 +120,8 @@ Switch Forwarding Table info
>
> See: ibtracert, ibroute, dump_lfts, dump_mfts, check_lft_balance, ibfindnodesusing
>
> -Peformance counters
> --------------------
> +Performance counters
> +--------------------
>
> See: ibqueryerrors, perfquery
>
> diff --git a/doc/rst/rdma-ndd.8.in.rst b/doc/rst/rdma-ndd.8.in.rst
> index 708a5ab..1d126db 100644
> --- a/doc/rst/rdma-ndd.8.in.rst
> +++ b/doc/rst/rdma-ndd.8.in.rst
> @@ -27,7 +27,7 @@ those changes.
> DETAILS
> =======
>
> -Full operation of this deamon requires kernels which support polling of the
> +Full operation of this daemon requires kernels which support polling of the
> procfs hostname file as well as libudev.
>
> If your system does not support either of these features, the daemon will set
> @@ -37,7 +37,7 @@ the Node Descriptions at start up and then sleep forever.
> Node Description format
> -----------------------
>
> -The deamon uses the nd_format configuration option within the ibdiags.conf
> +The daemon uses the nd_format configuration option within the ibdiags.conf
> file. %h and %d can be used as wildcards in that string to specify the dynamic
> use of <hostname> and <device> respectively.
>
> --
> 2.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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: [infiniband-diags PATCH 3/4] Update the shebang to point to /bin/bash
From: ira.weiny @ 2016-09-26 4:56 UTC (permalink / raw)
To: Ana Guerrero López; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160905214902.GA9765-2v28OwtJ5VXkvb5ro2YpKQ@public.gmane.org>
On Mon, Sep 05, 2016 at 11:49:02PM +0200, Ana Guerrero López wrote:
> The script contains a few bashims and it won't run in systems such as Debian
> and Ubuntu where /bin/sh is not a symlink to /bin/bash
>
> Signed-off-by: Ana Guerrero López <ana-kLLpUjk7vcnYtjvyW6yDsg@public.gmane.org>
Thanks Applied,
Ira
>
> ---
> doc/generate | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/doc/generate b/doc/generate
> index 816b66f..b1339f8 100755
> --- a/doc/generate
> +++ b/doc/generate
> @@ -1,4 +1,4 @@
> -#!/bin/sh
> +#!/bin/bash
>
> docdir=`dirname $0`
> mode=$1
> --
> 2.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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: [infiniband-diags PATCH 2/4] Replace INCLUDES with AM_CPPFLAGS
From: ira.weiny @ 2016-09-26 4:56 UTC (permalink / raw)
To: Ana Guerrero López; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160905214852.GA9682-2v28OwtJ5VXkvb5ro2YpKQ@public.gmane.org>
On Mon, Sep 05, 2016 at 11:48:52PM +0200, Ana Guerrero López wrote:
> Fixes the following warnings:
>
> Makefile.am:4: warning: 'INCLUDES' is the old name for 'AM_CPPFLAGS' (or '*_CPPFLAGS')
>
> libibnetdisc/Makefile.am:4: warning: 'INCLUDES' is the old name for 'AM_CPPFLAGS' (or '*_CPPFLAGS')
>
> Signed-off-by: Ana Guerrero López <ana-kLLpUjk7vcnYtjvyW6yDsg@public.gmane.org>
Thanks Applied,
Ira
> ---
> Makefile.am | 2 +-
> libibnetdisc/Makefile.am | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 3fea79d..1973702 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -1,7 +1,7 @@
> ACLOCAL_AMFLAGS = -I config
> SUBDIRS = libibnetdisc
>
> -INCLUDES = -I$(top_builddir)/include/ -I$(srcdir)/include -I$(includedir) \
> +AM_CPPFLAGS = -I$(top_builddir)/include/ -I$(srcdir)/include -I$(includedir) \
> -I$(includedir)/infiniband -I$(top_srcdir)/libibnetdisc/include
>
> if DEBUG
> diff --git a/libibnetdisc/Makefile.am b/libibnetdisc/Makefile.am
> index 2cfcc33..fee13ca 100644
> --- a/libibnetdisc/Makefile.am
> +++ b/libibnetdisc/Makefile.am
> @@ -1,7 +1,7 @@
>
> #SUBDIRS = .
>
> -INCLUDES = -I$(srcdir)/include -I$(includedir) -I$(includedir)/infiniband
> +AM_CPPFLAGS = -I$(srcdir)/include -I$(includedir) -I$(includedir)/infiniband
>
> lib_LTLIBRARIES = libibnetdisc.la
> sbin_PROGRAMS =
> --
> 2.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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: [infiniband-diags PATCH 1/4] Update AM_INIT_AUTOMAKE to use subdir-objects
From: ira.weiny @ 2016-09-26 4:56 UTC (permalink / raw)
To: Ana Guerrero López; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160905214844.GA9613-2v28OwtJ5VXkvb5ro2YpKQ@public.gmane.org>
On Mon, Sep 05, 2016 at 11:48:44PM +0200, Ana Guerrero López wrote:
> With this option, the objects are placed into the subdirectory of
> the build directory corresponding to the subdirectory of the source file.
> Fixes the following warning:
>
> Makefile.am:103: warning: source file 'src/ibdiag_common.c' is in a subdirectory,
> Makefile.am:103: but option 'subdir-objects' is disabled
> automake: warning: possible forward-incompatibility.
> automake: At least a source file is in a subdirectory, but the 'subdir-objects'
> automake: automake option hasn't been enabled. For now, the corresponding output
> automake: object file(s) will be placed in the top-level directory. However,
> automake: this behaviour will change in future Automake versions: they will
> automake: unconditionally cause object files to be placed in the same subdirectory
> automake: of the corresponding sources.
> automake: You are advised to start using 'subdir-objects' option throughout your
> automake: project, to avoid future incompatibilities.
>
> Signed-off-by: Ana Guerrero López <ana-kLLpUjk7vcnYtjvyW6yDsg@public.gmane.org>
Thanks Applied,
Ira
> ---
> configure.ac | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index 623bc4f..75dbb8d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -5,7 +5,7 @@ AC_INIT(infiniband-diags, 1.6.7, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org)
> AC_CONFIG_AUX_DIR(config)
> AC_CONFIG_MACRO_DIR(config)
> AM_CONFIG_HEADER(config.h)
> -AM_INIT_AUTOMAKE([foreign])
> +AM_INIT_AUTOMAKE([subdir-objects])
>
> AC_SUBST(RELEASE, ${RELEASE:-unknown})
> AC_SUBST(TARBALL, ${TARBALL:-${PACKAGE}-${VERSION}.tar.gz})
> --
> 2.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 02/16] IB/pvrdma: Add user-level shared functions
From: Adit Ranadive @ 2016-09-26 4:22 UTC (permalink / raw)
To: Leon Romanovsky
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
pv-drivers-pghWNbHTmq7QT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, jhansen-pghWNbHTmq7QT0dZR+AlfA,
asarwade-pghWNbHTmq7QT0dZR+AlfA,
georgezhang-pghWNbHTmq7QT0dZR+AlfA,
bryantan-pghWNbHTmq7QT0dZR+AlfA
In-Reply-To: <20160925072624.GV4088-2ukJVAZIZ/Y@public.gmane.org>
On Sun, Sep 25 2016 at 10:26:24AM +0300, Leon Romanovsky wrote:
> > On Sat, Sep 24, 2016 at 04:21:26PM -0700, Adit Ranadive wrote:
> > We share some common structures with the user-level driver. This patch adds
> > those structures and shared functions to traverse the QP/CQ rings.
<...>
> > +
> > +#include <linux/types.h>
> > +
> > +#define PVRDMA_UVERBS_ABI_VERSION 3
> > +#define PVRDMA_BOARD_ID 1
> > +#define PVRDMA_REV_ID 1
> >
> > Please don't add defines which you are not using in the library and the
> > two above are not in use.
> >
I'll move these to the pvrdma.h file.
<...>
> > diff --git a/include/uapi/rdma/pvrdma-uapi.h b/include/uapi/rdma/pvrdma-uapi.h
> > new file mode 100644
> > index 0000000..430d8a5
<...>
> > +
> > +#ifndef __PVRDMA_UAPI_H__
> > +#define __PVRDMA_UAPI_H__
> > +
> > +#include <linux/types.h>
> > +
> > +#define PVRDMA_VERSION 17
> >
> > What do you plan to do with this VERSION?
> > How is it related to ABI?
> >
Not related. This is only for the driver to know which APIs to support.
For example, an older driver would still be able to work with a newer
device. I can move this to pvrdma.h as well.
To be honest, I thought I can move this file into the uapi folder since
the structures here are shared with the user-level library. Based on
your comments in this thread and the other ones, I think it makes sense
to move this file back to the pvrdma driver folder and rename it
(pvrdma_wqe.h?) to avoid confusion. There might still be some duplicate
code (especially the UAR offsets and WQE structs) here and in our
user-level library.
Let me know if that makes sense.
> > +
> > +#define PVRDMA_UAR_HANDLE_MASK 0x00FFFFFF /* Bottom 24 bits. */
> > +#define PVRDMA_UAR_QP_OFFSET 0 /* Offset of QP doorbell. */
> > +#define PVRDMA_UAR_QP_SEND BIT(30) /* Send bit. */
> > +#define PVRDMA_UAR_QP_RECV BIT(31) /* Recv bit. */
> > +#define PVRDMA_UAR_CQ_OFFSET 4 /* Offset of CQ doorbell. */
> > +#define PVRDMA_UAR_CQ_ARM_SOL BIT(29) /* Arm solicited bit. */
> > +#define PVRDMA_UAR_CQ_ARM BIT(30) /* Arm bit. */
> > +#define PVRDMA_UAR_CQ_POLL BIT(31) /* Poll bit. */
> > +#define PVRDMA_INVALID_IDX -1 /* Invalid index. */
> >
> > +
> > +/* PVRDMA atomic compare and swap */
> > +struct pvrdma_exp_cmp_swap {
> >
> > _EXP_ looks very similar to MLNX_OFED naming convention.
> >
Yes, the operation was based on that. Any concerns?
I can rename this and the one below.
> > + __u64 swap_val;
> > + __u64 compare_val;
> > + __u64 swap_mask;
> > + __u64 compare_mask;
> > +};
> > +
> > +/* PVRDMA atomic fetch and add */
> > +struct pvrdma_exp_fetch_add {
> >
> > The same as above.
> >
> > + __u64 add_val;
> > + __u64 field_boundary;
> > +};
> > +
> > +/* PVRDMA address vector. */
> > +struct pvrdma_av {
> > + __u32 port_pd;
> > + __u32 sl_tclass_flowlabel;
> > + __u8 dgid[16];
> > + __u8 src_path_bits;
> > + __u8 gid_index;
> > + __u8 stat_rate;
> > + __u8 hop_limit;
> > + __u8 dmac[6];
> > + __u8 reserved[6];
> > +};
> > +
> > +/* PVRDMA scatter/gather entry */
> > +struct pvrdma_sge {
> > + __u64 addr;
> > + __u32 length;
> > + __u32 lkey;
> > +};
> > +
> > +/* PVRDMA receive queue work request */
> > +struct pvrdma_rq_wqe_hdr {
> > + __u64 wr_id; /* wr id */
> > + __u32 num_sge; /* size of s/g array */
> > + __u32 total_len; /* reserved */
> > +};
> > +/* Use pvrdma_sge (ib_sge) for receive queue s/g array elements. */
> > +
> > +/* PVRDMA send queue work request */
> > +struct pvrdma_sq_wqe_hdr {
> > + __u64 wr_id; /* wr id */
> > + __u32 num_sge; /* size of s/g array */
> > + __u32 total_len; /* reserved */
> > + __u32 opcode; /* operation type */
> > + __u32 send_flags; /* wr flags */
> > + union {
> > + __u32 imm_data;
> > + __u32 invalidate_rkey;
> > + } ex;
> > + __u32 reserved;
> > + union {
> > + struct {
> > + __u64 remote_addr;
> > + __u32 rkey;
> > + __u8 reserved[4];
> > + } rdma;
> > + struct {
> > + __u64 remote_addr;
> > + __u64 compare_add;
> > + __u64 swap;
> > + __u32 rkey;
> > + __u32 reserved;
> > + } atomic;
> > + struct {
> > + __u64 remote_addr;
> > + __u32 log_arg_sz;
> > + __u32 rkey;
> > + union {
> > + struct pvrdma_exp_cmp_swap cmp_swap;
> > + struct pvrdma_exp_fetch_add fetch_add;
> > + } wr_data;
> > + } masked_atomics;
> > + struct {
> > + __u64 iova_start;
> > + __u64 pl_pdir_dma;
> > + __u32 page_shift;
> > + __u32 page_list_len;
> > + __u32 length;
> > + __u32 access_flags;
> > + __u32 rkey;
> > + } fast_reg;
> > + struct {
> > + __u32 remote_qpn;
> > + __u32 remote_qkey;
> > + struct pvrdma_av av;
> > + } ud;
> > + } wr;
> > +};
> >
> > No, I have half-baked patch series which refactors this structure in kernel.
> > There is no need to put this structure in UAPI.
> >
This is specific to our device.. We do need to enqueue the WQE in this format
for the device to recognize it. This is the same format that the user-level
library will put the WQE in. As I said above, we can move this to the main
pvrdma driver directory if you prefer.
> > +/* Use pvrdma_sge (ib_sge) for send queue s/g array elements. */
> > +
> > +/* Completion queue element. */
> > +struct pvrdma_cqe {
> > + __u64 wr_id;
> > + __u64 qp;
> > + __u32 opcode;
> > + __u32 status;
> > + __u32 byte_len;
> > + __u32 imm_data;
> > + __u32 src_qp;
> > + __u32 wc_flags;
> > + __u32 vendor_err;
> > + __u16 pkey_index;
> > + __u16 slid;
> > + __u8 sl;
> > + __u8 dlid_path_bits;
> > + __u8 port_num;
> > + __u8 smac[6];
> > + __u8 reserved2[7]; /* Pad to next power of 2 (64). */
> > +};
> > +
> > +struct pvrdma_ring {
> > + atomic_t prod_tail; /* Producer tail. */
> > + atomic_t cons_head; /* Consumer head. */
> > +};
> > +
> > +struct pvrdma_ring_state {
> > + struct pvrdma_ring tx; /* Tx ring. */
> > + struct pvrdma_ring rx; /* Rx ring. */
> > +};
> > +
> > +static inline int pvrdma_idx_valid(__u32 idx, __u32 max_elems)
> > +{
> > + /* Generates fewer instructions than a less-than. */
> > + return (idx & ~((max_elems << 1) - 1)) == 0;
> > +}
> > +
> > +static inline __s32 pvrdma_idx(atomic_t *var, __u32 max_elems)
> > +{
> > + const unsigned int idx = atomic_read(var);
> > +
> > + if (pvrdma_idx_valid(idx, max_elems))
> > + return idx & (max_elems - 1);
> > + return PVRDMA_INVALID_IDX;
> > +}
> > +
> > +static inline void pvrdma_idx_ring_inc(atomic_t *var, __u32 max_elems)
> > +{
> > + __u32 idx = atomic_read(var) + 1; /* Increment. */
> >
> > It is definitely different atomic_read than you expect. From my grep
> > searches on my machine, linux kernel doesn't export in standard headers
> > the atomic_* functions and C has their implementation of that functions.
> >
This would probably change for the user-level library, so no need have this file
in UAPI.
> > +
> > + idx &= (max_elems << 1) - 1; /* Modulo size, flip gen. */
> > + atomic_set(var, idx);
> > +}
> > +
> > +static inline __s32 pvrdma_idx_ring_has_space(const struct pvrdma_ring *r,
> > + __u32 max_elems, __u32 *out_tail)
> > +{
> > + const __u32 tail = atomic_read(&r->prod_tail);
> > + const __u32 head = atomic_read(&r->cons_head);
> > +
> > + if (pvrdma_idx_valid(tail, max_elems) &&
> > + pvrdma_idx_valid(head, max_elems)) {
> > + *out_tail = tail & (max_elems - 1);
> > + return tail != (head ^ max_elems);
> > + }
> > + return PVRDMA_INVALID_IDX;
> > +}
> > +
> > +static inline __s32 pvrdma_idx_ring_has_data(const struct pvrdma_ring *r,
> > + __u32 max_elems, __u32 *out_head)
> > +{
> > + const __u32 tail = atomic_read(&r->prod_tail);
> > + const __u32 head = atomic_read(&r->cons_head);
> > +
> > + if (pvrdma_idx_valid(tail, max_elems) &&
> > + pvrdma_idx_valid(head, max_elems)) {
> > + *out_head = head & (max_elems - 1);
> > + return tail != head;
> > + }
> > + return PVRDMA_INVALID_IDX;
> > +}
> > +
> > +static inline bool pvrdma_idx_ring_is_valid_idx(const struct pvrdma_ring *r,
> > + __u32 max_elems, __u32 *idx)
> > +{
> > + const __u32 tail = atomic_read(&r->prod_tail);
> > + const __u32 head = atomic_read(&r->cons_head);
> > +
> > + if (pvrdma_idx_valid(tail, max_elems) &&
> > + pvrdma_idx_valid(head, max_elems) &&
> > + pvrdma_idx_valid(*idx, max_elems)) {
> > + if (tail > head && (*idx < tail && *idx >= head))
> > + return true;
> > + else if (head > tail && (*idx >= head || *idx < tail))
> > + return true;
> > + }
> > + return false;
> > +}
> > +
> > +#endif /* __PVRDMA_UAPI_H__ */
> >
> > I suggest completely remove this file from UAPI headers folder.
> >
I can move this back to the pvrdma driver folder.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH rdma-core 4/5] libocrdma: Move ocrdma's list implementation into common directory
From: Jason Gunthorpe @ 2016-09-25 22:05 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Christoph Hellwig, dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, yishaih-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <20160925161315.GD4088-2ukJVAZIZ/Y@public.gmane.org>
On Sun, Sep 25, 2016 at 07:13:15PM +0300, Leon Romanovsky wrote:
> On Sun, Sep 25, 2016 at 07:41:21AM -0700, Christoph Hellwig wrote:
> > This implementation was also copy and pasted from the Linux kernel
> > one, including the kernel document comments, and an incorrect license
> > was added to it. It should be removed just like the Mellanox copy.
>
> Awesome,
> Any suggestion where can we get list.h compliant with BSD license?
> Is this enough [1] ?
>
> [1]
> https://github.com/lattera/freebsd/blob/master/sys/ofed/include/linux/list.h
I recommend CCAN's version, it looks like it would be trivial to
convert to.
https://github.com/rustyrussell/ccan/blob/master/ccan/list/list.h
We should also try and use their macros for the other stuff your
migrated, eg their container_of has more static checking, as does the
min/max, and they have a min/max_t implementation.
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 4/5] libocrdma: Move ocrdma's list implementation into common directory
From: Bart Van Assche @ 2016-09-25 17:50 UTC (permalink / raw)
To: Christoph Hellwig, Leon Romanovsky
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
yishaih-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <20160925162203.GA32434-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
On 09/25/16 09:22, Christoph Hellwig wrote:
> On Sun, Sep 25, 2016 at 07:13:15PM +0300, Leon Romanovsky wrote:
>> Any suggestion where can we get list.h compliant with BSD license?
>> Is this enough [1] ?
>
> That one looks like someone who knew the Linux one very well spend
> a lot of effort obsfucating it after starting with the Linux version :)
>
> If I were the copyright holder of the file I'd be okay with that given
> that the list implementation is pretty much trivial anyway.
>
> But given that I don't know who owns every little bit of the Linux
> lists.h I'd simply avoid anything looking like it and use something
> like the BSD queue.h instead (which glibc also provides, but in a
> horribly outdated version).
How about using <list> from libstdc++? It is allowed to use libstdc++
headers to build non-GPL software. See also
https://gcc.gnu.org/onlinedocs/libstdc++/manual/license.html for
libstdc++ license information.
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
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