From: Vlastimil Babka <vbabka@suse.cz>
To: Linus Torvalds <torvalds@linux-foundation.org>,
Shakeel Butt <shakeelb@google.com>
Cc: Roman Gushchin <roman.gushchin@linux.dev>,
Josh Poimboeuf <jpoimboe@kernel.org>,
Jeff Layton <jlayton@kernel.org>,
Chuck Lever <chuck.lever@oracle.com>,
Johannes Weiner <hannes@cmpxchg.org>,
Michal Hocko <mhocko@kernel.org>,
linux-kernel@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
Tejun Heo <tj@kernel.org>,
Vasily Averin <vasily.averin@linux.dev>,
Michal Koutny <mkoutny@suse.com>,
Waiman Long <longman@redhat.com>,
Muchun Song <muchun.song@linux.dev>,
Jiri Kosina <jikos@kernel.org>,
cgroups@vger.kernel.org, linux-mm@kvack.org,
Howard McLauchlan <hmclauchlan@fb.com>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH RFC 1/4] fs/locks: Fix file lock cache accounting, again
Date: Tue, 30 Jan 2024 12:04:57 +0100 [thread overview]
Message-ID: <8fc59462-2940-4e60-95f1-2955a8c24ea0@suse.cz> (raw)
In-Reply-To: <6d5bb852-8703-4abf-a52b-90816bccbd7f@suse.cz>
On 1/26/24 10:50, Vlastimil Babka wrote:
> On 1/22/24 06:10, Linus Torvalds wrote:
>> On Wed, 17 Jan 2024 at 14:56, Shakeel Butt <shakeelb@google.com> wrote:
>>> >
>>> > So I don't see how we can make it really cheap (say, less than 5% overhead)
>>> > without caching pre-accounted objects.
>>>
>>> Maybe this is what we want. Now we are down to just SLUB, maybe such
>>> caching of pre-accounted objects can be in SLUB layer and we can
>>> decide to keep this caching per-kmem-cache opt-in or always on.
>>
>> So it turns out that we have another case of SLAB_ACCOUNT being quite
>> a big expense, and it's actually the normal - but failed - open() or
>> execve() case.
>>
>> See the thread at
>>
>> https://lore.kernel.org/all/CAHk-=whw936qzDLBQdUz-He5WK_0fRSWwKAjtbVsMGfX70Nf_Q@mail.gmail.com/
>>
>> and to see the effect in profiles, you can use this EXTREMELY stupid
>> test program:
>>
>> #include <fcntl.h>
>>
>> int main(int argc, char **argv)
>> {
>> for (int i = 0; i < 10000000; i++)
>> open("nonexistent", O_RDONLY);
>> }
>
> This reminded me I can see should_failslab() in the profiles (1.43% plus the
> overhead in its caller) even if it does nothing at all, and it's completely
> unconditional since commit 4f6923fbb352 ("mm: make should_failslab always
> available for fault injection").
>
> We discussed it briefly when Jens tried to change it in [1] to depend on
> CONFIG_FAILSLAB again. But now I think it should be even possible to leave
> it always available, but behind a static key. BPF or whoever else uses these
> error injection hooks would have to track how many users of them are active
> and manage the static key accordingly. Then it could be always available,
> but have no overhead when there's no active user? Other similars hooks could
> benefit from such an approach too?
>
> [1]
> https://lore.kernel.org/all/e01e5e40-692a-519c-4cba-e3331f173c82@kernel.dk/#t
Just for completeness, with the hunk below I've seen some 2% improvement on
the test program from Linus.
Of course something needs to operate the static key and I'm not familiar
enough with bpf and related machinery whether it's tracking users of the
injection points already where the static key toggling could be hooked.
diff --git a/mm/slub.c b/mm/slub.c
index 2ef88bbf56a3..da07b358d092 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3750,6 +3750,8 @@ noinline int should_failslab(struct kmem_cache *s, gfp_t gfpflags)
}
ALLOW_ERROR_INJECTION(should_failslab, ERRNO);
+static DEFINE_STATIC_KEY_FALSE(failslab_enabled);
+
static __fastpath_inline
struct kmem_cache *slab_pre_alloc_hook(struct kmem_cache *s,
struct list_lru *lru,
@@ -3760,8 +3762,10 @@ struct kmem_cache *slab_pre_alloc_hook(struct kmem_cache *s,
might_alloc(flags);
- if (unlikely(should_failslab(s, flags)))
- return NULL;
+ if (static_branch_unlikely(&failslab_enabled)) {
+ if (unlikely(should_failslab(s, flags)))
+ return NULL;
+ }
if (unlikely(!memcg_slab_pre_alloc_hook(s, lru, objcgp, size, flags)))
return NULL;
next prev parent reply other threads:[~2024-01-30 11:04 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-17 16:14 [PATCH RFC 0/4] Fix file lock cache accounting, again Josh Poimboeuf
2024-01-17 16:14 ` [PATCH RFC 1/4] fs/locks: " Josh Poimboeuf
2024-01-17 19:00 ` Jeff Layton
2024-01-17 19:39 ` Josh Poimboeuf
2024-01-17 20:20 ` Linus Torvalds
2024-01-17 21:02 ` Shakeel Butt
2024-01-17 22:20 ` Roman Gushchin
2024-01-17 22:56 ` Shakeel Butt
2024-01-22 5:10 ` Linus Torvalds
2024-01-22 17:38 ` Shakeel Butt
2024-01-26 9:50 ` Vlastimil Babka
2024-01-30 11:04 ` Vlastimil Babka [this message]
2024-01-19 7:47 ` Shakeel Butt
2024-01-17 21:19 ` Vlastimil Babka
2024-01-17 21:50 ` Roman Gushchin
2024-01-18 9:49 ` Michal Hocko
2024-01-17 16:14 ` [PATCH RFC 2/4] fs/locks: Add CONFIG_FLOCK_ACCOUNTING Josh Poimboeuf
2024-01-17 16:14 ` [PATCH RFC 3/4] mitigations: Expand 'mitigations=off' to include optional software mitigations Josh Poimboeuf
2024-01-17 16:14 ` [PATCH RFC 4/4] mitigations: Add flock cache accounting to 'mitigations=off' Josh Poimboeuf
2024-01-18 9:04 ` Michal Koutný
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8fc59462-2940-4e60-95f1-2955a8c24ea0@suse.cz \
--to=vbabka@suse.cz \
--cc=axboe@kernel.dk \
--cc=bpf@vger.kernel.org \
--cc=cgroups@vger.kernel.org \
--cc=chuck.lever@oracle.com \
--cc=hannes@cmpxchg.org \
--cc=hmclauchlan@fb.com \
--cc=jikos@kernel.org \
--cc=jlayton@kernel.org \
--cc=jpoimboe@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=longman@redhat.com \
--cc=mhocko@kernel.org \
--cc=mkoutny@suse.com \
--cc=muchun.song@linux.dev \
--cc=roman.gushchin@linux.dev \
--cc=shakeelb@google.com \
--cc=tj@kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=vasily.averin@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).