[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b94efb3f-8d37-c60c-5bf6-f87d41967da4@grimberg.me>
Date: Wed, 9 Aug 2023 11:00:18 +0300
From: Sagi Grimberg <sagi@...mberg.me>
To: Aurelien Aptel <aaptel@...dia.com>, linux-nvme@...ts.infradead.org,
netdev@...r.kernel.org, hch@....de, kbusch@...nel.org, axboe@...com,
chaitanyak@...dia.com, davem@...emloft.net, kuba@...nel.org
Cc: Or Gerlitz <ogerlitz@...dia.com>, aurelien.aptel@...il.com,
smalin@...dia.com, malin1024@...il.com, yorayz@...dia.com,
borisp@...dia.com, galshalom@...dia.com, mgurtovoy@...dia.com
Subject: Re: [PATCH v12 10/26] nvme-tcp: Deal with netdevice DOWN events
On 7/12/23 19:14, Aurelien Aptel wrote:
> From: Or Gerlitz <ogerlitz@...dia.com>
>
> For ddp setup/teardown and resync, the offloading logic
> uses HW resources at the NIC driver such as SQ and CQ.
>
> These resources are destroyed when the netdevice does down
> and hence we must stop using them before the NIC driver
> destroys them.
>
> Use netdevice notifier for that matter -- offloaded connections
> are stopped before the stack continues to call the NIC driver
> close ndo.
>
> We use the existing recovery flow which has the advantage
> of resuming the offload once the connection is re-set.
>
> This also buys us proper handling for the UNREGISTER event
> b/c our offloading starts in the UP state, and down is always
> there between up to unregister.
>
> Signed-off-by: Or Gerlitz <ogerlitz@...dia.com>
> Signed-off-by: Boris Pismenny <borisp@...dia.com>
> Signed-off-by: Ben Ben-Ishay <benishay@...dia.com>
> Signed-off-by: Yoray Zack <yorayz@...dia.com>
> Signed-off-by: Shai Malin <smalin@...dia.com>
> Signed-off-by: Aurelien Aptel <aaptel@...dia.com>
> Reviewed-by: Chaitanya Kulkarni <kch@...dia.com>
> ---
> drivers/nvme/host/tcp.c | 39 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index df58668cbad6..e68e5da3df76 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -221,6 +221,7 @@ struct nvme_tcp_ctrl {
>
> static LIST_HEAD(nvme_tcp_ctrl_list);
> static DEFINE_MUTEX(nvme_tcp_ctrl_mutex);
> +static struct notifier_block nvme_tcp_netdevice_nb;
> static struct workqueue_struct *nvme_tcp_wq;
> static const struct blk_mq_ops nvme_tcp_mq_ops;
> static const struct blk_mq_ops nvme_tcp_admin_mq_ops;
> @@ -3234,6 +3235,30 @@ static struct nvme_ctrl *nvme_tcp_create_ctrl(struct device *dev,
> return ERR_PTR(ret);
> }
>
> +static int nvme_tcp_netdev_event(struct notifier_block *this,
> + unsigned long event, void *ptr)
> +{
> + struct net_device *ndev = netdev_notifier_info_to_dev(ptr);
> + struct nvme_tcp_ctrl *ctrl;
> +
> + switch (event) {
> + case NETDEV_GOING_DOWN:
> + mutex_lock(&nvme_tcp_ctrl_mutex);
> + list_for_each_entry(ctrl, &nvme_tcp_ctrl_list, list) {
> + if (ndev == ctrl->offloading_netdev)
> + nvme_tcp_error_recovery(&ctrl->ctrl);
> + }
> + mutex_unlock(&nvme_tcp_ctrl_mutex);
> + flush_workqueue(nvme_reset_wq);
In what context is this called? because every time we flush a workqueue,
lockdep finds another reason to complain about something...
Otherwise looks good,
Reviewed-by: Sagi Grimberg <sagi@...mberg.me>
Powered by blists - more mailing lists