From: Roman Gushchin <guro@fb.com>
To: Daniel Borkmann <daniel@iogearbox.net>
Cc: <bpf@vger.kernel.org>, <ast@kernel.org>, <netdev@vger.kernel.org>,
<andrii@kernel.org>, <akpm@linux-foundation.org>,
<linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>,
<kernel-team@fb.com>
Subject: Re: [PATCH bpf-next v6 06/34] bpf: prepare for memcg-based memory accounting for bpf maps
Date: Tue, 17 Nov 2020 17:07:03 -0800 [thread overview]
Message-ID: <20201118010703.GC156448@carbon.DHCP.thefacebook.com> (raw)
In-Reply-To: <20201118004634.GA179309@carbon.dhcp.thefacebook.com>
On Tue, Nov 17, 2020 at 04:46:34PM -0800, Roman Gushchin wrote:
> On Wed, Nov 18, 2020 at 01:06:17AM +0100, Daniel Borkmann wrote:
> > On 11/17/20 4:40 AM, Roman Gushchin wrote:
> > > In the absolute majority of cases if a process is making a kernel
> > > allocation, it's memory cgroup is getting charged.
> > >
> > > Bpf maps can be updated from an interrupt context and in such
> > > case there is no process which can be charged. It makes the memory
> > > accounting of bpf maps non-trivial.
> > >
> > > Fortunately, after commit 4127c6504f25 ("mm: kmem: enable kernel
> > > memcg accounting from interrupt contexts") and b87d8cefe43c
> > > ("mm, memcg: rework remote charging API to support nesting")
> > > it's finally possible.
> > >
> > > To do it, a pointer to the memory cgroup of the process which created
> > > the map is saved, and this cgroup is getting charged for all
> > > allocations made from an interrupt context.
> > >
> > > Allocations made from a process context will be accounted in a usual way.
> > >
> > > Signed-off-by: Roman Gushchin <guro@fb.com>
> > > Acked-by: Song Liu <songliubraving@fb.com>
> > [...]
> > > +#ifdef CONFIG_MEMCG_KMEM
> > > +static __always_inline int __bpf_map_update_elem(struct bpf_map *map, void *key,
> > > + void *value, u64 flags)
> > > +{
> > > + struct mem_cgroup *old_memcg;
> > > + bool in_interrupt;
> > > + int ret;
> > > +
> > > + /*
> > > + * If update from an interrupt context results in a memory allocation,
> > > + * the memory cgroup to charge can't be determined from the context
> > > + * of the current task. Instead, we charge the memory cgroup, which
> > > + * contained a process created the map.
> > > + */
> > > + in_interrupt = in_interrupt();
> > > + if (in_interrupt)
> > > + old_memcg = set_active_memcg(map->memcg);
> > > +
> > > + ret = map->ops->map_update_elem(map, key, value, flags);
> > > +
> > > + if (in_interrupt)
> > > + set_active_memcg(old_memcg);
> > > +
> > > + return ret;
> >
> > Hmm, this approach here won't work, see also commit 09772d92cd5a ("bpf: avoid
> > retpoline for lookup/update/delete calls on maps") which removes the indirect
> > call, so the __bpf_map_update_elem() and therefore the set_active_memcg() is
> > not invoked for the vast majority of cases.
>
> I see. Well, the first option is to move these calls into map-specific update
> functions, but the list is relatively long:
> nsim_map_update_elem()
> cgroup_storage_update_elem()
> htab_map_update_elem()
> htab_percpu_map_update_elem()
> dev_map_update_elem()
> dev_map_hash_update_elem()
> trie_update_elem()
> cpu_map_update_elem()
> bpf_pid_task_storage_update_elem()
> bpf_fd_inode_storage_update_elem()
> bpf_fd_sk_storage_update_elem()
> sock_map_update_elem()
> xsk_map_update_elem()
>
> Alternatively, we can set the active memcg for the whole duration of bpf
> execution. It's simpler, but will add some overhead. Maybe we can somehow
> mark programs calling into update helpers and skip all others?
Actually, this is problematic if a program updates several maps, because
in theory they can belong to different cgroups.
So it seems that the first option is the way to go. Do you agree?
next prev parent reply other threads:[~2020-11-18 1:07 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-17 3:40 [PATCH bpf-next v6 00/34] bpf: switch to memcg-based memory accounting Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 01/34] mm: memcontrol: use helpers to read page's memcg data Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 02/34] mm: memcontrol/slab: use helpers to access slab page's memcg_data Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 03/34] mm: introduce page memcg flags Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 04/34] mm: convert page kmemcg type to a page memcg flag Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 05/34] bpf: memcg-based memory accounting for bpf progs Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 06/34] bpf: prepare for memcg-based memory accounting for bpf maps Roman Gushchin
2020-11-18 0:06 ` Daniel Borkmann
2020-11-18 0:46 ` Roman Gushchin
2020-11-18 1:07 ` Roman Gushchin [this message]
2020-11-18 1:11 ` Alexei Starovoitov
2020-11-18 1:11 ` Alexei Starovoitov
2020-11-18 1:28 ` Roman Gushchin
2020-11-18 10:22 ` Daniel Borkmann
2020-11-18 17:15 ` Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 07/34] bpf: " Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 08/34] bpf: refine memcg-based memory accounting for arraymap maps Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 09/34] bpf: refine memcg-based memory accounting for cpumap maps Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 10/34] bpf: memcg-based memory accounting for cgroup storage maps Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 11/34] bpf: refine memcg-based memory accounting for devmap maps Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 12/34] bpf: refine memcg-based memory accounting for hashtab maps Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 13/34] bpf: memcg-based memory accounting for lpm_trie maps Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 14/34] bpf: memcg-based memory accounting for bpf ringbuffer Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 15/34] bpf: memcg-based memory accounting for bpf local storage maps Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 16/34] bpf: refine memcg-based memory accounting for sockmap and sockhash maps Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 17/34] bpf: refine memcg-based memory accounting for xskmap maps Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 18/34] bpf: eliminate rlimit-based memory accounting for arraymap maps Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 19/34] bpf: eliminate rlimit-based memory accounting for bpf_struct_ops maps Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 20/34] bpf: eliminate rlimit-based memory accounting for cpumap maps Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 21/34] bpf: eliminate rlimit-based memory accounting for cgroup storage maps Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 22/34] bpf: eliminate rlimit-based memory accounting for devmap maps Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 23/34] bpf: eliminate rlimit-based memory accounting for hashtab maps Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 24/34] bpf: eliminate rlimit-based memory accounting for lpm_trie maps Roman Gushchin
2020-11-17 3:40 ` [PATCH bpf-next v6 25/34] bpf: eliminate rlimit-based memory accounting for queue_stack_maps maps Roman Gushchin
2020-11-17 3:41 ` [PATCH bpf-next v6 26/34] bpf: eliminate rlimit-based memory accounting for reuseport_array maps Roman Gushchin
2020-11-17 3:41 ` [PATCH bpf-next v6 27/34] bpf: eliminate rlimit-based memory accounting for bpf ringbuffer Roman Gushchin
2020-11-17 3:41 ` [PATCH bpf-next v6 28/34] bpf: eliminate rlimit-based memory accounting for sockmap and sockhash maps Roman Gushchin
2020-11-17 3:41 ` [PATCH bpf-next v6 29/34] bpf: eliminate rlimit-based memory accounting for stackmap maps Roman Gushchin
2020-11-17 3:41 ` [PATCH bpf-next v6 30/34] bpf: eliminate rlimit-based memory accounting for xskmap maps Roman Gushchin
2020-11-17 3:41 ` [PATCH bpf-next v6 31/34] bpf: eliminate rlimit-based memory accounting for bpf local storage maps Roman Gushchin
2020-11-17 3:41 ` [PATCH bpf-next v6 32/34] bpf: eliminate rlimit-based memory accounting infra for bpf maps Roman Gushchin
2020-11-17 3:41 ` [PATCH bpf-next v6 33/34] bpf: eliminate rlimit-based memory accounting for bpf progs Roman Gushchin
2020-11-17 3:41 ` [PATCH bpf-next v6 34/34] bpf: samples: do not touch RLIMIT_MEMLOCK Roman Gushchin
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=20201118010703.GC156448@carbon.DHCP.thefacebook.com \
--to=guro@fb.com \
--cc=akpm@linux-foundation.org \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=netdev@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.