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 86979C25B75 for ; Fri, 31 May 2024 23:39:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 072FF6B00A1; Fri, 31 May 2024 19:39:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 022406B00A2; Fri, 31 May 2024 19:39:50 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E2B666B00A4; Fri, 31 May 2024 19:39:50 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id C5ED06B00A1 for ; Fri, 31 May 2024 19:39:50 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 3B38C1409DF for ; Fri, 31 May 2024 23:39:50 +0000 (UTC) X-FDA: 82180310940.22.0D8E717 Received: from out-176.mta1.migadu.com (out-176.mta1.migadu.com [95.215.58.176]) by imf23.hostedemail.com (Postfix) with ESMTP id 550B1140010 for ; Fri, 31 May 2024 23:39:47 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b="fT/YcguP"; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf23.hostedemail.com: domain of roman.gushchin@linux.dev designates 95.215.58.176 as permitted sender) smtp.mailfrom=roman.gushchin@linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1717198787; 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=QLMB9tJ301/rLUk13wKnKotC95IcSNp342uZrlGl3Ns=; b=RXm1Y5+AkaC3ChO99kjzp20+w1x5HgT+xeLsiLnoqRUoKN2lxsLINhxqnV4RuMyQjgims2 v37386QjrvYU5ty1jJ7o/KRP9erfp0tWoej8neRJ1C/gqR+IOF5f9mQsnTk+s6DOO+9JuD mEsfjwl/hrdNdmAk0A8of9NH64pQ4RU= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b="fT/YcguP"; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf23.hostedemail.com: domain of roman.gushchin@linux.dev designates 95.215.58.176 as permitted sender) smtp.mailfrom=roman.gushchin@linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1717198787; a=rsa-sha256; cv=none; b=kfPCn6kNJOh/DFEudr1b14UObd24DdSGHse2weFEXDHUiG8xYa5W3TGsdFqKVkwG2XJHNg xqfxlRN2BhLsBVo+se6G2KYM6hdvnUzo+gY/WFrGbrD/AQG+TqC3ymJpuf08iuoTbuhMTm 5AF2FmwABJB5lE2IHSMynyy/GAsN7qs= X-Envelope-To: vbabka@suse.cz DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1717198785; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=QLMB9tJ301/rLUk13wKnKotC95IcSNp342uZrlGl3Ns=; b=fT/YcguPRBykza/O/N6SplR+kYvMGw63MZgVRIRCV13FZWCvYkSzvNVC0+ZXn0PLFysf1f dLYXEikbM7zhhyE4cY31XbHpskmn1I7FcfH5F03lqHDAQijs0mi9LWdL1vY3tKUsvTEPSN gc6t+IllOKGxK+nxCEIMdV6Hx0lFd+w= X-Envelope-To: akinobu.mita@gmail.com X-Envelope-To: cl@linux.com X-Envelope-To: rientjes@google.com X-Envelope-To: ast@kernel.org X-Envelope-To: daniel@iogearbox.net X-Envelope-To: andrii@kernel.org X-Envelope-To: naveen.n.rao@linux.ibm.com X-Envelope-To: anil.s.keshavamurthy@intel.com X-Envelope-To: davem@davemloft.net X-Envelope-To: mhiramat@kernel.org X-Envelope-To: rostedt@goodmis.org X-Envelope-To: mark.rutland@arm.com X-Envelope-To: jolsa@kernel.org X-Envelope-To: 42.hyeyoo@gmail.com X-Envelope-To: linux-kernel@vger.kernel.org X-Envelope-To: linux-mm@kvack.org X-Envelope-To: bpf@vger.kernel.org X-Envelope-To: linux-trace-kernel@vger.kernel.org Date: Fri, 31 May 2024 16:39:38 -0700 X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Roman Gushchin To: Vlastimil Babka Cc: Akinobu Mita , Christoph Lameter , David Rientjes , Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , "Naveen N. Rao" , Anil S Keshavamurthy , "David S. Miller" , Masami Hiramatsu , Steven Rostedt , Mark Rutland , Jiri Olsa , Hyeonggon Yoo <42.hyeyoo@gmail.com>, linux-kernel@vger.kernel.org, linux-mm@kvack.org, bpf@vger.kernel.org, linux-trace-kernel@vger.kernel.org Subject: Re: [PATCH RFC 0/4] static key support for error injection functions Message-ID: References: <20240531-fault-injection-statickeys-v1-0-a513fd0a9614@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20240531-fault-injection-statickeys-v1-0-a513fd0a9614@suse.cz> X-Migadu-Flow: FLOW_OUT X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 550B1140010 X-Stat-Signature: 96bfkbuwoesoptzukh54hof94397bhg4 X-HE-Tag: 1717198787-30707 X-HE-Meta: U2FsdGVkX19bPQVmETsjG9qXJpzstNTyPbmAqkE+t1Bdd/hi0kfvrhDt+ayCpcVUDMdunuLCd1dKiXQVyboHnAdGcEngCpYgUZpfJtothZ8S32J+cwUTTvcXdd+5MiyqpHz+hFrKEon/6zTMYP6lK14HKNP1fm5mvpLhMlI07GwdFxRcqLYNuJ0DojfiX8aE9JKeyVKhFuDHM8NB6NeQ6779DjI/Rnkk/LqRcB5gkHirHx9i87F06upbb3BoCveeK0nvjIuS61cH9kk0HPHinejohqEYt2CtNb4WIQt/GMxrT/NYUgI9J17ZdwXDJJLxSIyFvQMMgnUbj5g1YlgGRUkngppgiLnyk4nOVPasSibtrwAsg/PMAYReVDeKR+FqMI72t6q5a8cUgPB7fngtb+6DjB2C/+q00TXVh8IhEEX7uuHFxzvHvYQkTQRni1Az4kLC80uVVkyMbFJZcW7UzgzzrWAAnjAJbRvv1ayQHxrNzWIrqcUJLyqfFULT4Kj3YAkKsYQP4/9IWcj0CeDPH0dEuvf5tPyf8pjNpwjAVWFoTdVpVhbPVcNlwEuZS/mMrSmxLQqljz0iwLCkadCrRUqmxM/Bs/amvz56niCASoZzbT1YRWR/mlDzE/1bTLsmw1N7ug5G1yGNLieJ+ns20N1BU7y0m6vIBfNH1Jb1+LQfifpjxUme2XlLQx3rTotpcoZbq75M07KrCDTIZzW1QYTUz4eaQPqMC+2YL6RLU+mqBS8A63kzMbrkUv8BAuV71Ty8/tLLwBeDwpg6RbePFgtOudS+k4YwtWi1qntmnDJ5G4KovuOZGI2Byn0v2QdRUlgca7nTP8c2Y1kWC5+ytByJRcdFC5u46DXM3I/2oqm7nZRvQORZu70OKnItUODmXrBCpxqN96uvENeECvucwJgdOZXQ5aszTDGSnl4LYNLpgWKCbyk39y17mjsvIUs2ipghhaQcCvgU1vBFkf2 yW9zSPUz qZU2aj8rRIc0F5+ghr/pGBh5BZl5v4G+C8mSXopL0AEJa/MCz4kKY7e8G7iRA7EO2m5tNCjmX0RLj2dR86GDSjz2wjsNZtiovuAdX2xQ11EfpW1IvChoHABdFr0FD+1RjOGXETyhB9G5Av/rcyLRLftgsUeqVjv/O9jZ/UQFYtBE0O5x8zDxHxkw741w/E8L4f/DHF+SwQOq0gT8bNVm2Yv0bJNMYUAHVw5UhHVXDPd6SpmQ= 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 Fri, May 31, 2024 at 11:33:31AM +0200, Vlastimil Babka wrote: > Incomplete, help needed from ftrace/kprobe and bpf folks. > > As previously mentioned by myself [1] and others [2] the functions > designed for error injection can bring visible overhead in fastpaths > such as slab or page allocation, because even if nothing hooks into them > at a given moment, they are noninline function calls regardless of > CONFIG_ options since commits 4f6923fbb352 ("mm: make should_failslab > always available for fault injection") and af3b854492f3 > ("mm/page_alloc.c: allow error injection"). > > Live patching their callsites has been also suggested in both [1] and > [2] threads, and this is an attempt to do that with static keys that > guard the call sites. When disabled, the error injection functions still > exist and are noinline, but are not being called. Any of the existing > mechanisms that can inject errors should make sure to enable the > respective static key. I have added that support to some of them but > need help with the others. I think it's a clever idea and makes total sense! > > - the legacy fault injection, i.e. CONFIG_FAILSLAB and > CONFIG_FAIL_PAGE_ALLOC is handled in Patch 1, and can be passed the > address of the static key if it exists. The key will be activated if the > fault injection probability becomes non-zero, and deactivated in the > opposite transition. This also removes the overhead of the evaluation > (on top of the noninline function call) when these mechanisms are > configured in the kernel but unused at the moment. > > - the generic error injection using kretprobes with > override_function_with_return is handled in patch 2. The > ALLOW_ERROR_INJECTION() annotation is extended so that static key > address can be passed, and the framework controls it when error > injection is enabled or disabled in debugfs for the function. > > There are two more users I know of but am not familiar enough to fix up > myself. I hope people that are more familiar can help me here. > > - ftrace seems to be using override_function_with_return from > #define ftrace_override_function_with_return but I found no place > where the latter is used. I assume it might be hidden behind more > macro magic? But the point is if ftrace can be instructed to act like > an error injection, it would also have to use some form of metadata > (from patch 2 presumably?) to get to the static key and control it. > > If ftrace can only observe the function being called, maybe it > wouldn't be wrong to just observe nothing if the static key isn't > enabled because nobody is doing the fault injection? > > - bpftrace, as can be seen from the example in commit 4f6923fbb352 > description. I suppose bpf is already aware what functions the > currently loaded bpf programs hook into, so that it could look up the > static key and control it. Maybe using again the metadata from patch 2, > or extending its own, as I've noticed there's e.g. BTF_ID(func, > should_failslab) > > Now I realize maybe handling this at the k(ret)probe level would be > sufficient for all cases except the legacy fault injection from Patch 1? > Also wanted to note that by AFAIU by using the static_key_slow_dec/inc > API (as done in patches 1/2) should allow all mechanisms to coexist > naturally without fighting each other on the static key state, and also > handle the reference count for e.g. active probes or bpf programs if > there's no similar internal mechanism. > > Patches 3 and 4 implement the static keys for the two mm fault injection > sites in slab and page allocators. For a quick demonstration I've run a > VM and the simple test from [1] that stresses the slab allocator and got > this time before the series: > > real 0m8.349s > user 0m0.694s > sys 0m7.648s > > with perf showing > > 0.61% nonexistent [kernel.kallsyms] [k] should_failslab.constprop.0 > 0.00% nonexistent [kernel.kallsyms] [k] should_fail_alloc_page ▒ > > And after the series > > real 0m7.924s > user 0m0.727s > sys 0m7.191s Is "user" increase a measurement error or it's real? Otherwise, nice savings!