Re: [PATCH v2 02/15] IB/pvrdma: Add device command support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 12, 2016 at 12:36:32PM -0700, Adit Ranadive wrote:
> This patch enables posting Verb requests and receiving responses to/from
> the backend PVRDMA emulation layer.
> 
> Reviewed-by: Jorgen Hansen <jhansen@xxxxxxxxxx>
> Reviewed-by: George Zhang <georgezhang@xxxxxxxxxx>
> Reviewed-by: Aditya Sarwade <asarwade@xxxxxxxxxx>
> Reviewed-by: Bryan Tan <bryantan@xxxxxxxxxx>
> Signed-off-by: Adit Ranadive <aditr@xxxxxxxxxx>
> ---
>  drivers/infiniband/hw/pvrdma/pvrdma_cmd.c | 104 ++++++++++++++++++++++++++++++
>  1 file changed, 104 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..c22debf
> --- /dev/null
> +++ b/drivers/infiniband/hw/pvrdma/pvrdma_cmd.c
> @@ -0,0 +1,104 @@
> +/*
> + * 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 int pvrdma_cmd_recv(struct pvrdma_dev *dev, union pvrdma_cmd_resp *resp)
> +{
> +	dev_dbg(&dev->pdev->dev, "receive response from device\n");
> +
> +	spin_lock(&dev->cmd_lock);
> +	memcpy(resp, dev->resp_slot, sizeof(*resp));
> +	spin_unlock(&dev->cmd_lock);

In case your virtual HW supports more than one concurrent commands, if
cmd_lock protects cmd_slot i would expect to have extra resp_lock to
protect resp_slot.
btw, otherwise why not just lock the entire flow:
	memcpy(dev->cmd_slot, req, sizeof(*req));
	pvrdma_write_reg(dev, PVRDMA_REG_REQUEST, 0);
	pvrdma_read_reg(dev, PVRDMA_REG_ERR);
	memcpy(resp, dev->resp_slot, sizeof(*resp));

This might improve performances.

> +
> +	return 0;
> +}
> +
> +int
> +pvrdma_cmd_post(struct pvrdma_dev *dev, union pvrdma_cmd_req *req,
> +		bool expect_resp, union pvrdma_cmd_resp *resp)
> +{
> +	int err;
> +
> +	dev_dbg(&dev->pdev->dev, "post request to device\n");
> +
> +	/* Serializiation */
> +	down(&dev->cmd_sema);
> +
> +	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 (expect_resp) {
> +			err = wait_for_completion_interruptible_timeout(
> +				&dev->cmd_done,
> +				msecs_to_jiffies(PVRDMA_CMD_TIMEOUT));
> +			if (err == 0) {
> +				dev_warn(&dev->pdev->dev,
> +					 "completion timeout\n");
> +				err = -ETIMEDOUT;
> +			} else {
> +				err = pvrdma_cmd_recv(dev, resp);

Maybe it is a silly question but in case of two commands that issued almost
simultaneously, it possible that fast execution of second command will
reach this point first and the reasponse here is of the first one?

> +			}
> +		}
> +	} else {
> +		dev_warn(&dev->pdev->dev, "failed to write request %d\n", err);
> +	}
> +
> +	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@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux