[PATCH V3 2/2] nvme: Add support for FW activation without reset

[Arnav Dawn]

On Wednesday 21 June 2017 07:07 PM, Keith Busch wrote:
> On Wed, Jun 21, 2017 at 01:06:31PM +0530, Arnav Dawn wrote:
>> On Monday 19 June 2017 10:35 PM, Keith Busch wrote:
>>> On Sat, Jun 10, 2017 at 12:38:42PM +0530, Arnav Dawn wrote:
>>>
>>>> +				ctrl->fw_act_timeout = jiffies +
>>>> +					msecs_to_jiffies(ctrl->mtfa * 100);
>>> Instead of adding another field to the nvme_ctrl structure, just
>>> calculate the timeout in your nvme_fw_act_work function.
>> intention was to  set fw_act_timeout as soon as the AER is received.
>> Since work could be scheduled after some time, setting timeout in
>> work function would add that delay to it.
> This feature doesn't require such tight constraints. The 100ms sleep
> granularity in your CSTS.PP polling already exceeds the amount of time
> it takes for work to schedule.
i agree, i will update it in next version.
>>>> +			else
>>>> +				ctrl->fw_act_timeout = jiffies +
>>>> +					msecs_to_jiffies(admin_timeout * 1000);
>>>> +
>>>> +			schedule_delayed_work(&ctrl->fw_act_work, 0);
>>> If scheduling with 0 delay, why is this delayed work?
>> I used delayed work so i could use cancel_delayed_work, as cancel_work
>> was not available.
> If you really have a use for cancel_work, you could send a patch to
> export the symbol.
>
> In any case, that's probably not going to do what you want. The work can
> only be cancelled if it hasn't started, and since you start it without
> delay, the work will likely be running. Maybe you want to add some other
> criteria for the nvme_fw_act_work to end early, though I expect CSTS.PP
> to clear if f/w load failed.
you are right, if the device sends Firmware image load error AER,
CSTS.PP is probably cleared, and the nvme_fw_act_work will end.
so i think nothing needs to be done on FW Image load error AER.
>
> One last thing I noticed, it looks like the nvme_fw_act_work will break
> if an IO timeout occurs while it's running since that may reset the
> controller and restart IO queues.
could you please clarify more?
I think ,that, if an IO Timeout resets controller and restarts queue,
the nvme_ctrl_pp_status will return false, ending the nvme_fw_act_work.

Regards
Arnav Dawn