linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RESEND, PATCH 1/6] memcg: mark more functions/variables as static
@ 2012-01-06 20:57 Kirill A. Shutemov
  2012-01-06 20:57 ` [RESEND, PATCH 2/6] memcg: remove unused variable Kirill A. Shutemov
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Kirill A. Shutemov @ 2012-01-06 20:57 UTC (permalink / raw)
  To: linux-mm, cgroups
  Cc: linux-kernel, containers, KAMEZAWA Hiroyuki, Balbir Singh,
	Michal Hocko, Johannes Weiner, Kirill A. Shutemov

From: "Kirill A. Shutemov" <kirill@shutemov.name>

Based on sparse output.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Acked-by: Michal Hocko <mhocko@suse.cz>
---
 mm/memcontrol.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 90fdaeb..73f0c3e 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -59,7 +59,7 @@
 
 struct cgroup_subsys mem_cgroup_subsys __read_mostly;
 #define MEM_CGROUP_RECLAIM_RETRIES	5
-struct mem_cgroup *root_mem_cgroup __read_mostly;
+static struct mem_cgroup *root_mem_cgroup __read_mostly;
 
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
 /* Turned on only when memory cgroup is enabled && really_do_swap_account = 1 */
@@ -1558,7 +1558,7 @@ int mem_cgroup_select_victim_node(struct mem_cgroup *memcg)
  * unused nodes. But scan_nodes is lazily updated and may not cotain
  * enough new information. We need to do double check.
  */
-bool mem_cgroup_reclaimable(struct mem_cgroup *memcg, bool noswap)
+static bool mem_cgroup_reclaimable(struct mem_cgroup *memcg, bool noswap)
 {
 	int nid;
 
@@ -1593,7 +1593,7 @@ int mem_cgroup_select_victim_node(struct mem_cgroup *memcg)
 	return 0;
 }
 
-bool mem_cgroup_reclaimable(struct mem_cgroup *memcg, bool noswap)
+static bool mem_cgroup_reclaimable(struct mem_cgroup *memcg, bool noswap)
 {
 	return test_mem_cgroup_node_reclaimable(memcg, 0, noswap);
 }
@@ -1767,7 +1767,7 @@ static void memcg_oom_recover(struct mem_cgroup *memcg)
 /*
  * try to call OOM killer. returns false if we should exit memory-reclaim loop.
  */
-bool mem_cgroup_handle_oom(struct mem_cgroup *memcg, gfp_t mask)
+static bool mem_cgroup_handle_oom(struct mem_cgroup *memcg, gfp_t mask)
 {
 	struct oom_wait_info owait;
 	bool locked, need_to_kill;
@@ -3750,7 +3750,7 @@ try_to_free:
 	goto move_account;
 }
 
-int mem_cgroup_force_empty_write(struct cgroup *cont, unsigned int event)
+static int mem_cgroup_force_empty_write(struct cgroup *cont, unsigned int event)
 {
 	return mem_cgroup_force_empty(mem_cgroup_from_cont(cont), true);
 }
@@ -4017,7 +4017,7 @@ struct mcs_total_stat {
 	s64 stat[NR_MCS_STAT];
 };
 
-struct {
+static struct {
 	char *local_name;
 	char *total_name;
 } memcg_stat_strings[NR_MCS_STAT] = {
-- 
1.7.8.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RESEND, PATCH 2/6] memcg: remove unused variable
  2012-01-06 20:57 [RESEND, PATCH 1/6] memcg: mark more functions/variables as static Kirill A. Shutemov
@ 2012-01-06 20:57 ` Kirill A. Shutemov
  2012-01-06 20:57 ` [RESEND, PATCH 3/6] memcg: mark stat field of mem_cgroup struct as __percpu Kirill A. Shutemov
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Kirill A. Shutemov @ 2012-01-06 20:57 UTC (permalink / raw)
  To: linux-mm, cgroups
  Cc: linux-kernel, containers, KAMEZAWA Hiroyuki, Balbir Singh,
	Michal Hocko, Johannes Weiner, Kirill A. Shutemov

From: "Kirill A. Shutemov" <kirill@shutemov.name>

mm/memcontrol.c: In function ‘mc_handle_file_pte’:
mm/memcontrol.c:5206:16: warning: variable ‘inode’ set but not used [-Wunused-but-set-variable]

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Acked-by: Michal Hocko <mhocko@suse.cz>
---
 mm/memcontrol.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 73f0c3e..39c3d60 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5121,7 +5121,6 @@ static struct page *mc_handle_file_pte(struct vm_area_struct *vma,
 			unsigned long addr, pte_t ptent, swp_entry_t *entry)
 {
 	struct page *page = NULL;
-	struct inode *inode;
 	struct address_space *mapping;
 	pgoff_t pgoff;
 
@@ -5130,7 +5129,6 @@ static struct page *mc_handle_file_pte(struct vm_area_struct *vma,
 	if (!move_file())
 		return NULL;
 
-	inode = vma->vm_file->f_path.dentry->d_inode;
 	mapping = vma->vm_file->f_mapping;
 	if (pte_none(ptent))
 		pgoff = linear_page_index(vma, addr);
-- 
1.7.8.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RESEND, PATCH 3/6] memcg: mark stat field of mem_cgroup struct as __percpu
  2012-01-06 20:57 [RESEND, PATCH 1/6] memcg: mark more functions/variables as static Kirill A. Shutemov
  2012-01-06 20:57 ` [RESEND, PATCH 2/6] memcg: remove unused variable Kirill A. Shutemov
