All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Kleine-Budde <mkl@pengutronix.de>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>,
	linux-kernel@vger.kernel.org, arm@kernel.org,
	John Stultz <john.stultz@linaro.org>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	Andres Salomon <dilinger@queued.net>,
	Shawn Guo <shawn.guo@linaro.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: RFC: [PATCH] clocksource: tcb: fix min_delta calculation
Date: Thu, 20 Jun 2013 17:00:54 +0200	[thread overview]
Message-ID: <51C31926.3060400@pengutronix.de> (raw)
In-Reply-To: <51794385.1030308@pengutronix.de>

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

Hello Thomas,

On 04/25/2013 04:53 PM, Marc Kleine-Budde wrote:
> On 04/25/2013 04:21 PM, Thomas Gleixner wrote:
>> On Tue, 23 Apr 2013, Marc Kleine-Budde wrote:
>>> The commit
>>>
>>>     77cc982 clocksource: use clockevents_config_and_register() where possible
>>>
>>> switches from manually calculating min_delta_ns (and others) and
>>> clockevents_register_device() to automatic calculation via
>>> clockevents_config_and_register(). During this conversation the "+ 1" in
>>>
>>>     min_delta_ns = clockevent_delta2ns(1, &clkevt.clkevt) + 1;
>>>
>>> was lost. This leads to problems with schedule_delayed_work() with a delay of
>>> "1". Resulting in the work not scheduled in time.
>>
>> Errm. How is schedule_delayed_work() related to this?

Coming back to the this problem after some time.

I'm on at91sam9263 (armv5) using latest linus/master (v3.10-rc6-84-gc069114).
I can illustrate the problem running cyclictest.

Here's the output of unpatched linus/master:

> root@halo22:~ cyclictest -i 10000 -n -t 4
> policy: other/other: loadavg: 0.10 0.19 0.10 2/31 133
> 
> T: 0 (  130) P: 0 I:10000 C:  30767 Min:     57 Act:  165 Avg:909453 Max: 201897444
> T: 1 (  131) P: 0 I:10500 C:  29302 Min:     68 Act:  144 Avg:906651 Max: 204005666
> T: 2 (  132) P: 0 I:11000 C:  27970 Min:     79 Act:  188 Avg:905068 Max: 204008666
> T: 3 (  133) P: 0 I:11500 C:  26754 Min:     52 Act:  132 Avg:904438 Max: 203665111

Cyclictest runs for about two seconds, but then shows no output for
about two seconds, then it runs again, then stops and so on...

With my patch applied, the numbers are back to normal:

> root@halo22:~ cyclictest -i 10000 -n -t 4
> policy: other/other: loadavg: 0.34 0.15 0.06 2/32 109
> 
> T: 0 (  106) P: 0 I:10000 C:    292 Min:    112 Act:  208 Avg:  189 Max:    1715
> T: 1 (  107) P: 0 I:10500 C:    277 Min:    107 Act:  198 Avg:  201 Max:    1464
> T: 2 (  108) P: 0 I:11000 C:    265 Min:    104 Act:  148 Avg:  195 Max:    1530
> T: 3 (  109) P: 0 I:11500 C:    252 Min:    113 Act:  205 Avg:  167 Max:     269

Cyclictest keeps running without 2 second stops.

Is my patch a valid solution to the problem? If so, I'll write more 
specific commit message.

regards,
Marc

---

The commit

    77cc982 clocksource: use clockevents_config_and_register() where possible

switches from manually calculating min_delta_ns (and others) and
clockevents_register_device() to automatic calculation via
clockevents_config_and_register(). During this conversation the "+ 1" in

    min_delta_ns = clockevent_delta2ns(1, &clkevt.clkevt) + 1;

was lost. This leads to problems with schedule_delayed_work() with a delay of
"1". Resulting in the work not scheduled in time.

