[PATCH 4/4] nvme: Add two-pass shutdown support

[Jeremy Allison]

On Mon, Dec 25, 2023 at 11:58:04AM +0200, Sagi Grimberg wrote:
>On 12/21/23 19:22, Jeremy Allison wrote:
>>  EXPORT_SYMBOL_GPL(nvme_disable_ctrl);
>>+int nvme_wait_for_shutdown_cmpl(struct nvme_ctrl *ctrl)
>>+{
>>+	ctrl->ctrl_config &= ~NVME_CC_SHN_MASK;
>>+	ctrl->ctrl_config |= NVME_CC_SHN_NORMAL;
>
>Why is ctrl_config being set again?

Good catch. There's no need for that. I'll fix
in the next iteration, thanks !

>>+EXPORT_SYMBOL_GPL(nvme_wait_for_shutdown_cmpl);
>
>Why export the symbol?

nvme_wait_for_shutdown_cmpl() calls nvme_wait_ready(),
which is a static function defined in drivers/nvme/host/core.c.

The only way to inline the functionality of nvme_wait_for_shutdown_cmpl()
into nvme_shutdown_wait() is to export nvme_wait_ready() instead.

If you're OK with that I can remove the intermediate
function nvme_wait_for_shutdown_cmpl() (and indeed as
you've pointed out there's no need to set ctrl->ctrl_config
again).

>>+static void nvme_shutdown_wait(struct pci_dev *pdev)
>>+{
>>+	struct nvme_dev *dev = pci_get_drvdata(pdev);
>>+
>>+	mutex_lock(&dev->shutdown_lock);
>>+	nvme_wait_for_shutdown_cmpl(&dev->ctrl);
>
>If this is the only call-site? why not just open-code it here?

See above comment.

Thanks for the advice. I'll fix these up in the next
iteration (which probably won't be until after January
2nd due to the Christmas break).