Skip to content

Commit

Permalink
timekeeping: Fix HRTICK related deadlock from ntp lock changes
Browse files Browse the repository at this point in the history
Gerlando Falauto reported that when HRTICK is enabled, it is
possible to trigger system deadlocks. These were hard to
reproduce, as HRTICK has been broken in the past, but seemed
to be connected to the timekeeping_seq lock.

Since seqlock/seqcount's aren't supported w/ lockdep, I added
some extra spinlock based locking and triggered the following
lockdep output:

[   15.849182] ntpd/4062 is trying to acquire lock:
[   15.849765]  (&(&pool->lock)->rlock){..-...}, at: [<ffffffff810aa9b5>] __queue_work+0x145/0x480
[   15.850051]
[   15.850051] but task is already holding lock:
[   15.850051]  (timekeeper_lock){-.-.-.}, at: [<ffffffff810df6df>] do_adjtimex+0x7f/0x100

<snip>

[   15.850051] Chain exists of: &(&pool->lock)->rlock --> &p->pi_lock --> timekeeper_lock
[   15.850051]  Possible unsafe locking scenario:
[   15.850051]
[   15.850051]        CPU0                    CPU1
[   15.850051]        ----                    ----
[   15.850051]   lock(timekeeper_lock);
[   15.850051]                                lock(&p->pi_lock);
[   15.850051] lock(timekeeper_lock);
[   15.850051] lock(&(&pool->lock)->rlock);
[   15.850051]
[   15.850051]  *** DEADLOCK ***

The deadlock was introduced by 06c017f ("timekeeping:
Hold timekeepering locks in do_adjtimex and hardpps") in 3.10

This patch avoids this deadlock, by moving the call to
schedule_delayed_work() outside of the timekeeper lock
critical section.

Reported-by: Gerlando Falauto <[email protected]>
Tested-by: Lin Ming <[email protected]>
Signed-off-by: John Stultz <[email protected]>
Cc: Mathieu Desnoyers <[email protected]>
Cc: stable <[email protected]> #3.11, 3.10
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
  • Loading branch information
johnstultz-work authored and Ingo Molnar committed Sep 12, 2013
1 parent 5a8e01f commit 7bd3601
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 4 deletions.
1 change: 1 addition & 0 deletions include/linux/timex.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ extern int do_adjtimex(struct timex *);
extern void hardpps(const struct timespec *, const struct timespec *);

int read_current_timer(unsigned long *timer_val);
void ntp_notify_cmos_timer(void);

/* The clock frequency of the i8253/i8254 PIT */
#define PIT_TICK_RATE 1193182ul
Expand Down
6 changes: 2 additions & 4 deletions kernel/time/ntp.c
Original file line number Diff line number Diff line change
Expand Up @@ -516,13 +516,13 @@ static void sync_cmos_clock(struct work_struct *work)
schedule_delayed_work(&sync_cmos_work, timespec_to_jiffies(&next));
}

static void notify_cmos_timer(void)
void ntp_notify_cmos_timer(void)
{
schedule_delayed_work(&sync_cmos_work, 0);
}

#else
static inline void notify_cmos_timer(void) { }
void ntp_notify_cmos_timer(void) { }
#endif


Expand Down Expand Up @@ -687,8 +687,6 @@ int __do_adjtimex(struct timex *txc, struct timespec *ts, s32 *time_tai)
if (!(time_status & STA_NANO))
txc->time.tv_usec /= NSEC_PER_USEC;

notify_cmos_timer();

return result;
}

Expand Down
2 changes: 2 additions & 0 deletions kernel/time/timekeeping.c
Original file line number Diff line number Diff line change
Expand Up @@ -1703,6 +1703,8 @@ int do_adjtimex(struct timex *txc)
write_seqcount_end(&timekeeper_seq);
raw_spin_unlock_irqrestore(&timekeeper_lock, flags);

ntp_notify_cmos_timer();

return ret;
}

Expand Down

0 comments on commit 7bd3601

Please sign in to comment.