[PATCH 17/17] nvme: enable non-inline passthru commands

[Kanchan Joshi]

On Fri, Apr 1, 2022 at 8:14 AM Jens Axboe <axboe at kernel.dk> wrote:
> >>>>> Ok. If you are open to take new opcode/struct route, that is all we
> >>>>> require to pair with big-sqe and have this sorted. How about this -
> >>>>
> >>>> I would much, much, much prefer to support a bigger CQE.  Having
> >>>> a pointer in there just creates a fair amount of overhead and
> >>>> really does not fit into the model nvme and io_uring use.
> >>>
> >>> Sure, will post the code with bigger-cqe first.
> >>
> >> I can add the support, should be pretty trivial. And do the liburing
> >> side as well, so we have a sane base.
> >
> >  I will post the big-cqe based work today. It works with fio.
> >  It does not deal with liburing (which seems tricky), but hopefully it
> > can help us move forward anyway .
>
> Let's compare then, since I just did the support too :-)

:-) awesome

> Some limitations in what I pushed:
>
> 1) Doesn't support the inline completion path. Undecided if this is
> super important or not, the priority here for me was to not pollute the
> general completion path.
>
> 2) Doesn't support overflow. That can certainly be done, only
> complication here is that we need 2x64bit in the io_kiocb for that.
> Perhaps something can get reused for that, not impossible. But figured
> it wasn't important enough for a first run.

We have the handling in my version. But that part is not tested, since
that situation did not occur naturally.
Maybe it requires slowing down completion-reaping (in user-space) to
trigger that.

> I also did the liburing support, but haven't pushed it yet. That's
> another case where some care has to be taken to avoid makig the general
> path slower.
>
> Oh, it's here, usual branch:
>
> https://git.kernel.dk/cgit/linux-block/log/?h=io_uring-big-sqe
>
> and based on top of the pending 5.18 bits and the current 5.19 bits.
>
> >> Then I'd suggest to collapse a few of the patches in the series,
> >> the ones that simply modify or fix gaps in previous ones. Order
> >> the series so we build the support and then add nvme support
> >> nicely on top of that.
> >
> > I think we already did away with patches which were fixing only the
> > gaps. But yes, patches still add infra for features incrementally.
> > Do you mean having all io_uring infra (async, plug, poll) squashed
> > into a single io_uring patch?
>
> At least async and plug, I'll double check on the poll bit.

Sounds right, the plug should definitely go in the async one.