History

timekeeping: Fix HRTICK related deadlock from ntp lock changes

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 06c017fdd4dc48451a ("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.

John Stultz committed 13 years ago

Commit 7bd36011 parent 5a8e01f3 files · +5 −4Browse files

Showing the changes to kernel (2 of 3 files). Show all

Unified · Split

2 files
  1. kernel/time/ntp.c +2 −4
  2. kernel/time/timekeeping.c +2 −0
kernel/time/ntp.c+2−4
@@ -516,13 +516,13 @@ static void sync_cmos_clock(struct work_struct *work) · 515 unmodified lines
516 schedule_delayed_work(&sync_cmos_work, timespec_to_jiffies(&next));516 schedule_delayed_work(&sync_cmos_work, timespec_to_jiffies(&next));
517}517}
518 518
519static void notify_cmos_timer(void)519void ntp_notify_cmos_timer(void)
520{520{
521 schedule_delayed_work(&sync_cmos_work, 0);521 schedule_delayed_work(&sync_cmos_work, 0);
522}522}
523 523
524#else524#else
525static inline void notify_cmos_timer(void) { }525void ntp_notify_cmos_timer(void) { }
526#endif526#endif
527 527
528 528
@@ -687,8 +687,6 @@ int __do_adjtimex(struct timex *txc, struct timespec *ts, s32 *time_tai) · 158 unmodified lines
687 if (!(time_status & STA_NANO))687 if (!(time_status & STA_NANO))
688 txc->time.tv_usec /= NSEC_PER_USEC;688 txc->time.tv_usec /= NSEC_PER_USEC;
689 689
690 notify_cmos_timer();
691
692 return result;690 return result;
693}691}
694 692
kernel/time/timekeeping.c+2−0
@@ -1703,6 +1703,8 @@ int do_adjtimex(struct timex *txc) · 1702 unmodified lines
1703 write_seqcount_end(&timekeeper_seq);1703 write_seqcount_end(&timekeeper_seq);
1704 raw_spin_unlock_irqrestore(&timekeeper_lock, flags);1704 raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
1705 1705
1706 ntp_notify_cmos_timer();
1707
1706 return ret;1708 return ret;
1707}1709}
1708 1710