Re: v3.13-rc6+ regression (ARM board)

Hello Uwe,

quoted

quoted

quoted

There seems to be a regression in v3.13-rc6+ (up to current tip =
71ce176ee6ed1735b9a1160a5704a915d13849b1).

Board is Gateworks Cambria, CPU Intel IXP435 ARM big endian, gcc 4.7.3.
The board boots correctly and works (shell mostly, and SSHD) for about
50 seconds. After 52-54 seconds, it frozes dead without any console
(UART) output.

Merging 1ca7d67cf5d5a2aef26a8d9afd789006fa098347 with
85c3d2dd15be4d577a37ffb8bbbd019fc8e3280a = issue, but

merging 1ca7d67cf5d5a2aef26a8d9afd789006fa098347 with
85c3d2dd15be4d577a37ffb8bbbd019fc8e3280a~1 = no issue.

This means these two commits don't like each other:

commit 1ca7d67cf5d5a2aef26a8d9afd789006fa098347
Author: John Stultz [off-list ref]
Date:   Mon Oct 7 15:51:59 2013 -0700

    seqcount: Add lockdep functionality to seqcount/seqlock structures

    Currently seqlocks and seqcounts don't support lockdep.

    After running across a seqcount related deadlock in the timekeeping
    code, I used a less-refined and more focused variant of this patch
    to narrow down the cause of the issue.

    This is a first-pass attempt to properly enable lockdep functionality
    on seqlocks and seqcounts.

    Since seqcounts are used in the vdso gettimeofday code, I've provided
    non-lockdep accessors for those needs.

    I've also handled one case where there were nested seqlock writers
    and there may be more edge cases.

    Comments and feedback would be appreciated!

    Signed-off-by: John Stultz [off-list ref]
    Signed-off-by: Peter Zijlstra [off-list ref]
    Cc: Eric Dumazet [off-list ref]
    Cc: Li Zefan [off-list ref]
    Cc: Mathieu Desnoyers [off-list ref]
    Cc: Steven Rostedt [off-list ref]
    Cc: "David S. Miller" [off-list ref]
    Cc: netdev at vger.kernel.org
    Link: http://lkml.kernel.org/r/1381186321-4906-3-git-send-email-john.stultz at linaro.org
    Signed-off-by: Ingo Molnar [off-list ref]

 arch/x86/vdso/vclock_gettime.c |  8 ++++---- (not used on this machine)
 fs/dcache.c                    |  4 ++--
 fs/fs_struct.c                 |  2 +-
 include/linux/init_task.h      |  8 ++++----
 include/linux/lockdep.h        |  8 ++++++--
 include/linux/seqlock.h        | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 mm/filemap_xip.c               |  2 +-       (not used on this machine)
 7 files changed, 90 insertions(+), 21 deletions(-)

and:

commit 85c3d2dd15be4d577a37ffb8bbbd019fc8e3280a
Author: Stephen Boyd [off-list ref]
Date:   Thu Jul 18 16:21:15 2013 -0700

    sched_clock: Use seqcount instead of rolling our own

    We're going to increase the cyc value to 64 bits in the near
    future. Doing that is going to break the custom seqcount
    implementation in the sched_clock code because 64 bit numbers
    aren't guaranteed to be atomic. Replace the cyc_copy with a
    seqcount to avoid this problem.

    Cc: Russell King [off-list ref]
    Acked-by: Will Deacon [off-list ref]
    Signed-off-by: Stephen Boyd [off-list ref]
    Signed-off-by: John Stultz [off-list ref]

 kernel/time/sched_clock.c | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)
-- 
Krzysztof Halasa

Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

On Thu, Jan 2, 2014 at 4:07 AM, Krzysztof Ha?asa [off-list ref] wrote:

This means these two commits don't like each other:

    seqcount: Add lockdep functionality to seqcount/seqlock structures
    sched_clock: Use seqcount instead of rolling our own

Does something like this fix it for you?

  --- a/kernel/time/sched_clock.c
  +++ b/kernel/time/sched_clock.c
  @@ -36,6 +36,7 @@ core_param(irqtime, irqtime, int, 0400);

   static struct clock_data cd = {
          .mult   = NSEC_PER_SEC / HZ,
  +       .seq = SEQCNT_ZERO(cd.seq),
   };

   static u64 __read_mostly sched_clock_mask;

