[PATCH] nvme: use ctrl state accessor

[Keith Busch]

On Wed, Jan 24, 2024 at 10:19:50PM +0000, Chaitanya Kulkarni wrote:
> On 1/24/24 09:28, Keith Busch wrote:
> >   nvmf_ctlr_matches_baseopts(struct nvme_ctrl *ctrl,
> >   			struct nvmf_ctrl_options *opts)
> >   {
> > -	if (ctrl->state == NVME_CTRL_DELETING ||
> > -	    ctrl->state == NVME_CTRL_DELETING_NOIO ||
> > -	    ctrl->state == NVME_CTRL_DEAD ||
> > +	enum nvme_ctrl_state state = nvme_ctrl_state(ctrl);
> > +
> > +	if (state == NVME_CTRL_DELETING ||
> > +	    state == NVME_CTRL_DELETING_NOIO ||
> > +	    state == NVME_CTRL_DEAD ||
> >   	    strcmp(opts->subsysnqn, ctrl->opts->subsysnqn) ||
> >   	    strcmp(opts->host->nqn, ctrl->opts->host->nqn) ||
> >   	    !uuid_equal(&opts->host->id, &ctrl->opts->host->id))
> 
> since original code is accessing ctrl->state each time instead of
> separate variable, why not reading from it from nvme_ctrl_state() that
> will have READ_ONCE() for each call ?

I doubt the original compiled to code that really accessed the memory
address 3 times. We don't really want to do that either. We just want
the snapshot to be consistent throughout these checks.

> > @@ -156,7 +156,7 @@ void nvme_kick_requeue_lists(struct nvme_ctrl *ctrl)
> >   		if (!ns->head->disk)
> >   			continue;
> >   		kblockd_schedule_work(&ns->head->requeue_work);
> > -		if (ctrl->state == NVME_CTRL_LIVE)
> > +		if (nvme_ctrl_state(ns->ctrl) == NVME_CTRL_LIVE)
> >   			disk_uevent(ns->head->disk, KOBJ_CHANGE);
> >   	}
> >   	up_read(&ctrl->namespaces_rwsem);
> > @@ -223,13 +223,14 @@ void nvme_mpath_revalidate_paths(struct nvme_ns *ns)
> >   
> >   static bool nvme_path_is_disabled(struct nvme_ns *ns)
> >   {
> > +	enum nvme_ctrl_state state = nvme_ctrl_state(ctrl);
> > +
> 
> nvme_ctrl_state(ns->ctrl) is needed to compile the code, unless I'm 
> totally wrong
> here ...

Yep; I tested the wrong config. Will fixup, thanks for the catch.