Re: [PATCH net-next v5 2/3] sock: Always take memcg pressure into consideration

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

On Fri, Jun 2, 2023 at 10:42 PM Shakeel Butt <shakeelb@google.com> wrote:
>
> On Fri, Jun 02, 2023 at 04:11:34PM +0800, Abel Wu wrote:
> > The sk_under_memory_pressure() is called to check whether there is
> > memory pressure related to this socket. But now it ignores the net-
> > memcg's pressure if the proto of the socket doesn't care about the
> > global pressure, which may put burden on its memcg compaction or
> > reclaim path (also remember that socket memory is un-reclaimable).
> >
> > So always check the memcg's vm status to alleviate memstalls when
> > it's in pressure.
> >
>
> This is interesting. UDP is the only protocol which supports memory
> accounting (i.e. udp_memory_allocated) but it does not define
> memory_pressure. In addition, it does have sysctl_udp_mem. So
> effectively UDP supports a hard limit and ignores memcg pressure at the
> moment. This patch will change its behavior to consider memcg pressure
> as well. I don't have any objection but let's get opinion of UDP
> maintainer.

Others have more experience with memory pressure on UDP, for the
record. Paolo worked on UDP memory pressure in
https://lore.kernel.org/netdev/cover.1579281705.git.pabeni@redhat.com/

It does seem odd to me to modify sk_under_memory_pressure only. See
for instance its use in __sk_mem_raise_allocated:

        if (sk_has_memory_pressure(sk)) {
                u64 alloc;

                if (!sk_under_memory_pressure(sk))
                        return 1;

This is not even reached as sk_has_memory_pressure is false for UDP.
So this commit only affects the only other protocol-independent
caller, __sk_mem_reduce_allocated, to possibly call
sk_leave_memory_pressure if now under the global limit.

What is the expected behavioral change in practice of this commit?


> > Signed-off-by: Abel Wu <wuyun.abel@bytedance.com>
> > ---
> >  include/net/sock.h | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/net/sock.h b/include/net/sock.h
> > index 3f63253ee092..ad1895ffbc4a 100644
> > --- a/include/net/sock.h
> > +++ b/include/net/sock.h
> > @@ -1411,13 +1411,11 @@ static inline bool sk_has_memory_pressure(const struct sock *sk)
> >
> >  static inline bool sk_under_memory_pressure(const struct sock *sk)
> >  {
> > -     if (!sk->sk_prot->memory_pressure)
> > -             return false;
> > -
> >       if (mem_cgroup_under_socket_pressure(sk->sk_memcg))
> >               return true;
> >
> > -     return !!*sk->sk_prot->memory_pressure;
> > +     return sk->sk_prot->memory_pressure &&
> > +             *sk->sk_prot->memory_pressure;
> >  }
> >
> >  static inline long
> > --
> > 2.37.3
> >