All of lore.kernel.org
 help / color / mirror / Atom feed
From: Willy Tarreau <w@1wt.eu>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Kees Cook <keescook@chromium.org>,
	y2038 Mailman List <y2038@lists.linaro.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Jacek Anaszewski <jacek.anaszewski@gmail.com>,
	Pavel Machek <pavel@ucw.cz>,
	linux-leds@vger.kernel.org
Subject: Re: [PATCH] leds: ledtrig-activity: use ktime_get_boot_ns()
Date: Mon, 18 Jun 2018 20:29:13 +0200	[thread overview]
Message-ID: <20180618182913.GA30450@1wt.eu> (raw)
In-Reply-To: <CAK8P3a1K19WoEGaqrj6Fx0GYyj1jmA50dS+dMJuF7ugkwjCiug@mail.gmail.com>

Hi Arnd,

On Mon, Jun 18, 2018 at 05:47:28PM +0200, Arnd Bergmann wrote:
> On Mon, Jun 18, 2018 at 5:07 PM, Pavel Machek <pavel@ucw.cz> wrote:
> >> diff --git a/drivers/leds/trigger/ledtrig-activity.c b/drivers/leds/trigger/ledtrig-activity.c
> >> index 5081894082bd..589c1bc4d0b9 100644
> >> --- a/drivers/leds/trigger/ledtrig-activity.c
> >> +++ b/drivers/leds/trigger/ledtrig-activity.c
> >> @@ -37,7 +37,6 @@ static void led_activity_function(struct timer_list *t)
> >>       struct activity_data *activity_data = from_timer(activity_data, t,
> >>                                                        timer);
> >>       struct led_classdev *led_cdev = activity_data->led_cdev;
> >> -     struct timespec boot_time;
> >>       unsigned int target;
> >>       unsigned int usage;
> >>       int delay;
> >> @@ -57,7 +56,7 @@ static void led_activity_function(struct timer_list *t)
> >>               return;
> >>       }
> >>
> >> -     get_monotonic_boottime(&boot_time);
> >> +     curr_boot = ktime_get_boot_ns();
> >>
> >>       cpus = 0;
> >>       curr_used = 0;
> >> @@ -76,7 +75,6 @@ static void led_activity_function(struct timer_list *t)
> >>        * down to 16us, ensuring we won't overflow 32-bit computations below
> >>        * even up to 3k CPUs, while keeping divides cheap on smaller systems.
> >>        */
> >> -     curr_boot = timespec_to_ns(&boot_time) * cpus;
> >
> > Original code is pretty weird (notice the * cpus), so I'm
> > double-checking.
> 
> Ok, dropping the *cpus was not intentional, I'll repost a version that puts
> it that back. Thanks for pointing this out!

Pavel is right, the *cpus is intentional. curr_boot contains the cumulated
time for all CPUs in order to measure an average usage over all of them.
By keeping it scaled by #cpus we avoid useless divides.

Cheers,
Willy
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

WARNING: multiple messages have this Message-ID (diff)
From: Willy Tarreau <w@1wt.eu>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Pavel Machek <pavel@ucw.cz>,
	Jacek Anaszewski <jacek.anaszewski@gmail.com>,
	y2038 Mailman List <y2038@lists.linaro.org>,
	Kees Cook <keescook@chromium.org>,
	linux-leds@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] leds: ledtrig-activity: use ktime_get_boot_ns()
Date: Mon, 18 Jun 2018 20:29:13 +0200	[thread overview]
Message-ID: <20180618182913.GA30450@1wt.eu> (raw)
In-Reply-To: <CAK8P3a1K19WoEGaqrj6Fx0GYyj1jmA50dS+dMJuF7ugkwjCiug@mail.gmail.com>

Hi Arnd,

On Mon, Jun 18, 2018 at 05:47:28PM +0200, Arnd Bergmann wrote:
> On Mon, Jun 18, 2018 at 5:07 PM, Pavel Machek <pavel@ucw.cz> wrote:
> >> diff --git a/drivers/leds/trigger/ledtrig-activity.c b/drivers/leds/trigger/ledtrig-activity.c
> >> index 5081894082bd..589c1bc4d0b9 100644
> >> --- a/drivers/leds/trigger/ledtrig-activity.c
> >> +++ b/drivers/leds/trigger/ledtrig-activity.c
> >> @@ -37,7 +37,6 @@ static void led_activity_function(struct timer_list *t)
> >>       struct activity_data *activity_data = from_timer(activity_data, t,
> >>                                                        timer);
> >>       struct led_classdev *led_cdev = activity_data->led_cdev;
> >> -     struct timespec boot_time;
> >>       unsigned int target;
> >>       unsigned int usage;
> >>       int delay;
> >> @@ -57,7 +56,7 @@ static void led_activity_function(struct timer_list *t)
> >>               return;
> >>       }
> >>
> >> -     get_monotonic_boottime(&boot_time);
> >> +     curr_boot = ktime_get_boot_ns();
> >>
> >>       cpus = 0;
> >>       curr_used = 0;
> >> @@ -76,7 +75,6 @@ static void led_activity_function(struct timer_list *t)
> >>        * down to 16us, ensuring we won't overflow 32-bit computations below
> >>        * even up to 3k CPUs, while keeping divides cheap on smaller systems.
> >>        */
> >> -     curr_boot = timespec_to_ns(&boot_time) * cpus;
> >
> > Original code is pretty weird (notice the * cpus), so I'm
> > double-checking.
> 
> Ok, dropping the *cpus was not intentional, I'll repost a version that puts
> it that back. Thanks for pointing this out!

Pavel is right, the *cpus is intentional. curr_boot contains the cumulated
time for all CPUs in order to measure an average usage over all of them.
By keeping it scaled by #cpus we avoid useless divides.

Cheers,
Willy

  parent reply	other threads:[~2018-06-18 18:29 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-18 14:41 [PATCH] leds: ledtrig-activity: use ktime_get_boot_ns() Arnd Bergmann
2018-06-18 15:07 ` Pavel Machek
2018-06-18 15:47   ` Arnd Bergmann
2018-06-18 15:55     ` Pavel Machek
2018-06-18 18:29     ` Willy Tarreau [this message]
2018-06-18 18:29       ` Willy Tarreau

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=20180618182913.GA30450@1wt.eu \
    --to=w@1wt.eu \
    --cc=arnd@arndb.de \
    --cc=jacek.anaszewski@gmail.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=pavel@ucw.cz \
    --cc=y2038@lists.linaro.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.