All of lore.kernel.org
 help / color / mirror / Atom feed
From: santosh.shilimkar@ti.com (Santosh Shilimkar)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] sched_clock: fix postinit no sched_clock function check
Date: Wed, 2 Oct 2013 13:27:34 -0400	[thread overview]
Message-ID: <524C5786.2090008@ti.com> (raw)
In-Reply-To: <524C565C.8010709@codeaurora.org>

On Wednesday 02 October 2013 01:22 PM, Stephen Boyd wrote:
> On 10/02/13 10:14, Santosh Shilimkar wrote:
>> On Wednesday 02 October 2013 01:09 PM, Will Deacon wrote:
>>> On Wed, Oct 02, 2013 at 05:55:28PM +0100, Santosh Shilimkar wrote:
>>>> The sched_clock code uses 2 levels of function pointers, sched_clock_func()
>>>> and read_sched_clock() but the no sched_clock check in postinit() just
>>>> checks read_sched_clock().
>>>>
>>>> This leads to kernel falling back to jiffy based sched clock even in
>>>> presence of sched_clock_func() which is not desirable.
>>>>
>>>> Fix the postinit() check to avoid the issue. Probably the issue is hidden
>>>> so far on most of the arm SOCs because of already existing sched_clock
>>>> registrations apart from arch_timer sched_clock. One can reproduce the
>>>> issue by just have arch_timer as sched_clock
>>> Isn't this just an issue with the arch timer driver not calling
>>> setup_sched_clock? Instead, we munge around with sched_clock_func directly,
>>> which doesn't appear to be the way anybody else deals with this.
>>>
>> I thought about that option as well but was not sure since even in that case
>> the check is not complete. We just ensure that function is popullated.
> 
> Yes, nothing is actually broken because sched_clock_func() won't try to
> use the jiffy based read_sched_clock() function. I'm not sure we
> actually need this patch besides to remove a useless timer that updates
> the jiffy epoch. Can we wait until my 64-bit sched_clock patch series
> lands in 3.13? It looks like I still need an ack from Will or Catalin on
> the architected timer patch before the clocksource folks pick it up.
> 
Really... I have not created patch out of fun.
Its broken on my keystone machine at least where the sched_clock is
falling back on jiffy based sched_clock even in presence of arch_timer
sched_clock.

Regards,
Santosh

WARNING: multiple messages have this Message-ID (diff)
From: Santosh Shilimkar <santosh.shilimkar@ti.com>
To: Stephen Boyd <sboyd@codeaurora.org>
Cc: Will Deacon <will.deacon@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"arm@kernel.org" <arm@kernel.org>,
	John Stultz <john.stultz@linaro.org>,
	Russell King <linux@arm.linux.org.uk>,
	"rob.herring@calxeda.com" <rob.herring@calxeda.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Daniel Lezcano <daniel.lezcano@linaro.org>
Subject: Re: [PATCH] sched_clock: fix postinit no sched_clock function check
Date: Wed, 2 Oct 2013 13:27:34 -0400	[thread overview]
Message-ID: <524C5786.2090008@ti.com> (raw)
In-Reply-To: <524C565C.8010709@codeaurora.org>

On Wednesday 02 October 2013 01:22 PM, Stephen Boyd wrote:
> On 10/02/13 10:14, Santosh Shilimkar wrote:
>> On Wednesday 02 October 2013 01:09 PM, Will Deacon wrote:
>>> On Wed, Oct 02, 2013 at 05:55:28PM +0100, Santosh Shilimkar wrote:
>>>> The sched_clock code uses 2 levels of function pointers, sched_clock_func()
>>>> and read_sched_clock() but the no sched_clock check in postinit() just
>>>> checks read_sched_clock().
>>>>
>>>> This leads to kernel falling back to jiffy based sched clock even in
>>>> presence of sched_clock_func() which is not desirable.
>>>>
>>>> Fix the postinit() check to avoid the issue. Probably the issue is hidden
>>>> so far on most of the arm SOCs because of already existing sched_clock
>>>> registrations apart from arch_timer sched_clock. One can reproduce the
>>>> issue by just have arch_timer as sched_clock
>>> Isn't this just an issue with the arch timer driver not calling
>>> setup_sched_clock? Instead, we munge around with sched_clock_func directly,
>>> which doesn't appear to be the way anybody else deals with this.
>>>
>> I thought about that option as well but was not sure since even in that case
>> the check is not complete. We just ensure that function is popullated.
> 
> Yes, nothing is actually broken because sched_clock_func() won't try to
> use the jiffy based read_sched_clock() function. I'm not sure we
> actually need this patch besides to remove a useless timer that updates
> the jiffy epoch. Can we wait until my 64-bit sched_clock patch series
> lands in 3.13? It looks like I still need an ack from Will or Catalin on
> the architected timer patch before the clocksource folks pick it up.
> 
Really... I have not created patch out of fun.
Its broken on my keystone machine at least where the sched_clock is
falling back on jiffy based sched_clock even in presence of arch_timer
sched_clock.

Regards,
Santosh


  reply	other threads:[~2013-10-02 17:27 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-02 16:55 [PATCH] sched_clock: fix postinit no sched_clock function check Santosh Shilimkar
2013-10-02 16:55 ` Santosh Shilimkar
2013-10-02 17:09 ` Will Deacon
2013-10-02 17:09   ` Will Deacon
2013-10-02 17:14   ` Santosh Shilimkar
2013-10-02 17:14     ` Santosh Shilimkar
2013-10-02 17:22     ` Stephen Boyd
2013-10-02 17:22       ` Stephen Boyd
2013-10-02 17:27       ` Santosh Shilimkar [this message]
2013-10-02 17:27         ` Santosh Shilimkar
2013-10-02 17:42         ` Stephen Boyd
2013-10-02 17:42           ` Stephen Boyd
2013-10-02 17:48           ` Will Deacon
2013-10-02 17:48             ` Will Deacon
2013-10-02 18:07             ` Santosh Shilimkar
2013-10-02 18:07               ` Santosh Shilimkar
2013-10-09 23:59               ` John Stultz
2013-10-09 23:59                 ` John Stultz
2013-10-10  0:15                 ` Santosh Shilimkar
2013-10-10  0:15                   ` Santosh Shilimkar
2013-10-02 18:14           ` Rob Herring
2013-10-02 18:14             ` Rob Herring

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=524C5786.2090008@ti.com \
    --to=santosh.shilimkar@ti.com \
    --cc=linux-arm-kernel@lists.infradead.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.