On Mon, Jun 01, 2015 at 07:14:24PM +0300, Pavel Emelyanov wrote:
> On 06/01/2015 07:01 PM, Vladimir Davydov wrote:
> > 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.
>
> Yes, 200K is quite small amount. In this context.
>
> >> @@ -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?
>
> From what mm? From current->mm?
Why not? It cannot pass away in this context, can it?
> I tried to find for try_get_mm_cgroup_from_task but failed, this is
> the one I need :D
>
> >> +{
> >> + 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.
>
> OK, but I will introduce it anyway with the body you suggest :)
I beg you not to do that. Please. It's confusing. This function has
nothing to do with sockets in spite of its name. Please export
mem_cgroup_css and use css_put(mem_cgroup_css) inline as others do. It
will be much less intrusive. There is the only place you need it.
>
> >> 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?
>
> No, this constant can be different at uncharge time.
Right, I see.
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel
