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