On Fri, May 29, 2015 at 05:19:15PM +0300, Pavel Emelyanov wrote:
> Packet sockets have incoming queue of packets that is only limited
> with per-socket wmem buffer. Strictly speaking we should sum up
> all the queues and charge them into kmem once new packet arrives,
> but this will result in huge patch. Since there's typically quite
> a few of packet sockets in container (tcpdump) we can just forward
> charge the maximum socket rmem size into cg memory upon socket
> creation.

rmem_max ~ 200K

Small? If you say so, OK.

> 
> Signed-off-by: Pavel Emelyanov <[email protected]>
> 
> ---
>  include/linux/memcontrol.h |  2 ++
>  mm/memcontrol.c            | 20 +++++++++++++
>  net/packet/af_packet.c     | 74 
> +++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 95 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index d250663..9daaec2 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -412,6 +412,8 @@ struct sock;
>  #if defined(CONFIG_INET) && defined(CONFIG_MEMCG_KMEM)
>  void sock_update_memcg(struct sock *sk);
>  void sock_release_memcg(struct sock *sk);
> +struct mem_cgroup *sock_get_current_memcg(void);
> +void sock_put_memcg(struct mem_cgroup *);
>  #else
>  static inline void sock_update_memcg(struct sock *sk)
>  {
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index b498237..0e67616 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -554,6 +554,20 @@ void sock_update_memcg(struct sock *sk)
>  }
>  EXPORT_SYMBOL(sock_update_memcg);
>  
> +struct mem_cgroup *sock_get_current_memcg(void)

I don't like this funciton, because it has nothing to do with sockets.
May be, you'd better use try_get_mem_cgroup_from_mm?

> +{
> +     struct mem_cgroup *cg;
> +
> +     rcu_read_lock();
> +     cg = mem_cgroup_from_task(current);
> +     if (mem_cgroup_is_root(cg) || !css_tryget(&cg->css))
> +             cg = NULL;
> +     rcu_read_unlock();
> +
> +     return cg;
> +}
> +EXPORT_SYMBOL(sock_get_current_memcg);
> +
>  void sock_release_memcg(struct sock *sk)
>  {
>       if (mem_cgroup_sockets_enabled && sk->sk_cgrp) {
> @@ -564,6 +578,12 @@ void sock_release_memcg(struct sock *sk)
>       }
>  }
>  
> +void sock_put_memcg(struct mem_cgroup *cg)
> +{
> +     css_put(&cg->css);
> +}
> +EXPORT_SYMBOL(sock_put_memcg);
> +

I would use css_put(mem_cgroup_css(memcg)) instead. Awkward? Perhaps,
but memcg users do that, and you wouldn't have to introduce yet another
function.

>  struct cg_proto *tcp_proto_cgroup(struct mem_cgroup *memcg)
>  {
>       if (!memcg || mem_cgroup_is_root(memcg))
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index e8b5a0d..5a13dfa 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -2420,6 +2420,67 @@ static int packet_sendmsg(struct kiocb *iocb, struct 
> socket *sock,
>               return packet_snd(sock, msg, len);
>  }
>  
> +#ifdef CONFIG_MEMCG_KMEM
> +struct packet_sk_charge {
> +     struct mem_cgroup       *memcg;
> +     unsigned long           amt;
> +};
> +
> +static struct cg_proto *packet_sk_charge(void)
> +{
> +     struct packet_sk_charge *psc;
> +
> +     if (!mem_cgroup_sockets_enabled)
> +             return NULL;
> +
> +     psc = kmalloc(sizeof(*psc), GFP_KERNEL);
> +     if (!psc)
> +             return ERR_PTR(-ENOMEM);
> +
> +     psc->memcg = sock_get_current_memcg();
> +     if (psc->memcg) {
> +             /*
> +              * Forcedly charge the maximum amount of data this socket
> +              * may have. It's typically not huge and packet sockets are
> +              * rare guests in containers, so we don't disturb the memory
> +              * consumption much.
> +              */
> +             psc->amt = sysctl_rmem_max;

If psc->amt is constant, may be it'd better not to introduce struct
packet_sk_charge, but return memcg instead?

> +             memcg_charge_kmem_bytes(psc->memcg, psc->amt);
> +     } else {
> +             kfree(psc);
> +             psc = NULL;
> +     }
> +     rcu_read_unlock();

Stale?

> +
> +     /*
> +      * The sk->sk_cgrp is not used for packet sockets,
> +      * so we'll just put the smaller structure into it.
> +      */
> +     return (struct cg_proto *)psc;
> +}
> +
> +static void packet_sk_uncharge(struct cg_proto *cg)
> +{
> +     struct packet_sk_charge *psc = (struct packet_sk_charge *)cg;
> +
> +     if (psc) {
> +             memcg_uncharge_kmem_bytes(psc->memcg, psc->amt);
> +             sock_put_memcg(psc->memcg);
> +             kfree(psc);
> +     }
> +}
> +#else
> +static struct cg_proto *packet_sk_charge(void)
> +{
> +     return NULL;
> +}
> +
> +static void packet_sk_uncharge(struct sock *sk)

It does not match the definition above.
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to