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 97429D43349 for ; Thu, 7 Nov 2024 11:35:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1686E6B009A; Thu, 7 Nov 2024 06:35:55 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 118DC6B009B; Thu, 7 Nov 2024 06:35:55 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F22986B009D; Thu, 7 Nov 2024 06:35:54 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id D2B696B009A for ; Thu, 7 Nov 2024 06:35:54 -0500 (EST) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 8743A40596 for ; Thu, 7 Nov 2024 11:35:54 +0000 (UTC) X-FDA: 82759093032.19.0EC56D5 Received: from mail-ed1-f50.google.com (mail-ed1-f50.google.com [209.85.208.50]) by imf11.hostedemail.com (Postfix) with ESMTP id CBB5840008 for ; Thu, 7 Nov 2024 11:35:08 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=none; spf=pass (imf11.hostedemail.com: domain of breno.debian@gmail.com designates 209.85.208.50 as permitted sender) smtp.mailfrom=breno.debian@gmail.com; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1730979127; 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; bh=se/Mp7TgFQ1n2wTANvee737jkMqUK/eE4DjvvRz1c2U=; b=lN4WZAh+y41O63yj0ZV5EBg0RZ1qikmG3/S/vpnTU02tD/Vnfhu8FvilpYWBeiL0HVOdYL QNKDsydgpdBqJ540390581EJnHlQckqL5lhEQOESOYuU9VuGaJWH/5Wm8VbizWzJ8wUbNg 6q4bGZovlWVAPfTXEVDdq6dKl768Z0k= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=none; spf=pass (imf11.hostedemail.com: domain of breno.debian@gmail.com designates 209.85.208.50 as permitted sender) smtp.mailfrom=breno.debian@gmail.com; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1730979127; a=rsa-sha256; cv=none; b=z+7xobtNKlz4Q82H/icF+gy5/b39gfNWbUj24e0CPCoxtu3XpZb6ld3yckrVPcuknqMzlq Ib8pRLSLF7Dqy4euUGGuyJL2WfIGtGnnU8Zys8x3uoeTajbtqee4rW7A6dYJWhNVkciTaO SyzUSlXX7/fGuXBsowUc1IDFThejUdI= Received: by mail-ed1-f50.google.com with SMTP id 4fb4d7f45d1cf-5cefa22e9d5so937113a12.3 for ; Thu, 07 Nov 2024 03:35:52 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730979351; x=1731584151; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=se/Mp7TgFQ1n2wTANvee737jkMqUK/eE4DjvvRz1c2U=; b=Z+w/WE5+/n6iTpEp655VXfiQC/HHdbBKzSO6NY2hSxVUgL5FSLI7kZ5VP8tJSiUpq8 Gf+VumGxTXvfBTiKkrKMBtBBi+k/PjSkHHwJybCxze+neG3qvPSPZ7tlup3gvmwbjrpx JLgTmUw2onegQQmCvc3oo47IgjrmpbsGPD2007Q34LIVECk4toMYn+0+OS5maU7Xq3wC PgGMioucQ4nI6699W6Xt2UlyuV8gkw7mIMH/CXuub3GMrNrcHHEUgYdLtUp6e2Gkj100 ytuldvt0YziaxOJjj+jGjIGiOvuLRMTpUNo/OICIKdODLavsdOM7zOyFVM1V/oM7HvJT w1hg== X-Forwarded-Encrypted: i=1; AJvYcCVjyoQEYP5QGGpjxR0U/I/KUo38ubanSN1k1nrR5IQ+WclwWoz4Kh12D1hFHRo32QnLWhMHCoMGyg==@kvack.org X-Gm-Message-State: AOJu0Yw439eXlPqSdAZvepwioMN01BnXMWOJrvxjsgwyTz+d9M9LpKKx dlqFnr4A3ZrHl0pkNwgO5Qajjnee4a4AvtIqJfUYLkzwB3Xj3I5I X-Google-Smtp-Source: AGHT+IEtW2euZBjaf0HPobx7Tu/xvypqIcN8DQqJ7iuwfQr9wuHzlcMR9PCSG7w/pviG4WSUGR3VMw== X-Received: by 2002:a05:6402:2712:b0:5cf:4f2:e062 with SMTP id 4fb4d7f45d1cf-5cf04f2eb00mr1263365a12.8.1730979350939; Thu, 07 Nov 2024 03:35:50 -0800 (PST) Received: from gmail.com (fwdproxy-lla-007.fbsv.net. [2a03:2880:30ff:7::face:b00c]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5cf03b7e7f8sm683615a12.23.2024.11.07.03.35.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Nov 2024 03:35:50 -0800 (PST) Date: Thu, 7 Nov 2024 03:35:47 -0800 From: Breno Leitao To: Andrii Nakryiko Cc: Andrii Nakryiko , linux-trace-kernel@vger.kernel.org, peterz@infradead.org, oleg@redhat.com, rostedt@goodmis.org, mhiramat@kernel.org, bpf@vger.kernel.org, linux-kernel@vger.kernel.org, jolsa@kernel.org, paulmck@kernel.org, willy@infradead.org, surenb@google.com, akpm@linux-foundation.org, linux-mm@kvack.org Subject: Re: [PATCH v5 4/8] uprobes: travers uprobe's consumer list locklessly under SRCU protection Message-ID: <20241107-uncovered-swinging-bull-1e812e@leitao> References: <20240903174603.3554182-1-andrii@kernel.org> <20240903174603.3554182-5-andrii@kernel.org> <20241106-transparent-athletic-ammonite-586af8@leitao> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: CBB5840008 X-Stat-Signature: it6h4y3xfd9fxme7u7jmt3ujhbwt5x47 X-Rspam-User: X-HE-Tag: 1730979308-554903 X-HE-Meta: U2FsdGVkX1/cTrO9PvNegWmsTI1HkeN6eWpJFIoe4L6fmnkAoyked9YP+8qJLr1h9F91WloGlKw5fpkmdyvaVv5LkN0uPBQwk76huImYhfC6cdIsBmHYYHsU/SAosRiBnWbVvNSojwcKNR6E5mmEuTTyjulTyuSbZxG5MunhNfbxnPSSFpln0UTb6cnjTQ/syj4Pk4etWPYiUFRJUn1HQk10X/K0CQJymSEjE1LDUHunkKNfC/Vc38pjJywKLLPbOy0IIldlKfjw+TbDQqtCGToX8Sne1GKGf+1vuTxLVx1duf9nmf1JDWMaBvIQfDC4oO2qSlxSrwhFp9XBG5m08VYbeCLyoRXqhZEOPzeLVSqV5EYM28srdaUawlhhF0lCF1AB+6xy27f+3x2J1AIkvdWwubTgIavk+iwYaCG8QpAUlVnGVnokQi6sAinSSo/jzI3F8OE+odUmBQlzvD3YdQnSVvxjg8+iAAuVPaTHESG5/ZYSfehbeBiw4hfxqK1Lg7WZpwWbuHUvpHR8VNNi3xKk/vu/3bAp3zephz2kmS1fMPVDV1y/1FOEE1lPiJ1rOgwQPrdKrXMO3AKjp2Z9GgyuyNiGMsDa8fLdSYq7iA8Z/bo/NuS1KW8OKlBQ6gJrtdirfn1ieIXfnM2mqdLaAJxVO3uvqoY4v4+a6pOtNhqEGE5oF9KF7hbn/woszVwMXn+w0AouutbYRp770RFWILeulgz28Rci62uUkIQZBoOJguR1FRBObnC+1peqRYimF0Da9v+ot74gVGi00RgaKd4DAI3uNsujcNT5+X5G7ULemVAyRhj/GjhzFiM0Aa6+0RkfJFVE1DIVLO+shyitGBqeKvcDbEB19mfKJkl2TRRM0polsno9UrdWPh+KCJ2h0WG1WEZr8nX5Dkt6g2dEvww7/HAiCtcUBM4izft/ub58uexBERf6LWYt5sq6ktxFMPx4XTkqnfJP+l+zJeb uKLq46UV h1AJob9ppxg8TNB0uq0vVeOT2TSNQG+1cZg0hF0avEzTK+yEg1NWH1soLO5RxWTlBqVvNuXnuvkBcIPH0m0ymChNXzw+PkqANHlsEWmT5KbrwTZESFDQfEvdy2fUDOfD6v4+UuonXmknL3CY8qM4Ifv2k7G8DAgptauJEVIUXz1O3hehUsMeUdvfvoO7tPWvGJ4S8RdL6hJasUUN+uOdA6CJhytNdB5Dw3KqSTVVo2TEGZX/zVCR4m63yNUA/aTLrpxXy9t22zwSEde97at/me155CRp0hnTsA1Wuk8mPgrjiyWhBk9wkLsGZ+9AfJmVeLxVoEpaVbvZA9b3G6VgpRPmVcUcNLBAEn4YgmZdtvpOxv0YufBiF+kX/7eaDHlq1ecoKejmaLw8+S9BrVGsfOEc8D8s5scTR96WKBYBOR16IbnXRn6ndU1zbGFRJ91L4/tZIZ6dzf26OGSYkw0H+/H1/I/+5hTmNuyr3xIb8dvFV0yPDqFFiTZiSaIrhxveLsH8k 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: Hello Andrii, On Wed, Nov 06, 2024 at 08:25:25AM -0800, Andrii Nakryiko wrote: > On Wed, Nov 6, 2024 at 4:03 AM Breno Leitao wrote: > > On Tue, Sep 03, 2024 at 10:45:59AM -0700, Andrii Nakryiko wrote: > > > uprobe->register_rwsem is one of a few big bottlenecks to scalability of > > > uprobes, so we need to get rid of it to improve uprobe performance and > > > multi-CPU scalability. > > > > > > First, we turn uprobe's consumer list to a typical doubly-linked list > > > and utilize existing RCU-aware helpers for traversing such lists, as > > > well as adding and removing elements from it. > > > > > > For entry uprobes we already have SRCU protection active since before > > > uprobe lookup. For uretprobe we keep refcount, guaranteeing that uprobe > > > won't go away from under us, but we add SRCU protection around consumer > > > list traversal. > > > > I am seeing the following message in a kernel with RCU_PROVE_LOCKING: > > > > kernel/events/uprobes.c:937 RCU-list traversed without holding the required lock!! > > > > It seems the SRCU is not held, when coming from mmap_region -> > > uprobe_mmap. Here is the message I got in my debug kernel. (sorry for > > not decoding it, but, the stack trace is clear enough). > > > > WARNING: suspicious RCU usage > > 6.12.0-rc5-kbuilder-01152-gc688a96c432e #26 Tainted: G W E N > > ----------------------------- > > kernel/events/uprobes.c:938 RCU-list traversed without holding the required lock!! > > > > other info that might help us debug this: > > > > rcu_scheduler_active = 2, debug_locks = 1 > > 3 locks held by env/441330: > > #0: ffff00021c1bc508 (&mm->mmap_lock){++++}-{3:3}, at: vm_mmap_pgoff+0x84/0x1d0 > > #1: ffff800089f3ab48 (&uprobes_mmap_mutex[i]){+.+.}-{3:3}, at: uprobe_mmap+0x20c/0x548 > > #2: ffff0004e564c528 (&uprobe->consumer_rwsem){++++}-{3:3}, at: filter_chain+0x30/0xe8 > > > > stack backtrace: > > CPU: 4 UID: 34133 PID: 441330 Comm: env Kdump: loaded Tainted: G W E N 6.12.0-rc5-kbuilder-01152-gc688a96c432e #26 > > Tainted: [W]=WARN, [E]=UNSIGNED_MODULE, [N]=TEST > > Hardware name: Quanta S7GM 20S7GCU0010/S7G MB (CG1), BIOS 3D22 07/03/2024 > > Call trace: > > dump_backtrace+0x10c/0x198 > > show_stack+0x24/0x38 > > __dump_stack+0x28/0x38 > > dump_stack_lvl+0x74/0xa8 > > dump_stack+0x18/0x28 > > lockdep_rcu_suspicious+0x178/0x2c8 > > filter_chain+0xdc/0xe8 > > uprobe_mmap+0x2e0/0x548 > > mmap_region+0x510/0x988 > > do_mmap+0x444/0x528 > > vm_mmap_pgoff+0xf8/0x1d0 > > ksys_mmap_pgoff+0x184/0x2d8 > > > > > > That said, it seems we want to hold the SRCU, before reaching the > > filter_chain(). I hacked a bit, and adding the lock in uprobe_mmap() > > solves the problem, but, I might be missing something, since I am not familiar > > with this code. > > > > How does the following patch look like? > > > > commit 1bd7bcf03031ceca86fdddd8be2e5500497db29f > > Author: Breno Leitao > > Date: Mon Nov 4 06:53:31 2024 -0800 > > > > uprobes: Get SRCU lock before traverseing the list > > > > list_for_each_entry_srcu() is being called without holding the lock, > > which causes LOCKDEP (when enabled with RCU_PROVING) to complain such > > as: > > > > kernel/events/uprobes.c:937 RCU-list traversed without holding the required lock!! > > > > Get the SRCU uprobes_srcu lock before calling filter_chain(), which > > needs to have the SRCU lock hold, since it is going to call > > list_for_each_entry_srcu(). > > > > Signed-off-by: Breno Leitao > > Fixes: cc01bd044e6a ("uprobes: travers uprobe's consumer list locklessly under SRCU protection") > > > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > > index 4b52cb2ae6d62..cc9d4ddeea9a6 100644 > > --- a/kernel/events/uprobes.c > > +++ b/kernel/events/uprobes.c > > @@ -1391,6 +1391,7 @@ int uprobe_mmap(struct vm_area_struct *vma) > > struct list_head tmp_list; > > struct uprobe *uprobe, *u; > > struct inode *inode; > > + int srcu_idx; > > > > if (no_uprobe_events()) > > return 0; > > @@ -1409,6 +1410,7 @@ int uprobe_mmap(struct vm_area_struct *vma) > > > > mutex_lock(uprobes_mmap_hash(inode)); > > build_probe_list(inode, vma, vma->vm_start, vma->vm_end, &tmp_list); > > + srcu_idx = srcu_read_lock(&uprobes_srcu); > > Thanks for catching that (production testing FTW, right?!). Correct. I am running some hosts with RCU_PROVING and I am finding some cases where RCU protected areas are touched without holding the RCU read lock. > But I think you a) adding wrong RCU protection flavor (it has to be > rcu_read_lock_trace()/rcu_read_unlock_trace(), see uprobe_apply() for > an example) and b) I think this is the wrong place to add it. We > should add it inside filter_chain(). filter_chain() is called from > three places, only one of which is already RCU protected (that's the > handler_chain() case). But there is also register_for_each_vma(), > which needs RCU protection as well. Thanks for the guidance! My initial plan was to protect filter_chain(), but, handler_chain() already has the lock. Is it OK to get into a critical section in a nested form? The code will be something like: handle_swbp() { rcu_read_lock_trace(); handler_chain() { filter_chain() { rcu_read_lock_trace(); list_for_each_entry_rcu() rcu_read_lock_trace(); } } rcu_read_lock_trace(); } Is this nested locking fine? Thanks --breno