[PATCH RESEND v3] perf/core: Fix installing cgroup event into cpu

[PATCH RESEND v3] perf/core: Fix installing cgroup event into cpu

[linxiulei]

From: "leilei.lin" <leilei.lin@alibaba-inc.com>

Do not install cgroup event into the CPU context and schedule it
if the cgroup is not running on this CPU

While there is no task of cgroup running specified CPU, current
kernel still install cgroup event into CPU context that causes
another cgroup event can't be installed into this CPU.

This patch prevent scheduling events at __perf_install_in_context()
and installing events at list_update_cgroup_event() if cgroup isn't
running on specified CPU.

Signed-off-by: leilei.lin <leilei.lin@alibaba-inc.com>
---
 v2: Set cpuctx->cgrp only if the same cgroup is running on this
   CPU otherwise following events couldn't be activated immediately
 v3: Enhance the comments and commit message

 kernel/events/core.c | 49 ++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 13 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 4df5b69..fb5b167 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -933,31 +933,41 @@ list_update_cgroup_event(struct perf_event *event,
 {
 	struct perf_cpu_context *cpuctx;
 	struct list_head *cpuctx_entry;
+	struct perf_cgroup *cgrp;
 
 	if (!is_cgroup_event(event))
 		return;
 
-	if (add && ctx->nr_cgroups++)
-		return;
-	else if (!add && --ctx->nr_cgroups)
-		return;
 	/*
 	 * Because cgroup events are always per-cpu events,
 	 * this will always be called from the right CPU.
 	 */
 	cpuctx = __get_cpu_context(ctx);
-	cpuctx_entry = &cpuctx->cgrp_cpuctx_entry;
-	/* cpuctx->cgrp is NULL unless a cgroup event is active in this CPU .*/
-	if (add) {
-		struct perf_cgroup *cgrp = perf_cgroup_from_task(current, ctx);
+	cgrp = perf_cgroup_from_task(current, ctx);
 
-		list_add(cpuctx_entry, this_cpu_ptr(&cgrp_cpuctx_list));
-		if (cgroup_is_descendant(cgrp->css.cgroup, event->cgrp->css.cgroup))
+	/*
+	 * if only the cgroup is running on this cpu,
+	 * we put/remove this cgroup into cpu context.
+	 * Or it would case mismatch in following cgroup
+	 * events at event_filter_match()
+	 */
+	if (cgroup_is_descendant(cgrp->css.cgroup, event->cgrp->css.cgroup)) {
+		if (add)
 			cpuctx->cgrp = cgrp;
-	} else {
-		list_del(cpuctx_entry);
-		cpuctx->cgrp = NULL;
+		else
+			cpuctx->cgrp = NULL;
 	}
+
+	if (add && ctx->nr_cgroups++)
+		return;
+	else if (!add && --ctx->nr_cgroups)
+		return;
+
+	cpuctx_entry = &cpuctx->cgrp_cpuctx_entry;
+	if (add)
+		list_add(cpuctx_entry, this_cpu_ptr(&cgrp_cpuctx_list));
+	else
+		list_del(cpuctx_entry);
 }
 
 #else /* !CONFIG_CGROUP_PERF */
@@ -2284,6 +2294,7 @@ static int  __perf_install_in_context(void *info)
 	struct perf_event_context *ctx = event->ctx;
 	struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
 	struct perf_event_context *task_ctx = cpuctx->task_ctx;
+	struct perf_cgroup *cgrp;
 	bool reprogram = true;
 	int ret = 0;
 
@@ -2311,6 +2322,18 @@ static int  __perf_install_in_context(void *info)
 		raw_spin_lock(&task_ctx->lock);
 	}
 
+	if (is_cgroup_event(event)) {
+		/*
+		 * Only care about cgroup events.
+		 *
+		 * If only the task belongs to cgroup of this event,
+		 * we will continue the installment
+		 */
+		cgrp = perf_cgroup_from_task(current, ctx);
+		reprogram = cgroup_is_descendant(cgrp->css.cgroup,
+					event->cgrp->css.cgroup);
+	}
+
 	if (reprogram) {
 		ctx_sched_out(ctx, cpuctx, EVENT_TIME);
 		add_event_to_ctx(event, ctx);
-- 
2.8.4.31.g9ed660f

Re: [PATCH RESEND v3] perf/core: Fix installing cgroup event into cpu

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3876 bytes --]

Hi leilei.lin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.15 next-20180202]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/linxiulei-gmail-com/perf-core-Fix-installing-cgroup-event-into-cpu/20180203-133110
config: i386-randconfig-x071-201804 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   kernel/events/core.c: In function '__perf_install_in_context':
>> kernel/events/core.c:2332:10: error: implicit declaration of function 'perf_cgroup_from_task'; did you mean 'perf_cgroup_match'? [-Werror=implicit-function-declaration]
      cgrp = perf_cgroup_from_task(current, ctx);
             ^~~~~~~~~~~~~~~~~~~~~
             perf_cgroup_match
   kernel/events/core.c:2332:8: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
      cgrp = perf_cgroup_from_task(current, ctx);
           ^
