All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Whitcroft <apw@canonical.com>
To: Joe Perches <joe@perches.com>
Cc: Glauber Costa <glommer@parallels.com>,
	linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Michal Hocko <mhocko@suse.cz>,
	kamezawa.hiroyu@jp.fujitsu.com, Tejun Heo <tj@kernel.org>
Subject: Re: [PATCH 2/2] memcg: debugging facility to access dangling memcgs.
Date: Thu, 22 Nov 2012 15:02:04 +0000	[thread overview]
Message-ID: <20121122150204.GB30773@dm> (raw)
In-Reply-To: <1353592956.10902.5.camel@joe-AO722>

On Thu, Nov 22, 2012 at 06:02:36AM -0800, Joe Perches wrote:
> On Thu, 2012-11-22 at 17:53 +0400, Glauber Costa wrote:
> > On 11/22/2012 02:36 PM, Glauber Costa wrote:
> > >> @@ -5933,6 +6071,7 @@ static void __mem_cgroup_free(struct mem_cgroup *memcg)
> > >> >  	 * the cgroup_lock.
> > >> >  	 */
> > >> >  	disarm_static_keys(memcg);
> > >> > +
> > >> >  	if (size < PAGE_SIZE)
> > >> >  		kfree(memcg);
> > >> >  	else
> > 
> > Joe,
> > 
> > since you removed the code from my former e-mail:
> 
> Because you quoted a whole bunch of lines that
> included newlines 
> 
> > That one after "disarm_static_keys".
> 
> Vertical spacing has many uses within a functional
> block.  I don't see anything wrong with that one.
> 
> The one I suspected you meant was:
> 
> > +static inline void memcg_dangling_add(struct mem_cgroup *memcg)
> > +{
> > +
> > +     memcg->memcg_name = kstrdup(cgroup_name(memcg->css.cgroup), GFP_KERNEL);
> 
> I think a blank line after an initial { is suspect.


Yeah it is a bit odd to go changing the spacing in a function when you
arn't making any other change, but if the code is clearer with it so be
it.  The blank line at the start of a block is a little less obviously
ever useful.

-apw

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2012-11-22 15:02 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-22 10:29 [PATCH 0/2] Show information about dangling memcgs Glauber Costa
2012-11-22 10:29 ` [PATCH 1/2] cgroup: helper do determine group name Glauber Costa
2012-11-22 14:32   ` Tejun Heo
2012-11-23  8:55   ` Michal Hocko
2012-11-23 10:36     ` Michal Hocko
2012-11-22 10:29 ` [PATCH 2/2] memcg: debugging facility to access dangling memcgs Glauber Costa
2012-11-22 10:36   ` Glauber Costa
2012-11-22 13:53     ` Glauber Costa
2012-11-22 14:02       ` Joe Perches
2012-11-22 15:02         ` Andy Whitcroft [this message]
2012-11-23 17:24           ` [PATCH] checkpatch: Add --strict messages for blank lines around braces Joe Perches
2012-11-23  9:20   ` [PATCH 2/2] memcg: debugging facility to access dangling memcgs Michal Hocko
2012-11-23  9:33     ` Glauber Costa
2012-11-23 10:33       ` Michal Hocko
2012-11-23 10:37         ` Glauber Costa
2012-11-23 10:51           ` Michal Hocko
2012-11-23 14:00             ` Tejun Heo

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=20121122150204.GB30773@dm \
    --to=apw@canonical.com \
    --cc=akpm@linux-foundation.org \
    --cc=glommer@parallels.com \
    --cc=hannes@cmpxchg.org \
    --cc=joe@perches.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    --cc=tj@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.