From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DF4C4C4332F for ; Sun, 13 Feb 2022 06:57:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234132AbiBMG50 (ORCPT ); Sun, 13 Feb 2022 01:57:26 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:40700 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234051AbiBMG5K (ORCPT ); Sun, 13 Feb 2022 01:57:10 -0500 Received: from isilmar-4.linta.de (isilmar-4.linta.de [136.243.71.142]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 860105DE58 for ; Sat, 12 Feb 2022 22:57:05 -0800 (PST) X-isilmar-external: YES X-isilmar-external: YES X-isilmar-external: YES Received: from owl.dominikbrodowski.net (owl.brodo.linta [10.2.0.111]) by isilmar-4.linta.de (Postfix) with ESMTPSA id 22DCD20140B; Sun, 13 Feb 2022 06:57:00 +0000 (UTC) Received: by owl.dominikbrodowski.net (Postfix, from userid 1000) id E168B80ECF; Sun, 13 Feb 2022 07:55:37 +0100 (CET) Date: Sun, 13 Feb 2022 07:55:37 +0100 From: Dominik Brodowski To: "Jason A. Donenfeld" Cc: linux-kernel@vger.kernel.org, Theodore Ts'o Subject: Re: [PATCH 3/3] random: use trylock in irq handler rather than spinning Message-ID: References: <20220212231022.679926-1-Jason@zx2c4.com> <20220212231022.679926-4-Jason@zx2c4.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220212231022.679926-4-Jason@zx2c4.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am Sun, Feb 13, 2022 at 12:10:22AM +0100 schrieb Jason A. Donenfeld: > crng_pre_init_inject() (and prior crng_fast_load()) uses a trylock when > in fast mode, so that it never contends. We should be doing the same > when grabbing a spinlock for mixing into the entropy pool. So switch to > doing that before calling the underscored _mix_pool_bytes(). > > Cc: Dominik Brodowski > Cc: Theodore Ts'o > Signed-off-by: Jason A. Donenfeld > --- > drivers/char/random.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/drivers/char/random.c b/drivers/char/random.c > index 9a8e1bb9845d..ca224c3f2561 100644 > --- a/drivers/char/random.c > +++ b/drivers/char/random.c > @@ -1286,13 +1286,10 @@ void add_interrupt_randomness(int irq) > atomic_set(&fast_pool->count, 0); > fast_pool->last = now; > > - /* > - * Technically this call means that we're using a spinlock_t > - * in the IRQ handler, which isn't terrific for PREEMPT_RT. > - * However, this only happens during boot, and then never > - * again, so we live with it. > - */ > - mix_pool_bytes(&fast_pool->pool, sizeof(fast_pool->pool)); > + if (spin_trylock(&input_pool.lock)) { > + _mix_pool_bytes(&fast_pool->pool, sizeof(fast_pool->pool)); > + spin_unlock(&input_pool.lock); > + } You're still using a spinlock_t here, so I don't see a need to remove the comment. Also, I'm not super happy that the count is re-set to 0 but the input remains unused. Maybe the better approach here is, as discussed in the other thread, to always use the workqueue mechanism, which would allow us to streamline the code further. Thanks, Dominik