Re: [PATCHv3 1/2] Revert "nvme: make keep-alive synchronous operation"

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

On 11/4/24 06:01, Ming Lei wrote:
> On Sun, Nov 03, 2024 at 11:44:41PM +0530, Nilay Shroff wrote:
>> This reverts commit d06923670b5a5f609603d4a9fee4dec02d38de9c.
>>
>> It was realized that the fix implemented to contain the race condition
>> among the keep alive task and the fabric shutdown code path in the
>> commit d06923670b5ia ("nvme: make keep-alive synchronous operation")
>> is not optimal.
> 
> One good commit log is supposed to explain a bit why it isn't optimal
> instead of "It was realized" without any details.
> 
I thought the link included in the patch would help. But I think you were correct,
it would help if we include the brief description here in commit log.
No worries... I would add it and spin a new patch version.
>>
>> We also found that the above race condition is a regression caused due
>> to the changes implemented in commit a54a93d0e359 ("nvme: move stopping
>> keep-alive into nvme_uninit_ctrl()"). So we decided to revert the commit
>> d06923670b5a ("nvme: make keep-alive synchronous operation") and then
>> fix the regression.
>>
>> Link: https://lore.kernel.org/all/196f4013-3bbf-43ff-98b4-9cb2a96c20c2@grimberg.me/
>> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
> 
> The patch itself looks good:
> 
> Reviewed-by: Ming Lei <ming.lei@redhat.com>
> 
> 
Thanks,
--Nilay