Jason Wang
2022-Nov-14 07:53 UTC
[PATCH 6/7] vdpa/mlx5: Avoid using reslock in event_handler
On Sun, Nov 13, 2022 at 9:45 PM Eli Cohen <elic at nvidia.com> wrote:> > event_handler runs under atomic context and may not acquire reslock. We > can still guarantee that the handler won't be called after suspend by > clearing nb_registered, unregistering the handler and flushing the > workqueue. > > Signed-off-by: Eli Cohen <elic at nvidia.com> > --- > drivers/vdpa/mlx5/net/mlx5_vnet.c | 14 +++----------- > 1 file changed, 3 insertions(+), 11 deletions(-) > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c > index 6e6490c85be2..bebfba530247 100644 > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c > @@ -2872,8 +2872,8 @@ static int mlx5_vdpa_suspend(struct vdpa_device *vdev) > int i; > > down_write(&ndev->reslock); > - mlx5_notifier_unregister(mvdev->mdev, &ndev->nb); > ndev->nb_registered = false; > + mlx5_notifier_unregister(mvdev->mdev, &ndev->nb);I wonder why this can help anything. And if it does, we have simliar logic in mlx5_vdpa_dev_del() do we need to fix that as well? Thanks> flush_workqueue(ndev->mvdev.wq); > for (i = 0; i < ndev->cur_num_vqs; i++) { > mvq = &ndev->vqs[i]; > @@ -3051,7 +3051,7 @@ static void update_carrier(struct work_struct *work) > else > ndev->config.status &= cpu_to_mlx5vdpa16(mvdev, ~VIRTIO_NET_S_LINK_UP); > > - if (ndev->config_cb.callback) > + if (ndev->nb_registered && ndev->config_cb.callback) > ndev->config_cb.callback(ndev->config_cb.private); > > kfree(wqent); > @@ -3068,21 +3068,13 @@ static int event_handler(struct notifier_block *nb, unsigned long event, void *p > switch (eqe->sub_type) { > case MLX5_PORT_CHANGE_SUBTYPE_DOWN: > case MLX5_PORT_CHANGE_SUBTYPE_ACTIVE: > - down_read(&ndev->reslock); > - if (!ndev->nb_registered) { > - up_read(&ndev->reslock); > - return NOTIFY_DONE; > - } > wqent = kzalloc(sizeof(*wqent), GFP_ATOMIC); > - if (!wqent) { > - up_read(&ndev->reslock); > + if (!wqent) > return NOTIFY_DONE; > - } > > wqent->mvdev = &ndev->mvdev; > INIT_WORK(&wqent->work, update_carrier); > queue_work(ndev->mvdev.wq, &wqent->work); > - up_read(&ndev->reslock); > ret = NOTIFY_OK; > break; > default: > -- > 2.38.1 >