This patch fixes the problem by increasing the min_delta to "2" ticks.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/clocksource/tcb_clksrc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c
index 8a61872..7cf6dc7 100644
--- a/drivers/clocksource/tcb_clksrc.c
+++ b/drivers/clocksource/tcb_clksrc.c
@@ -197,7 +197,7 @@ static void __init setup_clkevents(struct atmel_tc *tc, int clk32k_divisor_idx)
 
 	clkevt.clkevt.cpumask = cpumask_of(0);
 
-	clockevents_config_and_register(&clkevt.clkevt, 32768, 1, 0xffff);
+	clockevents_config_and_register(&clkevt.clkevt, 32768, 2, 0xffff);
 
 	setup_irq(irq, &tc_irqaction);
 }
-- 1.8.2.rc2

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: mkl@pengutronix.de (Marc Kleine-Budde)
To: linux-arm-kernel@lists.infradead.org
Subject: RFC: [PATCH] clocksource: tcb: fix min_delta calculation
Date: Thu, 20 Jun 2013 17:00:54 +0200	[thread overview]
Message-ID: <51C31926.3060400@pengutronix.de> (raw)
In-Reply-To: <51794385.1030308@pengutronix.de>

Hello Thomas,

On 04/25/2013 04:53 PM, Marc Kleine-Budde wrote:
> On 04/25/2013 04:21 PM, Thomas Gleixner wrote:
>> On Tue, 23 Apr 2013, Marc Kleine-Budde wrote:
>>> The commit
>>>
>>>     77cc982 clocksource: use clockevents_config_and_register() where possible
>>>
>>> switches from manually calculating min_delta_ns (and others) and
>>> clockevents_register_device() to automatic calculation via
>>> clockevents_config_and_register(). During this conversation the "+ 1" in
>>>
>>>     min_delta_ns = clockevent_delta2ns(1, &clkevt.clkevt) + 1;
>>>
>>> was lost. This leads to problems with schedule_delayed_work() with a delay of
>>> "1". Resulting in the work not scheduled in time.
>>
>> Errm. How is schedule_delayed_work() related to this?

Coming back to the this problem after some time.

I'm on at91sam9263 (armv5) using latest linus/master (v3.10-rc6-84-gc069114).
I can illustrate the problem running cyclictest.

Here's the output of unpatched linus/master:

> root at halo22:~ cyclictest -i 10000 -n -t 4
> policy: other/other: loadavg: 0.10 0.19 0.10 2/31 133
> 
> T: 0 (  130) P: 0 I:10000 C:  30767 Min:     57 Act:  165 Avg:909453 Max: 201897444
> T: 1 (  131) P: 0 I:10500 C:  29302 Min:     68 Act:  144 Avg:906651 Max: 204005666
> T: 2 (  132) P: 0 I:11000 C:  27970 Min:     79 Act:  188 Avg:905068 Max: 204008666
> T: 3 (  133) P: 0 I:11500 C:  26754 Min:     52 Act:  132 Avg:904438 Max: 203665111

Cyclictest runs for about two seconds, but then shows no output for
about two seconds, then it runs again, then stops and so on...

With my patch applied, the numbers are back to normal:

> root at halo22:~ cyclictest -i 10000 -n -t 4
> policy: other/other: loadavg: 0.34 0.15 0.06 2/32 109
> 
> T: 0 (  106) P: 0 I:10000 C:    292 Min:    112 Act:  208 Avg:  189 Max:    1715
> T: 1 (  107) P: 0 I:10500 C:    277 Min:    107 Act:  198 Avg:  201 Max:    1464
> T: 2 (  108) P: 0 I:11000 C:    265 Min:    104 Act:  148 Avg:  195 Max:    1530
> T: 3 (  109) P: 0 I:11500 C:    252 Min:    113 Act:  205 Avg:  167 Max:     269

Cyclictest keeps running without 2 second stops.

Is my patch a valid solution to the problem? If so, I'll write more 
specific commit message.

regards,
Marc

---

The commit

    77cc982 clocksource: use clockevents_config_and_register() where possible

switches from manually calculating min_delta_ns (and others) and
clockevents_register_device() to automatic calculation via
clockevents_config_and_register(). During this conversation the "+ 1" in

    min_delta_ns = clockevent_delta2ns(1, &clkevt.clkevt) + 1;