(The above is not even compile-tested, because x86 doesn't use
GENERIC_SCHED_CLOCK. So I did the patch blindly, but I think you get
the idea..)

            Linus

On 01/02/2014 11:38 AM, Linus Torvalds wrote:

On Thu, Jan 2, 2014 at 4:07 AM, Krzysztof Ha?asa [off-list ref] wrote:

quoted

This means these two commits don't like each other:

    seqcount: Add lockdep functionality to seqcount/seqlock structures
    sched_clock: Use seqcount instead of rolling our own

Does something like this fix it for you?

  --- a/kernel/time/sched_clock.c
  +++ b/kernel/time/sched_clock.c
  @@ -36,6 +36,7 @@ core_param(irqtime, irqtime, int, 0400);

   static struct clock_data cd = {
          .mult   = NSEC_PER_SEC / HZ,
  +       .seq = SEQCNT_ZERO(cd.seq),
   };

   static u64 __read_mostly sched_clock_mask;

(The above is not even compile-tested, because x86 doesn't use
GENERIC_SCHED_CLOCK. So I did the patch blindly, but I think you get
the idea..)

Sheesh. Just finishing up holiday email backlog and Linus already has a
fix. :)

This looks like it should fix the issue, and does build for me.

Assuming it works for Krzysztof,

Acked-by: John Stultz <redacted>

I'll do another grep pass through -rc6 to make sure no other new
uninitialized seqlock usage was added.

thanks
-john

On 01/02/2014 12:03 PM, John Stultz wrote:

On 01/02/2014 11:38 AM, Linus Torvalds wrote:

quoted

On Thu, Jan 2, 2014 at 4:07 AM, Krzysztof Ha?asa [off-list ref] wrote:

quoted

This means these two commits don't like each other:

    seqcount: Add lockdep functionality to seqcount/seqlock structures
    sched_clock: Use seqcount instead of rolling our own

Does something like this fix it for you?

  --- a/kernel/time/sched_clock.c
  +++ b/kernel/time/sched_clock.c
  @@ -36,6 +36,7 @@ core_param(irqtime, irqtime, int, 0400);

   static struct clock_data cd = {
          .mult   = NSEC_PER_SEC / HZ,
  +       .seq = SEQCNT_ZERO(cd.seq),
   };

   static u64 __read_mostly sched_clock_mask;

(The above is not even compile-tested, because x86 doesn't use
GENERIC_SCHED_CLOCK. So I did the patch blindly, but I think you get
the idea..)

Sheesh. Just finishing up holiday email backlog and Linus already has a
fix. :)

This looks like it should fix the issue, and does build for me.

Assuming it works for Krzysztof,

So something else may be at play. Even with Linus' patch I reproduced a
similar hang here.

Still chasing it down, but it looks like a seqlock deadlock where we're
calling read while holding the lock.

thanks
-john

On 01/02/14 12:30, John Stultz wrote:

On 01/02/2014 12:03 PM, John Stultz wrote:

quoted

On 01/02/2014 11:38 AM, Linus Torvalds wrote:

quoted

On Thu, Jan 2, 2014 at 4:07 AM, Krzysztof Ha?asa [off-list ref] wrote:

quoted

This means these two commits don't like each other:

    seqcount: Add lockdep functionality to seqcount/seqlock structures
    sched_clock: Use seqcount instead of rolling our own

Does something like this fix it for you?

  --- a/kernel/time/sched_clock.c
  +++ b/kernel/time/sched_clock.c
  @@ -36,6 +36,7 @@ core_param(irqtime, irqtime, int, 0400);

   static struct clock_data cd = {
          .mult   = NSEC_PER_SEC / HZ,
  +       .seq = SEQCNT_ZERO(cd.seq),
   };

   static u64 __read_mostly sched_clock_mask;

(The above is not even compile-tested, because x86 doesn't use
GENERIC_SCHED_CLOCK. So I did the patch blindly, but I think you get
the idea..)

Sheesh. Just finishing up holiday email backlog and Linus already has a
fix. :)

