Re: [PATCH v2 3/3] nvme: use helper nvme_ctrl_state in nvme_keep_alive_finish function

[Nilay Shroff <nilay@linux.ibm.com>]

On 10/8/24 10:49, Damien Le Moal wrote:
> On 10/8/24 13:13, Nilay Shroff wrote:
>> We no more need acquiring ctrl->lock for accessing the NVMe controller
>> state and instead we can now use the helper nvme_ctrl_state. So replace
>> the ctrl->lock from nvme_keep_alive_finish function with nvme_ctrl_state
>> call.
>>
>> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
>> ---
>>  drivers/nvme/host/core.c | 7 ++-----
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index 736adbf65ef5..5a690cf16e5e 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -1296,10 +1296,10 @@ static void nvme_keep_alive_finish(struct request *rq,
>>  				blk_status_t status,
>>  				struct nvme_ctrl *ctrl)
>>  {
>> -	unsigned long flags;
>>  	bool startka = false;
>>  	unsigned long rtt = jiffies - (rq->deadline - rq->timeout);
>>  	unsigned long delay = nvme_keep_alive_work_period(ctrl);
>> +	enum nvme_ctrl_state state = nvme_ctrl_state(ctrl);
>>  
>>  	/*
>>  	 * Subtract off the keepalive RTT so nvme_keep_alive_work runs
>> @@ -1322,11 +1322,8 @@ static void nvme_keep_alive_finish(struct request *rq,
>>  
>>  	ctrl->ka_last_check_time = jiffies;
>>  	ctrl->comp_seen = false;
>> -	spin_lock_irqsave(&ctrl->lock, flags);
>> -	if (ctrl->state == NVME_CTRL_LIVE ||
>> -	    ctrl->state == NVME_CTRL_CONNECTING)
>> +	if (state == NVME_CTRL_LIVE || state == NVME_CTRL_CONNECTING)
>>  		startka = true;
> 
> 	startka = state == NVME_CTRL_LIVE || state == NVME_CTRL_CONNECTING;
> 
> But do you even need that variable now ? The below "if (startka)" could be
> replaced with "if (state == NVME_CTRL_LIVE || state == NVME_CTRL_CONNECTING)", no ?
> 
>> -	spin_unlock_irqrestore(&ctrl->lock, flags);
>>  	if (startka)
>>  		queue_delayed_work(nvme_wq, &ctrl->ka_work, delay);
>>  }
> 
> 
Oops yeah you were correct, we don't need @startka. I will spin a new version. 
Thanks for your comment!

--Nilay