@ 2012-01-06 20:57 ` Kirill A. Shutemov
  2012-01-06 20:57 ` [RESEND, PATCH 4/6] memcg: fix broken boolean expression Kirill A. Shutemov
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Kirill A. Shutemov @ 2012-01-06 20:57 UTC (permalink / raw)
  To: linux-mm, cgroups
  Cc: linux-kernel, containers, KAMEZAWA Hiroyuki, Balbir Singh,
	Michal Hocko, Johannes Weiner, Kirill A. Shutemov

From: "Kirill A. Shutemov" <kirill@shutemov.name>

It fixes a lot of sparse warnings.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Acked-by: Michal Hocko <mhocko@suse.cz>
---
 mm/memcontrol.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 39c3d60..831cdc4 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -282,7 +282,7 @@ struct mem_cgroup {
 	/*
 	 * percpu counter.
 	 */
-	struct mem_cgroup_stat_cpu *stat;
+	struct mem_cgroup_stat_cpu __percpu *stat;
 	/*
 	 * used when a cpu is offlined or other synchronizations
 	 * See mem_cgroup_read_stat().
-- 
1.7.8.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RESEND, PATCH 4/6] memcg: fix broken boolean expression
  2012-01-06 20:57 [RESEND, PATCH 1/6] memcg: mark more functions/variables as static Kirill A. Shutemov
  2012-01-06 20:57 ` [RESEND, PATCH 2/6] memcg: remove unused variable Kirill A. Shutemov
  2012-01-06 20:57 ` [RESEND, PATCH 3/6] memcg: mark stat field of mem_cgroup struct as __percpu Kirill A. Shutemov
@ 2012-01-06 20:57 ` Kirill A. Shutemov
  2012-01-09 14:04   ` Johannes Weiner
  2012-01-06 20:57 ` [RESEND, PATCH 5/6] memcg: drop redundant brackets Kirill A. Shutemov
  2012-01-06 20:57 ` [RESEND, PATCH 6/6] memcg: cleanup memcg_check_events() Kirill A. Shutemov
  4 siblings, 1 reply; 11+ messages in thread
From: Kirill A. Shutemov @ 2012-01-06 20:57 UTC (permalink / raw)
  To: linux-mm, cgroups
  Cc: linux-kernel, containers, KAMEZAWA Hiroyuki, Balbir Singh,
	Michal Hocko, Johannes Weiner, Kirill A. Shutemov, stable

From: "Kirill A. Shutemov" <kirill@shutemov.name>

action != CPU_DEAD || action != CPU_DEAD_FROZEN is always true.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Cc: <stable@kernel.org>
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Acked-by: Michal Hocko <mhocko@suse.cz>
---
 mm/memcontrol.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 831cdc4..0b5a3f8 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2085,7 +2085,7 @@ static int __cpuinit memcg_cpu_hotplug_callback(struct notifier_block *nb,
 		return NOTIFY_OK;
 	}
 
-	if ((action != CPU_DEAD) || action != CPU_DEAD_FROZEN)
+	if (action != CPU_DEAD && action != CPU_DEAD_FROZEN)
 		return NOTIFY_OK;
 
 	for_each_mem_cgroup(iter)
-- 
1.7.8.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RESEND, PATCH 5/6] memcg: drop redundant brackets
  2012-01-06 20:57 [RESEND, PATCH 1/6] memcg: mark more functions/variables as static Kirill A. Shutemov
                   ` (2 preceding siblings ...)
  2012-01-06 20:57 ` [RESEND, PATCH 4/6] memcg: fix broken boolean expression Kirill A. Shutemov
@ 2012-01-06 20:57 ` Kirill A. Shutemov
  2012-01-06 20:57 ` [RESEND, PATCH 6/6] memcg: cleanup memcg_check_events() Kirill A. Shutemov
  4 siblings, 0 replies; 11+ messages in thread
From: Kirill A. Shutemov @ 2012-01-06 20:57 UTC (permalink / raw)
  To: linux-mm, cgroups
  Cc: linux-kernel, containers, KAMEZAWA Hiroyuki, Balbir Singh,
	Michal Hocko, Johannes Weiner, Kirill A. Shutemov

From: "Kirill A. Shutemov" <kirill@shutemov.name>

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Acked-by: Michal Hocko <mhocko@suse.cz>
---
 mm/memcontrol.c |   39 ++++++++++++++++++++++-----------------
 1 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 0b5a3f8..2eddcb5 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -73,7 +73,7 @@ static int really_do_swap_account __initdata = 0;
 #endif
 
 #else
-#define do_swap_account		(0)
+#define do_swap_account		0
 #endif
 
 
@@ -113,9 +113,9 @@ enum mem_cgroup_events_target {
 	MEM_CGROUP_TARGET_NUMAINFO,
 	MEM_CGROUP_NTARGETS,
 };
-#define THRESHOLDS_EVENTS_TARGET (128)
-#define SOFTLIMIT_EVENTS_TARGET (1024)
-#define NUMAINFO_EVENTS_TARGET	(1024)
+#define THRESHOLDS_EVENTS_TARGET 128
+#define SOFTLIMIT_EVENTS_TARGET 1024
+#define NUMAINFO_EVENTS_TARGET	1024
 
 struct mem_cgroup_stat_cpu {
 	long count[MEM_CGROUP_STAT_NSTATS];
@@ -148,7 +148,7 @@ struct mem_cgroup_per_zone {
 						/* use container_of	   */
 };
 /* Macro for accessing counter */
