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@xxxxxxxxx>
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
