Re: [PATCH v6 15/18] nvme-fc: Hold inflight requests while in FENCING state

[Mohamed Khalfella]

On Tue 2026-10-20 14:21:59 +0200, Daniel Wagner wrote:
> On Sun, Sep 20, 2026 at 11:28:13AM -0700, Mohamed Khalfella wrote:
> >  static void
> > -nvme_fc_delete_association(struct nvme_fc_ctrl *ctrl)
> > +__nvme_fc_teardown_association(struct nvme_fc_ctrl *ctrl)
> >  {
> >  	struct nvmefc_ls_rcv_op *disls = NULL;
> >  	unsigned long flags;
> >  
> > -	if (!test_and_clear_bit(ASSOC_ACTIVE, &ctrl->flags))
> > -		return;
> > -
> >  	spin_lock_irqsave(&ctrl->lock, flags);
> >  	set_bit(FCCTRL_TERMIO, &ctrl->flags);
> > -	ctrl->iocnt = 0;
> > +	atomic_set(&ctrl->iocnt, 0);
> >  	spin_unlock_irqrestore(&ctrl->lock, flags);
> >  
> >  	__nvme_fc_abort_outstanding_ios(ctrl, false);
> > @@ -3287,11 +3328,18 @@ nvme_fc_delete_association(struct nvme_fc_ctrl *ctrl)
> >  	nvme_fc_abort_aen_ops(ctrl);
> >  
> >  	/* wait for all io that had to be aborted */
> > +	wait_event(ctrl->ioabort_wait, atomic_read(&ctrl->iocnt) == 0);
> >  	spin_lock_irq(&ctrl->lock);
> > -	wait_event_lock_irq(ctrl->ioabort_wait, ctrl->iocnt == 0, ctrl->lock);
> >  	clear_bit(FCCTRL_TERMIO, &ctrl->flags);
> >  	spin_unlock_irq(&ctrl->lock);
> >  
> > +	/*
> > +	 * Wait for a started nvme_fc_fcpio_done() to complete before we
> > +	 * proceed with cancelling inflight requests. This guarantees a
> > +	 * request is completed by one of the two codepaths, not both.
> > +	 */
> > +	synchronize_rcu();
> > +
> 
> The synchronize_rcu() protects against nvme_cancel_tagset() racing with
> an fcpio_done() for an op that completed normally and therefore wasn't
> counted in iocnt. Wouldn't it be clearer to make the hold explicit in
> op->state (e.g. FCPOP_STATE_HELD) and only complete held ops in
> nvme_fc_flush_held_requests(), instead of using RCU as a barrier for
> running callbacks?

Adding new FCPOP_STATE_HELD will result in a more invasive change and it
will result in nvme-fc no using nvme_cancel_tagset(). My intent with
this change is to bring nvme-fc closer to other transport and only
cancel inflight requests. synchronize_rcu() is a simpler in this case.