Re: /dev/random changes for 3.13

[Theodore Ts'o]

On Fri, Nov 15, 2013 at 01:58:05PM -0800, Linus Torvalds wrote:
> Ok, I finally got around to the random tree, but your proposed merge
> resolution makes no sense, so I didn't end up applying it.
> 
> > - -     r->entropy_total += nbits;
> >         if (!r->initialized && nbits > 0) {
> > +               r->entropy_total += nbits;
> 
> This part undoes your commit 6265e169cd31 ("random: push extra entropy
> to the output pools"), and the "entropy_total" field will now never be
> non-zero when "r->initialized" is set....

You're right, I totally screwed that up.  Part of the problem is that
I should have cleaned up the if statement in commit 6265e169cd31 so
that it looked like this:

        r->entropy_total += nbits;
        if (!r->initialized && (r->entropy_total > 128)) {
                        r->initialized = 1;
                        r->entropy_total = 0;
                }
        }

... and so at the top of the dev branch, it should have looked like
this:

        r->entropy_total += nbits;
        if (!r->initialized && (r->entropy_total > 128)) {
		if (r == &nonblocking_pool)
			pr_notice("random: %s pool is initialized\n", r->name);
		r->initialized = 1;
		r->entropy_total = 0;
        }

Instead, I had a more complicated structure which confused me when I
cleaned up the merge.

So I'm going to propose the following merge resolution:

diff --cc drivers/char/random.c
index cdf4cfb,4fe5609..0000000
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@@ -654,14 -601,12 +654,13 @@@ retry
  	if (cmpxchg(&r->entropy_count, orig, entropy_count) != orig)
  		goto retry;
  
 -	if (!r->initialized && nbits > 0) {
 -		r->entropy_total += nbits;
 -		if (r->entropy_total > 128) {
 -			r->initialized = 1;
 -			if (r == &nonblocking_pool)
 -				prandom_reseed_late();
 +	r->entropy_total += nbits;
- 	if (!r->initialized && nbits > 0) {
- 		if (r->entropy_total > 128) {
- 			if (r == &nonblocking_pool)
- 				pr_notice("random: %s pool is initialized\n",
- 					  r->name);
- 			r->initialized = 1;
- 			r->entropy_total = 0;
++	if (!r->initialized && (r->entropy_total > 128)) {
++		r->initialized = 1;
++		r->entropy_total = 0;
++		if (r == &nonblocking_pool) {
++			prandom_reseed_late();
++			pr_notice("random: %s pool is initialized\n", r->name);
  		}
  	}

It does fold a cleanup in with the merge resolution, but it's not
actually changing anything in terms of code behavior, and it makes the
resulting merge much more obvious.

Are you OK with that?

						- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/