Re: [PATCHSET 0/5] User mapped provided buffer rings

[Jens Axboe <axboe@kernel.dk>]

On 3/15/23 2:07?PM, Helge Deller wrote:
> On 3/15/23 21:03, Helge Deller wrote:
>> Hi Jens,
>>
>> Thanks for doing those fixes!
>>
>> On 3/14/23 18:16, Jens Axboe wrote:
>>> One issue that became apparent when running io_uring code on parisc is
>>> that for data shared between the application and the kernel, we must
>>> ensure that it's placed correctly to avoid aliasing issues that render
>>> it useless.
>>>
>>> The first patch in this series is from Helge, and ensures that the
>>> SQ/CQ rings are mapped appropriately. This makes io_uring actually work
>>> there.
>>>
>>> Patches 2..4 are prep patches for patch 5, which adds a variant of
>>> ring mapped provided buffers that have the kernel allocate the memory
>>> for them and the application mmap() it. This brings these mapped
>>> buffers in line with how the SQ/CQ rings are managed too.
>>>
>>> I'm not fully sure if this ONLY impacts archs that set SHM_COLOUR,
>>> of which there is only parisc, or if SHMLBA setting archs (of which
>>> there are others) are impact to any degree as well...
>>
>> It would be interesting to find out. I'd assume that other arches,
>> e.g. sparc, might have similiar issues.
>> Have you tested your patches on other arches as well?
> 
> By the way, I've now tested this series on current git head on an
> older parisc box (with PA8700 / PCX-W2 CPU).
> 
> Results of liburing testsuite:
> Tests timed out (1): <send-zerocopy.t> - (may not be a failure)
> Tests failed (5): <buf-ring.t> <file-verify.t> <poll-race-mshot.t> <ringbuf-read.t> <send_recvmsg.t>

send-zerocopy.t takes about ~20 seconds for me on modern hardware, so
that one likely just needs a longer timeout to work. Running it here on
my PA8900:

axboe@c8000 ~/g/liburing (master)> time test/send-zerocopy.t 

________________________________________________________
Executed in  115.08 secs    fish           external
   usr time   63.70 secs    1.08 millis   63.70 secs
   sys time   57.25 secs    4.26 millis   57.24 secs

which on that box is almost twice as long as the normal timeout for
the test script.

For file-verify.t, that one should work with the current tree. The issue
there is the use of registered buffers, and I added a parisc hack for
that. Maybe it's too specific to the PA8900 (the 128 byte stride). If
your tree does have:

commit 4c4fd1843bf284c0063c3a0f8822cb2d352b20c0 (origin/master, origin/HEAD, master)
Author: Jens Axboe <axboe@kernel.dk>
Date:   Wed Mar 15 11:34:54 2023 -0600

    test/file-verify: add dcache sync for parisc

then please experiment with that. 64 might be the correct value here and
I just got lucky with my testing...
be interesting to see 

For the remainder, they are all related to the buffer ring, which is
what is enabled by this series. But the tests don't use that yet, so
they will fail just like they do without the patch. In the
ring-buf-alloc branch of liburing there's the start of adding helpers to
setup the buffer rings, and then we can switch them to the mmap()
approach without much trouble. It's just not done yet, I will add a
patch in there to do that.

-- 
Jens Axboe