Build failures in -next due to use of __hrtimer_start_range_ns arm-ccn.c

Build failures in -next due to use of __hrtimer_start_range_ns arm-ccn.c

[Pawel Moll]

On Wed, 2015-05-13 at 15:51 +0100, Mark Brown wrote:
> On Wed, May 13, 2015 at 12:34:48PM +0100, Build bot for Mark Brown wrote:
> 
> > 	arm64-allmodconfig
> > ../drivers/bus/arm-ccn.c:924:3: error: implicit declaration of function '__hrtimer_start_range_ns' [-Werror=implicit-function-declaration]
> > 
> > 	arm-allmodconfig
> > ../drivers/bus/arm-ccn.c:924:3: error: implicit declaration of function '__hrtimer_start_range_ns' [-Werror=implicit-function-declaration]
> 
> Today's -next fails to build both arm and arm64 allmodconfig due to the
> above errors, introduced in commit ffa415245b8666c44d (bus: arm-ccn:
> cpumask attribute).  Judging from the name of the symbol it appears that
> the code is peering into hrtimer implementation details and indeed it
> was removed in commit 58f1f803f1d6ef9 (hrtimer: Get rid of
> __hrtimer_start_range_ns()) which has a commit message suggsting that
> this has indeed been explicitly removed and no new references should be
> added.

Right, the fix will be equivalent to
576b0704c9def6d54b3ae9e13b0b7567c713f568 "x86: perf: uncore: Use
hrtimer_start()"

http://marc.info/?l=linux-kernel&m=142904610220085

Will post it in a second, but not sure who should take it? The arm-soc
guys, as an additional ccn patch, or Thomas as part of his series?

Thanks for pointing this out!

Pawel

Build failures in -next due to use of __hrtimer_start_range_ns arm-ccn.c

[Thomas Gleixner]

On Wed, 13 May 2015, Pawel Moll wrote:
> On Wed, 2015-05-13 at 15:51 +0100, Mark Brown wrote:
> > On Wed, May 13, 2015 at 12:34:48PM +0100, Build bot for Mark Brown wrote:
> > 
> > > 	arm64-allmodconfig
> > > ../drivers/bus/arm-ccn.c:924:3: error: implicit declaration of function '__hrtimer_start_range_ns' [-Werror=implicit-function-declaration]
> > > 
> > > 	arm-allmodconfig
> > > ../drivers/bus/arm-ccn.c:924:3: error: implicit declaration of function '__hrtimer_start_range_ns' [-Werror=implicit-function-declaration]
> > 
> > Today's -next fails to build both arm and arm64 allmodconfig due to the
> > above errors, introduced in commit ffa415245b8666c44d (bus: arm-ccn:
> > cpumask attribute).  Judging from the name of the symbol it appears that
> > the code is peering into hrtimer implementation details and indeed it
> > was removed in commit 58f1f803f1d6ef9 (hrtimer: Get rid of
> > __hrtimer_start_range_ns()) which has a commit message suggsting that
> > this has indeed been explicitly removed and no new references should be
> > added.
> 
> Right, the fix will be equivalent to
> 576b0704c9def6d54b3ae9e13b0b7567c713f568 "x86: perf: uncore: Use
> hrtimer_start()"
> 
> http://marc.info/?l=linux-kernel&m=142904610220085
> 
> Will post it in a second, but not sure who should take it? The arm-soc
> guys, as an additional ccn patch, or Thomas as part of his series?

Route it through the same tree as the commit which introduces the
__hrtimer_start_range_ns usage

Thanks,

	tglx

[PATCH] bus: arm-ccn: Use hrtimer_start()

[Pawel Moll]

hrtimer_start() does not longer defer already expired timers to the
softirq. Get rid of the __hrtimer_start_range_ns() invocation.

Reported-by: Mark Brown <mark.brown@arm.com>
Signed-off-by: Pawel Moll <pawel.moll@arm.com>
---

Arnd, Olof, Kevin,

This patch solves the linux-next build issue reported by Mark Brown:

http://article.gmane.org/gmane.linux.kernel.next/35377

and is an equivalent of the following change:

http://marc.info/?l=linux-kernel&m=142904610220085

Could you, please, queue it?

Thanks!

 drivers/bus/arm-ccn.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/bus/arm-ccn.c b/drivers/bus/arm-ccn.c
