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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id E02D5C3ABC3 for ; Mon, 12 May 2025 17:16:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 40D706B0196; Mon, 12 May 2025 13:16:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 392996B0198; Mon, 12 May 2025 13:16:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 20D986B0199; Mon, 12 May 2025 13:16:44 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id F1C626B0196 for ; Mon, 12 May 2025 13:16:43 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id C6132594CD for ; Mon, 12 May 2025 17:16:45 +0000 (UTC) X-FDA: 83434910370.20.FDCBD65 Received: from mail-wr1-f48.google.com (mail-wr1-f48.google.com [209.85.221.48]) by imf25.hostedemail.com (Postfix) with ESMTP id D23A8A0010 for ; Mon, 12 May 2025 17:16:43 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=AhvI96TF; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf25.hostedemail.com: domain of alexei.starovoitov@gmail.com designates 209.85.221.48 as permitted sender) smtp.mailfrom=alexei.starovoitov@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1747070203; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=n9Ugkb47FGB9bw6za0PrdUHlAQDy5Ir+PZ8NxZxiXL4=; b=lAwg09ZPR/lUYOB/HJ0s0zoUTBXTUnZceLk6QLWKQ8uROAR3cMfKcxOPnOaUmNamoVyYGd pcBum3354CS5FicnVMH9I/WMYSrQKCQnmjJlYkfoeqkbJLSkliRBXdpD8hJzdIwZtALTVb hYDwcbTDYtdivU97qmc2hkSRFD6L0Tc= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1747070203; a=rsa-sha256; cv=none; b=JbdeELhgjbCEWDEOCyXJJjxqFga4aEjMKyF4nC7esj5AeLnl6R5cjL9I2ZadGyWVFGrHj4 3/coijsoLnorV5lzzfO98M/ZzpjLGODMRiy805+GxNNhxmiR11qyId3eFQkiwYXOPMP4CU vq9g5blOEntjXvx8RrhtPRG0nC2rQ24= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=AhvI96TF; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf25.hostedemail.com: domain of alexei.starovoitov@gmail.com designates 209.85.221.48 as permitted sender) smtp.mailfrom=alexei.starovoitov@gmail.com Received: by mail-wr1-f48.google.com with SMTP id ffacd0b85a97d-3a1d8c0966fso2950211f8f.1 for ; Mon, 12 May 2025 10:16:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1747070202; x=1747675002; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=n9Ugkb47FGB9bw6za0PrdUHlAQDy5Ir+PZ8NxZxiXL4=; b=AhvI96TFSyjfC2EVlBl7hlNksomYtsi6iUu/IDn9B53fJtu1Jph5imh+4mOUbZb8j4 smcnRLW+IwlG6LAf3doQsamRmNoESbYD5xqZ8u3MCFJRQdPHgv1mA/6oUCLOl1L/IHJv PYm8ecqnL3lGRJcqiSqPZqvIKlkSUfMiv4Dcg0qv1h5rXJsvA16Cv4ekqQETIW3vZb0t G+JdSXA0EdzLBeYQQPxsvP9oO9e1SSE77lFk2VQrFPFVYHml4qlfkL+A5CTZwZPALeFb JaVk2/Lk5w3Z2Ww1tr0+lO9ORAdSPMHN5ojNz6UUErrQZE0oItFm64FFF8/qaTP+ZBJ4 Fi3g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1747070202; x=1747675002; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=n9Ugkb47FGB9bw6za0PrdUHlAQDy5Ir+PZ8NxZxiXL4=; b=fs61eJijSOyTGEcSSJ/e31wTC1BkU/B78JizRC1QA7CGVYhVvc3sdAIjpck2gDqfPa 3dXnt1/H6N3L3wvv3MC6c3QbSEI9RZDpxNvKCr3TBve1ta7oMcihFaiZPUb49VPd6g97 /hnqNah/kdSEkekVmYeLbjBs2cV4JCD0KpG2MhQUHc/RidsgJIFcDoe0IBHeC/JOY6LO 5biLX1YERN2AJTfQDMBIWxbKwNT2o2JfygNBai6inLSRtPUpNCwnYs0fKHC7/MvQTdqW Uh74gnhDOrMumEp+QJTnhjYk2spii19s9y8fUJNQZGSxKTdK2ISBFXZo7JZwNsNYXIub 99RQ== X-Forwarded-Encrypted: i=1; AJvYcCVsfY+GRVY8tQfYElPCXhJKOXThjR67tsz2BnOnjltTfEnkHb5iC0UuNMMY/K0NSbE1MwWlyzTFuA==@kvack.org X-Gm-Message-State: AOJu0YxpFScLJ5I1H6M6NrNi5I+BAH2r+C5Vqv/OZbU6vc8l9a07CQZb P2607F/3j+BSXRZH8RaEJ7axKPfuiELr2BeYZ/Es4YDl6AWxvXdTiG/zz1FgHqBRF+r8xTfRTDK EnRSOqfN03GIRA3E6JDhLAnw3vPk= X-Gm-Gg: ASbGncuh5HDN1xDH+2EnT3Y9Sy4+07TYUXaYbTy2YBqGvxdDX5ovW48Mwc1z0xrB5hL zQf6uShsWxZyBUqzGx3ebEtDvqJpeULMvzfiLkOd75Eij5W+hphUypg3rAa+rAyHOQocVsQ6xrm GUXNZw5GMmZAg+OCc+CHMBNYUC6hK77VcGVWvJ12SI9lzXkm6rIyee5gLJMxA= X-Google-Smtp-Source: AGHT+IEFO81VYZtZULAu4VLScRjm7TCA581BtFO8mErq1xKitjODBjz5vRMDv2OewxxhQ263vCRx1tOAMJZGhpX5Yy0= X-Received: by 2002:a05:6000:420a:b0:39f:c05:c220 with SMTP id ffacd0b85a97d-3a340d2d334mr236282f8f.22.1747070202244; Mon, 12 May 2025 10:16:42 -0700 (PDT) MIME-Version: 1.0 References: <20250501032718.65476-1-alexei.starovoitov@gmail.com> <20250501032718.65476-5-alexei.starovoitov@gmail.com> <20250512140359.CDEasCj3@linutronix.de> In-Reply-To: <20250512140359.CDEasCj3@linutronix.de> From: Alexei Starovoitov Date: Mon, 12 May 2025 10:16:30 -0700 X-Gm-Features: AX0GCFsvjB7lchg81Xd9PQcdE0jHyptHMqB4ExvIEn_FR6Cg1YPKRo3RlQ9kpSg Message-ID: Subject: Re: [PATCH 4/6] locking/local_lock: Introduce local_lock_irqsave_check() To: Sebastian Andrzej Siewior Cc: bpf , linux-mm , Vlastimil Babka , Harry Yoo , Shakeel Butt , Michal Hocko , Andrii Nakryiko , Kumar Kartikeya Dwivedi , Andrew Morton , Peter Zijlstra , Steven Rostedt , Johannes Weiner , Matthew Wilcox Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: cugjh3tdiuwnunok9haswywrtsnrktp8 X-Rspam-User: X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: D23A8A0010 X-HE-Tag: 1747070203-783371 X-HE-Meta: U2FsdGVkX1/2xkU0AJv58aJKxL/UxtoL7gldVVc9vp8vjwOx9Y0g079oPHo4HWc1WZwyuR8SvFu/0Wkp1UPUo79X+gR/OdBwnmeUmRDFovhDmOKzTTCSiXBsLS6I5MpJyFoxIMu7Cfe0HsGCCBt7SIrTJ+Y9ky/2GozAwAfHMxQ0x3PcaREfGmAfNV+X0WTeBV70gAycqCuRzJZvMqJLLOBsUgdxAgGjrs9IBVO17jtQZjIlSQTXYObAoezweQoZ5Wm26chlHBPGibtKqv224i9d+DY+gKiTWtjVyANae2wgKSsN+prHpPcUkMG55UjNKVK/0dSJAqwc5oJIEOcNDwLKKT9u5mH7gu3pt7f+7scL6Adyga465dX7eGzQgNuM1ZYjCsdTw8Yo3pDiEi/hKFQ2quRrXo4+7z7spsITQB5D9CoPd+Cx81SlOexD2OzBpran5whGce5m+RnfjU4V8qao4De6W6/kpL2z0jlvm0BMxWadNHuWqn/XZ1zKi5k7xSvnI+pKAYpom97dnty6oOZZYG8pvX5zlKZ8PoVp8zPeddJmXd+8RCqaB3hB1DF/W2XtFIFhdkeg1DT5EK+8y/FBP5yPYchp8iFSnr+PKxYXox6AWgKj86s7ao8ms182JbcDRzhqUd4e/EzEIf10BMOziHF52Gp1lidRijwwVKnc9CKYHMj5Au3dcVL7Bm8lcAxwgYiyV3jjWmjjwOYpPiEWnIct7d6POLN/wzSb4RVzI4mn1BzdS3duTMzE7lZ+WbPY9gBZjVuZOYBgcQbdaHxeUix5TQz5hn7/74V78aVwoopaj2Mv1SO16sgFmmmrfzmRGx6K6ThJQ8nHRCkY93bLpDSlrlRsNTgzXJpT5j5tGf/TBn+DuUd+fipfewaZanYuue4SAXCQ6unauGxGJIdV5woQxe2lfx2hZ9hEy5VS7d65gNVpciQ+kEEnNHYiR5vBJudFHuY0132DrE2 wjXSMlUj 6mstpnuG0eetVOuj0z4NTZg6V5E/5snZ7g8Ugo6+CyELeXpkUTyDsru1ISfzrzjEdsyTrn1CbVcKeHqqiAWFor50vzVa2iEUErWg4x2M52G76nC0SrKbbIq7e68zx7IZ5cB8UxCSod8eskw7cMNsKHITDqwpTBBX88NU2jWmI3n+Rly4eA3VdBoLFT73TYxpXfIHeNmnU2GU+yQrKO/DtCCcLZVFPgK1XAwvx6C71OTjX7dgiB7T8qrL55rRa9V44d1nvAerJNmev8bJXjgowmLlBTYu22d8pxm1eQebwOiYVMPvIV6owX4O12uiF3zC6K1BoLQm0wDpAC6lIytJ2ZUCEPF74hKvbcnMkhQ3CRpKc1lRPE3gzp1NViLWuqV0Ba1XENF1fNWHIPks= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Mon, May 12, 2025 at 7:04=E2=80=AFAM Sebastian Andrzej Siewior wrote: > > On 2025-04-30 20:27:16 [-0700], Alexei Starovoitov wrote: > > --- a/include/linux/local_lock_internal.h > > +++ b/include/linux/local_lock_internal.h > > @@ -168,6 +168,15 @@ do { = \ > > /* preemption or migration must be disabled before calling __local_loc= k_is_locked */ > > #define __local_lock_is_locked(lock) READ_ONCE(this_cpu_ptr(lock)->acq= uired) > > > > +#define __local_lock_irqsave_check(lock, flags) = \ > > + do { = \ > > + if (IS_ENABLED(CONFIG_DEBUG_LOCK_ALLOC) && = \ > > + (!__local_lock_is_locked(lock) || in_nmi())) = \ > > + WARN_ON_ONCE(!__local_trylock_irqsave(lock, flags= )); \ > > + else = \ > > + __local_lock_irqsave(lock, flags); = \ > > + } while (0) > > + > > Hmm. If I see this right in SLUB then this is called from preemptible > context. Therefore the this_cpu_ptr() from __local_lock_is_locked() > should trigger a warning here. When preemptible the migration is disabled. So no warning. > This check variant provides only additional debugging and otherwise > behaves as local_lock_irqsave(). Therefore the in_nmi() should return > immediately with a WARN_ON() regardless if the lock is available or not > because the non-try variant should never be invoked from an NMI. non-try variant can be invoked from NMI, because the earlier __local_lock_is_locked() check tells us that the lock is not locked. And it's safe to do. And that's the main challenge here. local_lock_irqsave_check() macro fights lockdep here. > This looks like additional debug infrastructure that should be part of > local_lock_irqsave() itself, The pattern of if (!__local_lock_is_locked(lock)) { .. lots of code.. local_lock_irqsave(lock); is foreign to lockdep. Since it can be called from NMI the lockdep just hates it: [ 1021.956825] inconsistent {INITIAL USE} -> {IN-NMI} usage. ... [ 1021.956888] lock(per_cpu_ptr(&lock)); [ 1021.956890] [ 1021.956891] lock(per_cpu_ptr(&lock)); .. and technically lockdep is correct. For any normal lock it's a deadlock waiting to happen, but not here. Even without NMI the lockdep doesn't like it: [ 14.627331] page_alloc_kthr/1965 is trying to acquire lock: [ 14.627331] ffff8881f6ebe0f0 ((local_lock_t *)&c->lock){-.-.}-{3:3}, at: ___slab_alloc+0x9a9/0x1ab0 [ 14.627331] [ 14.627331] but task is already holding lock: [ 14.627331] ffff8881f6ebd490 ((local_lock_t *)&c->lock){-.-.}-{3:3}, at: ___slab_alloc+0xc7/0x1ab0 [ 14.627331] [ 14.627331] other info that might help us debug this: [ 14.627331] Possible unsafe locking scenario: [ 14.627331] [ 14.627331] CPU0 [ 14.627331] ---- [ 14.627331] lock((local_lock_t *)&c->lock); [ 14.627331] lock((local_lock_t *)&c->lock); When slub is holding lock ...bd490 we detect it with __local_lock_is_locked(), then we check that lock ..be0f0 is not locked, and proceed to acquire it, but lockdep will show the above splat. So local_lock_irqsave_check() is a workaround to avoid these two false positives from lockdep. Yours and Vlastimil's observation is correct, that ideally local_lock_irqsave() should just handle it, but I don't see how to do it. How can lockdep understand the if (!locked()) lock() pattern ? Such usage is correct only for per-cpu local lock when migration is disabled from check to acquire. Hence the macro is doing: if (IS_ENABLED(CONFIG_DEBUG_LOCK_ALLOC) && (!__local_lock_is_locked(lock) || in_nmi())) WARN_ON_ONCE(!__local_trylock_irqsave(lock, flags)); in_nmi() part is a workaround for the first lockdep splat and __local_lock_is_locked() is a workaround for 2nd lockdep splat, though the code did __local_lock_is_locked() check already. In your other email you wonder whether rt_mutex_base_is_locked() should be enough. It's not. We need to check: __local_lock_is_locked(__lock) \ rt_mutex_owner(&this_cpu_ptr(__lock)->lock) =3D=3D current Because the following sequence is normal in PREEMP_RT: kmalloc local_lock_irqsave(lock_A) preemption kmalloc_nolock if (is_locked(lock_A) =3D=3D true) retry: is_locked(lock_B) =3D=3D false local_lock_irqsave_check(lock_B) while lock_B could be locked on another CPU by a different task. So we cannot trylock(lock_B) here. Hence in PREEMPT_RT __local_lock_irqsave_check() is doing: WARN_ON_ONCE(__local_lock_is_locked(lock)); spin_lock(this_cpu_ptr((lock)));