netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Glauber Costa <glommer@parallels.com>
To: Greg Thelen <gthelen@google.com>
Cc: <linux-kernel@vger.kernel.org>, <paul@paulmenage.org>,
	<lizf@cn.fujitsu.com>, <kamezawa.hiroyu@jp.fujitsu.com>,
	<ebiederm@xmission.com>, <davem@davemloft.net>,
	<netdev@vger.kernel.org>, <linux-mm@kvack.org>,
	<kirill@shutemov.name>
Subject: Re: [PATCH v3 6/7] tcp buffer limitation: per-cgroup limit
Date: Sat, 24 Sep 2011 10:30:42 -0300	[thread overview]
Message-ID: <4E7DDB82.3030802@parallels.com> (raw)
In-Reply-To: <CAHH2K0Yuji2_2pMdzEaMvRx0KE7OOaoEGT+OK4gJgTcOPKuT9g@mail.gmail.com>

On 09/22/2011 03:01 AM, Greg Thelen wrote:
> On Sun, Sep 18, 2011 at 5:56 PM, Glauber Costa<glommer@parallels.com>  wrote:
>> +static inline bool mem_cgroup_is_root(struct mem_cgroup *mem)
>> +{
>> +       return (mem == root_mem_cgroup);
>> +}
>> +
>
> Why are you adding a copy of mem_cgroup_is_root().  I see one already
> in v3.0.  Was it deleted in a previous patch?

Already answered by another good samaritan.

>> +static int tcp_write_maxmem(struct cgroup *cgrp, struct cftype *cft, u64 val)
>> +{
>> +       struct mem_cgroup *sg = mem_cgroup_from_cont(cgrp);
>> +       struct mem_cgroup *parent = parent_mem_cgroup(sg);
>> +       struct net *net = current->nsproxy->net_ns;
>> +       int i;
>> +
>> +       if (!cgroup_lock_live_group(cgrp))
>> +               return -ENODEV;
>
> Why is cgroup_lock_live_cgroup() needed here?  Does it protect updates
> to sg->tcp_prot_mem[*]?
>
>> +static u64 tcp_read_maxmem(struct cgroup *cgrp, struct cftype *cft)
>> +{
>> +       struct mem_cgroup *sg = mem_cgroup_from_cont(cgrp);
>> +       u64 ret;
>> +
>> +       if (!cgroup_lock_live_group(cgrp))
>> +               return -ENODEV;
>
> Why is cgroup_lock_live_cgroup() needed here?  Does it protect updates
> to sg->tcp_max_memory?

No, that is not my understanding. My understanding is this lock is 
needed to protect against the cgroup just disappearing under our nose.

The task reading/writing it is not necessarily inside the cgroup 
(usually it is not...), so the mere fact of opening the file does not 
guarantee the cgroup will be kept alive. So we can grab a pointer - 
cgroup exists - and write to it - cgroup does not exist.

Or am I missing something ?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2011-09-24 13:30 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-19  0:56 [PATCH v3 0/7] per-cgroup tcp buffer pressure settings Glauber Costa
2011-09-19  0:56 ` [PATCH v3 1/7] Basic kernel memory functionality for the Memory Controller Glauber Costa
2011-09-21  2:23   ` Glauber Costa
2011-09-22  3:17     ` Balbir Singh
2011-09-22  3:19       ` Glauber Costa
2011-09-24 14:43       ` Glauber Costa
2011-09-27 10:06         ` Balbir Singh
2011-09-22  5:58   ` Greg Thelen
2011-09-26 10:34   ` KAMEZAWA Hiroyuki
2011-09-26 22:44     ` Glauber Costa
2011-09-26 23:18     ` Glauber Costa
2011-09-28  0:58       ` KAMEZAWA Hiroyuki
2011-09-28 12:03         ` Glauber Costa
2011-09-19  0:56 ` [PATCH v3 2/7] socket: initial cgroup code Glauber Costa
2011-09-21 18:47   ` Greg Thelen
2011-09-21 18:59     ` Glauber Costa
2011-09-22  6:00       ` Greg Thelen
2011-09-22 15:09         ` Balbir Singh
2011-09-24 13:33           ` Glauber Costa
2011-09-24 13:40           ` Glauber Costa
2011-09-24 14:45           ` Glauber Costa
2011-09-26 10:52             ` KAMEZAWA Hiroyuki
2011-09-26 22:47               ` Glauber Costa
2011-09-28  0:56                 ` KAMEZAWA Hiroyuki
2011-09-27 20:43               ` Glauber Costa
2011-09-19  0:56 ` [PATCH v3 3/7] foundations of per-cgroup memory pressure controlling Glauber Costa
2011-09-19  0:56 ` [PATCH v3 4/7] per-cgroup tcp buffers control Glauber Costa
2011-09-26 10:59   ` KAMEZAWA Hiroyuki
2011-09-26 22:48     ` Glauber Costa
2011-09-27  1:53     ` Glauber Costa
2011-09-28  1:09       ` KAMEZAWA Hiroyuki
2011-09-26 14:39   ` Andrew Vagin
2011-09-26 22:52     ` Glauber Costa
2011-09-19  0:56 ` [PATCH v3 5/7] per-netns ipv4 sysctl_tcp_mem Glauber Costa
2011-09-19  0:56 ` [PATCH v3 6/7] tcp buffer limitation: per-cgroup limit Glauber Costa
2011-09-22  6:01   ` Greg Thelen
2011-09-22  9:58     ` Kirill A. Shutemov
2011-09-22 15:44       ` Greg Thelen
2011-09-24 13:30     ` Glauber Costa [this message]
2011-09-26 11:02       ` KAMEZAWA Hiroyuki
2011-09-26 22:49         ` Glauber Costa
2011-09-22 23:08   ` Balbir Singh
2011-09-24 13:35     ` Glauber Costa
2011-09-24 16:58   ` Andi Kleen
2011-09-24 17:27     ` Glauber Costa
2011-09-28  2:29     ` Balbir Singh
2011-09-28  3:06       ` Andi Kleen
2011-09-19  0:56 ` [PATCH v3 7/7] Display current tcp memory allocation in kmem cgroup Glauber Costa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E7DDB82.3030802@parallels.com \
    --to=glommer@parallels.com \
    --cc=davem@davemloft.net \
    --cc=ebiederm@xmission.com \
    --cc=gthelen@google.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=kirill@shutemov.name \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lizf@cn.fujitsu.com \
    --cc=netdev@vger.kernel.org \
    --cc=paul@paulmenage.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).