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 32435C02180 for ; Thu, 16 Jan 2025 02:20:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 785826B007B; Wed, 15 Jan 2025 21:20:54 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7358B6B0082; Wed, 15 Jan 2025 21:20:54 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5D6BD280001; Wed, 15 Jan 2025 21:20:54 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 3E8E56B007B for ; Wed, 15 Jan 2025 21:20:54 -0500 (EST) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 9DA7DAECF8 for ; Thu, 16 Jan 2025 02:20:53 +0000 (UTC) X-FDA: 83011711986.06.3AF648A Received: from mail-wr1-f47.google.com (mail-wr1-f47.google.com [209.85.221.47]) by imf18.hostedemail.com (Postfix) with ESMTP id A39FE1C000B for ; Thu, 16 Jan 2025 02:20:51 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=c9zIlMwO; spf=pass (imf18.hostedemail.com: domain of alexei.starovoitov@gmail.com designates 209.85.221.47 as permitted sender) smtp.mailfrom=alexei.starovoitov@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1736994051; 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=hRlfV8HawfxpT8T4TI/pnqhjM8rBBcPb6CYaKC9QCFM=; b=2cmvncdQYON5XwxGknqn+8Tg5CkjupjeMhnHwq8lY0q9+XxfhKPJ0leGHBiwdqk/bEgxO3 UW3aDSjzcot11DtskMhXsWfQFP0N/Lu+VYHP/AjAC1yzrM+zTvSuPpSqdI45Ehj7FXVn+1 D4d469C3U0fTFV+zt8tGlW+8YKh3iW8= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=c9zIlMwO; spf=pass (imf18.hostedemail.com: domain of alexei.starovoitov@gmail.com designates 209.85.221.47 as permitted sender) smtp.mailfrom=alexei.starovoitov@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736994051; a=rsa-sha256; cv=none; b=fsVjaLHJmRj+cQYKGkSjVp0DUeol+KTPk80txo2H7leCMMOvge6GQ9NzSzEnYjvR0D79WL 8NaSlt8VxJtBLzEGt2UMm60R9XHglWCyOlzGZQtF2q5nm9RqkbsnMZwdEIIO3sA8kA6WTx rwlBb5Qf4bSFcvwZ8lj88Oot1ofAauc= Received: by mail-wr1-f47.google.com with SMTP id ffacd0b85a97d-3862d6d5765so213343f8f.3 for ; Wed, 15 Jan 2025 18:20:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736994050; x=1737598850; 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=hRlfV8HawfxpT8T4TI/pnqhjM8rBBcPb6CYaKC9QCFM=; b=c9zIlMwODq2JNqqrvEBrn6xYqNMZ5Z7Z4itRzVbtsf+ZJitL33EyVhKGPuRzx/R7Pr ZPX+1ZQ1yayozr0ZwdR25DHoVtggvrU4LbxlQtOTI5UodGN5VvxsPNYw4hIRU8Liie4N bV5V32wGvo4AFLSuTAYpyJu4wNUE3Kblp0mJL10MnIwDiG5TqSkLQHgm3bn2XDmTLFCK p1MexHih6tFpGJW5rMlYtwWsNBQdgrchkZkksOvRjsNKUIRxPZmjlKjRJ0S+HJc2keMo GKJTb+dBmSpFB0Z4J+CfMOv51A/Aj7euJHZibFNF1wM9WEsh/xB8FghoznZpJZ78oFj1 3Vmg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736994050; x=1737598850; 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=hRlfV8HawfxpT8T4TI/pnqhjM8rBBcPb6CYaKC9QCFM=; b=I6qsCh+F5cWGwZvKLCrMilS73plcYSdCVjOTVBcaCQsdlYMhfduraKhkZ/zVAnJUiV rbhqfrrZdTjHUCZ0T8COPbIj3pWeMSbhbCANdd6pyx+0OCNvSm5tp3gWG/jCV0cpwFoQ k0/AbrCZYiNhqnNNeFZoWIPVSe2i/Wgh8B+R3fYAo1+WRH7zSzu1Od6beCOPtP18SEgH 8ZM9AE0RHzgYaKPIeViwDGPnonKgEInhUJn4wZP0SIAgRLFk2CkRg/z8jtTZmqMpo38C y8i5HdOIPYsf1NHRYySR6DFdrRRl7kIF/U4l6FT6cMirsSYurMPvFU+8q35r6RUC1aMB SWKQ== X-Forwarded-Encrypted: i=1; AJvYcCUuDD71ilGI9C0PZyHJYDAoS2SL4XoPhOwu7pqyUg5r7tR2lT32dizNXOJAKVI4AP1rwmbKE2BqIQ==@kvack.org X-Gm-Message-State: AOJu0YwFuIo/094sR6NDqUCCtwmmICwVKJ6WqVxlvW3b/U5wJqSQFhPN J6vDxWVcOjxT9fXXnREasUAD2ZhETtQi6kNFLoqc/fN297HqtuBUJ53EZdgH2RY6kOf3ZP6K+9W 1MbegjLYHYYD8aCDIsGM9J5IrzkeC2AQY X-Gm-Gg: ASbGncs9RXhOm4c2UZOzMgqez/XtDmUKTV+BtcPUgJYQJYnbRsNNcNLatTOLDcNQf4Y TtEdtucRw/mEjexV4rg5XBrHlbtUvNAyApvWGj2i3pAtIBPv4bW/ZIQ== X-Google-Smtp-Source: AGHT+IH4UULK1HVLpczFGPJfQvxAuFwYt6Vz0YbS+lFQZfLRCjaI6ne7OABkBzzhchGKMb/a7imWAzY+HNYx20c31h0= X-Received: by 2002:a5d:64ed:0:b0:38a:9f27:82f2 with SMTP id ffacd0b85a97d-38a9f278491mr15270330f8f.49.1736994049689; Wed, 15 Jan 2025 18:20:49 -0800 (PST) MIME-Version: 1.0 References: <20250115021746.34691-1-alexei.starovoitov@gmail.com> <20250115021746.34691-4-alexei.starovoitov@gmail.com> In-Reply-To: From: Alexei Starovoitov Date: Wed, 15 Jan 2025 18:20:36 -0800 X-Gm-Features: AbW1kvauJUoZ1NXXwEjf8FCxtWpfLGkbHhkR9AY0EL8FcloeCpiUIxyjHXnZdkU Message-ID: Subject: Re: [PATCH bpf-next v5 3/7] locking/local_lock: Introduce local_trylock_irqsave() To: Vlastimil Babka Cc: bpf , Andrii Nakryiko , Kumar Kartikeya Dwivedi , Andrew Morton , Peter Zijlstra , Sebastian Sewior , Steven Rostedt , Hou Tao , Johannes Weiner , Shakeel Butt , Michal Hocko , Matthew Wilcox , Thomas Gleixner , Jann Horn , Tejun Heo , linux-mm , Kernel Team Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: A39FE1C000B X-Stat-Signature: p13nsj5knei4epytkqot8ekyr9xtywfe X-Rspam-User: X-HE-Tag: 1736994051-531851 X-HE-Meta: U2FsdGVkX18RlP7Qg9S5HJCZ43nukaZuxYRJp0p/dy9MHhxiBv7HZG3Q7jkr6YHJxlB/t+2ID7JPqzQLDn7ZQR8qinoAGcznq9THVzvA/rQT0eDND2PNhegzj+kazlQ5l6Ic7G9dsWViMXKwjyCquwVH7RFGB6CJ9gFPEWP/olNMPalrRF5E3tkC7Kii6gvJqtkBLugOgcyOkX33zz/yGJ7Fkt9rrt4rkvcdBN6yaw4uwKGMGTE7UwbAIz5mc5BWNAmr+bT+cuVtrZdIRpMEJGRMnN5yCMLr9cagVs5ZntRrBN/KFOK7njPEgdPfDoAdDJFzm+kA/Qdeko6UiCzlb8u73k+vVazvZmEJvsYzClD+CNuqAnAKnNFSXffITD2WfpJ4WA4nBN7KDGYIQrmiK5Mj/k1zpt/j1E+PL5+2XMuQt1UCcXZzdJb4UedcH5cm/JCNOkc84dTzDjNwFjreJkPpoi8qZ4Bmmxvc0w8XNYD8XalTMkI3kPw66Dhr0GW6NJUjGP2GCjhTaJ3aHWZsgGzVuNLPzN7zwZM0lTxSv55y4eG3m5G/eZ7IdTzGjSC2IyVG/iYy9GWT7ImINpDlOzzfgp3EIxGFaRKYFiv2aTbVD75zQ6AfBJ7C10kopxoTe63lZ7AuF/dT0clfj40V8kTI58+qejFveil4xSw2CdkIr+4kkerRSJzSyYOpyH7Ytn695v1Mo0lQDDjW0swQvLL6F9NxWg7kb3gl+JDChBW9foEB40L6gB7hAnAp7Jtvd3tSOU/z7nnHGFeqVoOgrSav5pDqW2JRaU8T8kkUDkWdXNKdtd8ecwLdVpoQvwL9NwsTxF0y13E6FkjEyPCJL8AVSnyETf6T0mH4/tFC2rOMUBc9dzMmZDp0KHwCI6Idblcy2hBB1+DlHsT4WZhtyR3FNDzwSpeLeFwkL5DKXcy9TmNWG6T97HIRiL8Nmed5Ipu10kuZAjUjXnRviuF Lj6k2a7l KtDS3numJNNu2ONP5461hnHogtsazD9y/wHorrWLX7viEJh2myjCicWN3zv1o5q7jDKIio2Cs6lMnvyb6h8pW3YpeUfKRuly28WghQtHYi9njXQUZzgxvmy7fRKLjPHTXqLdHlRoJz5oxDR1BQ5nP7sIyxG7yYkU1EbhvziQ5Ojruh2zeHHf7vMyH5rehlGFIWRhUDIgQ5IlFCbYyNuRfj4zAQrXycL9b/+zb2Hv3ZAJj65B+B2nwiXuVl+4BhJvUF2RKQM/YcAzPSZHEqMislnWLNsU+2KQrBz3beLbIZFYae+TbDLEuL9Pr02HgHH0kJMCrjwQWAs/bxPdLC+JjZinyD7ie40iMD+EgKXq76o1R8TerHDIKndHjPv+AejhmVrwhKuCMmMdkHqV/SjOrmr3p3ebkg8LSzxINr3/RL7/mP/A= 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 Wed, Jan 15, 2025 at 6:22=E2=80=AFAM Vlastimil Babka wr= ote: > > On 1/15/25 03:17, Alexei Starovoitov wrote: > > From: Alexei Starovoitov > > > > Similar to local_lock_irqsave() introduce local_trylock_irqsave(). > > This is inspired by 'struct local_tryirq_lock' in: > > https://lore.kernel.org/all/20241112-slub-percpu-caches-v1-5-ddc0bdc27e= 05@suse.cz/ > > Let's see what locking maintainers say about adding the flag to every > local_lock even if it doesn't use the trylock operation. As I replied to Sebastian there are very few users and hot users of local_lock like networking use it in RT only. local_lock_nested_bh() stays true nop in !RT. This patch doesn't change it. So active flag on !RT is not in critical path (at least as much I could study the code) > > Use spin_trylock in PREEMPT_RT when not in hard IRQ and not in NMI > > and fail instantly otherwise, since spin_trylock is not safe from IRQ > > due to PI issues. > > > > In !PREEMPT_RT use simple active flag to prevent IRQs or NMIs > > reentering locked region. > > > > Note there is no need to use local_inc for active flag. > > If IRQ handler grabs the same local_lock after READ_ONCE(lock->active) > > IRQ handler AFAICS can't do that since __local_trylock_irqsave() is the o= nly > trylock operation and it still does local_irq_save(). Could you have adde= d a > __local_trylock() operation instead? Guess not for your use case because = I > see in Patch 4 you want to use the local_unlock_irqrestore() universally = for > sections that are earlier locked either by local_trylock_irqsave() or > local_lock_irqsave(). But I wonder if those can be changed (will reply on > that patch). Pasting your reply from patch 4 here to reply to both... Yes, I'm only adding local_trylock_irqsave() and not local_trylock(), since memcg and slab are using local_lock_irqsave() in numerous places, and adding local_trylock() here would be just dead code. > The motivation in my case was to avoid the overhead of irqsave/restore on > !PREEMPT_RT. If there was a separate "flavor" of local_lock that would > support the trylock operation, I think it would not need the _irq and > _irqsave variants at all, and it would also avoid adding the "active" fla= g > on !PREEMPT_RT. Meanwhile on PREEMPT_RT, a single implementation could > likely handle both flavors with no downsides? I agree with the desire to use local_lock() in slab and memcg long term, but this is something that definitely should _not_ be done in this patch se= t. try_alloc_page() needs to learn to walk before we teach it to run. > The last line can practially only happen on RT, right? On non-RT irqsave > means we could only fail the trylock from a nmi and then we should have > gfp_flags that don't allow spinning. Correct. > So suppose we used local_trylock(), local_lock() and local_unlock() (no > _irqsave) instead, as I mentioned in reply to 3/7. The RT implementation > would be AFAICS the same. On !RT the trylock could now fail from a IRQ > context in addition to NMI context, but that should also have a gfp_mask > that does not allow spinning, so it should work fine. Also correct. > It would however mean converting all users of the lock, i.e. also > consume_obj_stock() etc., but AFAIU that will be necessary anyway to have > opportunistic slab allocations? Exactly. And as soon as we do that we start to conflict between trees. But the main concern is that change like that needs to be thoroughly analyzed. I'm not convinced that stock_lock as preempt_disable() will work for memcg. People do GFP_NOWAIT allocations from IRQ and assume it works. If memcg local_irqsave (aka local_lock_irqsave) is replaced with preempt_disable the IRQ can happen in the middle of memcg update of the counters, so ALL of stock_lock operations would have to local_TRYlock() with fallback in case IRQ kmalloc(GFP_NOWAIT) happens to reenter. Same issue with slub. local_lock_irqsave(&s->cpu_slab->lock) as irq disabled region works for kmalloc(GPF_NOWAIT) users. If it becomes preempt_disable I suspect it will break things. Like perf and bpf use irq_work do wakeups and allocations. slub's s->cpu_slab protected by preempt_disable would mean that 'perf record -a' will be triggering in the middle of slab partial, deactivate slab logic and perf will be doing wakups right there. I suspect it will be sad. While right now irq work handler will be called only after the last local_unlock_irqrestore enables irqs. So replacing local_lock_irqsave in slab and memcg with local_lock is not something to take lightly.