On 5/20/22 9:25 AM, Jason A. Donenfeld wrote:
> Hi Jens,
>
> On Fri, May 20, 2022 at 11:44:56AM +0200, Jason A. Donenfeld wrote:
>> Jens Axboe (3):
>> random: convert to using fops->read_iter()
>> random: convert to using fops->write_iter()
>> random: wire up fops->splice_{read,write}_iter()
>
> FYI, this series makes reads from /dev/urandom slower, from around 616
> MiB/s to 598 MiB/s on my system. That seems rather unfortunate.
How reproducible is that? That seems like a huge difference for the
change. How big are the reads?
> Are we sure we really want to do this and need to do this?
I'm very sure, otherwise we're just accepting that we're breaking real
world applications. Alternatively, you can keep the ->read() and add the
->read_iter() as well, but I'm a bit skeptical at your initial results
there.
--
Jens Axboe
On Fri, May 20, 2022 at 09:34:46AM -0600, Jens Axboe wrote:
> I'm very sure, otherwise we're just accepting that we're breaking real
> world applications.
"Breaking" as in "it used to work with earlier kernels, doesn't work with
recent ones"? Details, please...
Hi Jens,
On Fri, May 20, 2022 at 09:34:46AM -0600, Jens Axboe wrote:
> On 5/20/22 9:25 AM, Jason A. Donenfeld wrote:
> > Hi Jens,
> >
> > On Fri, May 20, 2022 at 11:44:56AM +0200, Jason A. Donenfeld wrote:
> >> Jens Axboe (3):
> >> random: convert to using fops->read_iter()
> >> random: convert to using fops->write_iter()
> >> random: wire up fops->splice_{read,write}_iter()
> >
> > FYI, this series makes reads from /dev/urandom slower, from around 616
> > MiB/s to 598 MiB/s on my system. That seems rather unfortunate.
>
> How reproducible is that? That seems like a huge difference for the
> change. How big are the reads?
Fairly reproducible. Actually, if anything, it reproduces consistently
with worst results; I chose the most favorable ones for the new code.
This isn't any fancy `perf` profiling, but just running:
$ pv /dev/urandom > /dev/null
From looking at strace, the read size appears to be 131072.
Jason
Hi Jens,
On Fri, May 20, 2022 at 09:34:46AM -0600, Jens Axboe wrote:
> On 5/20/22 9:25 AM, Jason A. Donenfeld wrote:
> > Are we sure we really want to do this and need to do this?
>
> I'm very sure, otherwise we're just accepting that we're breaking real
> world applications.
Would we really? I always thought splice() and copy_file_range() and
sendfile() were all kind of "special" in that they mostly do not work
for many things, and so all userspaces need fallback code. And the state
of "mostly not working" has always just been the norm. So broken today,
working tomorrow, broken next week would be par for the course? I might
be *super* wrong here, so feel free to say so, but this has been my
general impression.
Anyway, I do like the idea of supporting splice() and sendfile(). The
performance hit is just kind of sad.
Jason
On 5/20/22 9:39 AM, Jason A. Donenfeld wrote:
> Hi Jens,
>
> On Fri, May 20, 2022 at 09:34:46AM -0600, Jens Axboe wrote:
>> On 5/20/22 9:25 AM, Jason A. Donenfeld wrote:
>>> Hi Jens,
>>>
>>> On Fri, May 20, 2022 at 11:44:56AM +0200, Jason A. Donenfeld wrote:
>>>> Jens Axboe (3):
>>>> random: convert to using fops->read_iter()
>>>> random: convert to using fops->write_iter()
>>>> random: wire up fops->splice_{read,write}_iter()
>>>
>>> FYI, this series makes reads from /dev/urandom slower, from around 616
>>> MiB/s to 598 MiB/s on my system. That seems rather unfortunate.
>>
>> How reproducible is that? That seems like a huge difference for the
>> change. How big are the reads?
>
> Fairly reproducible. Actually, if anything, it reproduces consistently
> with worst results; I chose the most favorable ones for the new code.
> This isn't any fancy `perf` profiling, but just running:
>
> $ pv /dev/urandom > /dev/null
>
> From looking at strace, the read size appears to be 131072.
Ran 32, 1k, 4k here and it does seem to be down aboout 3%. Which is
definitely bigger than I expected, particularly for larger reads. If
anything, the 32b read seems comparably better than eg 1k or 4k, which
is also unexpected. Let me do a bit of profiling to see what is up.
If you're worried about it, I'd just keep the read/write and add the
iter variants on the side.
--
Jens Axboe
Hi Jens,
On Fri, May 20, 2022 at 09:44:25AM -0600, Jens Axboe wrote:
> Ran 32, 1k, 4k here and it does seem to be down aboout 3%. Which is
> definitely bigger than I expected, particularly for larger reads. If
> anything, the 32b read seems comparably better than eg 1k or 4k, which
> is also unexpected. Let me do a bit of profiling to see what is up.
Something to keep in mind wrt 32b is that for complicated crypto
reasons, the function has this logic:
- If len <= 32, generate one 64 byte block and give <= 32 bytes of it to
the caller.
- If len > 32, generate one 64 byte block, but give 0 of it to the
caller. Then generate ⌈len/64⌉ blocks for the caller.
Put together, this means:
- 1..32, 1 block
- 33..64, 2 blocks
- 65..128, 3 blocks
- 129..196, 4 blocks
So you get this sort of shelf where the amortization benefits don't
really kick in until after 3 blocks.
> If you're worried about it, I'd just keep the read/write and add the
> iter variants on the side.
Not a chance of that. These functions are already finicky as-is; I would
really hate to have to duplicate all of these paths.
Jason