Re: [PATCH] nvme: scan sequentially only when list scan unsupported

[Christoph Hellwig <hch@lst.de>]

On Fri, Nov 04, 2022 at 04:36:22PM -0600, Uday Shankar wrote:
> Currently, if nvme_scan_ns_list fails, nvme_scan_work will fall back to
> a sequential scan. nvme_scan_ns_list can fail for a variety of reasons,
> e.g. transient transport issue. And the resulting sequential scan can be
> extremely expensive on controllers reporting an NN value close to the
> maximum allowed (>4 billion). Avoid sequential scans wherever possible
> by only falling back to them if nvme_scan_ns_list fails due to
> controller non-support of Identify NS List.
> ---
> This would break devices that claim to support version NVME_VS(1, 1, 0)
> or above, but don't support Identify NS List. But it looks like we
> already have NVME_QUIRK_IDENTIFY_CNS to deal with that.

Yes.  I'm a little worried about changing something this fundamental
after such a long time, even if it is the right thing to do.  But at
least we get a warning when the Identify NS List fails, so between
that and the fact tht nvme_scan_ns_list logs an error I think we can
cope.

>  	mutex_lock(&ctrl->scan_lock);
> -	if (nvme_scan_ns_list(ctrl) != 0)
> +	if (nvme_scan_ns_list(ctrl) == -EOPNOTSUPP)
>  		nvme_scan_ns_sequential(ctrl);

Maybe make this a bit more clear by lifting the nvme_ctrl_limited_cns
check into nvme_scan_work, though:

	if (nvme_ctrl_limited_cns(ctrl))
		nvme_scan_ns_sequential(ctrl);
	else
		nvme_scan_ns_list(ctrl);

and drop the now superflous error return from nvme_scan_ns_list.