perf/core: Fix installing cgroup event into cpu

From: "leilei.lin" <[email protected]>

Do not install cgroup event into the CPU context 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.

Signed-off-by leilei.lin <[email protected]>

---
 kernel/events/core.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

kernel/events/core.c +7 -0

index 4df5b69..19c587b 100644
@@ -2284,6 +2284,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 +2312,19 @@ static int  __perf_install_in_context(void *info)
 		raw_spin_lock(&task_ctx->lock);
 	}
 
+	if (event->cgrp) {
+		/*
+		 * Only care about cgroup events.
+		 *
+		 * If only the task belongs to cgroup of this event,
+		 * we will continue the installment

Quoted 36 lines

From: "leilei.lin" <[email protected]>

Do not install cgroup event into the CPU context 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.

Signed-off-by leilei.lin <[email protected]>

---
 kernel/events/core.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

kernel/events/core.c +7 -0

index 4df5b69..19c587b 100644
@@ -2284,6 +2284,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 +2312,19 @@ static int  __perf_install_in_context(void *info)
              raw_spin_lock(&task_ctx->lock);
      }
 
+     if (event->cgrp) {
+             /*
+              * Only care about cgroup events.
+              *
+              * If only the task belongs to cgroup of this event,
+              * we will continue the installment

kernel/events/core.c static int __perf_install_in_context(void *info) · line 2330

Quoted 10 lines

kernel/events/core.c +7 -0

+              */
+             cgrp = perf_cgroup_from_task(current, ctx);
+             if (!cgroup_is_descendant(cgrp->css.cgroup,
+                                     event->cgrp->css.cgroup))
+                     goto unlock;
+     }
+
      if (reprogram) {
              ctx_sched_out(ctx, cpuctx, EVENT_TIME);
              add_event_to_ctx(event, ctx);

I think this is wrong. You're right in that we need not schedule it not, but the above also completely fails to add it to the context, leading to it never being scheduled ever.

At the very least we should do add_event_to_ctx() on it.

So the only thing you can do is pick 'reprogram' or not based on if the current cgrp is related to the event->ctx.

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-rc9 next-20180119]
[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/20180126-083327
config: x86_64-randconfig-g0-01260853 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4 reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64

All errors (new ones prefixed by >>):

kernel//events/core.c: In function '__perf_install_in_context':

   kernel//events/core.c:2315:11: error: 'struct perf_event' has no member named 'cgrp'
     if (event->cgrp) {
              ^

Quoted 1 line

Quoted 1 line

kernel//events/core.c:2322:3: error: implicit declaration of function 'perf_cgroup_from_task' [-Werror=implicit-function-declaration]

      cgrp = perf_cgroup_from_task(current, ctx);
      ^
   kernel//events/core.c:2322:8: warning: assignment makes pointer from integer without a cast
      cgrp = perf_cgroup_from_task(current, ctx);
           ^

Quoted 1 line

Quoted 1 line

kernel//events/core.c:2323:33: error: dereferencing pointer to incomplete type

      if (!cgroup_is_descendant(cgrp->css.cgroup,
                                    ^
   kernel//events/core.c:2324: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 +2322 kernel//events/core.c

2274
2275 /*

  2276   * Cross CPU call to install and enable a performance event
  2277   *
  2278   * Very similar to remote_function() + event_function() but cannot assume that
  2279   * things like ctx->is_active and cpuctx->task_ctx are set.
  2280   */
  2281  static int  __perf_install_in_context(void *info)
  2282  {
  2283          struct perf_event *event = info;
  2284          struct perf_event_context *ctx = event->ctx;
  2285          struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
  2286          struct perf_event_context *task_ctx = cpuctx->task_ctx;
  2287          struct perf_cgroup *cgrp;
  2288          bool reprogram = true;
  2289          int ret = 0;
  2290
  2291          raw_spin_lock(&cpuctx->ctx.lock);
  2292          if (ctx->task) {
  2293                  raw_spin_lock(&ctx->lock);
  2294                  task_ctx = ctx;
  2295
  2296                  reprogram = (ctx->task == current);
  2297
  2298                  /*
  2299                   * If the task is running, it must be running on this CPU,
  2300                   * otherwise we cannot reprogram things.
  2301                   *
  2302                   * If its not running, we don't care, ctx->lock will
  2303                   * serialize against it becoming runnable.
  2304                   */

2305 if (task_curr(ctx->task) && !reprogram) {
2306 ret = -ESRCH;
2307 goto unlock;
2308 }
2309
2310 WARN_ON_ONCE(reprogram && cpuctx->task_ctx && cpuctx->task_ctx != ctx);
2311 } else if (task_ctx) {
2312 raw_spin_lock(&task_ctx->lock);
2313 }
2314

Quoted 1 line

2315 if (event->cgrp) {

2316 /*

  2317                   * Only care about cgroup events.
  2318                   *
  2319                   * If only the task belongs to cgroup of this event,
  2320                   * we will continue the installment
  2321                   */

Quoted 2 lines

2322 cgrp = perf_cgroup_from_task(current, ctx);
2323 if (!cgroup_is_descendant(cgrp->css.cgroup,

2324 event->cgrp->css.cgroup))
2325 goto unlock;
2326 }
2327
2328 if (reprogram) {

  2329                  ctx_sched_out(ctx, cpuctx, EVENT_TIME);
  2330                  add_event_to_ctx(event, ctx);
  2331                  ctx_resched(cpuctx, task_ctx, get_event_type(event));

2332 } else {
2333 add_event_to_ctx(event, ctx);
2334 }
2335
2336 unlock:
2337 perf_ctx_unlock(cpuctx, task_ctx);
2338
2339 return ret;
2340 }
2341

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