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>
next prev parent 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: linkBe 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.