Re: parallel nvme format

[Niklas Cassel <Niklas.Cassel@wdc.com>]

On Thu, Dec 14, 2023 at 09:19:03AM -0800, Keith Busch wrote:
> 
> Hi Niklas!

Hello Keith!

> 
> Thanks for the report and detailed analysis. What do you think about
> this idea: if scan_lock is already locked, then some other thread is
> expecting to requeue scan_work shortly, so no need to proceed with the
> scan.

nvme_passthru_start() has:

if (effects & NVME_CMD_EFFECTS_CSE_MASK) {
	mutex_lock(&ctrl->scan_lock);


nvme_passthru_end() has:

if (effects & (NVME_CMD_EFFECTS_NIC | NVME_CMD_EFFECTS_NCC)) {
	nvme_queue_scan(ctrl);


What if the first passthru command (format), has
NVME_CMD_EFFECTS_NIC | NVME_CMD_EFFECTS_NCC
set, but the second passthru command (some other command),
satisfies: if (effects & NVME_CMD_EFFECTS_CSE_MASK)
but not:
if (effects & (NVME_CMD_EFFECTS_NIC | NVME_CMD_EFFECTS_NCC))

In that case, the scanning for the first command can get abandoned,
and the second command would never queue a new scan.

Perhaps we could introduce a new bit, NVME_CTRL_NEEDS_SCAN or similar?
(That gets cleared by nvme_scan_work(), after a scan has been performed.)

I guess for format command specifically, we already have
NVME_AER_NOTICE_NS_CHANGED, but that seems to be cleared before by
nvme_scan_work() before the mutex is acquired.
And I'm not sure if all command that have
effects & (NVME_CMD_EFFECTS_NIC | NVME_CMD_EFFECTS_NCC)
set will trigger a NVME_AER_NOTICE_NS_CHANGED AEN.


Kind regards,
Niklas