index df5f307..7d9879e 100644
--- a/drivers/bus/arm-ccn.c
+++ b/drivers/bus/arm-ccn.c
@@ -921,9 +921,8 @@ static void arm_ccn_pmu_event_start(struct perf_event *event, int flags)
 	 * attribute).
 	 */
 	if (!ccn->irq)
-		__hrtimer_start_range_ns(&ccn->dt.hrtimer,
-				arm_ccn_pmu_timer_period(), 0,
-				HRTIMER_MODE_REL_PINNED, 0);
+		hrtimer_start(&ccn->dt.hrtimer, arm_ccn_pmu_timer_period(),
+				HRTIMER_MODE_REL_PINNED);
 
 	/* Set the DT bus input, engaging the counter */
 	arm_ccn_pmu_xp_dt_config(event, 1);
-- 
2.1.0

[PATCH] bus: arm-ccn: Use hrtimer_start()

[Arnd Bergmann]

On Wednesday 13 May 2015 16:21:24 Pawel Moll wrote:
> diff --git a/drivers/bus/arm-ccn.c b/drivers/bus/arm-ccn.c
> index df5f307..7d9879e 100644
> --- a/drivers/bus/arm-ccn.c
> +++ b/drivers/bus/arm-ccn.c
> @@ -921,9 +921,8 @@ static void arm_ccn_pmu_event_start(struct perf_event *event, int flags)
>          * attribute).
>          */
>         if (!ccn->irq)
> -               __hrtimer_start_range_ns(&ccn->dt.hrtimer,
> -                               arm_ccn_pmu_timer_period(), 0,
> -                               HRTIMER_MODE_REL_PINNED, 0);
> +               hrtimer_start(&ccn->dt.hrtimer, arm_ccn_pmu_timer_period(),
> +                               HRTIMER_MODE_REL_PINNED);
>  
>         /* Set the DT bus input, engaging the counter */
>         arm_ccn_pmu_xp_dt_config(event, 1);

Is this correct by itself, or do we need to pull in the branch that
contains c6eb3f70d44828 ("hrtimer: Get rid of hrtimer softirq")
from Thomas? The other similar patches that Thomas did contain a
comment about the conversion being safe after hrtimer_start()
no longer uses a softirq, but it is still in use in 4.1-rc3.

	Arnd

[PATCH] bus: arm-ccn: Use hrtimer_start()

[Pawel Moll]

On Wed, 2015-05-13 at 16:32 +0100, Arnd Bergmann wrote:
> On Wednesday 13 May 2015 16:21:24 Pawel Moll wrote:
> > diff --git a/drivers/bus/arm-ccn.c b/drivers/bus/arm-ccn.c
> > index df5f307..7d9879e 100644
> > --- a/drivers/bus/arm-ccn.c
> > +++ b/drivers/bus/arm-ccn.c
> > @@ -921,9 +921,8 @@ static void arm_ccn_pmu_event_start(struct perf_event *event, int flags)
> >          * attribute).
> >          */
> >         if (!ccn->irq)
> > -               __hrtimer_start_range_ns(&ccn->dt.hrtimer,
> > -                               arm_ccn_pmu_timer_period(), 0,
> > -                               HRTIMER_MODE_REL_PINNED, 0);
> > +               hrtimer_start(&ccn->dt.hrtimer, arm_ccn_pmu_timer_period(),
> > +                               HRTIMER_MODE_REL_PINNED);
> >  
> >         /* Set the DT bus input, engaging the counter */
> >         arm_ccn_pmu_xp_dt_config(event, 1);
> 
> Is this correct by itself, or do we need to pull in the branch that
> contains c6eb3f70d44828 ("hrtimer: Get rid of hrtimer softirq")
> from Thomas? The other similar patches that Thomas did contain a
> comment about the conversion being safe after hrtimer_start()
> no longer uses a softirq, but it is still in use in 4.1-rc3.

That's why I asked who should carry this patch ;-)

Now, the answer to your question is: the result will not be worse than
the what was there before you pulled my updates, as the code was using
normal hrtimer_start(). It's just when I realised that it should be
pinned I looked at what x86 uncore pmu is doing and shamelessly (and
probably a bit mindlessly) copied the "do not wakeup" version from
there.

Pawel

