linux-kernel - Re: [PATCH V9] powercap/drivers/idle_injection: Add an idle injection framework

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 25 Jun 2018 10:41:31 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>
Cc:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Eduardo Valentin <edubezval@...il.com>,
        Javi Merino <javi.merino@...nel.org>,
        Leo Yan <leo.yan@...aro.org>,
        Kevin Wangtao <kevin.wangtao@...aro.org>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Rui Zhang <rui.zhang@...el.com>,
        Daniel Thompson <daniel.thompson@...aro.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Andrea Parri <andrea.parri@...rulasolutions.com>,
        "open list:POWER MANAGEMENT CORE" <linux-pm@...r.kernel.org>
Subject: Re: [PATCH V9] powercap/drivers/idle_injection: Add an idle injection framework

On Tue, Jun 19, 2018 at 3:23 PM, Daniel Lezcano
<daniel.lezcano@...aro.org> wrote:

[cut]

One more thing.

> +/**
> + * idle_injection_register - idle injection init routine
> + * @cpumask: the list of CPUs managed by the idle injection device
> + *
> + * This is the initialization function in charge of creating the
> + * initializing of the timer and allocate the structures. It does not
> + * starts the idle injection cycles.
> + *
> + * Return: NULL if an allocation fails.
> + */
> +struct idle_injection_device *idle_injection_register(struct cpumask *cpumask)
> +{
> +       struct idle_injection_device *ii_dev;
> +       int cpu, cpu_rb;
> +
> +       ii_dev = kzalloc(sizeof(*ii_dev) + cpumask_size(), GFP_KERNEL);
> +       if (!ii_dev)
> +               return NULL;
> +
> +       cpumask_copy(to_cpumask(ii_dev->cpumask), cpumask);
> +       hrtimer_init(&ii_dev->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +       ii_dev->timer.function = idle_injection_wakeup_fn;
> +
> +       for_each_cpu(cpu, to_cpumask(ii_dev->cpumask)) {
> +
> +               if (per_cpu(idle_injection_device, cpu)) {
> +                       pr_err("cpu%d is already registered\n", cpu);

If you print something like this, it should be clear what it is about
and what piece of code it comes from as there will be no context
around it it the log.

> +                       goto out_rollback_per_cpu;

Shorten the label perhaps?

> +               }
> +
> +               per_cpu(idle_injection_device, cpu) = ii_dev;
> +       }
> +
> +       return ii_dev;
> +
> +out_rollback_per_cpu:
> +       for_each_cpu(cpu_rb, to_cpumask(ii_dev->cpumask)) {
> +               if (cpu == cpu_rb)
> +                       break;
> +               per_cpu(idle_injection_device, cpu_rb) = NULL;
> +       }
> +
> +       kfree(ii_dev);
> +
> +       return NULL;
> +}
> +