This looks like it should fix the issue, and does build for me.

Assuming it works for Krzysztof,

So something else may be at play. Even with Linus' patch I reproduced a
similar hang here.

Still chasing it down, but it looks like a seqlock deadlock where we're
calling read while holding the lock.

Do you have tracing enabled? When I moved this code over to use
seqcounts it relied on the fact that the compiler wouldn't be generating
any function calls to the tracing code. Before seqcounts got lockdep
support it all collapsed down into sched_clock() due to the use of
inline on the seqlock API.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

On Thu, Jan 2, 2014 at 12:30 PM, John Stultz [off-list ref] wrote:

So something else may be at play. Even with Linus' patch I reproduced a
similar hang here.

Still chasing it down, but it looks like a seqlock deadlock where we're
calling read while holding the lock.

Hmm. Only with lockdep, right?

Does lockdep perhaps read the scheduler clock? Afaik, we have
lockstat_clock(), which uses local_clock(), which in turn translates
to sched_clock_cpu(smp_processor_id())..

So if that code now tries to read the scheduler clock when
update_sched_clock() is doing a update and has done a
write_seqcount_begin()...

    Linus

On 01/02/2014 12:42 PM, Stephen Boyd wrote:

On 01/02/14 12:30, John Stultz wrote:

quoted

On 01/02/2014 12:03 PM, John Stultz wrote:

quoted

On 01/02/2014 11:38 AM, Linus Torvalds wrote:

quoted

On Thu, Jan 2, 2014 at 4:07 AM, Krzysztof Ha?asa [off-list ref] wrote:

quoted

This means these two commits don't like each other:

    seqcount: Add lockdep functionality to seqcount/seqlock structures
    sched_clock: Use seqcount instead of rolling our own

Does something like this fix it for you?

  --- a/kernel/time/sched_clock.c
  +++ b/kernel/time/sched_clock.c
  @@ -36,6 +36,7 @@ core_param(irqtime, irqtime, int, 0400);

   static struct clock_data cd = {
          .mult   = NSEC_PER_SEC / HZ,
  +       .seq = SEQCNT_ZERO(cd.seq),
   };

   static u64 __read_mostly sched_clock_mask;

(The above is not even compile-tested, because x86 doesn't use
GENERIC_SCHED_CLOCK. So I did the patch blindly, but I think you get
the idea..)

Sheesh. Just finishing up holiday email backlog and Linus already has a
fix. :)

This looks like it should fix the issue, and does build for me.

Assuming it works for Krzysztof,

So something else may be at play. Even with Linus' patch I reproduced a
similar hang here.

Still chasing it down, but it looks like a seqlock deadlock where we're
calling read while holding the lock.

Do you have tracing enabled? When I moved this code over to use
seqcounts it relied on the fact that the compiler wouldn't be generating
any function calls to the tracing code. Before seqcounts got lockdep
support it all collapsed down into sched_clock() due to the use of
inline on the seqlock API.

Hrm. I have tracing compiled in, I'll see if disabling it avoids the issue.

If this is the problem, I'm guessing we may need to change it to use
read_seqcount_begin_no_lockdep() then.

But I still don't have a clear sense of exactly whats happening yet.

thanks
-john

On 01/02/2014 12:43 PM, Linus Torvalds wrote:

On Thu, Jan 2, 2014 at 12:30 PM, John Stultz [off-list ref] wrote:

quoted

So something else may be at play. Even with Linus' patch I reproduced a
similar hang here.

Still chasing it down, but it looks like a seqlock deadlock where we're
calling read while holding the lock.

Hmm. Only with lockdep, right?

Yep.

Does lockdep perhaps read the scheduler clock? Afaik, we have
lockstat_clock(), which uses local_clock(), which in turn translates
to sched_clock_cpu(smp_processor_id())..

So if that code now tries to read the scheduler clock when
update_sched_clock() is doing a update and has done a
write_seqcount_begin()...

Sigh. Deadlock by deadlock detection code.

