On Fri, Sep 05, 2025 at 02:42:01PM -0700, Roman Gushchin wrote: > 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. > Only the notification can be dropped and not the event (i.e. we are still incrementing the counters). Also for MAX only but I got your point. > So in my opinion using some delayed delivery mechanism is better > than just dropping these events. Let me see how doing this irq_work looks like and will update here.