was lost. This leads to problems with schedule_delayed_work() with a delay of
"1". Resulting in the work not scheduled in time.

This patch fixes the problem by increasing the min_delta to "2" ticks.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/clocksource/tcb_clksrc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c
index 8a61872..7cf6dc7 100644
--- a/drivers/clocksource/tcb_clksrc.c
+++ b/drivers/clocksource/tcb_clksrc.c
@@ -197,7 +197,7 @@ static void __init setup_clkevents(struct atmel_tc *tc, int clk32k_divisor_idx)
 
 	clkevt.clkevt.cpumask = cpumask_of(0);
 
-	clockevents_config_and_register(&clkevt.clkevt, 32768, 1, 0xffff);
+	clockevents_config_and_register(&clkevt.clkevt, 32768, 2, 0xffff);
 
 	setup_irq(irq, &tc_irqaction);
 }
-- 1.8.2.rc2

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 259 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130620/f746e3df/attachment.sig>

  reply	other threads:[~2013-06-20 15:01 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-12 11:50 [PATCH v2 0/3] Use helper clockevents_config_and_register() Shawn Guo
2013-01-12 11:50 ` Shawn Guo
2013-01-12 11:50 ` [PATCH v2 1/3] clockevents: export clockevents_config_and_register for module use Shawn Guo
2013-01-12 11:50   ` Shawn Guo
2013-01-12 11:50 ` [PATCH v2 2/3] ARM: use clockevents_config_and_register() where possible Shawn Guo
2013-01-12 11:50   ` Shawn Guo
2013-01-12 21:25   ` Jason Cooper
2013-01-12 21:25     ` Jason Cooper
2013-01-12 11:50 ` [PATCH v2 3/3] clocksource: " Shawn Guo
2013-01-12 11:50   ` Shawn Guo
2013-04-23 13:07   ` BUG: " Marc Kleine-Budde
2013-04-23 13:07     ` Marc Kleine-Budde
2013-04-23 13:08     ` RFC: [PATCH] clocksource: tcb: fix min_delta calculation Marc Kleine-Budde
2013-04-23 13:08       ` Marc Kleine-Budde
2013-04-23 13:11       ` Marc Kleine-Budde
2013-04-23 13:11         ` Marc Kleine-Budde
2013-04-23 13:44         ` Shawn Guo
2013-04-23 13:44           ` Shawn Guo
2013-04-23 13:50           ` Marc Kleine-Budde
2013-04-23 13:50             ` Marc Kleine-Budde
2013-04-25 13:36       ` Marc Kleine-Budde
2013-04-25 13:36         ` Marc Kleine-Budde
2013-04-25 14:18         ` Thomas Gleixner
2013-04-25 14:18           ` Thomas Gleixner
2013-04-25 14:21       ` Thomas Gleixner
2013-04-25 14:21         ` Thomas Gleixner
2013-04-25 14:53         ` Marc Kleine-Budde
2013-04-25 14:53           ` Marc Kleine-Budde
2013-06-20 15:00           ` Marc Kleine-Budde [this message]
2013-06-20 15:00             ` Marc Kleine-Budde
2013-09-09  8:18             ` Ronald Wahl
2013-09-09 13:56               ` Marc Kleine-Budde
2013-01-12 21:14 ` [PATCH v2 0/3] Use helper clockevents_config_and_register() Arnd Bergmann
2013-01-12 21:14   ` Arnd Bergmann
2013-01-14 11:23   ` Thomas Gleixner
2013-01-14 11:23     ` Thomas Gleixner
2013-01-14 18:16     ` Olof Johansson
2013-01-14 18:16       ` Olof Johansson

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=51C31926.3060400@pengutronix.de \
    --to=mkl@pengutronix.de \
    --cc=arm@kernel.org \
    --cc=dilinger@queued.net \
    --cc=john.stultz@linaro.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nicolas.ferre@atmel.com \
    --cc=shawn.guo@linaro.org \
    --cc=tglx@linutronix.de \
    /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.