Jason Wang
2022-Jun-08 08:14 UTC
[PATCH] vdpa: allow vdpa dev_del management operation to return failure
Hi Gautam: On Fri, Jun 3, 2022 at 6:34 PM Gautam Dawar <gautam.dawar at amd.com> wrote:> > Currently, the vdpa_nl_cmd_dev_del_set_doit() implementation allows > returning a value to depict the operation status but the return type > of dev_del() callback is void. So, any error while deleting the vdpa > device in the vdpa parent driver can't be returned to the management > layer.I wonder under which cognition we can hit an error in dev_del()? Thanks> This patch changes the return type of dev_del() callback to int to > allow returning an error code in case of failure. > > Signed-off-by: Gautam Dawar <gautam.dawar at amd.com> > --- > drivers/vdpa/ifcvf/ifcvf_main.c | 3 ++- > drivers/vdpa/mlx5/net/mlx5_vnet.c | 3 ++- > drivers/vdpa/vdpa.c | 11 ++++++++--- > drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 3 ++- > drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 3 ++- > drivers/vdpa/vdpa_user/vduse_dev.c | 3 ++- > include/linux/vdpa.h | 5 +++-- > 7 files changed, 21 insertions(+), 10 deletions(-) > > diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c > index 4366320fb68d..6a967935478b 100644 > --- a/drivers/vdpa/ifcvf/ifcvf_main.c > +++ b/drivers/vdpa/ifcvf/ifcvf_main.c > @@ -800,13 +800,14 @@ static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name, > return ret; > } > > -static void ifcvf_vdpa_dev_del(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev) > +static int ifcvf_vdpa_dev_del(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev) > { > struct ifcvf_vdpa_mgmt_dev *ifcvf_mgmt_dev; > > ifcvf_mgmt_dev = container_of(mdev, struct ifcvf_vdpa_mgmt_dev, mdev); > _vdpa_unregister_device(dev); > ifcvf_mgmt_dev->adapter = NULL; > + return 0; > } > > static const struct vdpa_mgmtdev_ops ifcvf_vdpa_mgmt_dev_ops = { > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c > index e0de44000d92..b06204c2f3e8 100644 > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > @@ -2775,7 +2775,7 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name, > return err; > } > > -static void mlx5_vdpa_dev_del(struct vdpa_mgmt_dev *v_mdev, struct vdpa_device *dev) > +static int mlx5_vdpa_dev_del(struct vdpa_mgmt_dev *v_mdev, struct vdpa_device *dev) > { > struct mlx5_vdpa_mgmtdev *mgtdev = container_of(v_mdev, struct mlx5_vdpa_mgmtdev, mgtdev); > struct mlx5_vdpa_dev *mvdev = to_mvdev(dev); > @@ -2788,6 +2788,7 @@ static void mlx5_vdpa_dev_del(struct vdpa_mgmt_dev *v_mdev, struct vdpa_device * > destroy_workqueue(wq); > _vdpa_unregister_device(dev); > mgtdev->ndev = NULL; > + return 0; > } > > static const struct vdpa_mgmtdev_ops mdev_ops = { > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c > index 2b75c00b1005..65dc8bf2f37f 100644 > --- a/drivers/vdpa/vdpa.c > +++ b/drivers/vdpa/vdpa.c > @@ -363,10 +363,11 @@ static int vdpa_match_remove(struct device *dev, void *data) > { > struct vdpa_device *vdev = container_of(dev, struct vdpa_device, dev); > struct vdpa_mgmt_dev *mdev = vdev->mdev; > + int err = 0; > > if (mdev == data) > - mdev->ops->dev_del(mdev, vdev); > - return 0; > + err = mdev->ops->dev_del(mdev, vdev); > + return err; > } > > void vdpa_mgmtdev_unregister(struct vdpa_mgmt_dev *mdev) > @@ -673,7 +674,11 @@ static int vdpa_nl_cmd_dev_del_set_doit(struct sk_buff *skb, struct genl_info *i > goto mdev_err; > } > mdev = vdev->mdev; > - mdev->ops->dev_del(mdev, vdev); > + err = mdev->ops->dev_del(mdev, vdev); > + if (err) { > + NL_SET_ERR_MSG_MOD(info->extack, "ops->dev_del failed"); > + goto dev_err; > + } > mdev_err: > put_device(dev); > dev_err: > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > index 42d401d43911..443d4b94268f 100644 > --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c > @@ -280,12 +280,13 @@ static int vdpasim_blk_dev_add(struct vdpa_mgmt_dev *mdev, const char *name, > return ret; > } > > -static void vdpasim_blk_dev_del(struct vdpa_mgmt_dev *mdev, > +static int vdpasim_blk_dev_del(struct vdpa_mgmt_dev *mdev, > struct vdpa_device *dev) > { > struct vdpasim *simdev = container_of(dev, struct vdpasim, vdpa); > > _vdpa_unregister_device(&simdev->vdpa); > + return 0; > } > > static const struct vdpa_mgmtdev_ops vdpasim_blk_mgmtdev_ops = { > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c > index d5324f6fd8c7..9e5a5ad34e65 100644 > --- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c > @@ -167,12 +167,13 @@ static int vdpasim_net_dev_add(struct vdpa_mgmt_dev *mdev, const char *name, > return ret; > } > > -static void vdpasim_net_dev_del(struct vdpa_mgmt_dev *mdev, > +static int vdpasim_net_dev_del(struct vdpa_mgmt_dev *mdev, > struct vdpa_device *dev) > { > struct vdpasim *simdev = container_of(dev, struct vdpasim, vdpa); > > _vdpa_unregister_device(&simdev->vdpa); > + return 0; > } > > static const struct vdpa_mgmtdev_ops vdpasim_net_mgmtdev_ops = { > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c > index f85d1a08ed87..33ff45e70ff7 100644 > --- a/drivers/vdpa/vdpa_user/vduse_dev.c > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c > @@ -1540,9 +1540,10 @@ static int vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name, > return 0; > } > > -static void vdpa_dev_del(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev) > +static int vdpa_dev_del(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev) > { > _vdpa_unregister_device(dev); > + return 0; > } > > static const struct vdpa_mgmtdev_ops vdpa_dev_mgmtdev_ops = { > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h > index 8943a209202e..e547c9dfdfce 100644 > --- a/include/linux/vdpa.h > +++ b/include/linux/vdpa.h > @@ -443,12 +443,13 @@ void vdpa_set_status(struct vdpa_device *vdev, u8 status); > * @mdev: parent device to use for device removal > * @dev: vdpa device to remove > * Driver need to remove the specified device by calling > - * _vdpa_unregister_device(). > + * _vdpa_unregister_device(). Driver must return 0 > + * on success or appropriate error code in failure case. > */ > struct vdpa_mgmtdev_ops { > int (*dev_add)(struct vdpa_mgmt_dev *mdev, const char *name, > const struct vdpa_dev_set_config *config); > - void (*dev_del)(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev); > + int (*dev_del)(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev); > }; > > /** > -- > 2.30.1 >