[PATCH v2] nvme-core: initialize status to NVME_SC_HOST_PATH_ERROR

[Keith Busch]

On Thu, Apr 15, 2021 at 03:11:13PM -0700, Yuanyuan Zhong wrote:
> > > It doesn't look like these types of errors are unique to nvme. Could we
> > > not just have blk_execute_rq() return an error instead?
> Looking at history of commit b7819b925918 ("block: remove the blk_execute_rq
> return value") and commit be549d491154 ("scsi: core: set result when the
> command cannot be dispatched"), it seems scsi code no longer have issue.

Those happened when 'struct request' carried an 'errors' field that a
caller could check. The struct doesn't have that anymore.

> > -extern void blk_execute_rq(struct gendisk *, struct request *, int);
> > +extern int blk_execute_rq(struct gendisk *, struct request *, int);
> Changing blk_execute_rq() prototype needs tree-wide callers update.

Callers aren't relying on an error now, so I don't think this suggestion
requires a tree-wide update. It might want an audit to see if existing
callers are susceptible to the same bug you found with NVMe, but
changing the return as suggested shouldn't introduce new issues.

> While it could be a fix, I'd wait for maintainers to chime in.

Yes, this does require concensus, but Linux is usually adaptable to
changing kernal APIs when it makes sense. I *think* my suggestion makes
sense, but I appreciate any feedback.
 
> I don't quite like initializing the status for every nvme command. However for
> such a long standing bug across multiple stable releases, I think it will be an
> easy backport for stable-tree.

Your proposal is easier for stable, however, the mainline fix doesn't
necessarily need to consider that. We can always deal with a back-port
if the mainline approach does not readily apply.