So yea, it looks like this is the case.. though I've not been able to
get a backtrace during the hang to totally validate it (I'm just using
qemu's info registers and looking at the pc and lr).


So I'm guessing we'll just have to disable the lockdep logic here, which
is a little sad, since I'm a little nervous about the generic
sched_clock's locking (ie: works ok for ARM, but its not NMI safe), and
having some better debugging tools there would be helpful.


Anyway, I'll send out a patch to disable the lockdep usage here shortly.

thanks
-john

Unforunately the seqlock lockdep enablmenet can't be used
in sched_clock, since the lockdep infrastructure eventually
calls into sched_clock, which causes a deadlock.

Thus, this patch adds _no_lockdep() seqlock methods for the
writer side, and changes all generic sched_clock usage to use
the _no_lockdep methods.

This solves the issue I was able to reproduce, but it would
be good to get Krzysztof to confirm it solves his problem.

Cc: Krzysztof Ha?asa <khalasa@piap.pl>
Cc: Uwe Kleine-K?nig <redacted>
Cc: Willy Tarreau <w@1wt.eu>
Cc: Ingo Molnar <mingo@kernel.org>,
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephen Boyd <redacted>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-arm-kernel at lists.infradead.org
Reported-by: Krzysztof Ha?asa <khalasa@piap.pl>
Signed-off-by: John Stultz <redacted>
---
 include/linux/seqlock.h   | 19 +++++++++++++++----
 kernel/time/sched_clock.c |  6 +++---
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h
index cf87a24..7664f68 100644
--- a/include/linux/seqlock.h
+++ b/include/linux/seqlock.h

@@ -206,14 +206,26 @@ static inline int read_seqcount_retry(const seqcount_t *s, unsigned start)
 }
 
 
+
+static inline void write_seqcount_begin_no_lockdep(seqcount_t *s)
+{
+	s->sequence++;
+	smp_wmb();
+}
+
+static inline void write_seqcount_end_no_lockdep(seqcount_t *s)
+{
+	smp_wmb();
+	s->sequence++;
+}
+
 /*
  * Sequence counter only version assumes that callers are using their
  * own mutexing.
  */
 static inline void write_seqcount_begin_nested(seqcount_t *s, int subclass)
 {
-	s->sequence++;
-	smp_wmb();
+	write_seqcount_begin_no_lockdep(s);
 	seqcount_acquire(&s->dep_map, subclass, 0, _RET_IP_);
 }
 

