Re: [PATCH v4] memcg: expose socket memory pressure in a cgroup

[Shakeel Butt <shakeel.butt@linux.dev>]

On Thu, Aug 14, 2025 at 06:27:22PM +0200, Matyas Hurtik wrote:
> On 8/7/25 10:52 PM, Shakeel Butt wrote:
> 
> > We definitely don't need a global lock. For memcg->net_pressure_lock, we
> > need to be very clear why we need this lock. Basically we are doing RMW
> > on memcg->socket_pressure and we want known 'consistently' how much
> > further we are pushing memcg->socket_pressure. In other words the
> > consistent value of diff. The lock is one way to get that consistent
> > diff. We can also play some atomic ops trick to get the consistent value
> > without lock but I don't think that complexity is worth it.
> 
> Hello,
> 
> 
> I tried implementing the second option, making the diff consistent using
> atomics.
> Would something like this work?
> 
> if (level > VMPRESSURE_LOW) {
>   unsigned long new_socket_pressure;
>   unsigned long old_socket_pressure;
>   unsigned long duration_to_add;
>   /*
>     * Let the socket buffer allocator know that
>     * we are having trouble reclaiming LRU pages.
>     *
>     * For hysteresis keep the pressure state
>     * asserted for a second in which subsequent
>     * pressure events can occur.
>     */
>   new_socket_pressure = jiffies + HZ;

Add an if condition here if old_socket_pressure is already equal to
the new_socket_pressure and skip all of the following.

>   old_socket_pressure = atomic_long_xchg(
>     &memcg->socket_pressure, new_socket_pressure);
> 
>   duration_to_add = jiffies_to_usecs(
>     min(new_socket_pressure - old_socket_pressure, HZ));

Here if duration_to_add is zero skip the upwards following traversal.

> 
>   do {
>     atomic_long_add(duration_to_add, &memcg->socket_pressure_duration);
>   } while ((memcg = parent_mem_cgroup(memcg)));
> }
> 
> memcg->socket_pressure would need to be changed into atomic_long_t,
> but we avoid adding the memcg->net_pressure_lock.

Awesome, seems fine to me.

> 
> > We don't need memcg->net_pressure_lock's protection for
> > sk_pressure_duration of the memcg and its ancestors if additions to
> > sk_pressure_duration are atomic.
> 
> With regards to the hierarchical propagation I noticed during testing that
> vmpressure() was sometimes called with memcgs, created for systemd oneshot
> services, that were at that time no longer present in the /sys/fs/cgroup
> tree.
> This then made their parent counters a lot larger than just sum of the
> subtree
> plus value of self. Would this behavior be correct?
> 

That is intentional. You can see couple of other similar monotonically
increasing stats in memory.stat like workkingset refaults and demotes.