[bug report] nvme-fc: fix io timeout to abort I/O

[bug report] nvme-fc: fix io timeout to abort I/O

[Dan Carpenter]

Hello James Smart,

Commit 52793d62a696 ("nvme-fc: fix io timeout to abort I/O") from Oct
16, 2020 (linux-next), leads to the following Smatch static checker
warning:

	drivers/nvme/host/fc.c:3239 nvme_fc_delete_association()
	warn: mixing irqsave and irq

drivers/nvme/host/fc.c
    3219 static void
    3220 nvme_fc_delete_association(struct nvme_fc_ctrl *ctrl)
    3221 {
    3222         struct nvmefc_ls_rcv_op *disls = NULL;
    3223         unsigned long flags;
    3224 
    3225         if (!test_and_clear_bit(ASSOC_ACTIVE, &ctrl->flags))
    3226                 return;
    3227 
    3228         spin_lock_irqsave(&ctrl->lock, flags);
    3229         set_bit(FCCTRL_TERMIO, &ctrl->flags);
    3230         ctrl->iocnt = 0;
    3231         spin_unlock_irqrestore(&ctrl->lock, flags);
                                                     ^^^^^
    3232 
    3233         __nvme_fc_abort_outstanding_ios(ctrl, false);
    3234 
    3235         /* kill the aens as they are a separate path */
    3236         nvme_fc_abort_aen_ops(ctrl);
    3237 
    3238         /* wait for all io that had to be aborted */
--> 3239         spin_lock_irq(&ctrl->lock);
    3240         wait_event_lock_irq(ctrl->ioabort_wait, ctrl->iocnt == 0, ctrl->lock);
    3241         clear_bit(FCCTRL_TERMIO, &ctrl->flags);
    3242         spin_unlock_irq(&ctrl->lock);
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This will enable IRQs

    3243 
    3244         nvme_fc_term_aen_ops(ctrl);
    3245 
    3246         /*
    3247          * send a Disconnect(association) LS to fc-nvme target
    3248          * Note: could have been sent at top of process, but
    3249          * cleaner on link traffic if after the aborts complete.
    3250          * Note: if association doesn't exist, association_id will be 0
    3251          */
    3252         if (ctrl->association_id)
    3253                 nvme_fc_xmt_disconnect_assoc(ctrl);
    3254 
    3255         spin_lock_irqsave(&ctrl->lock, flags);

So the flags here doesn't work as intended.  It will just enable IRQs
at the end.

    3256         ctrl->association_id = 0;
    3257         disls = ctrl->rcv_disconn;
    3258         ctrl->rcv_disconn = NULL;
    3259         spin_unlock_irqrestore(&ctrl->lock, flags);
    3260         if (disls)
    3261                 /*
    3262                  * if a Disconnect Request was waiting for a response, send
    3263                  * now that all ABTS's have been issued (and are complete).
    3264                  */
    3265                 nvme_fc_xmt_ls_rsp(disls);
    3266 
    3267         if (ctrl->ctrl.tagset) {
    3268                 nvme_fc_delete_hw_io_queues(ctrl);
    3269                 nvme_fc_free_io_queues(ctrl);
    3270         }
    3271 
    3272         __nvme_fc_delete_hw_queue(ctrl, &ctrl->queues[0], 0);
    3273         nvme_fc_free_queue(&ctrl->queues[0]);
    3274 
    3275         /* re-enable the admin_q so anything new can fast fail */
    3276         nvme_unquiesce_admin_queue(&ctrl->ctrl);
    3277 
    3278         /* resume the io queues so that things will fast fail */
    3279         nvme_unquiesce_io_queues(&ctrl->ctrl);
    3280 
    3281         nvme_fc_ctlr_inactive_on_rport(ctrl);
    3282 }

regards,
dan carpenter

Re: [bug report] nvme-fc: fix io timeout to abort I/O

[James Smart]

On 5/8/2024 7:43 AM, Dan Carpenter wrote:
> Hello James Smart,
> 
> Commit 52793d62a696 ("nvme-fc: fix io timeout to abort I/O") from Oct
> 16, 2020 (linux-next), leads to the following Smatch static checker
> warning:
> 
> 	drivers/nvme/host/fc.c:3239 nvme_fc_delete_association()
> 	warn: mixing irqsave and irq
> 

Dan,

fyi - I'm taking a look at this.

Thx

-- james