@@ -225,8 +237,7 @@ static inline void write_seqcount_begin(seqcount_t *s)
 static inline void write_seqcount_end(seqcount_t *s)
 {
 	seqcount_release(&s->dep_map, 1, _RET_IP_);
-	smp_wmb();
-	s->sequence++;
+	write_seqcount_end_no_lockdep(s);
 }
 
 /**

diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index 68b7993..13561a0 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c

@@ -74,7 +74,7 @@ unsigned long long notrace sched_clock(void)
 		return cd.epoch_ns;
 
 	do {
-		seq = read_seqcount_begin(&cd.seq);
+		seq = read_seqcount_begin_no_lockdep(&cd.seq);
 		epoch_cyc = cd.epoch_cyc;
 		epoch_ns = cd.epoch_ns;
 	} while (read_seqcount_retry(&cd.seq, seq));

@@ -99,10 +99,10 @@ static void notrace update_sched_clock(void)
 			  cd.mult, cd.shift);
 
 	raw_local_irq_save(flags);
-	write_seqcount_begin(&cd.seq);
+	write_seqcount_begin_no_lockdep(&cd.seq);
 	cd.epoch_ns = ns;
 	cd.epoch_cyc = cyc;
-	write_seqcount_end(&cd.seq);
+	write_seqcount_end_no_lockdep(&cd.seq);
 	raw_local_irq_restore(flags);
 }
 

-- 
1.8.3.2

On Thu, Jan 2, 2014 at 1:54 PM, John Stultz [off-list ref] wrote:

Unforunately the seqlock lockdep enablmenet can't be used
in sched_clock, since the lockdep infrastructure eventually
calls into sched_clock, which causes a deadlock.

Thus, this patch adds _no_lockdep() seqlock methods for the
writer side, and changes all generic sched_clock usage to use
the _no_lockdep methods.

Ugh.

On the x86 vclock_gettime() side, we only do this for the reader. Why
did you make the generic version do it for the writer too, adding the
necessity for those new operations? It's only the reader side that
doesn't want it.

Talking about the new operations, that "*_no_lockdep()" naming annoys
me. It doesn't match the spinlock naming, which is to just use
"raw_*()" instead. Wouldn't it be nice to make the naming be
consistent too? Especially when it's paired with raw_local_irq_save()
that shares that "raw_" model for non-checking stuff.

             Linus

On 01/02/2014 02:15 PM, Linus Torvalds wrote:

On Thu, Jan 2, 2014 at 1:54 PM, John Stultz [off-list ref] wrote:

quoted

Unforunately the seqlock lockdep enablmenet can't be used
in sched_clock, since the lockdep infrastructure eventually
calls into sched_clock, which causes a deadlock.

Thus, this patch adds _no_lockdep() seqlock methods for the
writer side, and changes all generic sched_clock usage to use
the _no_lockdep methods.

Ugh.

On the x86 vclock_gettime() side, we only do this for the reader. Why
did you make the generic version do it for the writer too, adding the
necessity for those new operations? It's only the reader side that
doesn't want it.

So the problem is that the update side calls the lockdep code which
calls sched_clock, which then deadlocks because the seqcount is odd
(held by the updater).

Thus we have to drop the lockdep usage in the updater as well.

On x86 vclock_gettime, we're in userspace, and that's why we can't call
the lockdep code. The update for that code however happens in kernel
space, so it doesn't have the same problem.

Talking about the new operations, that "*_no_lockdep()" naming annoys
me. It doesn't match the spinlock naming, which is to just use
"raw_*()" instead. Wouldn't it be nice to make the naming be
consistent too? Especially when it's paired with raw_local_irq_save()
that shares that "raw_" model for non-checking stuff.

Sure, I can change the naming. New patch to follow in a bit.

thanks
-john

Linus disliked the _no_lockdep() naming, so instead
use the more-consistent raw_* prefix to the non-lockdep
enabled seqcount methods.

This also adds raw_ methods for the write operations
as well, which will be utilized in a following patch.

Cc: Krzysztof Ha?asa <khalasa@piap.pl>
Cc: Uwe Kleine-K?nig <redacted>
Cc: Willy Tarreau <w@1wt.eu>
Cc: Ingo Molnar <mingo@kernel.org>,
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephen Boyd <redacted>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-arm-kernel at lists.infradead.org
Signed-off-by: John Stultz <redacted>
---
 arch/x86/vdso/vclock_gettime.c |  8 ++++----
 include/linux/seqlock.h        | 27 +++++++++++++++++++--------
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index 2ada505..eb5d7a5 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c

@@ -178,7 +178,7 @@ notrace static int __always_inline do_realtime(struct timespec *ts)
 
 	ts->tv_nsec = 0;
 	do {
-		seq = read_seqcount_begin_no_lockdep(&gtod->seq);
+		seq = raw_read_seqcount_begin(&gtod->seq);
 		mode = gtod->clock.vclock_mode;
 		ts->tv_sec = gtod->wall_time_sec;
 		ns = gtod->wall_time_snsec;

@@ -198,7 +198,7 @@ notrace static int do_monotonic(struct timespec *ts)
 
 	ts->tv_nsec = 0;
 	do {
-		seq = read_seqcount_begin_no_lockdep(&gtod->seq);
+		seq = raw_read_seqcount_begin(&gtod->seq);
 		mode = gtod->clock.vclock_mode;
 		ts->tv_sec = gtod->monotonic_time_sec;
 		ns = gtod->monotonic_time_snsec;

@@ -214,7 +214,7 @@ notrace static int do_realtime_coarse(struct timespec *ts)
 {
 	unsigned long seq;
 	do {
-		seq = read_seqcount_begin_no_lockdep(&gtod->seq);
+		seq = raw_read_seqcount_begin(&gtod->seq);
 		ts->tv_sec = gtod->wall_time_coarse.tv_sec;
 		ts->tv_nsec = gtod->wall_time_coarse.tv_nsec;
 	} while (unlikely(read_seqcount_retry(&gtod->seq, seq)));

@@ -225,7 +225,7 @@ notrace static int do_monotonic_coarse(struct timespec *ts)
 {
 	unsigned long seq;
 	do {
-		seq = read_seqcount_begin_no_lockdep(&gtod->seq);
+		seq = raw_read_seqcount_begin(&gtod->seq);
 		ts->tv_sec = gtod->monotonic_time_coarse.tv_sec;
 		ts->tv_nsec = gtod->monotonic_time_coarse.tv_nsec;
 	} while (unlikely(read_seqcount_retry(&gtod->seq, seq)));

diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h
index cf87a24..535f158 100644
--- a/include/linux/seqlock.h
+++ b/include/linux/seqlock.h

@@ -117,15 +117,15 @@ repeat:
 }
 
 /**
- * read_seqcount_begin_no_lockdep - start seq-read critical section w/o lockdep
+ * raw_read_seqcount_begin - start seq-read critical section w/o lockdep
  * @s: pointer to seqcount_t
  * Returns: count to be passed to read_seqcount_retry
  *
- * read_seqcount_begin_no_lockdep opens a read critical section of the given
+ * raw_read_seqcount_begin opens a read critical section of the given
  * seqcount, but without any lockdep checking. Validity of the critical
  * section is tested by checking read_seqcount_retry function.
  */
