Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-09 Thread George Spelvin
> Yes, but again, if we're only adding a single bit's worth of entropy > credit, then at worst we'll only be off by one. And if the race is a > 50-50 proposition (and I know life is not that simple; for example, it > doesn't deal with N-way races), then we might not even be off by one. :-)

Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-09 Thread Theodore Ts'o
On Mon, Jun 09, 2014 at 11:04:26AM -0400, George Spelvin wrote: > > What could be a problem is if we get really unlucky (an interrupt > > update happening at the same time as an update from rngd), it's > > possible we could end up with an entropy addition getting lost. It's > > not such a big

Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-09 Thread George Spelvin
> Actually, if you look very closely, or take a look at the commit > description for 902c098a3663de, you'll see that we're actually > updating the entropy pool from add_interrupt_randomness() w/o taking > any locks. So that's actually not a problem. Arrgh... I could swear I saw a path that

Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-09 Thread Theodore Ts'o
On Mon, Jun 09, 2014 at 12:03:55AM -0400, George Spelvin wrote: > > That seems... noticeable. Causing iterrupt latency problems is defintiely > a theoretical extrapolation, however. Actually, if you look very closely, or take a look at the commit description for 902c098a3663de, you'll see that

Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-09 Thread George Spelvin
> Sigh, adventures in "unable to mount root filesystem" currently underway. PEBKAC resolved. (I had CONFIG_ATA=m for some testing, but forgot to change it back to y when compiling a kernel I expected to boot.) 32 MiB write: Without patch With patch 0 readers: 0.495523

Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-09 Thread George Spelvin
Sigh, adventures in unable to mount root filesystem currently underway. PEBKAC resolved. (I had CONFIG_ATA=m for some testing, but forgot to change it back to y when compiling a kernel I expected to boot.) 32 MiB write: Without patch With patch 0 readers: 0.495523

Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-09 Thread Theodore Ts'o
On Mon, Jun 09, 2014 at 12:03:55AM -0400, George Spelvin wrote: That seems... noticeable. Causing iterrupt latency problems is defintiely a theoretical extrapolation, however. Actually, if you look very closely, or take a look at the commit description for 902c098a3663de, you'll see that

Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-09 Thread George Spelvin
Actually, if you look very closely, or take a look at the commit description for 902c098a3663de, you'll see that we're actually updating the entropy pool from add_interrupt_randomness() w/o taking any locks. So that's actually not a problem. Arrgh... I could swear I saw a path that locked,

Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-09 Thread Theodore Ts'o
On Mon, Jun 09, 2014 at 11:04:26AM -0400, George Spelvin wrote: What could be a problem is if we get really unlucky (an interrupt update happening at the same time as an update from rngd), it's possible we could end up with an entropy addition getting lost. It's not such a big deal if a

Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-09 Thread George Spelvin
Yes, but again, if we're only adding a single bit's worth of entropy credit, then at worst we'll only be off by one. And if the race is a 50-50 proposition (and I know life is not that simple; for example, it doesn't deal with N-way races), then we might not even be off by one. :-) Indeed.

Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-08 Thread George Spelvin
Sigh, adventures in "unable to mount root filesystem" currently underway. Tested on a different computer without patches (half size, since it's an older machine): Writing 16 MiB: 16777216 bytes (17 MB) copied, 0.289169 s, 58.0 MB/s 16777216 bytes (17 MB) copied, 0.289378 s, 58.0 MB/s Writing

Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-08 Thread George Spelvin
> Summarizing that, time to feed in 32 MiB of zeros (from user-space): > > 0 concurrent reads: 0.356898 0.357693 > 1 concurrent read: 0.505941 0.509075(+42%) > 2 concurrent reads: 0.662240 0.657055(+84%) Er, wait a minute... I'm not sure which kernel (patched or unpatched) I

Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-08 Thread George Spelvin
> Which writer are you worried about, specifically? A userspace write > to /dev/random from rgnd? Actually, the part I'm actually worried about is latency in add_interrupt_randomness. But I may have not understood how the locking works. There's a per-CPU fast pool which isn't locked at all,

Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-08 Thread Theodore Ts'o
On Sun, Jun 08, 2014 at 08:05:24PM -0400, George Spelvin wrote: > > The problem I started trying to solve is readers (entropy extractors) > monopolizing the pool lock and stalling writers (entropy mixers), which > are supposed to be fast and low overhead. Which writer are you worried about,

[RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-08 Thread George Spelvin
I have a few assorted cleanups in-work, but I was thinking about the following and it's led to some thinking about the (non-)usefulness of the out[] buffer from _mix_pool_bytes. The problem I started trying to solve is readers (entropy extractors) monopolizing the pool lock and stalling writers

[RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-08 Thread George Spelvin
I have a few assorted cleanups in-work, but I was thinking about the following and it's led to some thinking about the (non-)usefulness of the out[] buffer from _mix_pool_bytes. The problem I started trying to solve is readers (entropy extractors) monopolizing the pool lock and stalling writers

Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-08 Thread Theodore Ts'o
On Sun, Jun 08, 2014 at 08:05:24PM -0400, George Spelvin wrote: The problem I started trying to solve is readers (entropy extractors) monopolizing the pool lock and stalling writers (entropy mixers), which are supposed to be fast and low overhead. Which writer are you worried about,

Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-08 Thread George Spelvin
Which writer are you worried about, specifically? A userspace write to /dev/random from rgnd? Actually, the part I'm actually worried about is latency in add_interrupt_randomness. But I may have not understood how the locking works. There's a per-CPU fast pool which isn't locked at all,

Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-08 Thread George Spelvin
Summarizing that, time to feed in 32 MiB of zeros (from user-space): 0 concurrent reads: 0.356898 0.357693 1 concurrent read: 0.505941 0.509075(+42%) 2 concurrent reads: 0.662240 0.657055(+84%) Er, wait a minute... I'm not sure which kernel (patched or unpatched) I did

Re: [RFC PATCH] drivers/char/random.c: Is reducing locking range like this safe?

2014-06-08 Thread George Spelvin
Sigh, adventures in unable to mount root filesystem currently underway. Tested on a different computer without patches (half size, since it's an older machine): Writing 16 MiB: 16777216 bytes (17 MB) copied, 0.289169 s, 58.0 MB/s 16777216 bytes (17 MB) copied, 0.289378 s, 58.0 MB/s Writing