[PATCH] bus: arm-ccn: Use hrtimer_start()

[Arnd Bergmann]

On Wednesday 13 May 2015 16:47:07 Pawel Moll wrote:
> On Wed, 2015-05-13 at 16:32 +0100, Arnd Bergmann wrote:
> > On Wednesday 13 May 2015 16:21:24 Pawel Moll wrote:
> > > diff --git a/drivers/bus/arm-ccn.c b/drivers/bus/arm-ccn.c
> > > index df5f307..7d9879e 100644
> > > --- a/drivers/bus/arm-ccn.c
> > > +++ b/drivers/bus/arm-ccn.c
> > > @@ -921,9 +921,8 @@ static void arm_ccn_pmu_event_start(struct perf_event *event, int flags)
> > >          * attribute).
> > >          */
> > >         if (!ccn->irq)
> > > -               __hrtimer_start_range_ns(&ccn->dt.hrtimer,
> > > -                               arm_ccn_pmu_timer_period(), 0,
> > > -                               HRTIMER_MODE_REL_PINNED, 0);
> > > +               hrtimer_start(&ccn->dt.hrtimer, arm_ccn_pmu_timer_period(),
> > > +                               HRTIMER_MODE_REL_PINNED);
> > >  
> > >         /* Set the DT bus input, engaging the counter */
> > >         arm_ccn_pmu_xp_dt_config(event, 1);
> > 
> > Is this correct by itself, or do we need to pull in the branch that
> > contains c6eb3f70d44828 ("hrtimer: Get rid of hrtimer softirq")
> > from Thomas? The other similar patches that Thomas did contain a
> > comment about the conversion being safe after hrtimer_start()
> > no longer uses a softirq, but it is still in use in 4.1-rc3.
> 
> That's why I asked who should carry this patch 
> 
> Now, the answer to your question is: the result will not be worse than
> the what was there before you pulled my updates, as the code was using
> normal hrtimer_start(). It's just when I realised that it should be
> pinned I looked at what x86 uncore pmu is doing and shamelessly (and
> probably a bit mindlessly) copied the "do not wakeup" version from
> there.

Ok, got it. I've rewritten the commit message now to reflect this,
and have applied your patch on top of the next/drivers branch.

Thanks a lot for the fast roundtrip in fixing it. Hopefully, tomorrow's
linux-next works better now.

	Arnd

commit a78a3c8ef715b94de3ab27633866d4bde3e2226f
Author: Pawel Moll <pawel.moll@arm.com>
Date:   Wed May 13 16:21:24 2015 +0100

    bus: arm-ccn: Use hrtimer_start() again
    
    hrtimer_start() will no longer defer already expired timers to the
    softirq in 4.2, and the __hrtimer_start_range_ns() function is
    getting removed, causing build errors when both the tip tree and
    the arm-ccn changes are merged.
    
    This changes the code back to using hrtimer_start, which will
    do the right thing after this branch gets merged with the
    timers update from tip.
    
    As pointed out after a discussion on the mailing list, the result will
    not be worse than the what was there before you pulled my updates, as
    the code was using normal hrtimer_start(). It's just when I realised
    that it should be pinned I looked at what x86 uncore pmu is doing and
    shamelessly (and probably a bit mindlessly) copied the "do not wakeup"
    version from there.
    
    Reported-by: Mark Brown <mark.brown@arm.com>
    Signed-off-by: Pawel Moll <pawel.moll@arm.com>
    Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/bus/arm-ccn.c b/drivers/bus/arm-ccn.c
index df5f307f3244..7d9879e166cf 100644
--- a/drivers/bus/arm-ccn.c
+++ b/drivers/bus/arm-ccn.c
@@ -921,9 +921,8 @@ static void arm_ccn_pmu_event_start(struct perf_event *event, int flags)
 	 * attribute).
 	 */
 	if (!ccn->irq)
-		__hrtimer_start_range_ns(&ccn->dt.hrtimer,
-				arm_ccn_pmu_timer_period(), 0,
-				HRTIMER_MODE_REL_PINNED, 0);
+		hrtimer_start(&ccn->dt.hrtimer, arm_ccn_pmu_timer_period(),
+				HRTIMER_MODE_REL_PINNED);
 
 	/* Set the DT bus input, engaging the counter */
 	arm_ccn_pmu_xp_dt_config(event, 1);