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 E829DC3ABC3 for ; Tue, 13 May 2025 06:58:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E6B866B0085; Tue, 13 May 2025 02:58:48 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E1BFD6B0088; Tue, 13 May 2025 02:58:48 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C70926B0089; Tue, 13 May 2025 02:58:48 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id A3E9E6B0085 for ; Tue, 13 May 2025 02:58:48 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id DB6B51219DD for ; Tue, 13 May 2025 06:58:49 +0000 (UTC) X-FDA: 83436981978.03.C526133 Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.223.131]) by imf21.hostedemail.com (Postfix) with ESMTP id 54E321C000D for ; Tue, 13 May 2025 06:58:47 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=ohnsBIxQ; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=jRHEwBNT; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=y8U+0rYT; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b="ku25K/j6"; spf=pass (imf21.hostedemail.com: domain of vbabka@suse.cz designates 195.135.223.131 as permitted sender) smtp.mailfrom=vbabka@suse.cz; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1747119527; 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=KHFYjp+NZF69gFs84lEm0Tqe7hwBiQDRrO4TAqTmZlQ=; b=zVDB602Q04M4Sg7DyxRmRJQkFHU0BVFGFHCVoZIhMN8D+NUmeGHwdjg/5hqj/rZpSXtisg 3ei3C7h9WN/JE7ggvQ8L2bBriJCQugrJlHV1BiTeFKuCFZ24JCJXbjJiXlHMxPwLNfjZN+ LrDyF4cXcu2/a9q68tMlH3ITu2FJjho= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=ohnsBIxQ; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=jRHEwBNT; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=y8U+0rYT; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b="ku25K/j6"; spf=pass (imf21.hostedemail.com: domain of vbabka@suse.cz designates 195.135.223.131 as permitted sender) smtp.mailfrom=vbabka@suse.cz; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1747119527; a=rsa-sha256; cv=none; b=ucvTtWzmMzseVH9hA8yR5DXwZ2H8oBMYBz7c65lY1imgLSHEWPbz/iMpYMiCJtP0EbW6qu sfFb37OF5cQDKoEAVmOhyBZ2ltmRe/1IbsMxuvQWcG8IeqyFbhgh2AbhLHuTrnffqo458b LJR2wo24+eYQIdYssv57H57kUlTI8ow= Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 30DC11F387; Tue, 13 May 2025 06:58:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1747119525; h=from:from:reply-to: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; bh=KHFYjp+NZF69gFs84lEm0Tqe7hwBiQDRrO4TAqTmZlQ=; b=ohnsBIxQcoYJSBP3KrzPYNO2BCmMLFsVATlnVsJkBuGBj22MmV/LWs+jL4+DHCwmCTm6jn L+TrH8zO4yL2r87BYdcKuwbPdwlF5DjnJ9VAI3cU7RfL6qBqBiG4sk99jh9qSU5pc33YGb L0k4a5krbsR9biXHOQza5hPRcWfHeg8= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1747119525; h=from:from:reply-to: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; bh=KHFYjp+NZF69gFs84lEm0Tqe7hwBiQDRrO4TAqTmZlQ=; b=jRHEwBNTm5Ue38W7au7yMQMQn2neYPyeuN9KC30Y/+zZja8nSUCSsu08yDkM6iQsLDG5df DJvL2jyPx7L2rRDQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1747119524; h=from:from:reply-to: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; bh=KHFYjp+NZF69gFs84lEm0Tqe7hwBiQDRrO4TAqTmZlQ=; b=y8U+0rYThE7ed7Ek6inI7M/CyA7vBD+f5TmW+fqf7/kzC0Fr55Q6dPU5TCfcrFVwsW6N6l G9fXF+vG5/P+oPHggb6ZPV9Dj6OdKxctL2n6z8EIUexnQg6FMh5Z9DyxxrKrQigguiU1hQ 5bcCTAcdTw+Y+yNffoBN4wXtlhCoaqU= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1747119524; h=from:from:reply-to: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; bh=KHFYjp+NZF69gFs84lEm0Tqe7hwBiQDRrO4TAqTmZlQ=; b=ku25K/j6M6jzfOGavHwWzq0FvTjkfBJJy33xh6FLnU97iTk5vMDRc0OUV0BUARztcFPB+N LyeBSw63c0MSIaCQ== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id F37E8137E8; Tue, 13 May 2025 06:58:43 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id 1HqNOaPtImhFUAAAD6G6ig (envelope-from ); Tue, 13 May 2025 06:58:43 +0000 Message-ID: <737d8993-b3c7-4ed5-8872-20c62ab81572@suse.cz> Date: Tue, 13 May 2025 08:58:43 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 4/6] locking/local_lock: Introduce local_lock_irqsave_check() To: Alexei Starovoitov , Sebastian Andrzej Siewior Cc: bpf , linux-mm , Harry Yoo , Shakeel Butt , Michal Hocko , Andrii Nakryiko , Kumar Kartikeya Dwivedi , Andrew Morton , Peter Zijlstra , Steven Rostedt , Johannes Weiner , Matthew Wilcox References: <20250501032718.65476-1-alexei.starovoitov@gmail.com> <20250501032718.65476-5-alexei.starovoitov@gmail.com> <20250512140359.CDEasCj3@linutronix.de> Content-Language: en-US From: Vlastimil Babka In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Action: no action X-Stat-Signature: pbctugtd3o5d6zw8a1xznrxrakkhekqk X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 54E321C000D X-HE-Tag: 1747119527-944939 X-HE-Meta: U2FsdGVkX185pmy0mqRDLlwocycW1QLX/XUSTLbAkFQP+UfcdTYYtG0sBjiAYOwhDcpnodSl4i+9X7yx7CBMn8Y3fSRk2wuS7+eLy4GogYDdyUaDrzDmc9K4KNinlUGMWd4i98FT0NmXAdMHi7CNvJj1lf/TnOlJBKIWYCqJMzgyS1y0hPWndahf1j3+agTu3Zpq25uSr0p1B4cMDDeomm+LvJfxBPH3c9hzrA77hL317VMtUqBEerH0ImwTgyo8FWHkoz5u8lERBuwOXDc7PLPH5omiQ4pDZEj4bjGSopNzlyhCxw/DBrhRvFI1YmeaIRwNT5r7SF3XTjpptSm8J/cFyu3Ba4EHXRcoX6hgRgj0BMIIMGU/jNGbhjU0xXOTgmPLQp8oJaBAhoQkVqBNoe26zIMSotGdC/qSpaR4/kGq9u7aeBiuBQtacuOMbagAAI8y7h9ZpKoY/xiH385aQ2AdMVY647bJeeSKJoLbirzuG42R7DQNKdhKM6NMn3+hur6h4Is9li3xOlF+qFCEVgdGish7f58oQLRPz0Jh3lcn2lat1k/w+RDEr/T958SwLJt5ZsvHkorQoq6Nrfbd4qPzqX1KNo4xXcuUPpfzHjBrw+zKU/9GEga04hzeF6C8mhUhmyPisQIRctcjBnXBQUWYhewUI3RCpmek/EwbrIGJ2Ukz7BGe82+jG7wrRfbl/Q43kZunZryYgr1qpOp2kunk92o29MMfsNl2x74odur3aB52ypePUQ7IWdcvcrg9MnDPVEkMQMaUHa4TJgVgY3/B8udk6jg1Kah60xkSJoQCYrzR183zJ8lWZBPeCaCaMTpvYgzOEMCeBFqJONvRSVuQBf9Qt0/tOfvkP3a4uHiBtDKFVY0RQdaD6VHwIK8l+TpPY/cMaLxpGfujrKCbAhak+CmO+BjkCek3QqeopocgdMYm+o6xkFjqlAqgWRSJhIcMlgo8yb7p8rlCNkm xtQQKVcn ikZNHFk+/cYmdoWbNaQtBEAPPnKofGknLTm9JmxfWc99P/bGPYZuEvJJ5N+FYEjRZMDZVBqeDsfvy74C00oUDyZ23Sd3jL3xW+TWVq13dgeNvHZ66TJb5o+6GYfNyfEcAHI5lnz21oQqZ06BuXARkaVf7yA7pGDPPqH95f0QPUMXYyWrKFpF/HZTShnG18ADz8EE0qoPpgBlNRsXqTG2sLvvdJrIuiFRPRPdxQPxXWFrRypMCkAHpkRqXGfe2LgM9gujLvcdXrff5uazSPTMNiMjNMATzTBR16+0EdTOMA+zoEBShNHYglUNdtnKP93Xk+2MJCmDswiyPN6cSMt7+rK5TwMXKsDIcoFGJeQCSOjBr/kAzNsTPkz9QZeK7aKRjlo1SUCRwntCndCbyvEBgDBPLSzRXogWw5GhLMNBYdYL819Y= 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 5/12/25 19:16, Alexei Starovoitov wrote: > On Mon, May 12, 2025 at 7:04 AM 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_lock_is_locked */ >> > #define __local_lock_is_locked(lock) READ_ONCE(this_cpu_ptr(lock)->acquired) >> > >> > +#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. Thanks, I think I finally understand the issue and why a _check variant is necessary. As a general note as this is so tricky, having more details in comments and commit messages can't hurt so we can understand it sooner :) Again this would be all simpler if we could just use trylock instead of _check(), but then we need to handle the fallbacks. And AFAIU on RT trylock can fail "spuriously", i.e. when we don't really preempt ourselves, as we discussed in that memcg thread. > 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. So here's where this would be useful to have that info in a comment. However, I wonder about it, as the code uses __local_trylock_irqsave(), so lockdep should see it as an opportunistic attempt and not splat as that trylock alone should be avoiding deadlock - if not we might have a bug in the lockdep bits of trylock. > 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) == current > > Because the following sequence is normal in PREEMP_RT: > kmalloc > local_lock_irqsave(lock_A) > preemption > kmalloc_nolock > if (is_locked(lock_A) == true) > retry: is_locked(lock_B) == 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)));