>> kernel/events/core.c:2333:40: error: dereferencing pointer to incomplete type 'struct perf_cgroup'
      reprogram = cgroup_is_descendant(cgrp->css.cgroup,
                                           ^~
>> kernel/events/core.c:2334:11: error: 'struct perf_event' has no member named 'cgrp'
         event->cgrp->css.cgroup);
              ^~
   cc1: some warnings being treated as errors

vim +2332 kernel/events/core.c

  2284	
  2285	/*
  2286	 * Cross CPU call to install and enable a performance event
  2287	 *
  2288	 * Very similar to remote_function() + event_function() but cannot assume that
  2289	 * things like ctx->is_active and cpuctx->task_ctx are set.
  2290	 */
  2291	static int  __perf_install_in_context(void *info)
  2292	{
  2293		struct perf_event *event = info;
  2294		struct perf_event_context *ctx = event->ctx;
  2295		struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
  2296		struct perf_event_context *task_ctx = cpuctx->task_ctx;
  2297		struct perf_cgroup *cgrp;
  2298		bool reprogram = true;
  2299		int ret = 0;
  2300	
  2301		raw_spin_lock(&cpuctx->ctx.lock);
  2302		if (ctx->task) {
  2303			raw_spin_lock(&ctx->lock);
  2304			task_ctx = ctx;
  2305	
  2306			reprogram = (ctx->task == current);
  2307	
  2308			/*
  2309			 * If the task is running, it must be running on this CPU,
  2310			 * otherwise we cannot reprogram things.
  2311			 *
  2312			 * If its not running, we don't care, ctx->lock will
  2313			 * serialize against it becoming runnable.
  2314			 */
  2315			if (task_curr(ctx->task) && !reprogram) {
  2316				ret = -ESRCH;
  2317				goto unlock;
  2318			}
  2319	
  2320			WARN_ON_ONCE(reprogram && cpuctx->task_ctx && cpuctx->task_ctx != ctx);
  2321		} else if (task_ctx) {
  2322			raw_spin_lock(&task_ctx->lock);
  2323		}
  2324	
  2325		if (is_cgroup_event(event)) {
  2326			/*
  2327			 * Only care about cgroup events.
  2328			 *
  2329			 * If only the task belongs to cgroup of this event,
  2330			 * we will continue the installment
  2331			 */
> 2332			cgrp = perf_cgroup_from_task(current, ctx);
> 2333			reprogram = cgroup_is_descendant(cgrp->css.cgroup,
> 2334						event->cgrp->css.cgroup);
  2335		}
  2336	
  2337		if (reprogram) {
  2338			ctx_sched_out(ctx, cpuctx, EVENT_TIME);
  2339			add_event_to_ctx(event, ctx);
  2340			ctx_resched(cpuctx, task_ctx, get_event_type(event));
  2341		} else {
  2342			add_event_to_ctx(event, ctx);
  2343		}
  2344	
  2345	unlock:
  2346		perf_ctx_unlock(cpuctx, task_ctx);
  2347	
  2348		return ret;
  2349	}
  2350	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31734 bytes --]