-#define MEM_CGROUP_ZSTAT(mz, idx)	((mz)->count[(idx)])
+#define MEM_CGROUP_ZSTAT(mz, idx)	((mz)->count[idx])
 
 struct mem_cgroup_per_node {
 	struct mem_cgroup_per_zone zoneinfo[MAX_NR_ZONES];
@@ -337,8 +337,8 @@ static bool move_file(void)
  * Maximum loops in mem_cgroup_hierarchical_reclaim(), used for soft
  * limit reclaim to prevent infinite loops, if they ever occur.
  */
-#define	MEM_CGROUP_MAX_RECLAIM_LOOPS		(100)
-#define	MEM_CGROUP_MAX_SOFT_LIMIT_RECLAIM_LOOPS	(2)
+#define	MEM_CGROUP_MAX_RECLAIM_LOOPS		100
+#define	MEM_CGROUP_MAX_SOFT_LIMIT_RECLAIM_LOOPS	2
 
 enum charge_type {
 	MEM_CGROUP_CHARGE_TYPE_CACHE = 0,
@@ -351,14 +351,19 @@ enum charge_type {
 };
 
 /* for encoding cft->private value on file */
-#define _MEM			(0)
-#define _MEMSWAP		(1)
-#define _OOM_TYPE		(2)
-#define MEMFILE_PRIVATE(x, val)	(((x) << 16) | (val))
-#define MEMFILE_TYPE(val)	(((val) >> 16) & 0xffff)
+
+enum mem_type {
+	_MEM = 0,
+	_MEMSWAP,
+	_OOM_TYPE,
+	_KMEM,
+};
+
+#define MEMFILE_PRIVATE(x, val)	((x) << 16 | val)
+#define MEMFILE_TYPE(val)	((val) >> 16 & 0xffff)
 #define MEMFILE_ATTR(val)	((val) & 0xffff)
 /* Used for OOM nofiier */
-#define OOM_CONTROL		(0)
+#define OOM_CONTROL		0
 
 /*
  * Reclaim flags for mem_cgroup_hierarchical_reclaim
@@ -1898,7 +1903,7 @@ struct memcg_stock_pcp {
 	unsigned int nr_pages;
 	struct work_struct work;
 	unsigned long flags;
-#define FLUSHING_CACHED_CHARGE	(0)
+#define FLUSHING_CACHED_CHARGE	0
 };
 static DEFINE_PER_CPU(struct memcg_stock_pcp, memcg_stock);
 static DEFINE_MUTEX(percpu_charge_mutex);
@@ -2079,7 +2084,7 @@ static int __cpuinit memcg_cpu_hotplug_callback(struct notifier_block *nb,
 	struct memcg_stock_pcp *stock;
 	struct mem_cgroup *iter;
 
-	if ((action == CPU_ONLINE)) {
+	if (action == CPU_ONLINE) {
 		for_each_mem_cgroup(iter)
 			synchronize_mem_cgroup_on_move(iter, cpu);
 		return NOTIFY_OK;
@@ -2443,8 +2448,8 @@ static void __mem_cgroup_commit_charge(struct mem_cgroup *memcg,
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 
-#define PCGF_NOCOPY_AT_SPLIT ((1 << PCG_LOCK) | (1 << PCG_MOVE_LOCK) |\
-			(1 << PCG_MIGRATION))
+#define PCGF_NOCOPY_AT_SPLIT (1 << PCG_LOCK | 1 << PCG_MOVE_LOCK |\
+		1 << PCG_MIGRATION)
 /*
  * Because tail pages are not marked as "used", set it. We're under
  * zone->lru_lock, 'splitting on pmd' and compound_lock.
-- 
1.7.8.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RESEND, PATCH 6/6] memcg: cleanup memcg_check_events()
  2012-01-06 20:57 [RESEND, PATCH 1/6] memcg: mark more functions/variables as static Kirill A. Shutemov
                   ` (3 preceding siblings ...)
  2012-01-06 20:57 ` [RESEND, PATCH 5/6] memcg: drop redundant brackets Kirill A. Shutemov
@ 2012-01-06 20:57 ` Kirill A. Shutemov
  2012-01-09 13:41   ` Johannes Weiner
  4 siblings, 1 reply; 11+ messages in thread
From: Kirill A. Shutemov @ 2012-01-06 20:57 UTC (permalink / raw)
  To: linux-mm, cgroups
  Cc: linux-kernel, containers, KAMEZAWA Hiroyuki, Balbir Singh,
	Michal Hocko, Johannes Weiner, Kirill A. Shutemov

From: "Kirill A. Shutemov" <kirill@shutemov.name>

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 mm/memcontrol.c |   42 ++++++++++++++++++++++++------------------
 1 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 2eddcb5..0a13afa 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -108,11 +108,12 @@ enum mem_cgroup_events_index {
  * than using jiffies etc. to handle periodic memcg event.
  */
 enum mem_cgroup_events_target {
-	MEM_CGROUP_TARGET_THRESH,
-	MEM_CGROUP_TARGET_SOFTLIMIT,
-	MEM_CGROUP_TARGET_NUMAINFO,
-	MEM_CGROUP_NTARGETS,
+	MEM_CGROUP_TARGET_THRESH	= BIT(1),
+	MEM_CGROUP_TARGET_SOFTLIMIT	= BIT(2),
+	MEM_CGROUP_TARGET_NUMAINFO	= BIT(3),
 };
+#define MEM_CGROUP_NTARGETS 3
+
 #define THRESHOLDS_EVENTS_TARGET 128
 #define SOFTLIMIT_EVENTS_TARGET 1024
 #define NUMAINFO_EVENTS_TARGET	1024
@@ -734,7 +735,7 @@ static unsigned long mem_cgroup_nr_lru_pages(struct mem_cgroup *memcg,
 	return total;
 }
 
-static bool mem_cgroup_event_ratelimit(struct mem_cgroup *memcg,
+static int mem_cgroup_event_ratelimit(struct mem_cgroup *memcg,
 				       enum mem_cgroup_events_target target)
 {
 	unsigned long val, next;
@@ -757,9 +758,9 @@ static bool mem_cgroup_event_ratelimit(struct mem_cgroup *memcg,
 			break;
 		}
 		__this_cpu_write(memcg->stat->targets[target], next);
-		return true;
+		return target;
 	}
-	return false;
+	return 0;
 }
 
 /*
@@ -768,29 +769,34 @@ static bool mem_cgroup_event_ratelimit(struct mem_cgroup *memcg,
  */
 static void memcg_check_events(struct mem_cgroup *memcg, struct page *page)
 {
+	int flags;
+
 	preempt_disable();
-	/* threshold event is triggered in finer grain than soft limit */
-	if (unlikely(mem_cgroup_event_ratelimit(memcg,
-						MEM_CGROUP_TARGET_THRESH))) {
-		bool do_softlimit, do_numainfo;
+	flags = mem_cgroup_event_ratelimit(memcg, MEM_CGROUP_TARGET_THRESH);
 
-		do_softlimit = mem_cgroup_event_ratelimit(memcg,
+	/*
+	 * Threshold event is triggered in finer grain than soft limit
+	 * and numainfo
+	 */
+	if (unlikely(flags)) {
+		flags |= mem_cgroup_event_ratelimit(memcg,
 						MEM_CGROUP_TARGET_SOFTLIMIT);
 #if MAX_NUMNODES > 1
-		do_numainfo = mem_cgroup_event_ratelimit(memcg,
+		flags |= mem_cgroup_event_ratelimit(memcg,
 						MEM_CGROUP_TARGET_NUMAINFO);
 #endif
-		preempt_enable();
+	}
+	preempt_enable();
 
+	if (unlikely(flags)) {
 		mem_cgroup_threshold(memcg);
-		if (unlikely(do_softlimit))
+		if (unlikely(flags & MEM_CGROUP_TARGET_SOFTLIMIT))
 			mem_cgroup_update_tree(memcg, page);
 #if MAX_NUMNODES > 1
-		if (unlikely(do_numainfo))
+		if (unlikely(flags & MEM_CGROUP_TARGET_NUMAINFO))
 			atomic_inc(&memcg->numainfo_events);
 #endif
-	} else
-		preempt_enable();
+	}
 }
 
 struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont)
-- 
1.7.8.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [RESEND, PATCH 6/6] memcg: cleanup memcg_check_events()
  2012-01-06 20:57 ` [RESEND, PATCH 6/6] memcg: cleanup memcg_check_events() Kirill A. Shutemov
@ 2012-01-09 13:41   ` Johannes Weiner
  2012-01-16 11:59     ` Kirill A. Shutemov
  0 siblings, 1 reply; 11+ messages in thread
From: Johannes Weiner @ 2012-01-09 13:41 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: linux-mm, cgroups, linux-kernel, containers, KAMEZAWA Hiroyuki,
	Balbir Singh, Michal Hocko

On Fri, Jan 06, 2012 at 10:57:52PM +0200, Kirill A. Shutemov wrote:
> From: "Kirill A. Shutemov" <kirill@shutemov.name>
> 
> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> ---
>  mm/memcontrol.c |   42 ++++++++++++++++++++++++------------------
>  1 files changed, 24 insertions(+), 18 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 2eddcb5..0a13afa 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -108,11 +108,12 @@ enum mem_cgroup_events_index {
>   * than using jiffies etc. to handle periodic memcg event.
>   */
>  enum mem_cgroup_events_target {
> -	MEM_CGROUP_TARGET_THRESH,
> -	MEM_CGROUP_TARGET_SOFTLIMIT,
> -	MEM_CGROUP_TARGET_NUMAINFO,
> -	MEM_CGROUP_NTARGETS,
> +	MEM_CGROUP_TARGET_THRESH	= BIT(1),
> +	MEM_CGROUP_TARGET_SOFTLIMIT	= BIT(2),
> +	MEM_CGROUP_TARGET_NUMAINFO	= BIT(3),
>  };
> +#define MEM_CGROUP_NTARGETS 3

That really asks for the next guy forgetting to increase the number
when adding another bit.

> @@ -734,7 +735,7 @@ static unsigned long mem_cgroup_nr_lru_pages(struct mem_cgroup *memcg,
>  	return total;
>  }
>  
> -static bool mem_cgroup_event_ratelimit(struct mem_cgroup *memcg,
> +static int mem_cgroup_event_ratelimit(struct mem_cgroup *memcg,
>  				       enum mem_cgroup_events_target target)
>  {
>  	unsigned long val, next;
> @@ -757,9 +758,9 @@ static bool mem_cgroup_event_ratelimit(struct mem_cgroup *memcg,
>  			break;
>  		}
>  		__this_cpu_write(memcg->stat->targets[target], next);
> -		return true;
> +		return target;
>  	}
> -	return false;
> +	return 0;

Really weird interface - I'll return what you passed in, or zero...?

> @@ -768,29 +769,34 @@ static bool mem_cgroup_event_ratelimit(struct mem_cgroup *memcg,
>   */
>  static void memcg_check_events(struct mem_cgroup *memcg, struct page *page)
>  {
> +	int flags;
> +
>  	preempt_disable();
> -	/* threshold event is triggered in finer grain than soft limit */
> -	if (unlikely(mem_cgroup_event_ratelimit(memcg,
> -						MEM_CGROUP_TARGET_THRESH))) {
> -		bool do_softlimit, do_numainfo;
> +	flags = mem_cgroup_event_ratelimit(memcg, MEM_CGROUP_TARGET_THRESH);
>  
> -		do_softlimit = mem_cgroup_event_ratelimit(memcg,
> +	/*
> +	 * Threshold event is triggered in finer grain than soft limit
> +	 * and numainfo
> +	 */
> +	if (unlikely(flags)) {
> +		flags |= mem_cgroup_event_ratelimit(memcg,
>  						MEM_CGROUP_TARGET_SOFTLIMIT);
>  #if MAX_NUMNODES > 1
> -		do_numainfo = mem_cgroup_event_ratelimit(memcg,
> +		flags |= mem_cgroup_event_ratelimit(memcg,
>  						MEM_CGROUP_TARGET_NUMAINFO);
>  #endif
> -		preempt_enable();
> +	}
> +	preempt_enable();
>  
> +	if (unlikely(flags)) {
>  		mem_cgroup_threshold(memcg);
> -		if (unlikely(do_softlimit))
> +		if (unlikely(flags & MEM_CGROUP_TARGET_SOFTLIMIT))
>  			mem_cgroup_update_tree(memcg, page);
>  #if MAX_NUMNODES > 1
> -		if (unlikely(do_numainfo))
> +		if (unlikely(flags & MEM_CGROUP_TARGET_NUMAINFO))
>  			atomic_inc(&memcg->numainfo_events);
>  #endif
> -	} else
> -		preempt_enable();
> +	}
>  }

I'm about to remove the soft limit part of this code, so we'll be able
to condense this back into a single #if block again, anyway.

I would much prefer having the extra #if in the code over this patch
just to silence the warning for now.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RESEND, PATCH 4/6] memcg: fix broken boolean expression
  2012-01-06 20:57 ` [RESEND, PATCH 4/6] memcg: fix broken boolean expression Kirill A. Shutemov
@ 2012-01-09 14:04   ` Johannes Weiner
  2012-01-16 11:54     ` Kirill A. Shutemov
  0 siblings, 1 reply; 11+ messages in thread
From: Johannes Weiner @ 2012-01-09 14:04 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: linux-mm, cgroups, linux-kernel, containers, KAMEZAWA Hiroyuki,
	Balbir Singh, Michal Hocko, stable

On Fri, Jan 06, 2012 at 10:57:50PM +0200, Kirill A. Shutemov wrote:
> From: "Kirill A. Shutemov" <kirill@shutemov.name>
> 
> action != CPU_DEAD || action != CPU_DEAD_FROZEN is always true.
> 
> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> Cc: <stable@kernel.org>

I think you don't need to actually CC stable via email.  If you
include that tag, they will pick it up once the patch hits mainline.

The changelog is too terse, doubly so for a patch that should go into
stable.  How is the code supposed to work?  What are the consequences
of the bug?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RESEND, PATCH 4/6] memcg: fix broken boolean expression
  2012-01-09 14:04   ` Johannes Weiner
@ 2012-01-16 11:54     ` Kirill A. Shutemov
  2012-01-19 13:44       ` Johannes Weiner
  0 siblings, 1 reply; 11+ messages in thread
From: Kirill A. Shutemov @ 2012-01-16 11:54 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: linux-mm, cgroups, linux-kernel, containers, KAMEZAWA Hiroyuki,
	Balbir Singh, Michal Hocko, stable

On Mon, Jan 09, 2012 at 03:04:04PM +0100, Johannes Weiner wrote:
> On Fri, Jan 06, 2012 at 10:57:50PM +0200, Kirill A. Shutemov wrote:
> > From: "Kirill A. Shutemov" <kirill@shutemov.name>
> > 
> > action != CPU_DEAD || action != CPU_DEAD_FROZEN is always true.
> > 
> > Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> > Cc: <stable@kernel.org>
> 
> I think you don't need to actually CC stable via email.  If you
> include that tag, they will pick it up once the patch hits mainline.

I don't think it's a problem for stable@.

> 
> The changelog is too terse, doubly so for a patch that should go into
> stable.  How is the code supposed to work?  What are the consequences
> of the bug?

Is it enough?

---

>From fe1c2f2dc1abf63cce12017e2d9031cf67f0a161 Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <kirill@shutemov.name>
Date: Sat, 24 Dec 2011 04:12:53 +0200
Subject: [PATCH 4/6] memcg: fix broken boolean expression

action != CPU_DEAD || action != CPU_DEAD_FROZEN is always true.

We should return at the point if CPU doesn't go away. Otherwise drain
all counters and stocks from the CPU.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Cc: <stable@kernel.org>
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Acked-by: Michal Hocko <mhocko@suse.cz>
---
 mm/memcontrol.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 4ed6737..513ae04 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2095,9 +2095,11 @@ static int __cpuinit memcg_cpu_hotplug_callback(struct notifier_block *nb,
 		return NOTIFY_OK;
 	}
 
-	if ((action != CPU_DEAD) || action != CPU_DEAD_FROZEN)
+	if (action != CPU_DEAD && action != CPU_DEAD_FROZEN)
 		return NOTIFY_OK;
 
+	/* CPU goes away */
+
 	for_each_mem_cgroup(iter)
 		mem_cgroup_drain_pcp_counter(iter, cpu);
 
-- 
1.7.8.3

-- 
 Kirill A. Shutemov

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [RESEND, PATCH 6/6] memcg: cleanup memcg_check_events()
  2012-01-09 13:41   ` Johannes Weiner
@ 2012-01-16 11:59     ` Kirill A. Shutemov
  0 siblings, 0 replies; 11+ messages in thread
From: Kirill A. Shutemov @ 2012-01-16 11:59 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: linux-mm, cgroups, linux-kernel, containers, KAMEZAWA Hiroyuki,
	Balbir Singh, Michal Hocko

On Mon, Jan 09, 2012 at 02:41:08PM +0100, Johannes Weiner wrote:
> On Fri, Jan 06, 2012 at 10:57:52PM +0200, Kirill A. Shutemov wrote:
> > From: "Kirill A. Shutemov" <kirill@shutemov.name>
> > 
> > Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>

...

> I'm about to remove the soft limit part of this code, so we'll be able
> to condense this back into a single #if block again, anyway.
> 
> I would much prefer having the extra #if in the code over this patch
> just to silence the warning for now.

The patch is informative only. I agree with Michal Hocko. It introduce too
much noise to fix one warning. Just ignore it.

-- 
 Kirill A. Shutemov

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RESEND, PATCH 4/6] memcg: fix broken boolean expression
  2012-01-16 11:54     ` Kirill A. Shutemov
@ 2012-01-19 13:44       ` Johannes Weiner
  0 siblings, 0 replies; 11+ messages in thread
From: Johannes Weiner @ 2012-01-19 13:44 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: linux-mm, cgroups, linux-kernel, containers, KAMEZAWA Hiroyuki,
	Balbir Singh, Michal Hocko, stable

On Mon, Jan 16, 2012 at 01:54:16PM +0200, Kirill A. Shutemov wrote:
> On Mon, Jan 09, 2012 at 03:04:04PM +0100, Johannes Weiner wrote:
> > On Fri, Jan 06, 2012 at 10:57:50PM +0200, Kirill A. Shutemov wrote:
> > > From: "Kirill A. Shutemov" <kirill@shutemov.name>
> > > 
> > > action != CPU_DEAD || action != CPU_DEAD_FROZEN is always true.
> > > 
> > > Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> > > Cc: <stable@kernel.org>
> > 
> > I think you don't need to actually CC stable via email.  If you
> > include that tag, they will pick it up once the patch hits mainline.
> 
> I don't think it's a problem for stable@.
> 
> > 
> > The changelog is too terse, doubly so for a patch that should go into
> > stable.  How is the code supposed to work?  What are the consequences
> > of the bug?
> 
> Is it enough?

I think so, thank you.

> >From fe1c2f2dc1abf63cce12017e2d9031cf67f0a161 Mon Sep 17 00:00:00 2001
> From: "Kirill A. Shutemov" <kirill@shutemov.name>
> Date: Sat, 24 Dec 2011 04:12:53 +0200
> Subject: [PATCH 4/6] memcg: fix broken boolean expression
> 
> action != CPU_DEAD || action != CPU_DEAD_FROZEN is always true.
> 
> We should return at the point if CPU doesn't go away. Otherwise drain
> all counters and stocks from the CPU.
> 
> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> Cc: <stable@kernel.org>
> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Acked-by: Michal Hocko <mhocko@suse.cz>

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

But without the stable Cc, I guess :)

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2012-01-19 13:44 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-06 20:57 [RESEND, PATCH 1/6] memcg: mark more functions/variables as static Kirill A. Shutemov
2012-01-06 20:57 ` [RESEND, PATCH 2/6] memcg: remove unused variable Kirill A. Shutemov
2012-01-06 20:57 ` [RESEND, PATCH 3/6] memcg: mark stat field of mem_cgroup struct as __percpu Kirill A. Shutemov
2012-01-06 20:57 ` [RESEND, PATCH 4/6] memcg: fix broken boolean expression Kirill A. Shutemov
2012-01-09 14:04   ` Johannes Weiner
2012-01-16 11:54     ` Kirill A. Shutemov
2012-01-19 13:44       ` Johannes Weiner
2012-01-06 20:57 ` [RESEND, PATCH 5/6] memcg: drop redundant brackets Kirill A. Shutemov
2012-01-06 20:57 ` [RESEND, PATCH 6/6] memcg: cleanup memcg_check_events() Kirill A. Shutemov
2012-01-09 13:41   ` Johannes Weiner
2012-01-16 11:59     ` Kirill A. Shutemov

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).