From: PrasannaKumar Muralidharan Subject: Re: [PATCH v2] hwrng: Clean up RNG list when last hwrng is unregistered Date: Sun, 17 Dec 2017 15:19:37 +0530 Message-ID: References: <151336772989.60446.16322941485803241258.stgit@sosxen2.amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" , thomas.lendacky@amd.com, Herbert Xu , "David S . Miller" To: Gary R Hook Return-path: Received: from mail-it0-f66.google.com ([209.85.214.66]:33091 "EHLO mail-it0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751005AbdLQJti (ORCPT ); Sun, 17 Dec 2017 04:49:38 -0500 Received: by mail-it0-f66.google.com with SMTP id o130so2590225itg.0 for ; Sun, 17 Dec 2017 01:49:38 -0800 (PST) In-Reply-To: Sender: linux-crypto-owner@vger.kernel.org List-ID: On 17 December 2017 at 14:53, PrasannaKumar Muralidharan wrote: > Hi Gary, > > Some minor comments below. > > On 16 December 2017 at 01:25, Gary R Hook wrote: >> >> Commit 142a27f0a731 added support for a "best" RNG, and in doing so >> introduced a hang from rmmod/modprobe -r when the last RNG on the list >> was unloaded. > > Nice catch. Thanks for fixing this. > >> When the hwrng list is depleted, return the global variables to their >> original state and decrement all references to the object. >> >> Fixes: 142a27f0a731 ("hwrng: core - Reset user selected rng by writing "" to rng_current") > > Please cc the commit author (in this case its me) so that this patch > gets noticed easily. > >> Signed-off-by: Gary R Hook >> --- >> >> Changes since v1: fix misspelled word in subject >> >> drivers/char/hw_random/core.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c >> index 657b8770b6b9..91bb98c42a1c 100644 >> --- a/drivers/char/hw_random/core.c >> +++ b/drivers/char/hw_random/core.c >> @@ -306,6 +306,10 @@ static int enable_best_rng(void) >> ret = ((new_rng == current_rng) ? 0 : set_current_rng(new_rng)); >> if (!ret) >> cur_rng_set_by_user = 0; >> + } else { >> + drop_current_rng(); > > When the hwrng list is empty just set current_rng = NULL instead of > calling drop_current_rng(). > >> + cur_rng_set_by_user = 0; >> + ret = 0; >> } >> >> return ret; >> > > Regards, > PrasannaKumar I am fine with the code as is. Reviewed-by: PrasannaKumar Muralidharan Regards, PrasannaKumar