Re: [PATCH v2] nvme: skip the zoned limits update if the zone info query failed

[Chris S]

Hi Christoph,

Thanks for the review.

> Did you do error injections to get here?

Yes, on the device side rather than in the kernel.  The splat comes
from a fuzzing setup where an emulated NVMe controller rewrites
completion status codes.  In the wire log of the run that crashed, the
Identify with CNS 05h and CSI ZNS -- the one nvme_query_zone_info()
issues -- appears twice: it completed successfully at boot, and on a
later rescan the same command was completed with SCT 0 / SC 24h
instead.  That positive status is what falls through the "ret < 0"
check.

> The most common case for this is probably during revalidation of some
> kind, and then we'll just keep the old values.  Which in general
> should be fine, but might be worth mentioning in this comment?

Yes, that is what happens.  queue_limits_start_update() hands back a
copy of the current limits, and nvme_set_chunk_sectors() returns early
for an already zoned queue, so skipping the update leaves the
previously validated zone geometry in place.  I will spell that out in
the comment.

> Also we should probably log a message that we either just register
> the handle or keep the old values?

Will do in v3.  blk_queue_is_zoned() on the not yet committed queue
tells the two cases apart, so the message can say which one it is.  I
was going to use dev_warn() to match the neighbouring zone code -- tell
me if you would rather have dev_warn_once(), since this runs on every
revalidation and a device that keeps failing the command would repeat
it.

Thanks,
Chao