Shakeel Butt <shakeel.butt@xxxxxxxxx> writes: > On Fri, Sep 05, 2025 at 02:20:46PM -0700, Roman Gushchin wrote: >> Shakeel Butt <shakeel.butt@xxxxxxxxx> writes: >> >> > Generally memcg charging is allowed from all the contexts including NMI >> > where even spinning on spinlock can cause locking issues. However one >> > call chain was missed during the addition of memcg charging from any >> > context support. That is try_charge_memcg() -> memcg_memory_event() -> >> > cgroup_file_notify(). >> > >> > The possible function call tree under cgroup_file_notify() can acquire >> > many different spin locks in spinning mode. Some of them are >> > cgroup_file_kn_lock, kernfs_notify_lock, pool_workqeue's lock. So, let's >> > just skip cgroup_file_notify() from memcg charging if the context does >> > not allow spinning. >> >> Hmm, what about OOM events? Losing something like MEMCG_LOW doesn't look >> like a bit deal, but OOM events can be way more important. >> >> Should we instead preserve the event (e.g. as a pending_event_mask) and >> raise it on the next occasion / from a different context? >> > > Thanks for the review. For now only MAX can happen in non-spinning > context. All others only happen in process context. Maybe with BPF OOM, > OOM might be possible in a different context (is that what you are > thinking?). I think we can add the complexity of preserving the event > when the actual need arise. No, I haven't thought about any particular use case, just a bit worried about silently dropping some events. It might be not an issue now, but might be easy to miss a moment when it becomes a problem. So in my opinion using some delayed delivery mechanism is better than just dropping these events.