Re: [PATCH RESEND v3] perf/core: Fix installing cgroup event into cpu

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3831 bytes --]

Hi leilei.lin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.15 next-20180202]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/linxiulei-gmail-com/perf-core-Fix-installing-cgroup-event-into-cpu/20180203-133110
config: i386-randconfig-s0-201804 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   kernel/events/core.c: In function '__perf_install_in_context':
>> kernel/events/core.c:2332:10: error: implicit declaration of function 'perf_cgroup_from_task' [-Werror=implicit-function-declaration]
      cgrp = perf_cgroup_from_task(current, ctx);
             ^~~~~~~~~~~~~~~~~~~~~
   kernel/events/core.c:2332:8: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
      cgrp = perf_cgroup_from_task(current, ctx);
           ^
   kernel/events/core.c:2333:40: error: dereferencing pointer to incomplete type 'struct perf_cgroup'
      reprogram = cgroup_is_descendant(cgrp->css.cgroup,
                                           ^~
   kernel/events/core.c:2334:11: error: 'struct perf_event' has no member named 'cgrp'
         event->cgrp->css.cgroup);
              ^~
   cc1: some warnings being treated as errors

vim +/perf_cgroup_from_task +2332 kernel/events/core.c

  2284	
  2285	/*
  2286	 * Cross CPU call to install and enable a performance event
  2287	 *
  2288	 * Very similar to remote_function() + event_function() but cannot assume that
  2289	 * things like ctx->is_active and cpuctx->task_ctx are set.
  2290	 */
  2291	static int  __perf_install_in_context(void *info)
  2292	{
  2293		struct perf_event *event = info;
  2294		struct perf_event_context *ctx = event->ctx;
  2295		struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
  2296		struct perf_event_context *task_ctx = cpuctx->task_ctx;
  2297		struct perf_cgroup *cgrp;
  2298		bool reprogram = true;
  2299		int ret = 0;
  2300	
  2301		raw_spin_lock(&cpuctx->ctx.lock);
  2302		if (ctx->task) {
  2303			raw_spin_lock(&ctx->lock);
  2304			task_ctx = ctx;
  2305	
  2306			reprogram = (ctx->task == current);
  2307	
  2308			/*
  2309			 * If the task is running, it must be running on this CPU,
  2310			 * otherwise we cannot reprogram things.
  2311			 *
  2312			 * If its not running, we don't care, ctx->lock will
  2313			 * serialize against it becoming runnable.
  2314			 */
  2315			if (task_curr(ctx->task) && !reprogram) {
  2316				ret = -ESRCH;
  2317				goto unlock;
  2318			}
  2319	
  2320			WARN_ON_ONCE(reprogram && cpuctx->task_ctx && cpuctx->task_ctx != ctx);
  2321		} else if (task_ctx) {
  2322			raw_spin_lock(&task_ctx->lock);
  2323		}
  2324	
  2325		if (is_cgroup_event(event)) {
  2326			/*
  2327			 * Only care about cgroup events.
  2328			 *
  2329			 * If only the task belongs to cgroup of this event,
  2330			 * we will continue the installment
  2331			 */
> 2332			cgrp = perf_cgroup_from_task(current, ctx);
  2333			reprogram = cgroup_is_descendant(cgrp->css.cgroup,
  2334						event->cgrp->css.cgroup);
  2335		}
  2336	
  2337		if (reprogram) {
  2338			ctx_sched_out(ctx, cpuctx, EVENT_TIME);
  2339			add_event_to_ctx(event, ctx);
  2340			ctx_resched(cpuctx, task_ctx, get_event_type(event));
  2341		} else {
  2342			add_event_to_ctx(event, ctx);
  2343		}
  2344	
  2345	unlock:
  2346		perf_ctx_unlock(cpuctx, task_ctx);
  2347	
  2348		return ret;
  2349	}
  2350	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30878 bytes --]