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 2C33FC433F5 for ; Thu, 17 Feb 2022 17:33:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243738AbiBQRd2 (ORCPT ); Thu, 17 Feb 2022 12:33:28 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:43852 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232550AbiBQRd0 (ORCPT ); Thu, 17 Feb 2022 12:33:26 -0500 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 087A515F353 for ; Thu, 17 Feb 2022 09:33:11 -0800 (PST) Date: Thu, 17 Feb 2022 18:33:07 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1645119188; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=g46W2ujMzZ0SbZmNOAXSHQ2Kv0SmQ9Y1FWBTwikBicA=; b=wRZEerD+adqSR/+xcb9EvZcSBM/QVOiCFzR7BoBpPdygu+sGYrXbnnOPidqSLhnw/qGxu0 uRIRqBQDjcC2GRl7MuYdoo1lKa4lSdPu9xowRDXjwYTx6MKi6p/vFhCXOMoyNPd56v0poe WEh1H7BP0/0pQvL8DRMup0hFeIE/FEmH08hJh+XPG96j5XPCPjlWw1A2nBYiNriMNQC2Gv DeHdEhu2fSs7ZMAD767TyKd4IOOddTvzG0EP09Y1If5MJeohKmTuezWSNYMtluzfv9DamJ xnTse5y7x3Jb/oPS1XSjaUn72laui2gFJewyGCQRy5lhUSXZ3Jm09UfiK1FRQA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1645119188; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=g46W2ujMzZ0SbZmNOAXSHQ2Kv0SmQ9Y1FWBTwikBicA=; b=lcoMp3hftWA+yXVSqoHe+fD5HGlbPTHdTHa10wrGiw6nhwwCJlz2TSWoTd2EDwYhV1heiu a2HgKToD99OgJgDQ== From: Sebastian Andrzej Siewior To: "Jason A. Donenfeld" Cc: Jann Horn , Andy Lutomirski , Boqun Feng , Will Deacon , Peter Zijlstra , Ingo Molnar , Waiman Long , Sultan Alsawaf , Theodore Ts'o , Andy Lutomirski , Jonathan =?utf-8?Q?Neusch=C3=A4fer?= , LKML , Thomas Gleixner Subject: Re: [PATCH v3] random: remove batched entropy locking Message-ID: References: <20220204155142.56419-1-Jason@zx2c4.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2022-02-16 21:58:14 [+0100], Jason A. Donenfeld wrote: > > Why raw_cpu_ptr? include/linux/percpu-defs.h says about raw_*() operations: > > > > * Operations for contexts where we do not want to do any checks for > > * preemptions. Unless strictly necessary, always use [__]this_cpu_*() > > * instead. > > > > So when I see a raw_*() percpu thing, I read it as "it is expected > > that we can migrate after this point (or we're in some really weird > > context where the normal context check doesn't work)". Is that > > incorrect? > > If it says "contexts where we do not want to do any checks for > preemptions", then that would apply here I would think? We're taking a > local lock, which means afterwards there are no preemptions. For > context, the code that got committed after Sebastian's final review > is: > > local_lock_irqsave(&batched_entropy_u32.lock, flags); > batch = raw_cpu_ptr(&batched_entropy_u32); > > However, I think most other code uses this_cpu_ptr() instead? So not sure. It depends what you are looking for. raw_cpu_ptr(&batched_entropy_u32) give you the pointer to &batched_entropy_u32 of _this_ CPU - the CPU you are currently running. this_cpu_ptr() does the same except that it has smp_processor_id() in it. smp_processor_id() will yell at you (given you enabled CONFIG_DEBUG_PREEMPT) if the code can migrate to another CPU. So it will yell at you unless: - you disable preemption / migration so code can't migrate - you run on a per-CPU thread i.e. a thread which is bound to a single CPU and therefore can't migrate to another. The local_lock_irqsave() acquires a lock / disables interrupt and therefore can't migrate. So I suggested to use raw_cpu_ptr() as an optimisation in the debug-case. If you disable preemption before accessing a per-CPU variable with this_cpu_ptr() then you _need_ to ensure that nobody is accessing the variable from in-IRQ context. Nobody will yell at you. But if you use a local-lock then you get lockdep annotation and lockdep will complain if you use that variable always in process context and sometimes in-IRQ. > Jason Sebastian