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

Chris S <[email protected]>
Newsgroups org.infradead.lists.linux-nvme,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <CACd_6n0FoZo88kqzN0X-n6aOXKTeqDL5XwdgEaxGVkfGti2gkg@mail.gmail.com>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.