-static inline unsigned read_seqcount_begin_no_lockdep(const seqcount_t *s)
+static inline unsigned raw_read_seqcount_begin(const seqcount_t *s)
 {
 	unsigned ret = __read_seqcount_begin(s);
 	smp_rmb();

@@ -144,7 +144,7 @@ static inline unsigned read_seqcount_begin_no_lockdep(const seqcount_t *s)
 static inline unsigned read_seqcount_begin(const seqcount_t *s)
 {
 	seqcount_lockdep_reader_access(s);
-	return read_seqcount_begin_no_lockdep(s);
+	return raw_read_seqcount_begin(s);
 }
 
 /**

@@ -206,14 +206,26 @@ static inline int read_seqcount_retry(const seqcount_t *s, unsigned start)
 }
 
 
+
+static inline void raw_write_seqcount_begin(seqcount_t *s)
+{
+	s->sequence++;
+	smp_wmb();
+}
+
+static inline void raw_write_seqcount_end(seqcount_t *s)
+{
+	smp_wmb();
+	s->sequence++;
+}
+
 /*
  * Sequence counter only version assumes that callers are using their
  * own mutexing.
  */
 static inline void write_seqcount_begin_nested(seqcount_t *s, int subclass)
 {
-	s->sequence++;
-	smp_wmb();
+	raw_write_seqcount_begin(s);
 	seqcount_acquire(&s->dep_map, subclass, 0, _RET_IP_);
 }
 

@@ -225,8 +237,7 @@ static inline void write_seqcount_begin(seqcount_t *s)
 static inline void write_seqcount_end(seqcount_t *s)
 {
 	seqcount_release(&s->dep_map, 1, _RET_IP_);
-	smp_wmb();
-	s->sequence++;
+	raw_write_seqcount_end(s);
 }
 
 /**

-- 
1.8.3.2

Unforunately the seqlock lockdep enablmenet can't be used
in sched_clock, since the lockdep infrastructure eventually
calls into sched_clock, which causes a deadlock.

Thus, this patch changes all generic sched_clock usage
to use the raw_* methods.

Cc: Krzysztof Ha?asa <khalasa@piap.pl>
Cc: Uwe Kleine-K?nig <redacted>
Cc: Willy Tarreau <w@1wt.eu>
Cc: Ingo Molnar <mingo@kernel.org>,
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephen Boyd <redacted>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-arm-kernel at lists.infradead.org
Reported-by: Krzysztof Ha?asa <khalasa@piap.pl>
Signed-off-by: John Stultz <redacted>
---
 kernel/time/sched_clock.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index 68b7993..0abb364 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c

@@ -74,7 +74,7 @@ unsigned long long notrace sched_clock(void)
 		return cd.epoch_ns;
 
 	do {
-		seq = read_seqcount_begin(&cd.seq);
+		seq = raw_read_seqcount_begin(&cd.seq);
 		epoch_cyc = cd.epoch_cyc;
 		epoch_ns = cd.epoch_ns;
 	} while (read_seqcount_retry(&cd.seq, seq));

@@ -99,10 +99,10 @@ static void notrace update_sched_clock(void)
 			  cd.mult, cd.shift);
 
 	raw_local_irq_save(flags);
-	write_seqcount_begin(&cd.seq);
+	raw_write_seqcount_begin(&cd.seq);
 	cd.epoch_ns = ns;
 	cd.epoch_cyc = cyc;
-	write_seqcount_end(&cd.seq);
+	raw_write_seqcount_end(&cd.seq);
 	raw_local_irq_restore(flags);
 }
 

-- 
1.8.3.2

On 01/02/14 15:11, John Stultz wrote:

Linus disliked the _no_lockdep() naming, so instead
use the more-consistent raw_* prefix to the non-lockdep
enabled seqcount methods.

This also adds raw_ methods for the write operations
as well, which will be utilized in a following patch.

Cc: Krzysztof Ha?asa <khalasa@piap.pl>
Cc: Uwe Kleine-K?nig <redacted>
Cc: Willy Tarreau <w@1wt.eu>
Cc: Ingo Molnar <mingo@kernel.org>,
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephen Boyd <redacted>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-arm-kernel at lists.infradead.org
Signed-off-by: John Stultz <redacted>

Reviewed-by: Stephen Boyd <redacted>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

On 01/02/14 15:11, John Stultz wrote:

Unforunately the seqlock lockdep enablmenet can't be used

s/Unforunately/Unfortunately/
s/enablmenet/enablement/

in sched_clock, since the lockdep infrastructure eventually
calls into sched_clock, which causes a deadlock.

Thus, this patch changes all generic sched_clock usage
to use the raw_* methods.

Cc: Krzysztof Ha?asa <khalasa@piap.pl>
Cc: Uwe Kleine-K?nig <redacted>
Cc: Willy Tarreau <w@1wt.eu>
Cc: Ingo Molnar <mingo@kernel.org>,
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephen Boyd <redacted>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-arm-kernel at lists.infradead.org
Reported-by: Krzysztof Ha?asa <khalasa@piap.pl>
Signed-off-by: John Stultz <redacted>

Reviewed-by: Stephen Boyd <redacted>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

On Thu, Jan 2, 2014 at 3:11 PM, John Stultz [off-list ref] wrote:

Linus disliked the _no_lockdep() naming, so instead
use the more-consistent raw_* prefix to the non-lockdep
enabled seqcount methods.

This also adds raw_ methods for the write operations
as well, which will be utilized in a following patch.

Ack on this and on 2/2. I'm assuming I'll get them through the -tip
tree, which is where the problem came from. No?

               Linus

On 01/02/2014 04:50 PM, Linus Torvalds wrote:

On Thu, Jan 2, 2014 at 3:11 PM, John Stultz [off-list ref] wrote:

quoted

Linus disliked the _no_lockdep() naming, so instead
use the more-consistent raw_* prefix to the non-lockdep
enabled seqcount methods.

This also adds raw_ methods for the write operations
as well, which will be utilized in a following patch.

Ack on this and on 2/2. I'm assuming I'll get them through the -tip
tree, which is where the problem came from. No?

It seems Peter or Ingo are still on holiday. Hopefully they will be back
on Monday to queue these.

thanks
-john

On Fri, Jan 03, 2014 at 04:28:31PM -0800, John Stultz wrote:

On 01/02/2014 04:50 PM, Linus Torvalds wrote:

quoted

On Thu, Jan 2, 2014 at 3:11 PM, John Stultz [off-list ref] wrote:

quoted

Linus disliked the _no_lockdep() naming, so instead
use the more-consistent raw_* prefix to the non-lockdep
enabled seqcount methods.

This also adds raw_ methods for the write operations
as well, which will be utilized in a following patch.

Ack on this and on 2/2. I'm assuming I'll get them through the -tip
tree, which is where the problem came from. No?

It seems Peter or Ingo are still on holiday. Hopefully they will be back
on Monday to queue these.

Done! thanks!