Shakeel Butt <shakeel.butt@xxxxxxxxx> writes: > 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. Thanks! If it won't work out for some reason, maybe at least explicitly narrow it down to the MEMCG_MAX events.
