Re: [PATCH v6 08/18] nvme: Implement cross-controller reset recovery

[Daniel Wagner]

On Tue, Oct 20, 2026 at 04:56:24PM -0700, Mohamed Khalfella wrote:
> > This part could be made a bit simpler and there is no need for the local
> > completed variable:
> > 
> > [...]
> >         if (!wait_for_completion_timeout(&ccr.complete, tmo))
> >                 ret = -ETIMEDOUT;
> > 
> > out:
> >         spin_lock_irqsave(&sctrl->lock, flags);
> >         list_del(&ccr.list);
> >         spin_unlock_irqrestore(&sctrl->lock, flags);
> > 
> >         if (ret || !completion_done(&ccr.complete))
> >                 return ret;
> >         if (ccr.ccrs == NVME_CCR_STATUS_SUCCESS)
> >                 return 0;
> >         return -EREMOTEIO;
> > }
> 
> You are right, this is deleting "completed" local variable at the
> expense of checking the ccr.complete again. I find the current code
> easier to read and reason about. I prefer to keep it as it is.

The point is that with the local variable, the reader has to track back
and work out where it is set. I had to do exactly that while reviewing.
Putting !completion_done() in the condition makes it self-documenting
and ties it directly to the wait_for_completion_timeout() above.