Re: [PATCH] hw_random: move add_early_randomness() out of rng_mutex

From: Laurent Vivier <lvivier@redhat.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org,
	Matt Mackall <mpm@selenic.com>
Subject: Re: [PATCH] hw_random: move add_early_randomness() out of rng_mutex
Date: Fri, 4 Oct 2019 17:32:39 +0200	[thread overview]
Message-ID: <dbb2c7a8-3fe6-9af4-7d14-4d45eccd8305@redhat.com> (raw)
In-Reply-To: <20191004142603.GA29787@gondor.apana.org.au>

On 04/10/2019 16:26, Herbert Xu wrote:
> On Thu, Sep 12, 2019 at 03:30:22PM +0200, Laurent Vivier wrote:
>>
>> @@ -496,19 +510,24 @@ int hwrng_register(struct hwrng *rng)
>>  			goto out_unlock;
>>  	}
>>  
>> -	if (old_rng && !rng->init) {
>> +	new_rng = rng;
>> +	kref_get(&new_rng->ref);
>> +out_unlock:
>> +	mutex_unlock(&rng_mutex);
>> +
>> +	if (new_rng) {
>> +		if (new_rng != old_rng || !rng->init) {
> 
> Is this really supposed to be || instead of &&?

original code calls add_early_randomness():

1- everytime a new device is plugged in except if there is an init
  function (because if there is an init function it has not been
  called and it is needed to be able to use the device)

2- everytime current device is changed, unconditionally
   because in this case the init function has already been called.
   (in hwrng_init() in set_current_rng())

"new_rng != old_rng" is for 2-, and "!rng->init" is for 1-.

So, yes, it's supposed to be "||".

Thanks,
Laurent