Re: [PATCH 00/17] io_uring passthru over nvme

[Kanchan Joshi <joshiiitr@gmail.com>]

On Sat, Mar 12, 2022 at 7:57 AM Adam Manzanares
<a.manzanares@samsung.com> wrote:
>
> On Fri, Mar 11, 2022 at 03:35:04PM -0800, Adam Manzanares wrote:
> > On Fri, Mar 11, 2022 at 08:43:24AM -0800, Luis Chamberlain wrote:
> > > On Thu, Mar 10, 2022 at 03:35:02PM +0530, Kanchan Joshi wrote:
> > > > On Thu, Mar 10, 2022 at 1:59 PM Christoph Hellwig <hch@lst.de> wrote:
> > > > >
> > > > > What branch is this against?
> > > > Sorry I missed that in the cover.
> > > > Two options -
> > > > (a) https://urldefense.com/v3/__https://protect2.fireeye.com/v1/url?k=03500d22-5ccb341f-0351866d-0cc47a31309a-6f95e6932e414a1d&q=1&e=4ca7b05e-2fe6-40d9-bbcf-a4ed687eca9f&u=https*3A*2F*2Fgit.kernel.dk*2Fcgit*2Flinux-block*2Flog*2F*3Fh*3Dio_uring-big-sqe__;JSUlJSUlJSUl!!EwVzqGoTKBqv-0DWAJBm!FujuZ927K3fuIklgYjkWtodmdQnQyBqOw4Ge4M08DU_0oD5tPm0-wS2SZg0MDh8_2-U9$
> > > > first patch ("128 byte sqe support") is already there.
> > > > (b) for-next (linux-block), series will fit on top of commit 9e9d83faa
> > > > ("io_uring: Remove unneeded test in io_run_task_work_sig")
> > > >
> > > > > Do you have a git tree available?
> > > > Not at the moment.
> > > >
> > > > @Jens: Please see if it is possible to move patches to your
> > > > io_uring-big-sqe branch (and maybe rename that to big-sqe-pt.v1).
> > >
> > > Since Jens might be busy, I've put up a tree with all this stuff:
> > >
> > > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20220311-io-uring-cmd__;!!EwVzqGoTKBqv-0DWAJBm!FujuZ927K3fuIklgYjkWtodmdQnQyBqOw4Ge4M08DU_0oD5tPm0-wS2SZg0MDiTF0Q7F$
> > >
> > > It is based on option (b) mentioned above, I took linux-block for-next
> > > and reset the tree to commit "io_uring: Remove unneeded test in
> > > io_run_task_work_sig" before applying the series.
> >
> > FYI I can be involved in testing this and have added some colleagues that can
> > help in this regard. We have been using some form of this work for several
> > months now and haven't had any issues. That being said some simple tests I have
> > are not currently working with the above git tree :). I will work to get this
> > resolved and post an update here.
>
> Sorry for the noise, I jumped up the stack too quickly with my tests. The
> "simple test" actually depends on several pieces of SW not related to the
> kernel.

Did you read the cover letter? It's not the same *user-interface* as
the previous series.
If you did not modify those out-of-kernel layers for the new
interface, you're bound to see what you saw.
If you did, please specify what the simple test was. I'll fix that in v2.

Otherwise, the throwaway remark "simple tests not working" - only
infers this series is untested. Nothing could be further from the
truth.
Rather this series is more robust than the previous one.

Let me expand bit more on testing part that's already there in cover:

fio -iodepth=256 -rw=randread -ioengine=io_uring -bs=512 -numjobs=1
-runtime=60 -group_reporting -iodepth_batch_submit=64
-iodepth_batch_complete_min=1 -iodepth_batch_complete_max=64
-fixedbufs=1 -hipri=1 -sqthread_poll=0 -filename=/dev/ng0n1
-name=io_uring_256 -uring_cmd=1

When I reduce the above command-line to do single IO, I call that a simple test.
Simple test that touches almost *everything* that patches build (i.e
async, fixed-buffer, plugging, fixed-buffer, bio-cache, polling).
And larger tests combine these knobs in various ways, QD ranging from
1, 2, 4...upto 256; on general and perf-optimized kernel config; with
big-sqe and normal-sqe (pointer one). And all this is repeated on the
block interface (regular io) too, which covers the regression part.
Sure, I can add more tests for checking regression. But no, I don't
expect any simple test to fail. And that applies to Luis' tree as
well. Tried that too again.


-- 
Joshi