[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <1549028430.1478776.1648575992.104E2731@webmail.messagingengine.com>
Date: Fri, 01 Feb 2019 15:40:30 +0200
From: Martynas <m@...bda.lt>
To: Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc: netdev@...r.kernel.org, ys114321@...il.com, ast@...nel.org,
daniel@...earbox.net, Yonghong Song <yhs@...com>
Subject: Re: [PATCH v2 bpf-next] bpf: add optional memory accounting for maps
On Thu, Jan 31, 2019, at 10:04 PM, Jakub Kicinski wrote:
> On Thu, 31 Jan 2019 10:38:01 +0100, Martynas Pumputis wrote:
> > Previously, memory allocated for a map was not accounted. Therefore,
> > this memory could not be taken into consideration by the cgroups
> > memory controller.
> >
> > This patch introduces the "BPF_F_ACCOUNT_MEM" flag which enables
> > the memory accounting for a map, and it can be set during
> > the map creation ("BPF_MAP_CREATE") in "map_flags".
>
> What should happen for no-prealloc maps?
I wanted to be consistent with "bpf_map_precharge_memlock". So, as such map elements are not charged against RLIMIT_MEMLOCK, I haven't enabled accounting for them (yet).
> Would it make some sense to
> charge the max map size to the user and not each allocation?
Hmm, but that would not reflect a real memory usage, and it could potentially lead to some troubles. E.g. a process with tight cgroup mem limits gets OOM'd because we have pre-charged for what it actually doesn't use.
> Or
> perhaps remember the owner to be able to charge the data path
> allocations which don't happen in process context as well?
In use cases I am aware of all map updates happen within the same context, i.e. in the same cgroup. So, that cgroup becomes the "owner" of the allocations.
Another issue is when we pin a map (regardless whether it was prealloc'd or not). In this case, if a process which created the map
gets restarted and continues to use the map, then it is not being charged. But as long as the process runs in the same cgroup, it should be fine.
Powered by blists - more mailing lists