* [3.13 PATCH 0/2] random: bytes vs. bits @ 2013-12-07 2:27 Greg Price 2013-12-07 2:27 ` [3.13 PATCH 1/2] random: entropy_bytes is actually bits Greg Price 2013-12-07 2:28 ` [PATCH 2/2] random: clarify bits/bytes in wakeup thresholds Greg Price 0 siblings, 2 replies; 5+ messages in thread From: Greg Price @ 2013-12-07 2:27 UTC (permalink / raw) To: Theodore Ts'o; +Cc: linux-kernel The new when-to-push logic in v3.13-rc1~61^2~6 "random: push extra entropy to the output pools" uses the name "entropy_bytes" for a quantity that's actually in bits. This results in confusing, buggy-looking lines both where the variable is set and where it's used. Fortunately it's all consistent if the variable is understood as counting bits, so the behavior is fine. This code is new in v3.13-rc1, so fixing the name seems in order for 3.13. The first patch here just does that. One cause of the mistake is probably that the names of random_read_wakeup_thresh and random_write_wakeup_thresh don't indicate what units they're in. This is an especially acute issue now that we have three units for entropy in different places -- bytes, bits, and 1 / (1 << ENTROPY_SHIFT) fractions of a bit. The second patch clarifies the units in both names. (Maybe other names should be clarified too.) This may as well be post-3.13. Cheers, Greg Greg Price (2): random: entropy_bytes is actually bits random: clarify bits/bytes in wakeup thresholds drivers/char/random.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) -- 1.8.3.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [3.13 PATCH 1/2] random: entropy_bytes is actually bits 2013-12-07 2:27 [3.13 PATCH 0/2] random: bytes vs. bits Greg Price @ 2013-12-07 2:27 ` Greg Price 2013-12-07 14:50 ` Theodore Ts'o 2013-12-07 2:28 ` [PATCH 2/2] random: clarify bits/bytes in wakeup thresholds Greg Price 1 sibling, 1 reply; 5+ messages in thread From: Greg Price @ 2013-12-07 2:27 UTC (permalink / raw) To: Theodore Ts'o; +Cc: linux-kernel The variable 'entropy_bytes' is set from an expression that actually counts bits. Fortunately it's also only compared to values that also count bits. Rename it accordingly. Signed-off-by: Greg Price <price@mit.edu> --- drivers/char/random.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index 429b75bb6..dc9b9af20 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -666,10 +666,10 @@ retry: r->entropy_total, _RET_IP_); if (r == &input_pool) { - int entropy_bytes = entropy_count >> ENTROPY_SHIFT; + int entropy_bits = entropy_count >> ENTROPY_SHIFT; /* should we wake readers? */ - if (entropy_bytes >= random_read_wakeup_thresh) { + if (entropy_bits >= random_read_wakeup_thresh) { wake_up_interruptible(&random_read_wait); kill_fasync(&fasync, SIGIO, POLL_IN); } @@ -678,7 +678,7 @@ retry: * forth between them, until the output pools are 75% * full. */ - if (entropy_bytes > random_write_wakeup_thresh && + if (entropy_bits > random_write_wakeup_thresh && r->initialized && r->entropy_total >= 2*random_read_wakeup_thresh) { static struct entropy_store *last = &blocking_pool; -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [3.13 PATCH 1/2] random: entropy_bytes is actually bits 2013-12-07 2:27 ` [3.13 PATCH 1/2] random: entropy_bytes is actually bits Greg Price @ 2013-12-07 14:50 ` Theodore Ts'o 0 siblings, 0 replies; 5+ messages in thread From: Theodore Ts'o @ 2013-12-07 14:50 UTC (permalink / raw) To: Greg Price; +Cc: linux-kernel On Fri, Dec 06, 2013 at 09:27:52PM -0500, Greg Price wrote: > The variable 'entropy_bytes' is set from an expression that actually > counts bits. Fortunately it's also only compared to values that also > count bits. Rename it accordingly. > > Signed-off-by: Greg Price <price@mit.edu> Thanks, applied. - Ted ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] random: clarify bits/bytes in wakeup thresholds 2013-12-07 2:27 [3.13 PATCH 0/2] random: bytes vs. bits Greg Price 2013-12-07 2:27 ` [3.13 PATCH 1/2] random: entropy_bytes is actually bits Greg Price @ 2013-12-07 2:28 ` Greg Price 2013-12-07 19:20 ` Theodore Ts'o 1 sibling, 1 reply; 5+ messages in thread From: Greg Price @ 2013-12-07 2:28 UTC (permalink / raw) To: Theodore Ts'o; +Cc: linux-kernel These are a recurring cause of confusion, so rename them to hopefully be clearer. Signed-off-by: Greg Price <price@mit.edu> --- drivers/char/random.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/drivers/char/random.c b/drivers/char/random.c index dc9b9af20..3f87a7e88 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -295,14 +295,14 @@ * The minimum number of bits of entropy before we wake up a read on * /dev/random. Should be enough to do a significant reseed. */ -static int random_read_wakeup_thresh = 64; +static int random_read_wakeup_bits = 64; /* * If the entropy count falls under this number of bits, then we * should wake up processes which are selecting or polling on write * access to /dev/random. */ -static int random_write_wakeup_thresh = 28 * OUTPUT_POOL_WORDS; +static int random_write_wakeup_bits = 28 * OUTPUT_POOL_WORDS; /* * The minimum number of seconds between urandom pool resending. We @@ -669,7 +669,7 @@ retry: int entropy_bits = entropy_count >> ENTROPY_SHIFT; /* should we wake readers? */ - if (entropy_bits >= random_read_wakeup_thresh) { + if (entropy_bits >= random_read_wakeup_bits) { wake_up_interruptible(&random_read_wait); kill_fasync(&fasync, SIGIO, POLL_IN); } @@ -678,9 +678,9 @@ retry: * forth between them, until the output pools are 75% * full. */ - if (entropy_bits > random_write_wakeup_thresh && + if (entropy_bits > random_write_wakeup_bits && r->initialized && - r->entropy_total >= 2*random_read_wakeup_thresh) { + r->entropy_total >= 2*random_read_wakeup_bits) { static struct entropy_store *last = &blocking_pool; struct entropy_store *other = &blocking_pool; @@ -924,19 +924,19 @@ static void _xfer_secondary_pool(struct entropy_store *r, size_t nbytes) { __u32 tmp[OUTPUT_POOL_WORDS]; - /* For /dev/random's pool, always leave two wakeup worth's BITS */ - int rsvd = r->limit ? 0 : random_read_wakeup_thresh/4; + /* For /dev/random's pool, always leave two wakeups' worth */ + int rsvd_bytes = r->limit ? 0 : random_read_wakeup_bits / 4; int bytes = nbytes; - /* pull at least as many as BYTES as wakeup BITS */ - bytes = max_t(int, bytes, random_read_wakeup_thresh / 8); + /* pull at least as much as a wakeup */ + bytes = max_t(int, bytes, random_read_wakeup_bits / 8); /* but never more than the buffer size */ bytes = min_t(int, bytes, sizeof(tmp)); trace_xfer_secondary_pool(r->name, bytes * 8, nbytes * 8, ENTROPY_BITS(r), ENTROPY_BITS(r->pull)); bytes = extract_entropy(r->pull, tmp, bytes, - random_read_wakeup_thresh / 8, rsvd); + random_read_wakeup_bits / 8, rsvd_bytes); mix_pool_bytes(r, tmp, bytes, NULL); credit_entropy_bits(r, bytes*8); } @@ -952,7 +952,7 @@ static void push_to_pool(struct work_struct *work) struct entropy_store *r = container_of(work, struct entropy_store, push_work); BUG_ON(!r); - _xfer_secondary_pool(r, random_read_wakeup_thresh/8); + _xfer_secondary_pool(r, random_read_wakeup_bits/8); trace_push_to_pool(r->name, r->entropy_count >> ENTROPY_SHIFT, r->pull->entropy_count >> ENTROPY_SHIFT); } @@ -1004,7 +1004,7 @@ retry: goto retry; if ((r->entropy_count >> ENTROPY_SHIFT) - < random_write_wakeup_thresh) + < random_write_wakeup_bits) wakeup_write = 1; } spin_unlock_irqrestore(&r->lock, flags); @@ -1313,7 +1313,7 @@ random_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos) wait_event_interruptible(random_read_wait, ENTROPY_BITS(&input_pool) >= - random_read_wakeup_thresh); + random_read_wakeup_bits); if (signal_pending(current)) { retval = -ERESTARTSYS; @@ -1358,9 +1358,9 @@ random_poll(struct file *file, poll_table * wait) poll_wait(file, &random_read_wait, wait); poll_wait(file, &random_write_wait, wait); mask = 0; - if (ENTROPY_BITS(&input_pool) >= random_read_wakeup_thresh) + if (ENTROPY_BITS(&input_pool) >= random_read_wakeup_bits) mask |= POLLIN | POLLRDNORM; - if (ENTROPY_BITS(&input_pool) < random_write_wakeup_thresh) + if (ENTROPY_BITS(&input_pool) < random_write_wakeup_bits) mask |= POLLOUT | POLLWRNORM; return mask; } @@ -1583,7 +1583,7 @@ struct ctl_table random_table[] = { }, { .procname = "read_wakeup_threshold", - .data = &random_read_wakeup_thresh, + .data = &random_read_wakeup_bits, .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec_minmax, @@ -1592,7 +1592,7 @@ struct ctl_table random_table[] = { }, { .procname = "write_wakeup_threshold", - .data = &random_write_wakeup_thresh, + .data = &random_write_wakeup_bits, .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec_minmax, -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] random: clarify bits/bytes in wakeup thresholds 2013-12-07 2:28 ` [PATCH 2/2] random: clarify bits/bytes in wakeup thresholds Greg Price @ 2013-12-07 19:20 ` Theodore Ts'o 0 siblings, 0 replies; 5+ messages in thread From: Theodore Ts'o @ 2013-12-07 19:20 UTC (permalink / raw) To: Greg Price; +Cc: linux-kernel On Fri, Dec 06, 2013 at 09:28:03PM -0500, Greg Price wrote: > These are a recurring cause of confusion, so rename them to > hopefully be clearer. > > Signed-off-by: Greg Price <price@mit.edu> Thanks, applied with some merge fix ups to get the patch to apply. - Ted ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-12-07 19:20 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-12-07 2:27 [3.13 PATCH 0/2] random: bytes vs. bits Greg Price 2013-12-07 2:27 ` [3.13 PATCH 1/2] random: entropy_bytes is actually bits Greg Price 2013-12-07 14:50 ` Theodore Ts'o 2013-12-07 2:28 ` [PATCH 2/2] random: clarify bits/bytes in wakeup thresholds Greg Price 2013-12-07 19:20 ` Theodore Ts'o
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox