[PATCHv2] nvme: use ctrl state accessor

[Keith Busch]

On Fri, Jan 26, 2024 at 08:25:38AM +0000, Chaitanya Kulkarni wrote:
> On 1/25/24 07:52, Keith Busch wrote:
> > From: Keith Busch <kbusch at kernel.org>
> >
> > The ctrl->state value is updated in another thread using WRITE_ONCE, so
> > ensure all the readers use the appropriate accessor.
> >
> > Signed-off-by: Keith Busch <kbusch at kernel.org>
> > ---
> >
> 
> Keith,
> 
> Not sure if nvme_check_ready() needs nvme_ctrl_state() ?
> 
> consider :-
> 
> * Thread A running timeout handler :-
> nvme_timeout()
>   ends up resetting ctrl state at the end of function using WRITE_ONCE() :-
> 
>   disable:
>             nvme_change_ctrl_state(&dev->ctrl, NVME_CTRL_RESETTING)
>              WRITE_ONCE()
> 
> * Thread B issuing I/O :-
> nvme_queue_rq() or nvme_prep_rq_batch()
>   nvme_check_ready()
>    ctrl->state == NVME_CTRL_LIVE
> 
> Since timeout handler's nvme_change_ctrl_state() call will result in
> updating ctrl->state with WRITE_ONCE(), hence reader in the 
> nvme_check_ready()
> should also read using nvme_ctrl_state()->READ_ONCE() ?
> 
> if it is not needed please ignore above question, looks good to me ..

It should be a READ_ONCE() here. Consider it's an inline function that's
called in a loop (at least from pci), so a compiler may optimize the
read so that it happens just once instead of at each iteration as
expected.

I missed this one, I'll get it added for the next version.