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 C9591C25B74 for ; Sun, 2 Jun 2024 14:20:05 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EDE036B0095; Sun, 2 Jun 2024 10:20:04 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E8DC36B0098; Sun, 2 Jun 2024 10:20:04 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D55E96B009A; Sun, 2 Jun 2024 10:20:04 -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 B6EE56B0095 for ; Sun, 2 Jun 2024 10:20:04 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 5DEDC1201B6 for ; Sun, 2 Jun 2024 14:20:04 +0000 (UTC) X-FDA: 82186157928.05.BD5BD71 Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf22.hostedemail.com (Postfix) with ESMTP id EAC3FC0012 for ; Sun, 2 Jun 2024 14:20:00 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=aeZtNGj8; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf22.hostedemail.com: domain of mhiramat@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=mhiramat@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1717338001; 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=n4Gy1wG/JmMqWQguQkD4zzKsZX/xfdTS44731Isyo7g=; b=bWkevMHpbg+Xtguehxp2n2ujfEK2i6JW4MO3VRi4oUMpIJ4QLvSkQuXrirZA5iFrfndPDQ LKaUHI7YrwrA0s5D8iyhTX77zdcLRJTyQrYplI2jxJdFca8tgy+gnSUPYC+w3p7mNOLtTM BcdOuuafxCLWAQwcVRAV27qf0bYDEuk= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1717338001; a=rsa-sha256; cv=none; b=jToYpIolQej1MbPQFhfxu9gNOncmZaLNAHM23uUxQhcfpaEDgNdc5ugzN4WtPXUsvhJ81k 8W/19nHC+8y/kCUrqBMZGy0SB539JBvVrKecmMU00EZQhHBKc7q+zm56vdQeCpmCOBETY6 FFq038QOcFI+5rfl3MW+BLy1geukPPY= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=aeZtNGj8; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf22.hostedemail.com: domain of mhiramat@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=mhiramat@kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 892ADCE0A25; Sun, 2 Jun 2024 14:19:57 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id ADE04C2BBFC; Sun, 2 Jun 2024 14:19:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1717337996; bh=SeH9oWFSVri/bYiL+0r6iXfVRwtJYCiDV4spRm85XH0=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=aeZtNGj80P6dBrMVhzxBiC8mLZ9bYoUrqmjdD1dEvU+6K0j5kGcQ3+RwXldhxqspA x+cNLqDPTlDhsmZ/k06h+3sQeO9EMu2CpvFATRODDEt7O0Cpwf/7sSAOzeGLePKgw0 uFfoLJ2++QZQUHQIPo88cMO/TxUy4IEphA6QNOzR6QJd2uVfjbV+3cA4WPU8c5BJlh c7u9OznZuzTZtyEIRsBExkYRo45mdJnj9zH61RljTC6obWI55KX5iXtvw90b3q3pFf PkarPFFsgewNvIHhvk0+MX/EAIhp9CPaLsEoyZXtftQv83jaCBu0ThMALHgJHXtEYS xOey6ph3aoZKw== Date: Sun, 2 Jun 2024 23:19:50 +0900 From: Masami Hiramatsu (Google) 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" , Steven Rostedt , Mark Rutland , Jiri Olsa , Roman Gushchin , 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 2/4] error-injection: support static keys around injectable functions Message-Id: <20240602231950.cbb7bc65fce96934fb10dc06@kernel.org> In-Reply-To: <20240531-fault-injection-statickeys-v1-2-a513fd0a9614@suse.cz> References: <20240531-fault-injection-statickeys-v1-0-a513fd0a9614@suse.cz> <20240531-fault-injection-statickeys-v1-2-a513fd0a9614@suse.cz> X-Mailer: Sylpheed 3.7.0 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Stat-Signature: wjpjt7wxhqjhr4rm6zizzt4jhkwnebre X-Rspamd-Queue-Id: EAC3FC0012 X-Rspam-User: X-Rspamd-Server: rspam01 X-HE-Tag: 1717338000-564924 X-HE-Meta: U2FsdGVkX1/lfNZQtXoAg0hNyTcHg/q0SvjDvxsUZDew+M1Y9PiD6qtZNKg9cwwbeM6MWdG5NSTiNWyjwSZ+BrM5e66IfpLBtmxmr4A2MifwpA8qvMMRPj60P055G+nXjN2uCknxZ3TAVQSOMCRMx5o7velfXTG85nBZ0dWMJaandR6CtrgOakMsepEvTMftoMdUIEPDs9L5s6rSn4+V8SeqEJyhBK7tlH/iEWVVdBD5D0CVwJiKLjkQnmS7w7WCMH9bCQk0KPZkK4+1OZWhszZjzC8Wlw71HxzHfYZm+7tkGX+YKKomSEwT2YjSIl0a9RhZxLjb3t+oW4vCeCr70fUUVHp1539BjjgLdKPdiMbNcflszUTIaQmQ7EDetPq3d9rYUHbItfdiNasq0MNryPXiSvHEc8x2MPyScix7M3DgLLVO04b5WHfFO+nCJgEIcxF7br/0ESqOOH8IMsqOjdEligZLgYlKp99OP9nbnzl8fOhaUJwykQEB7O0uUXlnEV6M0YIfEsqvvXpaAXevxYG5VWwqqHQlGym08SM+yuJdQXZR8uZV6OSvSNTJmjt3vf0FLcb5BoCo3F+hNmtMqrAhOop5+OwHBqMVCQeSYW6EA4ttXWFacQiRfjf12C075hBS4kVNAQv1IqNAuBf7FmW0SqNgW+AQy6awGBXC/fTyee6Cq7ale2OHSyFlY4jiRv5uN69j02zhP2h8GhMc48MRWzmVPS2gMnjCVc189Rn6RZ/sDhTPEikrsL6M7URMmFx4+eqyPtS/qOyBZTiqqIM55gYUEwHx4hjVW46B5EK/nN7Mr/QjiWq3wjOzRdBr1fFTUGZkDKusP4vEXD2quFj8GdGOGRMt3cxnb7lzwmD25VuutSWilH3Dc7yBIbmMx1VHVma35X2aPWCvnzI7EZcGiNRhFGh0IFK0UoTkAmGL7YGrbje0lxeZqpSINNN7///+ZlS94G8f8siEw7e rm3PiSYI +HHm11tGaoYILJuBCyXmftjsV8mmteybF1AbxnhK1F+6x8GpjTLpDeOGwgL3pXmTPuguEvagyoAXCRrl88EaCFOj1kFJnKJsi8A4qwjvdKwg6aBRqPwB96ZaLuWoc7lvOjORBfk12P/3+nvvRDvue3/+yuC2ECpPaqAlhtk7Ir155laLL5WrKgvbsi4EIKtBncaGlZVoT7g26RfjvynnP9aLnK5kd8548QuRmeT/I7+wHGvnEhmHywO8bJBoycySOKaAxNyt/P6r7DAomJqkIoac+AvUcrOr15OXUSRFniS9ep/q3PGDLOW4ThRVLaTNi2X4T1GZ0WjBy0qawYJBKpVJ/xWO7NXvUg2vfPT5LwHpoONWEN7aHDIP1Y49IquW5VjLE 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, 31 May 2024 11:33:33 +0200 Vlastimil Babka wrote: > Error injectable functions cannot be inlined and since some are called > from hot paths, this incurrs overhead even if no error injection is > enabled for them. > > To remove this overhead when disabled, allow the callsites of error > injectable functions to put the calls behind a static key, which the > framework can control when error injection is enabled or disabled for > the function. > > Introduce a new ALLOW_ERROR_INJECTION_KEY() macro that adds a parameter > with the static key's address, and store it in struct > error_injection_entry. This new field has caused a mismatch when > populating the injection list from the _error_injection_whitelist > section with the current STRUCT_ALIGN(), so change the alignment to 8. > > During the population, copy the key's address also to struct ei_entry, > and make it possible to retrieve it along with the error type by > get_injectable_error_type(). > > Finally, make the processing of writes to the debugfs inject file enable > the static key when the function is added to the injection list, and > disable when removed. > > Signed-off-by: Vlastimil Babka > --- > include/asm-generic/error-injection.h | 13 ++++++++++++- > include/asm-generic/vmlinux.lds.h | 2 +- > include/linux/error-injection.h | 9 ++++++--- > kernel/fail_function.c | 22 +++++++++++++++++++--- > lib/error-inject.c | 6 +++++- > 5 files changed, 43 insertions(+), 9 deletions(-) > > diff --git a/include/asm-generic/error-injection.h b/include/asm-generic/error-injection.h > index b05253f68eaa..eed2731f3820 100644 > --- a/include/asm-generic/error-injection.h > +++ b/include/asm-generic/error-injection.h > @@ -12,6 +12,7 @@ enum { > > struct error_injection_entry { > unsigned long addr; > + unsigned long static_key_addr; > int etype; > }; > > @@ -25,16 +26,26 @@ struct pt_regs; > * 'Error Injectable Functions' section. > */ > #define ALLOW_ERROR_INJECTION(fname, _etype) \ > -static struct error_injection_entry __used \ > +static struct error_injection_entry __used __aligned(8) \ > __section("_error_injection_whitelist") \ > _eil_addr_##fname = { \ > .addr = (unsigned long)fname, \ > .etype = EI_ETYPE_##_etype, \ > } > > +#define ALLOW_ERROR_INJECTION_KEY(fname, _etype, key) \ > +static struct error_injection_entry __used __aligned(8) \ > + __section("_error_injection_whitelist") \ > + _eil_addr_##fname = { \ > + .addr = (unsigned long)fname, \ > + .static_key_addr = (unsigned long)key, \ > + .etype = EI_ETYPE_##_etype, \ > + } > + > void override_function_with_return(struct pt_regs *regs); > #else > #define ALLOW_ERROR_INJECTION(fname, _etype) > +#define ALLOW_ERROR_INJECTION_KEY(fname, _etype, key) > > static inline void override_function_with_return(struct pt_regs *regs) { } > #endif > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h > index 5703526d6ebf..1b15a0af2a00 100644 > --- a/include/asm-generic/vmlinux.lds.h > +++ b/include/asm-generic/vmlinux.lds.h > @@ -248,7 +248,7 @@ > > #ifdef CONFIG_FUNCTION_ERROR_INJECTION > #define ERROR_INJECT_WHITELIST() \ > - STRUCT_ALIGN(); \ > + . = ALIGN(8); \ > BOUNDED_SECTION(_error_injection_whitelist) > #else > #define ERROR_INJECT_WHITELIST() > diff --git a/include/linux/error-injection.h b/include/linux/error-injection.h > index 20e738f4eae8..bec81b57a9d5 100644 > --- a/include/linux/error-injection.h > +++ b/include/linux/error-injection.h > @@ -6,10 +6,12 @@ > #include > #include > > +struct static_key; > + > #ifdef CONFIG_FUNCTION_ERROR_INJECTION > > -extern bool within_error_injection_list(unsigned long addr); > -extern int get_injectable_error_type(unsigned long addr); > +bool within_error_injection_list(unsigned long addr); > +int get_injectable_error_type(unsigned long addr, struct static_key **key_addr); This seems like an add-hoc change. Since this is called in a cold path (only used when adding new function), can you add new `struct static_key *get_injection_key(unsigned long addr)` to find the static_key from the address? Other part looks good to me. Thank you, -- Masami Hiramatsu (Google)