Re: [PATCH v2] fs: fsnotify: account fsnotify metadata to kmemcg

[Jan Kara]

On Sat 28-10-17 02:22:18, Yang Shi wrote:
> If some process generates events into a huge or unlimit event queue, but no
> listener read them, they may consume significant amount of memory silently
> until oom happens or some memory pressure issue is raised.
> It'd better to account those slab caches in memcg so that we can get heads
> up before the problematic process consume too much memory silently.
> 
> But, the accounting might be heuristic if the producer is in the different
> memcg from listener if the listener doesn't read the events. Due to the
> current design of kmemcg, who does the allocation, who gets the accounting.
> 
> Signed-off-by: Yang Shi <yang.s@xxxxxxxxxxxxxxx>
> ---
> v1 --> v2:
> * Updated commit log per Amir's suggestion

I'm sorry but I don't think this solution is acceptable. I understand that
in some cases (and you likely run one of these) the result may *happen* to
be the desired one but in other cases, you might be charging wrong memcg
and so misbehaving process in memcg A can effectively cause a DoS attack on
a process in memcg B.

If you have a setup in which notification events can consume considerable
amount of resources, you are doing something wrong I think. Standard event
queue length is limited, overall events are bounded to consume less than 1
MB. If you have unbounded queue, the process has to be CAP_SYS_ADMIN and
presumably it has good reasons for requesting unbounded queue and it should
know what it is doing.

So maybe we could come up with some better way to control amount of
resources consumed by notification events but for that we lack more
information about your use case. And I maintain that the solution should
account events to the consumer, not the producer...

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR