[PATCH 2/5] nvme-core: add a function to submit a cancel command

[Maurizio Lombardi]

Hello Sagi,

ne 12. 5. 2024 v 16:06 odesílatel Sagi Grimberg <sagi at grimberg.me> napsal:
> > +                                            BLK_MQ_REQ_NOWAIT |
> > +                                            BLK_MQ_REQ_RESERVED,
> > +                                            sqid - 1);
>
> I think we need to account for a new reserved commands that is now
> can be taken outside of connect. Should this be reserved at all?

That is true, I should add some reserved tags, and yes, they have to
be reserved because
blk_mq_alloc_request_hctx() enforces it to keep the low level allocator simple.

> meaning even if we account for it, there are potentially more and more
> requests that we'd want to cancel...

Yes, so the question is how many tags should be reserved for cancel commands.

I received a suggestion to make Cancel command work the following way:

- The first timeout on a specific I/O queue triggers a cancel in
"single command" mode, aborting the command
that timed out.
- If another command times out on the same I/O queue while the fist
cancel is still executing, send a second cancel command in
"multiple command" mode, aborting all of the outstanding commands on
the I/O queue.

but this could potentially require 2 * queue_count reserved tags, so
tens of reserved tags for a command that maybe will be seldom used;
also, the implementation will be more complex.

Another possibility is to just use Cancel in "single command" mode,
similarly to how the normal abort command works and limit it
with a variable in the nvme_ctrl structure, as we do with
"abort_limit" (it could be called "cancel_limit").
This would prevent us from exhausting the pool of reserved tags, thus
avoiding the problems like with commit de105068fead while keeping the
implementation very simple;
if we reach "cancel_limit" we can always fallback to a controller
reset. On the other hand, we would use only a subset of the features
Cancel has to offer.

Maurizio