[tip:timers/core] timer stats: Add a 'Collection: active/inactive ' line to timer usage statistics
Commit-ID: 2cb763614c1c5baef58045af9304265075f22d0a Gitweb: http://git.kernel.org/tip/2cb763614c1c5baef58045af9304265075f22d0a Author: Dong Zhu AuthorDate: Thu, 10 Oct 2013 15:56:18 +0800 Committer: Ingo Molnar CommitDate: Thu, 10 Oct 2013 09:59:25 +0200 timer stats: Add a 'Collection: active/inactive' line to timer usage statistics We can enable/disable timer statistics collection via: echo [1|0] > /proc/timers_stats and it would be nice if apps had the ability to check what the current collection status is. This patch adds a 'Collection: active/inactive' line to display the current timer collection status. Also bump up the timer stats version to v0.3. Signed-off-by: Dong Zhu Cc: John Stultz Link: http://lkml.kernel.org/r/20131010075618.gh2...@zhudong.nay.redhat.com [ Improved the changelog and the code. ] Signed-off-by: Ingo Molnar --- kernel/time/timer_stats.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c index 0b537f2..1fb08f2 100644 --- a/kernel/time/timer_stats.c +++ b/kernel/time/timer_stats.c @@ -298,15 +298,15 @@ static int tstats_show(struct seq_file *m, void *v) period = ktime_to_timespec(time); ms = period.tv_nsec / 100; - seq_puts(m, "Timer Stats Version: v0.2\n"); + seq_puts(m, "Timer Stats Version: v0.3\n"); seq_printf(m, "Sample period: %ld.%03ld s\n", period.tv_sec, ms); if (atomic_read(_count)) - seq_printf(m, "Overflow: %d entries\n", - atomic_read(_count)); + seq_printf(m, "Overflow: %d entries\n", atomic_read(_count)); + seq_printf(m, "Collection: %s\n", timer_stats_active ? "active" : "inactive"); for (i = 0; i < nr_entries; i++) { entry = entries + i; - if (entry->timer_flag & TIMER_STATS_FLAG_DEFERRABLE) { + if (entry->timer_flag & TIMER_STATS_FLAG_DEFERRABLE) { seq_printf(m, "%4luD, %5d %-16s ", entry->count, entry->pid, entry->comm); } else { -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4] timer stats: add a 'status' line to timer usage statistics
> > > I suspect we could do something like: > > > > > > seq_printf("Status: collection %s\n", timer_stats_active ? "enabled" : > > > "disabled"); > > > > > > and save a bit of kernel image size? > > > > > > Also, please bump up the version to v0.3, to give parsers a chance. > > I think you forgot about this suggestion of mine. > Oh, oops, sorry about that. I modified the patch again as below, hope it could be applied this time. >From 92a67f74766e02b5f4702b1af2af9861bf70d60b Mon Sep 17 00:00:00 2001 From: Dong Zhu Date: Thu, 10 Oct 2013 15:48:29 +0800 We can enable|disable timer statistics collection (via echo [1|0] > /proc/timers_stats), this patch adds a 'status' line to display the current timer collection status. In the meantime I also bump up the timer stats version to v0.3. Signed-off-by: Dong Zhu --- kernel/time/timer_stats.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c index 0b537f2..7c31ab7 100644 --- a/kernel/time/timer_stats.c +++ b/kernel/time/timer_stats.c @@ -298,11 +298,13 @@ static int tstats_show(struct seq_file *m, void *v) period = ktime_to_timespec(time); ms = period.tv_nsec / 100; - seq_puts(m, "Timer Stats Version: v0.2\n"); + seq_puts(m, "Timer Stats Version: v0.3\n"); seq_printf(m, "Sample period: %ld.%03ld s\n", period.tv_sec, ms); if (atomic_read(_count)) seq_printf(m, "Overflow: %d entries\n", atomic_read(_count)); + seq_printf(m, "Status: collection %s\n", + timer_stats_active ? "enabled" : "disabled"); for (i = 0; i < nr_entries; i++) { entry = entries + i; -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v0.3] timer stats: add a 'status' line to timer usage statistics
> > Agree, I modified this patch and resubmited it again, Could you help > > reviewing it again ? Thanks ! > > > > From 263c40abea8011c82582b2d671ae783b26f44bd5 Mon Sep 17 00:00:00 2001 > > From: Dong Zhu > > Date: Thu, 10 Oct 2013 13:46:08 +0800 > > > > We can enable|disable timer statistics collection (via echo [1|0] > > > /proc/timers_stats), this patch adds a 'status' line to display the > > current timer collection status. > > > > Signed-off-by: Dong Zhu > > --- > > kernel/time/timer_stats.c | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c > > index 0b537f2..bac5e91 100644 > > --- a/kernel/time/timer_stats.c > > +++ b/kernel/time/timer_stats.c > > @@ -303,6 +303,10 @@ static int tstats_show(struct seq_file *m, void *v) > > if (atomic_read(_count)) > > seq_printf(m, "Overflow: %d entries\n", > > atomic_read(_count)); > > + if (timer_stats_active) > > + seq_puts(m, "Status: collection active\n"); > > + else > > + seq_puts(m, "Status: collection disabled\n"); > > I suspect we could do something like: > > seq_printf("Status: collection %s\n", timer_stats_active ? "enabled" : > "disabled"); > > and save a bit of kernel image size? > > Also, please bump up the version to v0.3, to give parsers a chance. > > Otherwise it looks good to me. > Thanks for your comments, I refined this patch as below: >From 1035e4e2b7ff28b1b2fccd407929179b5de8fbd4 Mon Sep 17 00:00:00 2001 From: Dong Zhu Date: Thu, 10 Oct 2013 14:48:24 +0800 We can enable|disable timer statistics collection (via echo [1|0] > /proc/timers_stats), this patch adds a 'status' line to display the current timer collection status. Signed-off-by: Dong Zhu --- kernel/time/timer_stats.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c index 0b537f2..5236b45 100644 --- a/kernel/time/timer_stats.c +++ b/kernel/time/timer_stats.c @@ -303,6 +303,8 @@ static int tstats_show(struct seq_file *m, void *v) if (atomic_read(_count)) seq_printf(m, "Overflow: %d entries\n", atomic_read(_count)); + seq_printf(m, "Status: collection %s\n", + timer_stats_active ? "enabled" : "disabled"); for (i = 0; i < nr_entries; i++) { entry = entries + i; -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] timer stats: add a 'status' line to timer usage statistics
Hi Ingo and John, On Thu, Oct 10, 2013 at 06:35:19AM +0200, Ingo Molnar wrote: > > * Dong Zhu wrote: > > > From f41628c61d8a9172677ba33a55b61e37ce28f7a6 Mon Sep 17 00:00:00 2001 > > From: Dong Zhu > > Date: Thu, 10 Oct 2013 10:38:13 +0800 > > > > When we stop timer statistics collection (via echo 0 > > > /proc/timers_stats), the statistics data is still exported as if it were > > correct, which can cause applicaitons to misuse the statistics. > > What misuse do you mean? > > > This patch resets the statistics when we stop collecting them, to avoid > > this problem. > > Well, this loses the handy 'snapshot' property of /proc/timer_stats. > Before this change one could do: > > echo 1 > /proc/timers_stats > sleep 60 # run system workload > echo 0 > /proc/timers_stats > > and examine the 1-minute collection result without it changing. Your > change, if I understand it correctly, zeroes it all out. Yes, I am wrong about it. Thanks for pointing this out and pretty sorry for confusing you John. > > Instead of this change I'd suggest adding a 'status' line, with two > outputs: > > Status: collection active > > Status: collection disabled > Agree, I modified this patch and resubmited it again, Could you help reviewing it again ? Thanks ! >From 263c40abea8011c82582b2d671ae783b26f44bd5 Mon Sep 17 00:00:00 2001 From: Dong Zhu Date: Thu, 10 Oct 2013 13:46:08 +0800 We can enable|disable timer statistics collection (via echo [1|0] > /proc/timers_stats), this patch adds a 'status' line to display the current timer collection status. Signed-off-by: Dong Zhu --- kernel/time/timer_stats.c | 4 1 file changed, 4 insertions(+) diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c index 0b537f2..bac5e91 100644 --- a/kernel/time/timer_stats.c +++ b/kernel/time/timer_stats.c @@ -303,6 +303,10 @@ static int tstats_show(struct seq_file *m, void *v) if (atomic_read(_count)) seq_printf(m, "Overflow: %d entries\n", atomic_read(_count)); + if (timer_stats_active) + seq_puts(m, "Status: collection active\n"); + else + seq_puts(m, "Status: collection disabled\n"); for (i = 0; i < nr_entries; i++) { entry = entries + i; -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] timer stats: add a 'status' line to timer usage statistics
Hi Ingo and John, On Thu, Oct 10, 2013 at 06:35:19AM +0200, Ingo Molnar wrote: * Dong Zhu bluezhud...@gmail.com wrote: From f41628c61d8a9172677ba33a55b61e37ce28f7a6 Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Thu, 10 Oct 2013 10:38:13 +0800 When we stop timer statistics collection (via echo 0 /proc/timers_stats), the statistics data is still exported as if it were correct, which can cause applicaitons to misuse the statistics. What misuse do you mean? This patch resets the statistics when we stop collecting them, to avoid this problem. Well, this loses the handy 'snapshot' property of /proc/timer_stats. Before this change one could do: echo 1 /proc/timers_stats sleep 60 # run system workload echo 0 /proc/timers_stats and examine the 1-minute collection result without it changing. Your change, if I understand it correctly, zeroes it all out. Yes, I am wrong about it. Thanks for pointing this out and pretty sorry for confusing you John. Instead of this change I'd suggest adding a 'status' line, with two outputs: Status: collection active Status: collection disabled Agree, I modified this patch and resubmited it again, Could you help reviewing it again ? Thanks ! From 263c40abea8011c82582b2d671ae783b26f44bd5 Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Thu, 10 Oct 2013 13:46:08 +0800 We can enable|disable timer statistics collection (via echo [1|0] /proc/timers_stats), this patch adds a 'status' line to display the current timer collection status. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- kernel/time/timer_stats.c | 4 1 file changed, 4 insertions(+) diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c index 0b537f2..bac5e91 100644 --- a/kernel/time/timer_stats.c +++ b/kernel/time/timer_stats.c @@ -303,6 +303,10 @@ static int tstats_show(struct seq_file *m, void *v) if (atomic_read(overflow_count)) seq_printf(m, Overflow: %d entries\n, atomic_read(overflow_count)); + if (timer_stats_active) + seq_puts(m, Status: collection active\n); + else + seq_puts(m, Status: collection disabled\n); for (i = 0; i nr_entries; i++) { entry = entries + i; -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v0.3] timer stats: add a 'status' line to timer usage statistics
Agree, I modified this patch and resubmited it again, Could you help reviewing it again ? Thanks ! From 263c40abea8011c82582b2d671ae783b26f44bd5 Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Thu, 10 Oct 2013 13:46:08 +0800 We can enable|disable timer statistics collection (via echo [1|0] /proc/timers_stats), this patch adds a 'status' line to display the current timer collection status. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- kernel/time/timer_stats.c | 4 1 file changed, 4 insertions(+) diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c index 0b537f2..bac5e91 100644 --- a/kernel/time/timer_stats.c +++ b/kernel/time/timer_stats.c @@ -303,6 +303,10 @@ static int tstats_show(struct seq_file *m, void *v) if (atomic_read(overflow_count)) seq_printf(m, Overflow: %d entries\n, atomic_read(overflow_count)); + if (timer_stats_active) + seq_puts(m, Status: collection active\n); + else + seq_puts(m, Status: collection disabled\n); I suspect we could do something like: seq_printf(Status: collection %s\n, timer_stats_active ? enabled : disabled); and save a bit of kernel image size? Also, please bump up the version to v0.3, to give parsers a chance. Otherwise it looks good to me. Thanks for your comments, I refined this patch as below: From 1035e4e2b7ff28b1b2fccd407929179b5de8fbd4 Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Thu, 10 Oct 2013 14:48:24 +0800 We can enable|disable timer statistics collection (via echo [1|0] /proc/timers_stats), this patch adds a 'status' line to display the current timer collection status. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- kernel/time/timer_stats.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c index 0b537f2..5236b45 100644 --- a/kernel/time/timer_stats.c +++ b/kernel/time/timer_stats.c @@ -303,6 +303,8 @@ static int tstats_show(struct seq_file *m, void *v) if (atomic_read(overflow_count)) seq_printf(m, Overflow: %d entries\n, atomic_read(overflow_count)); + seq_printf(m, Status: collection %s\n, + timer_stats_active ? enabled : disabled); for (i = 0; i nr_entries; i++) { entry = entries + i; -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4] timer stats: add a 'status' line to timer usage statistics
I suspect we could do something like: seq_printf(Status: collection %s\n, timer_stats_active ? enabled : disabled); and save a bit of kernel image size? Also, please bump up the version to v0.3, to give parsers a chance. I think you forgot about this suggestion of mine. Oh, oops, sorry about that. I modified the patch again as below, hope it could be applied this time. From 92a67f74766e02b5f4702b1af2af9861bf70d60b Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Thu, 10 Oct 2013 15:48:29 +0800 We can enable|disable timer statistics collection (via echo [1|0] /proc/timers_stats), this patch adds a 'status' line to display the current timer collection status. In the meantime I also bump up the timer stats version to v0.3. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- kernel/time/timer_stats.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c index 0b537f2..7c31ab7 100644 --- a/kernel/time/timer_stats.c +++ b/kernel/time/timer_stats.c @@ -298,11 +298,13 @@ static int tstats_show(struct seq_file *m, void *v) period = ktime_to_timespec(time); ms = period.tv_nsec / 100; - seq_puts(m, Timer Stats Version: v0.2\n); + seq_puts(m, Timer Stats Version: v0.3\n); seq_printf(m, Sample period: %ld.%03ld s\n, period.tv_sec, ms); if (atomic_read(overflow_count)) seq_printf(m, Overflow: %d entries\n, atomic_read(overflow_count)); + seq_printf(m, Status: collection %s\n, + timer_stats_active ? enabled : disabled); for (i = 0; i nr_entries; i++) { entry = entries + i; -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[tip:timers/core] timer stats: Add a 'Collection: active/inactive ' line to timer usage statistics
Commit-ID: 2cb763614c1c5baef58045af9304265075f22d0a Gitweb: http://git.kernel.org/tip/2cb763614c1c5baef58045af9304265075f22d0a Author: Dong Zhu bluezhud...@gmail.com AuthorDate: Thu, 10 Oct 2013 15:56:18 +0800 Committer: Ingo Molnar mi...@kernel.org CommitDate: Thu, 10 Oct 2013 09:59:25 +0200 timer stats: Add a 'Collection: active/inactive' line to timer usage statistics We can enable/disable timer statistics collection via: echo [1|0] /proc/timers_stats and it would be nice if apps had the ability to check what the current collection status is. This patch adds a 'Collection: active/inactive' line to display the current timer collection status. Also bump up the timer stats version to v0.3. Signed-off-by: Dong Zhu bluezhud...@gmail.com Cc: John Stultz john.stu...@linaro.org Link: http://lkml.kernel.org/r/20131010075618.gh2...@zhudong.nay.redhat.com [ Improved the changelog and the code. ] Signed-off-by: Ingo Molnar mi...@kernel.org --- kernel/time/timer_stats.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c index 0b537f2..1fb08f2 100644 --- a/kernel/time/timer_stats.c +++ b/kernel/time/timer_stats.c @@ -298,15 +298,15 @@ static int tstats_show(struct seq_file *m, void *v) period = ktime_to_timespec(time); ms = period.tv_nsec / 100; - seq_puts(m, Timer Stats Version: v0.2\n); + seq_puts(m, Timer Stats Version: v0.3\n); seq_printf(m, Sample period: %ld.%03ld s\n, period.tv_sec, ms); if (atomic_read(overflow_count)) - seq_printf(m, Overflow: %d entries\n, - atomic_read(overflow_count)); + seq_printf(m, Overflow: %d entries\n, atomic_read(overflow_count)); + seq_printf(m, Collection: %s\n, timer_stats_active ? active : inactive); for (i = 0; i nr_entries; i++) { entry = entries + i; - if (entry-timer_flag TIMER_STATS_FLAG_DEFERRABLE) { + if (entry-timer_flag TIMER_STATS_FLAG_DEFERRABLE) { seq_printf(m, %4luD, %5d %-16s , entry-count, entry-pid, entry-comm); } else { -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RESEND] timer stats: reset entries when disable the timer usage statistics
>From f41628c61d8a9172677ba33a55b61e37ce28f7a6 Mon Sep 17 00:00:00 2001 From: Dong Zhu Date: Thu, 10 Oct 2013 10:38:13 +0800 When we stop timer statistics collection (via echo 0 > /proc/timers_stats), the statistics data is still exported as if it were correct, which can cause applicaitons to misuse the statistics. This patch resets the statistics when we stop collecting them, to avoid this problem. Signed-off-by: Dong Zhu --- kernel/time/timer_stats.c | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c index 0b537f2..43f05e7 100644 --- a/kernel/time/timer_stats.c +++ b/kernel/time/timer_stats.c @@ -371,6 +371,7 @@ static ssize_t tstats_write(struct file *file, const char __user *buf, switch (ctl[0]) { case '0': if (timer_stats_active) { + reset_entries(); timer_stats_active = 0; time_stop = ktime_get(); sync_access(); -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RESEND] timer stats: reset entries when disable the timer usage statistics
From f41628c61d8a9172677ba33a55b61e37ce28f7a6 Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Thu, 10 Oct 2013 10:38:13 +0800 When we stop timer statistics collection (via echo 0 /proc/timers_stats), the statistics data is still exported as if it were correct, which can cause applicaitons to misuse the statistics. This patch resets the statistics when we stop collecting them, to avoid this problem. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- kernel/time/timer_stats.c | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c index 0b537f2..43f05e7 100644 --- a/kernel/time/timer_stats.c +++ b/kernel/time/timer_stats.c @@ -371,6 +371,7 @@ static ssize_t tstats_write(struct file *file, const char __user *buf, switch (ctl[0]) { case '0': if (timer_stats_active) { + reset_entries(); timer_stats_active = 0; time_stop = ktime_get(); sync_access(); -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ptp: add range check on n_samples
>From d4eb97e8d5def76d46167c91059147e2c7d33433 Mon Sep 17 00:00:00 2001 When using PTP_SYS_OFFSET ioctl to measure the time offset between the PHC and system clock, we need to specify the number of measurements, the valid value of n_samples is between 1 to 25. If n_samples <= 0 or > 25 it makes no sense, so this patch intends to add a range check. Signed-off-by: Dong Zhu --- drivers/ptp/ptp_chardev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c index 34a0c60..4e85b23 100644 --- a/drivers/ptp/ptp_chardev.c +++ b/drivers/ptp/ptp_chardev.c @@ -104,7 +104,8 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg) err = -EFAULT; break; } - if (sysoff->n_samples > PTP_MAX_SAMPLES) { + if (sysoff->n_samples <= 0 || + sysoff->n_samples > PTP_MAX_SAMPLES) { err = -EINVAL; break; } -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ptp: add range check on n_samples
From d4eb97e8d5def76d46167c91059147e2c7d33433 Mon Sep 17 00:00:00 2001 When using PTP_SYS_OFFSET ioctl to measure the time offset between the PHC and system clock, we need to specify the number of measurements, the valid value of n_samples is between 1 to 25. If n_samples = 0 or 25 it makes no sense, so this patch intends to add a range check. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- drivers/ptp/ptp_chardev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c index 34a0c60..4e85b23 100644 --- a/drivers/ptp/ptp_chardev.c +++ b/drivers/ptp/ptp_chardev.c @@ -104,7 +104,8 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg) err = -EFAULT; break; } - if (sysoff-n_samples PTP_MAX_SAMPLES) { + if (sysoff-n_samples = 0 || + sysoff-n_samples PTP_MAX_SAMPLES) { err = -EINVAL; break; } -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next v4] ptp: add the PTP_SYS_OFFSET ioctl to the testptp program
Hi Richard, I developed a new patch and added a method to estimate the time offset between system and phc clock. Could you help reviewing it again ? If it do make sense I hope it could be accepted. Thanks ! >From e524e3b68f3df3cd91acd814490d092bad05386b Mon Sep 17 00:00:00 2001 This patch add a method into testptp.c to measure the time offset between phc and system clock through the ioctl PTP_SYS_OFFSET. Signed-off-by: Dong Zhu --- Documentation/ptp/testptp.c | 65 +++-- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/Documentation/ptp/testptp.c b/Documentation/ptp/testptp.c index f59ded0..a74d0a8 100644 --- a/Documentation/ptp/testptp.c +++ b/Documentation/ptp/testptp.c @@ -100,6 +100,11 @@ static long ppb_to_scaled_ppm(int ppb) return (long) (ppb * 65.536); } +static int64_t pctns(struct ptp_clock_time *t) +{ + return t->sec * 10LL + t->nsec; +} + static void usage(char *progname) { fprintf(stderr, @@ -112,6 +117,8 @@ static void usage(char *progname) " -f val adjust the ptp clock frequency by 'val' ppb\n" " -g get the ptp clock time\n" " -h prints this message\n" + " -k val measure the time offset between system and phc clock\n" + "for 'val' times (Maximum 25)\n" " -p val enable output with a period of 'val' nanoseconds\n" " -P val enable or disable (val=1|0) the system clock PPS\n" " -s set the ptp clock time from the system time\n" @@ -133,8 +140,12 @@ int main(int argc, char *argv[]) struct itimerspec timeout; struct sigevent sigevent; + struct ptp_clock_time *pct; + struct ptp_sys_offset *sysoff; + + char *progname; - int c, cnt, fd; + int i, c, cnt, fd; char *device = DEVICE; clockid_t clkid; @@ -144,14 +155,19 @@ int main(int argc, char *argv[]) int extts = 0; int gettime = 0; int oneshot = 0; + int pct_offset = 0; + int n_samples = 0; int periodic = 0; int perout = -1; int pps = -1; int settime = 0; + int64_t t1, t2, tp; + int64_t interval, offset; + progname = strrchr(argv[0], '/'); progname = progname ? 1+progname : argv[0]; - while (EOF != (c = getopt(argc, argv, "a:A:cd:e:f:ghp:P:sSt:v"))) { + while (EOF != (c = getopt(argc, argv, "a:A:cd:e:f:ghk:p:P:sSt:v"))) { switch (c) { case 'a': oneshot = atoi(optarg); @@ -174,6 +190,10 @@ int main(int argc, char *argv[]) case 'g': gettime = 1; break; + case 'k': + pct_offset = 1; + n_samples = atoi(optarg); + break; case 'p': perout = atoi(optarg); break; @@ -376,6 +396,47 @@ int main(int argc, char *argv[]) } } + if (pct_offset) { + if (n_samples <= 0 || n_samples > 25) { + puts("n_samples should be between 1 and 25"); + usage(progname); + return -1; + } + + sysoff = calloc(1, sizeof(*sysoff)); + if (!sysoff) { + perror("calloc"); + return -1; + } + sysoff->n_samples = n_samples; + + if (ioctl(fd, PTP_SYS_OFFSET, sysoff)) + perror("PTP_SYS_OFFSET"); + else + puts("system and phc clock time offset request okay"); + + pct = >ts[0]; + for (i = 0; i < sysoff->n_samples; i++) { + t1 = pctns(pct+2*i); + tp = pctns(pct+2*i+1); + t2 = pctns(pct+2*i+2); + interval = t2 - t1; + offset = (t2 + t1) / 2 - tp; + + printf("system time: %ld.%ld\n", + (pct+2*i)->sec, (pct+2*i)->nsec); + printf("phctime: %ld.%ld\n", + (pct+2*i+1)->sec, (pct+2*i+1)->nsec); + printf("system time: %ld.%ld\n", + (pct+2*i+2)->sec, (pct+2*i+2)->nsec); + printf("system/phc clock time offset is %ld ns\n" + "system clock time delay is %ld ns\n", + offset, interval); + }
Re: [PATCH net-next v4] ptp: add the PTP_SYS_OFFSET ioctl to the testptp program
Hi Richard, I developed a new patch and added a method to estimate the time offset between system and phc clock. Could you help reviewing it again ? If it do make sense I hope it could be accepted. Thanks ! From e524e3b68f3df3cd91acd814490d092bad05386b Mon Sep 17 00:00:00 2001 This patch add a method into testptp.c to measure the time offset between phc and system clock through the ioctl PTP_SYS_OFFSET. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- Documentation/ptp/testptp.c | 65 +++-- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/Documentation/ptp/testptp.c b/Documentation/ptp/testptp.c index f59ded0..a74d0a8 100644 --- a/Documentation/ptp/testptp.c +++ b/Documentation/ptp/testptp.c @@ -100,6 +100,11 @@ static long ppb_to_scaled_ppm(int ppb) return (long) (ppb * 65.536); } +static int64_t pctns(struct ptp_clock_time *t) +{ + return t-sec * 10LL + t-nsec; +} + static void usage(char *progname) { fprintf(stderr, @@ -112,6 +117,8 @@ static void usage(char *progname) -f val adjust the ptp clock frequency by 'val' ppb\n -g get the ptp clock time\n -h prints this message\n +-k val measure the time offset between system and phc clock\n + for 'val' times (Maximum 25)\n -p val enable output with a period of 'val' nanoseconds\n -P val enable or disable (val=1|0) the system clock PPS\n -s set the ptp clock time from the system time\n @@ -133,8 +140,12 @@ int main(int argc, char *argv[]) struct itimerspec timeout; struct sigevent sigevent; + struct ptp_clock_time *pct; + struct ptp_sys_offset *sysoff; + + char *progname; - int c, cnt, fd; + int i, c, cnt, fd; char *device = DEVICE; clockid_t clkid; @@ -144,14 +155,19 @@ int main(int argc, char *argv[]) int extts = 0; int gettime = 0; int oneshot = 0; + int pct_offset = 0; + int n_samples = 0; int periodic = 0; int perout = -1; int pps = -1; int settime = 0; + int64_t t1, t2, tp; + int64_t interval, offset; + progname = strrchr(argv[0], '/'); progname = progname ? 1+progname : argv[0]; - while (EOF != (c = getopt(argc, argv, a:A:cd:e:f:ghp:P:sSt:v))) { + while (EOF != (c = getopt(argc, argv, a:A:cd:e:f:ghk:p:P:sSt:v))) { switch (c) { case 'a': oneshot = atoi(optarg); @@ -174,6 +190,10 @@ int main(int argc, char *argv[]) case 'g': gettime = 1; break; + case 'k': + pct_offset = 1; + n_samples = atoi(optarg); + break; case 'p': perout = atoi(optarg); break; @@ -376,6 +396,47 @@ int main(int argc, char *argv[]) } } + if (pct_offset) { + if (n_samples = 0 || n_samples 25) { + puts(n_samples should be between 1 and 25); + usage(progname); + return -1; + } + + sysoff = calloc(1, sizeof(*sysoff)); + if (!sysoff) { + perror(calloc); + return -1; + } + sysoff-n_samples = n_samples; + + if (ioctl(fd, PTP_SYS_OFFSET, sysoff)) + perror(PTP_SYS_OFFSET); + else + puts(system and phc clock time offset request okay); + + pct = sysoff-ts[0]; + for (i = 0; i sysoff-n_samples; i++) { + t1 = pctns(pct+2*i); + tp = pctns(pct+2*i+1); + t2 = pctns(pct+2*i+2); + interval = t2 - t1; + offset = (t2 + t1) / 2 - tp; + + printf(system time: %ld.%ld\n, + (pct+2*i)-sec, (pct+2*i)-nsec); + printf(phctime: %ld.%ld\n, + (pct+2*i+1)-sec, (pct+2*i+1)-nsec); + printf(system time: %ld.%ld\n, + (pct+2*i+2)-sec, (pct+2*i+2)-nsec); + printf(system/phc clock time offset is %ld ns\n + system clock time delay is %ld ns\n, + offset, interval); + } + + free(sysoff); + } + close(fd); return 0; } -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More
Re: [PATCH net-next v3] ptp: add the PTP_SYS_OFFSET ioctl to the testptp program
Hi Richard, Thanks for your comments, I modified the patch and resubmit it again: >From 7636f69b74c34eca14c85fd2d518da6044b94f53 Mon Sep 17 00:00:00 2001 From: Dong Zhu Date: Sun, 15 Sep 2013 17:12:52 +0800 This patch add a method into testptp.c to measure the time offset between phc and system clock through the ioctl PTP_SYS_OFFSET. Signed-off-by: Dong Zhu --- Documentation/ptp/testptp.c | 40 ++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/Documentation/ptp/testptp.c b/Documentation/ptp/testptp.c index f59ded0..b6f6f47 100644 --- a/Documentation/ptp/testptp.c +++ b/Documentation/ptp/testptp.c @@ -112,6 +112,8 @@ static void usage(char *progname) " -f val adjust the ptp clock frequency by 'val' ppb\n" " -g get the ptp clock time\n" " -h prints this message\n" + " -k val measure the time offset between phc and system clock\n" + "for 'val' times (Maximum 25)\n" " -p val enable output with a period of 'val' nanoseconds\n" " -P val enable or disable (val=1|0) the system clock PPS\n" " -s set the ptp clock time from the system time\n" @@ -133,8 +135,12 @@ int main(int argc, char *argv[]) struct itimerspec timeout; struct sigevent sigevent; + struct ptp_clock_time *pct; + struct ptp_sys_offset *sysoff; + + char *progname; - int c, cnt, fd; + int i, c, cnt, fd; char *device = DEVICE; clockid_t clkid; @@ -144,6 +150,8 @@ int main(int argc, char *argv[]) int extts = 0; int gettime = 0; int oneshot = 0; + int offset = 0; + int n_samples = 0; int periodic = 0; int perout = -1; int pps = -1; @@ -151,7 +159,7 @@ int main(int argc, char *argv[]) progname = strrchr(argv[0], '/'); progname = progname ? 1+progname : argv[0]; - while (EOF != (c = getopt(argc, argv, "a:A:cd:e:f:ghp:P:sSt:v"))) { + while (EOF != (c = getopt(argc, argv, "a:A:cd:e:f:ghk:p:P:sSt:v"))) { switch (c) { case 'a': oneshot = atoi(optarg); @@ -174,6 +182,10 @@ int main(int argc, char *argv[]) case 'g': gettime = 1; break; + case 'k': + offset = 1; + n_samples = atoi(optarg); + break; case 'p': perout = atoi(optarg); break; @@ -376,6 +388,30 @@ int main(int argc, char *argv[]) } } + if (offset) { + sysoff = calloc(1, sizeof(*sysoff)); + if (!sysoff) { + perror("calloc"); + return -1; + } + sysoff->n_samples = n_samples; + + if (ioctl(fd, PTP_SYS_OFFSET, sysoff)) + perror("PTP_SYS_OFFSET"); + else + puts("phc and system clock time offset request okay"); + + pct = >ts[0]; + for (i = 0; i < sysoff->n_samples; i++, pct++) { + printf("system time: %ld.%ld\n", pct->sec, pct->nsec); + pct++; + printf("phctime: %ld.%ld\n", pct->sec, pct->nsec); + } + printf("system time: %ld.%ld\n", pct->sec, pct->nsec); + + free(sysoff); + } + close(fd); return 0; } -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next v3] ptp: add the PTP_SYS_OFFSET ioctl to the testptp program
Hi Richard, Thanks for your comments, I modified the patch and resubmit it again: From 7636f69b74c34eca14c85fd2d518da6044b94f53 Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Sun, 15 Sep 2013 17:12:52 +0800 This patch add a method into testptp.c to measure the time offset between phc and system clock through the ioctl PTP_SYS_OFFSET. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- Documentation/ptp/testptp.c | 40 ++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/Documentation/ptp/testptp.c b/Documentation/ptp/testptp.c index f59ded0..b6f6f47 100644 --- a/Documentation/ptp/testptp.c +++ b/Documentation/ptp/testptp.c @@ -112,6 +112,8 @@ static void usage(char *progname) -f val adjust the ptp clock frequency by 'val' ppb\n -g get the ptp clock time\n -h prints this message\n +-k val measure the time offset between phc and system clock\n + for 'val' times (Maximum 25)\n -p val enable output with a period of 'val' nanoseconds\n -P val enable or disable (val=1|0) the system clock PPS\n -s set the ptp clock time from the system time\n @@ -133,8 +135,12 @@ int main(int argc, char *argv[]) struct itimerspec timeout; struct sigevent sigevent; + struct ptp_clock_time *pct; + struct ptp_sys_offset *sysoff; + + char *progname; - int c, cnt, fd; + int i, c, cnt, fd; char *device = DEVICE; clockid_t clkid; @@ -144,6 +150,8 @@ int main(int argc, char *argv[]) int extts = 0; int gettime = 0; int oneshot = 0; + int offset = 0; + int n_samples = 0; int periodic = 0; int perout = -1; int pps = -1; @@ -151,7 +159,7 @@ int main(int argc, char *argv[]) progname = strrchr(argv[0], '/'); progname = progname ? 1+progname : argv[0]; - while (EOF != (c = getopt(argc, argv, a:A:cd:e:f:ghp:P:sSt:v))) { + while (EOF != (c = getopt(argc, argv, a:A:cd:e:f:ghk:p:P:sSt:v))) { switch (c) { case 'a': oneshot = atoi(optarg); @@ -174,6 +182,10 @@ int main(int argc, char *argv[]) case 'g': gettime = 1; break; + case 'k': + offset = 1; + n_samples = atoi(optarg); + break; case 'p': perout = atoi(optarg); break; @@ -376,6 +388,30 @@ int main(int argc, char *argv[]) } } + if (offset) { + sysoff = calloc(1, sizeof(*sysoff)); + if (!sysoff) { + perror(calloc); + return -1; + } + sysoff-n_samples = n_samples; + + if (ioctl(fd, PTP_SYS_OFFSET, sysoff)) + perror(PTP_SYS_OFFSET); + else + puts(phc and system clock time offset request okay); + + pct = sysoff-ts[0]; + for (i = 0; i sysoff-n_samples; i++, pct++) { + printf(system time: %ld.%ld\n, pct-sec, pct-nsec); + pct++; + printf(phctime: %ld.%ld\n, pct-sec, pct-nsec); + } + printf(system time: %ld.%ld\n, pct-sec, pct-nsec); + + free(sysoff); + } + close(fd); return 0; } -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ptp: add the PTP_SYS_OFFSET ioctl to the testptp program clock
On Sat, Sep 14, 2013 at 04:31:46PM +0200, Richard Cochran wrote: > On Sat, Sep 14, 2013 at 04:03:06PM +0800, Dong Zhu wrote: > > This patch add a method into testptp.c to measure the time offset > > between phc and system clock through the ioctl PTP_SYS_OFFSET. > > > > This is a nice addition to the testptp program. I do have a few > comments, below. > Thanks very much for your comments, I have modified the patch as below, Cuold you have a look at it again ? Any comments would be appreciated. >From 655b45785a85599d5fff5eb3b8d9b49b72f2991f Mon Sep 17 00:00:00 2001 From: Dong Zhu Date: Sat, 14 Sep 2013 23:32:14 +0800 This patch add a method into testptp.c to measure the time offset between phc and system clock through the ioctl PTP_SYS_OFFSET. Signed-off-by: Dong Zhu --- Documentation/ptp/testptp.c | 40 ++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/Documentation/ptp/testptp.c b/Documentation/ptp/testptp.c index f59ded0..8acdc70 100644 --- a/Documentation/ptp/testptp.c +++ b/Documentation/ptp/testptp.c @@ -112,6 +112,8 @@ static void usage(char *progname) " -f val adjust the ptp clock frequency by 'val' ppb\n" " -g get the ptp clock time\n" " -h prints this message\n" + " -k val measure the time offset between phc and system clock " + "for 'val' times (Maximum 25)\n" " -p val enable output with a period of 'val' nanoseconds\n" " -P val enable or disable (val=1|0) the system clock PPS\n" " -s set the ptp clock time from the system time\n" @@ -133,8 +135,12 @@ int main(int argc, char *argv[]) struct itimerspec timeout; struct sigevent sigevent; + struct ptp_clock_time *pct; + struct ptp_sys_offset *sysoff; + + char *progname; - int c, cnt, fd; + int i, c, cnt, fd; char *device = DEVICE; clockid_t clkid; @@ -144,6 +150,8 @@ int main(int argc, char *argv[]) int extts = 0; int gettime = 0; int oneshot = 0; + int offset = 0; + int n_samples = 0; int periodic = 0; int perout = -1; int pps = -1; @@ -151,7 +159,7 @@ int main(int argc, char *argv[]) progname = strrchr(argv[0], '/'); progname = progname ? 1+progname : argv[0]; - while (EOF != (c = getopt(argc, argv, "a:A:cd:e:f:ghp:P:sSt:v"))) { + while (EOF != (c = getopt(argc, argv, "a:A:cd:e:f:ghk:p:P:sSt:v"))) { switch (c) { case 'a': oneshot = atoi(optarg); @@ -174,6 +182,10 @@ int main(int argc, char *argv[]) case 'g': gettime = 1; break; + case 'k': + offset = 1; + n_samples = atoi(optarg); + break; case 'p': perout = atoi(optarg); break; @@ -376,6 +388,30 @@ int main(int argc, char *argv[]) } } + if (offset) { + sysoff = calloc(1, sizeof(*sysoff)); + if (!sysoff) { + perror("calloc"); + return -1; + } + sysoff->n_samples = n_samples; + + if (ioctl(fd, PTP_SYS_OFFSET, sysoff)) + perror("PTP_SYS_OFFSET"); + else + puts("phc and system clock time offset request okay"); + + pct = >ts[0]; + for (i = 0; i < sysoff->n_samples; i++, pct++) { + printf("system time: %ld.%ld\n", pct->sec, pct->nsec); + pct++; + printf("phctime: %ld.%ld\n", pct->sec, pct->nsec); + } + printf("system time: %ld.%ld\n", pct->sec, pct->nsec); + + free(sysoff); + } + close(fd); return 0; } -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ptp: measure the time offset between PHC and system clock
This patch add a method into testptp.c to measure the time offset between phc and system clock through the ioctl PTP_SYS_OFFSET. Signed-off-by: Dong Zhu --- Documentation/ptp/testptp.c | 40 ++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/Documentation/ptp/testptp.c b/Documentation/ptp/testptp.c index f59ded0..72bb030 100644 --- a/Documentation/ptp/testptp.c +++ b/Documentation/ptp/testptp.c @@ -112,6 +112,7 @@ static void usage(char *progname) " -f val adjust the ptp clock frequency by 'val' ppb\n" " -g get the ptp clock time\n" " -h prints this message\n" + " -k val measure the time offset between PHC and system clock\n" " -p val enable output with a period of 'val' nanoseconds\n" " -P val enable or disable (val=1|0) the system clock PPS\n" " -s set the ptp clock time from the system time\n" @@ -133,8 +134,12 @@ int main(int argc, char *argv[]) struct itimerspec timeout; struct sigevent sigevent; + struct ptp_clock_time *pct; + struct ptp_sys_offset *sysoff; + + char *progname; - int c, cnt, fd; + int i, c, cnt, fd; char *device = DEVICE; clockid_t clkid; @@ -144,6 +149,8 @@ int main(int argc, char *argv[]) int extts = 0; int gettime = 0; int oneshot = 0; + int offset = 0; + int n_samples = 0; int periodic = 0; int perout = -1; int pps = -1; @@ -151,7 +158,7 @@ int main(int argc, char *argv[]) progname = strrchr(argv[0], '/'); progname = progname ? 1+progname : argv[0]; - while (EOF != (c = getopt(argc, argv, "a:A:cd:e:f:ghp:P:sSt:v"))) { + while (EOF != (c = getopt(argc, argv, "a:A:cd:e:f:ghk:p:P:sSt:v"))) { switch (c) { case 'a': oneshot = atoi(optarg); @@ -174,6 +181,10 @@ int main(int argc, char *argv[]) case 'g': gettime = 1; break; + case 'k': + offset = 1; + n_samples = atoi(optarg); + break; case 'p': perout = atoi(optarg); break; @@ -376,6 +387,31 @@ int main(int argc, char *argv[]) } } + if (offset) { + sysoff = calloc(1, sizeof(*sysoff)); + if (!sysoff) { + perror("calloc"); + return -1; + } + sysoff->n_samples = n_samples; + + if (ioctl(fd, PTP_SYS_OFFSET, sysoff)) + perror("PTP_SYS_OFFSET"); + else + puts("time offset between PHC and +system clock request okay"); + + pct = >ts[0]; + for (i = 0; i < sysoff->n_samples; i++, pct++) { + printf("system time: %ld.%ld\n", pct->sec, pct->nsec); + pct++; + printf("phctime: %ld.%ld\n\n", pct->sec, pct->nsec); + } + printf("system time: %ld.%ld\n", pct->sec, pct->nsec); + + free(sysoff); + } + close(fd); return 0; } -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ptp: measure the time offset between PHC and system clock
This patch add a method into testptp.c to measure the time offset between phc and system clock through the ioctl PTP_SYS_OFFSET. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- Documentation/ptp/testptp.c | 40 ++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/Documentation/ptp/testptp.c b/Documentation/ptp/testptp.c index f59ded0..72bb030 100644 --- a/Documentation/ptp/testptp.c +++ b/Documentation/ptp/testptp.c @@ -112,6 +112,7 @@ static void usage(char *progname) -f val adjust the ptp clock frequency by 'val' ppb\n -g get the ptp clock time\n -h prints this message\n +-k val measure the time offset between PHC and system clock\n -p val enable output with a period of 'val' nanoseconds\n -P val enable or disable (val=1|0) the system clock PPS\n -s set the ptp clock time from the system time\n @@ -133,8 +134,12 @@ int main(int argc, char *argv[]) struct itimerspec timeout; struct sigevent sigevent; + struct ptp_clock_time *pct; + struct ptp_sys_offset *sysoff; + + char *progname; - int c, cnt, fd; + int i, c, cnt, fd; char *device = DEVICE; clockid_t clkid; @@ -144,6 +149,8 @@ int main(int argc, char *argv[]) int extts = 0; int gettime = 0; int oneshot = 0; + int offset = 0; + int n_samples = 0; int periodic = 0; int perout = -1; int pps = -1; @@ -151,7 +158,7 @@ int main(int argc, char *argv[]) progname = strrchr(argv[0], '/'); progname = progname ? 1+progname : argv[0]; - while (EOF != (c = getopt(argc, argv, a:A:cd:e:f:ghp:P:sSt:v))) { + while (EOF != (c = getopt(argc, argv, a:A:cd:e:f:ghk:p:P:sSt:v))) { switch (c) { case 'a': oneshot = atoi(optarg); @@ -174,6 +181,10 @@ int main(int argc, char *argv[]) case 'g': gettime = 1; break; + case 'k': + offset = 1; + n_samples = atoi(optarg); + break; case 'p': perout = atoi(optarg); break; @@ -376,6 +387,31 @@ int main(int argc, char *argv[]) } } + if (offset) { + sysoff = calloc(1, sizeof(*sysoff)); + if (!sysoff) { + perror(calloc); + return -1; + } + sysoff-n_samples = n_samples; + + if (ioctl(fd, PTP_SYS_OFFSET, sysoff)) + perror(PTP_SYS_OFFSET); + else + puts(time offset between PHC and +system clock request okay); + + pct = sysoff-ts[0]; + for (i = 0; i sysoff-n_samples; i++, pct++) { + printf(system time: %ld.%ld\n, pct-sec, pct-nsec); + pct++; + printf(phctime: %ld.%ld\n\n, pct-sec, pct-nsec); + } + printf(system time: %ld.%ld\n, pct-sec, pct-nsec); + + free(sysoff); + } + close(fd); return 0; } -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ptp: add the PTP_SYS_OFFSET ioctl to the testptp program clock
On Sat, Sep 14, 2013 at 04:31:46PM +0200, Richard Cochran wrote: On Sat, Sep 14, 2013 at 04:03:06PM +0800, Dong Zhu wrote: This patch add a method into testptp.c to measure the time offset between phc and system clock through the ioctl PTP_SYS_OFFSET. This is a nice addition to the testptp program. I do have a few comments, below. Thanks very much for your comments, I have modified the patch as below, Cuold you have a look at it again ? Any comments would be appreciated. From 655b45785a85599d5fff5eb3b8d9b49b72f2991f Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Sat, 14 Sep 2013 23:32:14 +0800 This patch add a method into testptp.c to measure the time offset between phc and system clock through the ioctl PTP_SYS_OFFSET. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- Documentation/ptp/testptp.c | 40 ++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/Documentation/ptp/testptp.c b/Documentation/ptp/testptp.c index f59ded0..8acdc70 100644 --- a/Documentation/ptp/testptp.c +++ b/Documentation/ptp/testptp.c @@ -112,6 +112,8 @@ static void usage(char *progname) -f val adjust the ptp clock frequency by 'val' ppb\n -g get the ptp clock time\n -h prints this message\n +-k val measure the time offset between phc and system clock + for 'val' times (Maximum 25)\n -p val enable output with a period of 'val' nanoseconds\n -P val enable or disable (val=1|0) the system clock PPS\n -s set the ptp clock time from the system time\n @@ -133,8 +135,12 @@ int main(int argc, char *argv[]) struct itimerspec timeout; struct sigevent sigevent; + struct ptp_clock_time *pct; + struct ptp_sys_offset *sysoff; + + char *progname; - int c, cnt, fd; + int i, c, cnt, fd; char *device = DEVICE; clockid_t clkid; @@ -144,6 +150,8 @@ int main(int argc, char *argv[]) int extts = 0; int gettime = 0; int oneshot = 0; + int offset = 0; + int n_samples = 0; int periodic = 0; int perout = -1; int pps = -1; @@ -151,7 +159,7 @@ int main(int argc, char *argv[]) progname = strrchr(argv[0], '/'); progname = progname ? 1+progname : argv[0]; - while (EOF != (c = getopt(argc, argv, a:A:cd:e:f:ghp:P:sSt:v))) { + while (EOF != (c = getopt(argc, argv, a:A:cd:e:f:ghk:p:P:sSt:v))) { switch (c) { case 'a': oneshot = atoi(optarg); @@ -174,6 +182,10 @@ int main(int argc, char *argv[]) case 'g': gettime = 1; break; + case 'k': + offset = 1; + n_samples = atoi(optarg); + break; case 'p': perout = atoi(optarg); break; @@ -376,6 +388,30 @@ int main(int argc, char *argv[]) } } + if (offset) { + sysoff = calloc(1, sizeof(*sysoff)); + if (!sysoff) { + perror(calloc); + return -1; + } + sysoff-n_samples = n_samples; + + if (ioctl(fd, PTP_SYS_OFFSET, sysoff)) + perror(PTP_SYS_OFFSET); + else + puts(phc and system clock time offset request okay); + + pct = sysoff-ts[0]; + for (i = 0; i sysoff-n_samples; i++, pct++) { + printf(system time: %ld.%ld\n, pct-sec, pct-nsec); + pct++; + printf(phctime: %ld.%ld\n, pct-sec, pct-nsec); + } + printf(system time: %ld.%ld\n, pct-sec, pct-nsec); + + free(sysoff); + } + close(fd); return 0; } -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] posix_cpu_timers: fix timer never expires when executes clock_nanosleep
Hi Stanislaw, Thansk for your info. On Thu, Aug 01, 2013 at 01:30:50PM +0200, Stanislaw Gruszka wrote: > Hi Dong Zhu > > On Thu, Aug 01, 2013 at 06:10:19PM +0800, Dong Zhu wrote: > > diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c > > index c7f31aa..cc03290 100644 > > --- a/kernel/posix-cpu-timers.c > > +++ b/kernel/posix-cpu-timers.c > > @@ -1413,9 +1413,9 @@ static int posix_cpu_nsleep(const clockid_t > > which_clock, int flags, > > /* > > * Diagnose required errors first. > > */ > > - if (CPUCLOCK_PERTHREAD(which_clock) && > > - (CPUCLOCK_PID(which_clock) == 0 || > > -CPUCLOCK_PID(which_clock) == current->pid)) > > + if (CPUCLOCK_PID(which_clock) == current->pid || > > + (CPUCLOCK_PERTHREAD(which_clock) && > > +CPUCLOCK_PID(which_clock) == 0)) > > return -EINVAL; > > Nope, this is wrong. We have to allow own pid process clock, because it > can be used correctly on multi-threaded processes. Own tid thread clock Yes, you are right, I really neglected this point. > has no sense and we correctly return -EINVAL in such case. > > We could possibly add check for own pid together with check if process > consist of one thread, but that is too complicated IMHO especially > taking into account that threads on the process can be destroyed and > created dynamically. > Agree, really so complicated. -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] posix_cpu_timers: fix timer never expires when executes clock_nanosleep
>From c7439b90b0794c016b29356f0e232f7413ef7b60 Mon Sep 17 00:00:00 2001 From: Dong Zhu Date: Thu, 1 Aug 2013 11:39:04 +0800 When use the current process pid as the clockid, then executes clock_nanosleep syscall the timer will never expire. Kernel should prevent user doing like this and this patch is supposed to fix it.I wrote a simple case to test it: #include #include #include #include #define CPU_CLOCK_PROF 0 #define CPU_CLOCK_VIRT 1 #define CPU_CLOCK_SCHED 2 #define CPU_CLOCK_THREAD 4 #define PID_TO_CLOCKID(pid, clock) ((~(clockid_t) (pid) << 3) | (clockid_t) (clock)) int main(void) { int ret; pid_t pid; clockid_t clk; struct timespec ts; ts.tv_sec = 1; ts.tv_nsec = 0; pid = getpid(); clk = PID_TO_CLOCKID(pid, CPU_CLOCK_PROF); if ((ret = clock_nanosleep(clk, 0, , NULL)) != 0) { perror("clock_nanosleep"); return ret; } return 0; } Signed-off-by: Dong Zhu --- kernel/posix-cpu-timers.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c index c7f31aa..cc03290 100644 --- a/kernel/posix-cpu-timers.c +++ b/kernel/posix-cpu-timers.c @@ -1413,9 +1413,9 @@ static int posix_cpu_nsleep(const clockid_t which_clock, int flags, /* * Diagnose required errors first. */ - if (CPUCLOCK_PERTHREAD(which_clock) && - (CPUCLOCK_PID(which_clock) == 0 || -CPUCLOCK_PID(which_clock) == current->pid)) + if (CPUCLOCK_PID(which_clock) == current->pid || + (CPUCLOCK_PERTHREAD(which_clock) && +CPUCLOCK_PID(which_clock) == 0)) return -EINVAL; error = do_cpu_nanosleep(which_clock, flags, rqtp, ); -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] posix_cpu_timers: fix timer never expires when executes clock_nanosleep
From c7439b90b0794c016b29356f0e232f7413ef7b60 Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Thu, 1 Aug 2013 11:39:04 +0800 When use the current process pid as the clockid, then executes clock_nanosleep syscall the timer will never expire. Kernel should prevent user doing like this and this patch is supposed to fix it.I wrote a simple case to test it: #include time.h #include errno.h #include unistd.h #include sys/types.h #define CPU_CLOCK_PROF 0 #define CPU_CLOCK_VIRT 1 #define CPU_CLOCK_SCHED 2 #define CPU_CLOCK_THREAD 4 #define PID_TO_CLOCKID(pid, clock) ((~(clockid_t) (pid) 3) | (clockid_t) (clock)) int main(void) { int ret; pid_t pid; clockid_t clk; struct timespec ts; ts.tv_sec = 1; ts.tv_nsec = 0; pid = getpid(); clk = PID_TO_CLOCKID(pid, CPU_CLOCK_PROF); if ((ret = clock_nanosleep(clk, 0, ts, NULL)) != 0) { perror(clock_nanosleep); return ret; } return 0; } Signed-off-by: Dong Zhu bluezhud...@gmail.com --- kernel/posix-cpu-timers.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c index c7f31aa..cc03290 100644 --- a/kernel/posix-cpu-timers.c +++ b/kernel/posix-cpu-timers.c @@ -1413,9 +1413,9 @@ static int posix_cpu_nsleep(const clockid_t which_clock, int flags, /* * Diagnose required errors first. */ - if (CPUCLOCK_PERTHREAD(which_clock) - (CPUCLOCK_PID(which_clock) == 0 || -CPUCLOCK_PID(which_clock) == current-pid)) + if (CPUCLOCK_PID(which_clock) == current-pid || + (CPUCLOCK_PERTHREAD(which_clock) +CPUCLOCK_PID(which_clock) == 0)) return -EINVAL; error = do_cpu_nanosleep(which_clock, flags, rqtp, it); -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] posix_cpu_timers: fix timer never expires when executes clock_nanosleep
Hi Stanislaw, Thansk for your info. On Thu, Aug 01, 2013 at 01:30:50PM +0200, Stanislaw Gruszka wrote: Hi Dong Zhu On Thu, Aug 01, 2013 at 06:10:19PM +0800, Dong Zhu wrote: diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c index c7f31aa..cc03290 100644 --- a/kernel/posix-cpu-timers.c +++ b/kernel/posix-cpu-timers.c @@ -1413,9 +1413,9 @@ static int posix_cpu_nsleep(const clockid_t which_clock, int flags, /* * Diagnose required errors first. */ - if (CPUCLOCK_PERTHREAD(which_clock) - (CPUCLOCK_PID(which_clock) == 0 || -CPUCLOCK_PID(which_clock) == current-pid)) + if (CPUCLOCK_PID(which_clock) == current-pid || + (CPUCLOCK_PERTHREAD(which_clock) +CPUCLOCK_PID(which_clock) == 0)) return -EINVAL; Nope, this is wrong. We have to allow own pid process clock, because it can be used correctly on multi-threaded processes. Own tid thread clock Yes, you are right, I really neglected this point. has no sense and we correctly return -EINVAL in such case. We could possibly add check for own pid together with check if process consist of one thread, but that is too complicated IMHO especially taking into account that threads on the process can be destroyed and created dynamically. Agree, really so complicated. -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] timer stats: reset entries when disable the timer usage statistics
On Thu, Jun 20, 2013 at 06:30:48PM +0800, Dong Zhu wrote: > On Mon, Jun 10, 2013 at 03:57:45PM +0800, Dong Zhu wrote: > > From 4dbb760213856dc382241db456c1c6487694837c Mon Sep 17 00:00:00 2001 > > From: Dong Zhu > > Date: Mon, 10 Jun 2013 15:09:27 +0800 > > > > we can start/stop data collection by using : > > echo [1|0] > /proc/timer_stats > > when we stop the data collection,hrtimer will not update the statistics > > for a timer any more,but we could still check the timer usage > > statistics,unfortunately the data is not the latest, the inaccurate data > > might make us confusing. So the purpose of this patch is to reset the > > entries once disable operation is executed.In this way we will never > > see the inaccurate timer usage statistics. > > > > Signed-off-by: Dong Zhu > > --- > > kernel/time/timer_stats.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c > > index 0b537f2..43f05e7 100644 > > --- a/kernel/time/timer_stats.c > > +++ b/kernel/time/timer_stats.c > > @@ -371,6 +371,7 @@ static ssize_t tstats_write(struct file *file, const > > char __user *buf, > > switch (ctl[0]) { > > case '0': > > if (timer_stats_active) { > > + reset_entries(); > > timer_stats_active = 0; > > time_stop = ktime_get(); > > sync_access(); > > -- > > 1.7.11.7 > Hi John and Thomas, A long time has passed and still no reply here. I think this patch makes sense so could you plz accept it ? Thanks! BTW, any comments would be appreciated. -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] timer stats: reset entries when disable the timer usage statistics
On Thu, Jun 20, 2013 at 06:30:48PM +0800, Dong Zhu wrote: On Mon, Jun 10, 2013 at 03:57:45PM +0800, Dong Zhu wrote: From 4dbb760213856dc382241db456c1c6487694837c Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Mon, 10 Jun 2013 15:09:27 +0800 we can start/stop data collection by using : echo [1|0] /proc/timer_stats when we stop the data collection,hrtimer will not update the statistics for a timer any more,but we could still check the timer usage statistics,unfortunately the data is not the latest, the inaccurate data might make us confusing. So the purpose of this patch is to reset the entries once disable operation is executed.In this way we will never see the inaccurate timer usage statistics. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- kernel/time/timer_stats.c | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c index 0b537f2..43f05e7 100644 --- a/kernel/time/timer_stats.c +++ b/kernel/time/timer_stats.c @@ -371,6 +371,7 @@ static ssize_t tstats_write(struct file *file, const char __user *buf, switch (ctl[0]) { case '0': if (timer_stats_active) { + reset_entries(); timer_stats_active = 0; time_stop = ktime_get(); sync_access(); -- 1.7.11.7 Hi John and Thomas, A long time has passed and still no reply here. I think this patch makes sense so could you plz accept it ? Thanks! BTW, any comments would be appreciated. -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] timer stats: reset entries when disable the timer usage statistics
On Mon, Jun 10, 2013 at 03:57:45PM +0800, Dong Zhu wrote: > From 4dbb760213856dc382241db456c1c6487694837c Mon Sep 17 00:00:00 2001 > From: Dong Zhu > Date: Mon, 10 Jun 2013 15:09:27 +0800 > > we can start/stop data collection by using : > echo [1|0] > /proc/timer_stats > when we stop the data collection,hrtimer will not update the statistics > for a timer any more,but we could still check the timer usage > statistics,unfortunately the data is not the latest, the inaccurate data > might make us confusing. So the purpose of this patch is to reset the > entries once disable operation is executed.In this way we will never > see the inaccurate timer usage statistics. > > Signed-off-by: Dong Zhu > --- > kernel/time/timer_stats.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c > index 0b537f2..43f05e7 100644 > --- a/kernel/time/timer_stats.c > +++ b/kernel/time/timer_stats.c > @@ -371,6 +371,7 @@ static ssize_t tstats_write(struct file *file, const char > __user *buf, > switch (ctl[0]) { > case '0': > if (timer_stats_active) { > + reset_entries(); > timer_stats_active = 0; > time_stop = ktime_get(); > sync_access(); > -- > 1.7.11.7 Hi all, Any comments here ? -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] timer stats: reset entries when disable the timer usage statistics
On Mon, Jun 10, 2013 at 03:57:45PM +0800, Dong Zhu wrote: From 4dbb760213856dc382241db456c1c6487694837c Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Mon, 10 Jun 2013 15:09:27 +0800 we can start/stop data collection by using : echo [1|0] /proc/timer_stats when we stop the data collection,hrtimer will not update the statistics for a timer any more,but we could still check the timer usage statistics,unfortunately the data is not the latest, the inaccurate data might make us confusing. So the purpose of this patch is to reset the entries once disable operation is executed.In this way we will never see the inaccurate timer usage statistics. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- kernel/time/timer_stats.c | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c index 0b537f2..43f05e7 100644 --- a/kernel/time/timer_stats.c +++ b/kernel/time/timer_stats.c @@ -371,6 +371,7 @@ static ssize_t tstats_write(struct file *file, const char __user *buf, switch (ctl[0]) { case '0': if (timer_stats_active) { + reset_entries(); timer_stats_active = 0; time_stop = ktime_get(); sync_access(); -- 1.7.11.7 Hi all, Any comments here ? -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] timer stats: reset entries when disable the timer usage statistics
>From 4dbb760213856dc382241db456c1c6487694837c Mon Sep 17 00:00:00 2001 From: Dong Zhu Date: Mon, 10 Jun 2013 15:09:27 +0800 we can start/stop data collection by using : echo [1|0] > /proc/timer_stats when we stop the data collection,hrtimer will not update the statistics for a timer any more,but we could still check the timer usage statistics,unfortunately the data is not the latest, the inaccurate data might make us confusing. So the purpose of this patch is to reset the entries once disable operation is executed.In this way we will never see the inaccurate timer usage statistics. Signed-off-by: Dong Zhu --- kernel/time/timer_stats.c | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c index 0b537f2..43f05e7 100644 --- a/kernel/time/timer_stats.c +++ b/kernel/time/timer_stats.c @@ -371,6 +371,7 @@ static ssize_t tstats_write(struct file *file, const char __user *buf, switch (ctl[0]) { case '0': if (timer_stats_active) { + reset_entries(); timer_stats_active = 0; time_stop = ktime_get(); sync_access(); -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] timer stats: reset entries when disable the timer usage statistics
From 4dbb760213856dc382241db456c1c6487694837c Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Mon, 10 Jun 2013 15:09:27 +0800 we can start/stop data collection by using : echo [1|0] /proc/timer_stats when we stop the data collection,hrtimer will not update the statistics for a timer any more,but we could still check the timer usage statistics,unfortunately the data is not the latest, the inaccurate data might make us confusing. So the purpose of this patch is to reset the entries once disable operation is executed.In this way we will never see the inaccurate timer usage statistics. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- kernel/time/timer_stats.c | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel/time/timer_stats.c b/kernel/time/timer_stats.c index 0b537f2..43f05e7 100644 --- a/kernel/time/timer_stats.c +++ b/kernel/time/timer_stats.c @@ -371,6 +371,7 @@ static ssize_t tstats_write(struct file *file, const char __user *buf, switch (ctl[0]) { case '0': if (timer_stats_active) { + reset_entries(); timer_stats_active = 0; time_stop = ktime_get(); sync_access(); -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] igb: add a method to get the nic hw time stamping policy
Hi, > I modified this patch and added the method to igb_get_ts_info function. > For 82576 nic, through this method we can easily check which type of packets > are time stamped now, such as (HWTSTAMP_FILTER_PTP_V1_L4_SYNC and > HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ), then change or disable it up to your > requests. > I think it is convenient.The origial GET_TS_INFO method can only show the > device’s > time stamping capabilities which nics supported. Through this method we can easily check which type of packets are timestamped currently, it is useful because that we use the hwstamp_ctl set time stamping policy first then we could verify what type of packets does the nic timestamp and then do some other testing(regression) for each policy.No matter PTP is running or not we can make sure whether timestamp packets could cause other general network problems through viewing and setting the timestamp policy. My original idea is that: The implementation of this method is calling the ioctl call, in order not to break the userland ABI I use the flags field of hwtstamp_config, as it has no definition yet. - For I350 it only supports two types HWTSTAMP_FILTER_NONE and HWTSTAMP_FILTER_ALL for timestamping. It is easy to handle just return the value to flags. - For 82576NS it has more individual filters, so I need to do the judement in the igb_ptp_hwtstamp_ioctl function for different nics, the code of judgement might be reduplicated with igb_get_ts_info,then do the OR operation for filters then return to the flags. Could tell me whether the above method(ioctl) is feasible and better than this one (ethtool)? IMO checking this through the ethtool GET_TS_INFO is so convenient without breaking any userland ABIs. Do you think so ? > > I use the tx_reserved[0] and rx_reserved[0] to restore the hwtstamp_tx_types > and > hwtstamp_rx_filters enumeration values. > > I test it on I350 and 82576NS nics and it works as expect. > > Could help reviewing it again ? Any comments would be appreciated. > > > From 8a12932fd2a3bb5ca904bc72b20140247a5d81be Mon Sep 17 00:00:00 2001 > From: Dong Zhu > Date: Mon, 13 May 2013 17:27:59 +0800 > > Currently kernel only support setting the hw time stamping policy > through ioctl,now add a method to check which packets(Outgoing and > Incoming) are time stamped by nic. > > Add this to igb_get_ts_info, we can query this by using the GET_TS_INFO > ethtool command. Testing on I350 and 82576NS it seems work well. > > Signed-off-by: Dong Zhu > --- > drivers/net/ethernet/intel/igb/igb_ethtool.c | 78 > +++- > include/uapi/linux/ethtool.h | 3 ++ > 2 files changed, 79 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c > b/drivers/net/ethernet/intel/igb/igb_ethtool.c > index 7876240..49486b8 100644 > --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c > +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c > @@ -2327,6 +2327,8 @@ static int igb_get_ts_info(struct net_device *dev, > struct ethtool_ts_info *info) > { > struct igb_adapter *adapter = netdev_priv(dev); > + struct e1000_hw *hw = >hw; > + u32 regval; > > switch (adapter->hw.mac.type) { > case e1000_82575: > @@ -2360,10 +2362,29 @@ static int igb_get_ts_info(struct net_device *dev, > > info->rx_filters = 1 << HWTSTAMP_FILTER_NONE; > > + regval = rd32(E1000_TSYNCTXCTL); > + if (regval & E1000_TSYNCTXCTL_ENABLED) > + info->tx_reserved[0] = 1 << HWTSTAMP_TX_ON; > + else > + info->tx_reserved[0] = 1 << HWTSTAMP_TX_OFF; > + > + regval = rd32(E1000_TSYNCRXCTL); > + > /* 82576 does not support timestamping all packets. */ > - if (adapter->hw.mac.type >= e1000_82580) > + if (adapter->hw.mac.type >= e1000_82580) { > info->rx_filters |= 1 << HWTSTAMP_FILTER_ALL; > - else > + > + if (!(regval & E1000_TSYNCRXCTL_ENABLED)) > + info->rx_reserved[0] = > + 1 << HWTSTAMP_FILTER_NONE; > + else if (E1000_TSYNCRXCTL_TYPE_ALL == > + (regval & E1000_TSYNCRXCTL_TYPE_MASK)) > + info->rx_reserved[0] = 1 << HWTSTAMP_FILTER_ALL; > + else > + return -ERANGE; > + > + return 0; > + } else { > info->rx_filters |= >
Re: [PATCH] igb: add a method to get the nic hw time stamping policy
Hi, I modified this patch and added the method to igb_get_ts_info function. For 82576 nic, through this method we can easily check which type of packets are time stamped now, such as (HWTSTAMP_FILTER_PTP_V1_L4_SYNC and HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ), then change or disable it up to your requests. I think it is convenient.The origial GET_TS_INFO method can only show the device’s time stamping capabilities which nics supported. Through this method we can easily check which type of packets are timestamped currently, it is useful because that we use the hwstamp_ctl set time stamping policy first then we could verify what type of packets does the nic timestamp and then do some other testing(regression) for each policy.No matter PTP is running or not we can make sure whether timestamp packets could cause other general network problems through viewing and setting the timestamp policy. My original idea is that: The implementation of this method is calling the ioctl call, in order not to break the userland ABI I use the flags field of hwtstamp_config, as it has no definition yet. - For I350 it only supports two types HWTSTAMP_FILTER_NONE and HWTSTAMP_FILTER_ALL for timestamping. It is easy to handle just return the value to flags. - For 82576NS it has more individual filters, so I need to do the judement in the igb_ptp_hwtstamp_ioctl function for different nics, the code of judgement might be reduplicated with igb_get_ts_info,then do the OR operation for filters then return to the flags. Could tell me whether the above method(ioctl) is feasible and better than this one (ethtool)? IMO checking this through the ethtool GET_TS_INFO is so convenient without breaking any userland ABIs. Do you think so ? I use the tx_reserved[0] and rx_reserved[0] to restore the hwtstamp_tx_types and hwtstamp_rx_filters enumeration values. I test it on I350 and 82576NS nics and it works as expect. Could help reviewing it again ? Any comments would be appreciated. From 8a12932fd2a3bb5ca904bc72b20140247a5d81be Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Mon, 13 May 2013 17:27:59 +0800 Currently kernel only support setting the hw time stamping policy through ioctl,now add a method to check which packets(Outgoing and Incoming) are time stamped by nic. Add this to igb_get_ts_info, we can query this by using the GET_TS_INFO ethtool command. Testing on I350 and 82576NS it seems work well. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- drivers/net/ethernet/intel/igb/igb_ethtool.c | 78 +++- include/uapi/linux/ethtool.h | 3 ++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c index 7876240..49486b8 100644 --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c @@ -2327,6 +2327,8 @@ static int igb_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info) { struct igb_adapter *adapter = netdev_priv(dev); + struct e1000_hw *hw = adapter-hw; + u32 regval; switch (adapter-hw.mac.type) { case e1000_82575: @@ -2360,10 +2362,29 @@ static int igb_get_ts_info(struct net_device *dev, info-rx_filters = 1 HWTSTAMP_FILTER_NONE; + regval = rd32(E1000_TSYNCTXCTL); + if (regval E1000_TSYNCTXCTL_ENABLED) + info-tx_reserved[0] = 1 HWTSTAMP_TX_ON; + else + info-tx_reserved[0] = 1 HWTSTAMP_TX_OFF; + + regval = rd32(E1000_TSYNCRXCTL); + /* 82576 does not support timestamping all packets. */ - if (adapter-hw.mac.type = e1000_82580) + if (adapter-hw.mac.type = e1000_82580) { info-rx_filters |= 1 HWTSTAMP_FILTER_ALL; - else + + if (!(regval E1000_TSYNCRXCTL_ENABLED)) + info-rx_reserved[0] = + 1 HWTSTAMP_FILTER_NONE; + else if (E1000_TSYNCRXCTL_TYPE_ALL == + (regval E1000_TSYNCRXCTL_TYPE_MASK)) + info-rx_reserved[0] = 1 HWTSTAMP_FILTER_ALL; + else + return -ERANGE; + + return 0; + } else { info-rx_filters |= (1 HWTSTAMP_FILTER_PTP_V1_L4_SYNC) | (1 HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ) | @@ -2373,6 +2394,59 @@ static int igb_get_ts_info(struct net_device *dev, (1 HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ) | (1 HWTSTAMP_FILTER_PTP_V2_EVENT); + if (!(regval
Re: [PATCH] igb: add a method to get the nic hw time stamping policy
> You could use the flags field, as it has no definition yet. > > But you still need to explain why this new functionality is needed in > the first place: > > - You can query an interface's time stamping capabilities with the > GET_TS_INFO ethtool command. > Hi, I modified this patch and added the method to igb_get_ts_info function. For 82576 nic, through this method we can easily check which type of packets are time stamped now, such as (HWTSTAMP_FILTER_PTP_V1_L4_SYNC and HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ), then change or disable it up to your requests. I think it is convenient.The origial GET_TS_INFO method can only show the device’s time stamping capabilities which nics supported. I use the tx_reserved[0] and rx_reserved[0] to restore the hwtstamp_tx_types and hwtstamp_rx_filters enumeration values. Due to the limitation of 80 characters one line, I have to move the switch out of else judegment. I test it on I350 and 82576NS nics and it works as expect. Could help reviewing it again ? Any comments would be appreciated. >From 8a12932fd2a3bb5ca904bc72b20140247a5d81be Mon Sep 17 00:00:00 2001 From: Dong Zhu Date: Mon, 13 May 2013 17:27:59 +0800 Currently kernel only support setting the hw time stamping policy through ioctl,now add a method to check which packets(Outgoing and Incoming) are time stamped by nic. Add this to igb_get_ts_info, we can query this by using the GET_TS_INFO ethtool command. Testing on I350 and 82576NS it seems work well. Signed-off-by: Dong Zhu --- drivers/net/ethernet/intel/igb/igb_ethtool.c | 78 +++- include/uapi/linux/ethtool.h | 3 ++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c index 7876240..49486b8 100644 --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c @@ -2327,6 +2327,8 @@ static int igb_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info) { struct igb_adapter *adapter = netdev_priv(dev); + struct e1000_hw *hw = >hw; + u32 regval; switch (adapter->hw.mac.type) { case e1000_82575: @@ -2360,10 +2362,29 @@ static int igb_get_ts_info(struct net_device *dev, info->rx_filters = 1 << HWTSTAMP_FILTER_NONE; + regval = rd32(E1000_TSYNCTXCTL); + if (regval & E1000_TSYNCTXCTL_ENABLED) + info->tx_reserved[0] = 1 << HWTSTAMP_TX_ON; + else + info->tx_reserved[0] = 1 << HWTSTAMP_TX_OFF; + + regval = rd32(E1000_TSYNCRXCTL); + /* 82576 does not support timestamping all packets. */ - if (adapter->hw.mac.type >= e1000_82580) + if (adapter->hw.mac.type >= e1000_82580) { info->rx_filters |= 1 << HWTSTAMP_FILTER_ALL; - else + + if (!(regval & E1000_TSYNCRXCTL_ENABLED)) + info->rx_reserved[0] = + 1 << HWTSTAMP_FILTER_NONE; + else if (E1000_TSYNCRXCTL_TYPE_ALL == + (regval & E1000_TSYNCRXCTL_TYPE_MASK)) + info->rx_reserved[0] = 1 << HWTSTAMP_FILTER_ALL; + else + return -ERANGE; + + return 0; + } else { info->rx_filters |= (1 << HWTSTAMP_FILTER_PTP_V1_L4_SYNC) | (1 << HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ) | @@ -2373,6 +2394,59 @@ static int igb_get_ts_info(struct net_device *dev, (1 << HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ) | (1 << HWTSTAMP_FILTER_PTP_V2_EVENT); + if (!(regval & E1000_TSYNCRXCTL_ENABLED)) { + info->rx_reserved[0] = + 1 << HWTSTAMP_FILTER_NONE; + return 0; + } + } + + switch (regval & E1000_TSYNCRXCTL_TYPE_MASK) { + case E1000_TSYNCRXCTL_TYPE_L4_V1: + regval = rd32(E1000_TSYNCRXCFG); + if (E1000_TSYNCRXCFG_PTP_V1_SYNC_MESSAGE == + (regval & +E1000_TSYNCRXCFG_PTP_V1_CTRLT_MASK)) + info->rx_reserved[0] = + (1 << HWTSTAMP_FILTER_PTP_V1_L4_SYNC); + else if (E1000_TSYNCRXCFG_PTP_V1_DELAY_REQ_MESSAGE +
Re: [PATCH] igb: add a method to get the nic hw time stamping policy
You could use the flags field, as it has no definition yet. But you still need to explain why this new functionality is needed in the first place: - You can query an interface's time stamping capabilities with the GET_TS_INFO ethtool command. Hi, I modified this patch and added the method to igb_get_ts_info function. For 82576 nic, through this method we can easily check which type of packets are time stamped now, such as (HWTSTAMP_FILTER_PTP_V1_L4_SYNC and HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ), then change or disable it up to your requests. I think it is convenient.The origial GET_TS_INFO method can only show the device’s time stamping capabilities which nics supported. I use the tx_reserved[0] and rx_reserved[0] to restore the hwtstamp_tx_types and hwtstamp_rx_filters enumeration values. Due to the limitation of 80 characters one line, I have to move the switch out of else judegment. I test it on I350 and 82576NS nics and it works as expect. Could help reviewing it again ? Any comments would be appreciated. From 8a12932fd2a3bb5ca904bc72b20140247a5d81be Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Mon, 13 May 2013 17:27:59 +0800 Currently kernel only support setting the hw time stamping policy through ioctl,now add a method to check which packets(Outgoing and Incoming) are time stamped by nic. Add this to igb_get_ts_info, we can query this by using the GET_TS_INFO ethtool command. Testing on I350 and 82576NS it seems work well. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- drivers/net/ethernet/intel/igb/igb_ethtool.c | 78 +++- include/uapi/linux/ethtool.h | 3 ++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c index 7876240..49486b8 100644 --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c @@ -2327,6 +2327,8 @@ static int igb_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info) { struct igb_adapter *adapter = netdev_priv(dev); + struct e1000_hw *hw = adapter-hw; + u32 regval; switch (adapter-hw.mac.type) { case e1000_82575: @@ -2360,10 +2362,29 @@ static int igb_get_ts_info(struct net_device *dev, info-rx_filters = 1 HWTSTAMP_FILTER_NONE; + regval = rd32(E1000_TSYNCTXCTL); + if (regval E1000_TSYNCTXCTL_ENABLED) + info-tx_reserved[0] = 1 HWTSTAMP_TX_ON; + else + info-tx_reserved[0] = 1 HWTSTAMP_TX_OFF; + + regval = rd32(E1000_TSYNCRXCTL); + /* 82576 does not support timestamping all packets. */ - if (adapter-hw.mac.type = e1000_82580) + if (adapter-hw.mac.type = e1000_82580) { info-rx_filters |= 1 HWTSTAMP_FILTER_ALL; - else + + if (!(regval E1000_TSYNCRXCTL_ENABLED)) + info-rx_reserved[0] = + 1 HWTSTAMP_FILTER_NONE; + else if (E1000_TSYNCRXCTL_TYPE_ALL == + (regval E1000_TSYNCRXCTL_TYPE_MASK)) + info-rx_reserved[0] = 1 HWTSTAMP_FILTER_ALL; + else + return -ERANGE; + + return 0; + } else { info-rx_filters |= (1 HWTSTAMP_FILTER_PTP_V1_L4_SYNC) | (1 HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ) | @@ -2373,6 +2394,59 @@ static int igb_get_ts_info(struct net_device *dev, (1 HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ) | (1 HWTSTAMP_FILTER_PTP_V2_EVENT); + if (!(regval E1000_TSYNCRXCTL_ENABLED)) { + info-rx_reserved[0] = + 1 HWTSTAMP_FILTER_NONE; + return 0; + } + } + + switch (regval E1000_TSYNCRXCTL_TYPE_MASK) { + case E1000_TSYNCRXCTL_TYPE_L4_V1: + regval = rd32(E1000_TSYNCRXCFG); + if (E1000_TSYNCRXCFG_PTP_V1_SYNC_MESSAGE == + (regval +E1000_TSYNCRXCFG_PTP_V1_CTRLT_MASK)) + info-rx_reserved[0] = + (1 HWTSTAMP_FILTER_PTP_V1_L4_SYNC); + else if (E1000_TSYNCRXCFG_PTP_V1_DELAY_REQ_MESSAGE + == (regval + E1000_TSYNCRXCFG_PTP_V1_CTRLT_MASK)) + info-rx_reserved[0
Re: [PATCH] igb: add a method to get the nic hw time stamping policy
On Sun, May 12, 2013 at 07:24:46PM +0200, Richard Cochran wrote: > On Sun, May 12, 2013 at 10:25:55PM +0800, Dong Zhu wrote: > > Thanks for your pointing out my mistakes of CodingStyle. > > > > > > struct hwtstamp_config { > > > >+int rw; > > > > My initial idea was that the type of rw should be enum like tx_type, but I > > am > > not sure whther it is necessary to define a new enum, if this patch could > > be accpeted I will ask someone about the rw. At that time I will change > > the type of rw to bool or define a new enum, then convert the if to > > switch if necessary. > > You cannot add any new field at all. That would break a userland ABI. > Thanks for your info Richard. Can I use the 'flags' which members of hwtstamp_config to judge the ioctl request instead ? If not could you plz give me a new way to resolve this issue ? Thanks a lot ! -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] igb: add a method to get the nic hw time stamping policy
Thanks for your pointing out my mistakes of CodingStyle. > > struct hwtstamp_config { > >+int rw; My initial idea was that the type of rw should be enum like tx_type, but I am not sure whther it is necessary to define a new enum, if this patch could be accpeted I will ask someone about the rw. At that time I will change the type of rw to bool or define a new enum, then convert the if to switch if necessary. Patch after modifying: >From cf337e7863af66428554785c32a9840fafaa3492 Mon Sep 17 00:00:00 2001 From: Dong Zhu Date: Sun, 12 May 2013 21:57:57 +0800 Currently kernel only support setting the hw time stamping policy through ioctl,now add a method to check which packets(Outgoing and Incoming) are time stamped by nic. Signed-off-by: Dong Zhu --- drivers/net/ethernet/intel/igb/igb_ptp.c | 28 include/uapi/linux/net_tstamp.h | 4 +++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c index 7e8c477..4ea091d 100644 --- a/drivers/net/ethernet/intel/igb/igb_ptp.c +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c @@ -577,6 +577,33 @@ int igb_ptp_hwtstamp_ioctl(struct net_device *netdev, if (config.flags) return -EINVAL; + if (config.rw == 0) { + goto set_policy; + } else if (config.rw == 1) { + if (config.tx_type || config.rx_filter) + return -EINVAL; + + regval = rd32(E1000_TSYNCTXCTL); + if (regval & E1000_TSYNCTXCTL_ENABLED) + config.tx_type = HWTSTAMP_TX_ON; + else + config.tx_type = HWTSTAMP_TX_OFF; + + regval = rd32(E1000_TSYNCRXCTL); + if (!(regval & E1000_TSYNCRXCTL_ENABLED)) + config.rx_filter = HWTSTAMP_FILTER_NONE; + else if (E1000_TSYNCRXCTL_TYPE_ALL == + (regval & E1000_TSYNCRXCTL_TYPE_MASK)) + config.rx_filter = HWTSTAMP_FILTER_ALL; + else + return -ERANGE; + + goto end; + } else { + return -EINVAL; + } + +set_policy: switch (config.tx_type) { case HWTSTAMP_TX_OFF: tsync_tx_ctl = 0; @@ -707,6 +734,7 @@ int igb_ptp_hwtstamp_ioctl(struct net_device *netdev, regval = rd32(E1000_RXSTMPL); regval = rd32(E1000_RXSTMPH); +end: return copy_to_user(ifr->ifr_data, , sizeof(config)) ? -EFAULT : 0; } diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h index ae5df12..77147da 100644 --- a/include/uapi/linux/net_tstamp.h +++ b/include/uapi/linux/net_tstamp.h @@ -28,9 +28,10 @@ enum { /** * struct hwtstamp_config - %SIOCSHWTSTAMP parameter * + * @rw:0/1 represents set/get the hw time stamp policy * @flags: no flags defined right now, must be zero * @tx_type: one of HWTSTAMP_TX_* - * @rx_type: one of one of HWTSTAMP_FILTER_* + * @rx_filter: one of one of HWTSTAMP_FILTER_* * * %SIOCSHWTSTAMP expects a ifreq with a ifr_data pointer to * this structure. dev_ifsioc() in the kernel takes care of the @@ -39,6 +40,7 @@ enum { * 32 and 64 bit systems, don't break this! */ struct hwtstamp_config { + int rw; int flags; int tx_type; int rx_filter; -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] igb: add a method to get the nic hw time stamping policy
On Sat, May 11, 2013 at 05:31:43PM +0200, Richard Cochran wrote: > On Sat, May 11, 2013 at 10:02:19PM +0800, Dong Zhu wrote: > > > > Currently kernel only support setting the hw time stamping policy > > through ioctl,now add a method to check which packets(Outgoing and > > Incoming) are time stamped by nic. > > I don't really see a use case here. Applications needing time stamping > should just set the policy that they need. I think it is necessary to check which type of packets are stamped by nic. For I350 (igb), it only support HWTSTAMP_FILTER_NONE and HWTSTAMP_FILTER_ALL of outgoing packets.For 82599EB(ixgbe), we can check more. I add a new member of hwtstamp_config to judge the ioctl request is read or write, we can specify the rw in the userspace using hwstamp_ctl application to get the time stamped info. -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] igb: add a method to get the nic hw time stamping policy
On Sat, May 11, 2013 at 05:31:43PM +0200, Richard Cochran wrote: On Sat, May 11, 2013 at 10:02:19PM +0800, Dong Zhu wrote: Currently kernel only support setting the hw time stamping policy through ioctl,now add a method to check which packets(Outgoing and Incoming) are time stamped by nic. I don't really see a use case here. Applications needing time stamping should just set the policy that they need. I think it is necessary to check which type of packets are stamped by nic. For I350 (igb), it only support HWTSTAMP_FILTER_NONE and HWTSTAMP_FILTER_ALL of outgoing packets.For 82599EB(ixgbe), we can check more. I add a new member of hwtstamp_config to judge the ioctl request is read or write, we can specify the rw in the userspace using hwstamp_ctl application to get the time stamped info. -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] igb: add a method to get the nic hw time stamping policy
Thanks for your pointing out my mistakes of CodingStyle. struct hwtstamp_config { +int rw; My initial idea was that the type of rw should be enum like tx_type, but I am not sure whther it is necessary to define a new enum, if this patch could be accpeted I will ask someone about the rw. At that time I will change the type of rw to bool or define a new enum, then convert the if to switch if necessary. Patch after modifying: From cf337e7863af66428554785c32a9840fafaa3492 Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Sun, 12 May 2013 21:57:57 +0800 Currently kernel only support setting the hw time stamping policy through ioctl,now add a method to check which packets(Outgoing and Incoming) are time stamped by nic. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- drivers/net/ethernet/intel/igb/igb_ptp.c | 28 include/uapi/linux/net_tstamp.h | 4 +++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c index 7e8c477..4ea091d 100644 --- a/drivers/net/ethernet/intel/igb/igb_ptp.c +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c @@ -577,6 +577,33 @@ int igb_ptp_hwtstamp_ioctl(struct net_device *netdev, if (config.flags) return -EINVAL; + if (config.rw == 0) { + goto set_policy; + } else if (config.rw == 1) { + if (config.tx_type || config.rx_filter) + return -EINVAL; + + regval = rd32(E1000_TSYNCTXCTL); + if (regval E1000_TSYNCTXCTL_ENABLED) + config.tx_type = HWTSTAMP_TX_ON; + else + config.tx_type = HWTSTAMP_TX_OFF; + + regval = rd32(E1000_TSYNCRXCTL); + if (!(regval E1000_TSYNCRXCTL_ENABLED)) + config.rx_filter = HWTSTAMP_FILTER_NONE; + else if (E1000_TSYNCRXCTL_TYPE_ALL == + (regval E1000_TSYNCRXCTL_TYPE_MASK)) + config.rx_filter = HWTSTAMP_FILTER_ALL; + else + return -ERANGE; + + goto end; + } else { + return -EINVAL; + } + +set_policy: switch (config.tx_type) { case HWTSTAMP_TX_OFF: tsync_tx_ctl = 0; @@ -707,6 +734,7 @@ int igb_ptp_hwtstamp_ioctl(struct net_device *netdev, regval = rd32(E1000_RXSTMPL); regval = rd32(E1000_RXSTMPH); +end: return copy_to_user(ifr-ifr_data, config, sizeof(config)) ? -EFAULT : 0; } diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h index ae5df12..77147da 100644 --- a/include/uapi/linux/net_tstamp.h +++ b/include/uapi/linux/net_tstamp.h @@ -28,9 +28,10 @@ enum { /** * struct hwtstamp_config - %SIOCSHWTSTAMP parameter * + * @rw:0/1 represents set/get the hw time stamp policy * @flags: no flags defined right now, must be zero * @tx_type: one of HWTSTAMP_TX_* - * @rx_type: one of one of HWTSTAMP_FILTER_* + * @rx_filter: one of one of HWTSTAMP_FILTER_* * * %SIOCSHWTSTAMP expects a struct ifreq with a ifr_data pointer to * this structure. dev_ifsioc() in the kernel takes care of the @@ -39,6 +40,7 @@ enum { * 32 and 64 bit systems, don't break this! */ struct hwtstamp_config { + int rw; int flags; int tx_type; int rx_filter; -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] igb: add a method to get the nic hw time stamping policy
On Sun, May 12, 2013 at 07:24:46PM +0200, Richard Cochran wrote: On Sun, May 12, 2013 at 10:25:55PM +0800, Dong Zhu wrote: Thanks for your pointing out my mistakes of CodingStyle. struct hwtstamp_config { +int rw; My initial idea was that the type of rw should be enum like tx_type, but I am not sure whther it is necessary to define a new enum, if this patch could be accpeted I will ask someone about the rw. At that time I will change the type of rw to bool or define a new enum, then convert the if to switch if necessary. You cannot add any new field at all. That would break a userland ABI. Thanks for your info Richard. Can I use the 'flags' which members of hwtstamp_config to judge the ioctl request instead ? If not could you plz give me a new way to resolve this issue ? Thanks a lot ! -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] igb: add a method to get the nic hw time stamping policy
>From e6a55411486de8a09b859e73140bf35c0ee36047 Mon Sep 17 00:00:00 2001 From: Dong Zhu Date: Sat, 11 May 2013 21:44:54 +0800 Subject: [PATCH] igb: add a method to get the nic hw time stamping policy Currently kernel only support setting the hw time stamping policy through ioctl,now add a method to check which packets(Outgoing and Incoming) are time stamped by nic. Signed-off-by: Dong Zhu --- drivers/net/ethernet/intel/igb/igb_ptp.c | 29 + include/uapi/linux/net_tstamp.h | 4 +++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c index 7e8c477..8c06346 100644 --- a/drivers/net/ethernet/intel/igb/igb_ptp.c +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c @@ -577,6 +577,34 @@ int igb_ptp_hwtstamp_ioctl(struct net_device *netdev, if (config.flags) return -EINVAL; + if (config.rw == 0) + goto set_policy; + else if (config.rw == 1) { + if (config.tx_type || config.rx_filter) + return -EINVAL; + + regval = rd32(E1000_TSYNCTXCTL); + + if (regval & E1000_TSYNCTXCTL_ENABLED) + config.tx_type = HWTSTAMP_TX_ON; + else + config.tx_type = HWTSTAMP_TX_OFF; + + regval = rd32(E1000_TSYNCRXCTL); + + if (!(regval & E1000_TSYNCRXCTL_ENABLED)) + config.rx_filter = HWTSTAMP_FILTER_NONE; + else if (E1000_TSYNCRXCTL_TYPE_ALL == + (regval & E1000_TSYNCRXCTL_TYPE_MASK)) + config.rx_filter = HWTSTAMP_FILTER_ALL; + else + return -ERANGE; + + goto end; + } else + return -EINVAL; + +set_policy: switch (config.tx_type) { case HWTSTAMP_TX_OFF: tsync_tx_ctl = 0; @@ -707,6 +735,7 @@ int igb_ptp_hwtstamp_ioctl(struct net_device *netdev, regval = rd32(E1000_RXSTMPL); regval = rd32(E1000_RXSTMPH); +end: return copy_to_user(ifr->ifr_data, , sizeof(config)) ? -EFAULT : 0; } diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h index ae5df12..77147da 100644 --- a/include/uapi/linux/net_tstamp.h +++ b/include/uapi/linux/net_tstamp.h @@ -28,9 +28,10 @@ enum { /** * struct hwtstamp_config - %SIOCSHWTSTAMP parameter * + * @rw:0/1 represents set/get the hw time stamp policy * @flags: no flags defined right now, must be zero * @tx_type: one of HWTSTAMP_TX_* - * @rx_type: one of one of HWTSTAMP_FILTER_* + * @rx_filter: one of one of HWTSTAMP_FILTER_* * * %SIOCSHWTSTAMP expects a ifreq with a ifr_data pointer to * this structure. dev_ifsioc() in the kernel takes care of the @@ -39,6 +40,7 @@ enum { * 32 and 64 bit systems, don't break this! */ struct hwtstamp_config { + int rw; int flags; int tx_type; int rx_filter; -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] igb: add a method to get the nic hw time stamping policy
From e6a55411486de8a09b859e73140bf35c0ee36047 Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Sat, 11 May 2013 21:44:54 +0800 Subject: [PATCH] igb: add a method to get the nic hw time stamping policy Currently kernel only support setting the hw time stamping policy through ioctl,now add a method to check which packets(Outgoing and Incoming) are time stamped by nic. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- drivers/net/ethernet/intel/igb/igb_ptp.c | 29 + include/uapi/linux/net_tstamp.h | 4 +++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c index 7e8c477..8c06346 100644 --- a/drivers/net/ethernet/intel/igb/igb_ptp.c +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c @@ -577,6 +577,34 @@ int igb_ptp_hwtstamp_ioctl(struct net_device *netdev, if (config.flags) return -EINVAL; + if (config.rw == 0) + goto set_policy; + else if (config.rw == 1) { + if (config.tx_type || config.rx_filter) + return -EINVAL; + + regval = rd32(E1000_TSYNCTXCTL); + + if (regval E1000_TSYNCTXCTL_ENABLED) + config.tx_type = HWTSTAMP_TX_ON; + else + config.tx_type = HWTSTAMP_TX_OFF; + + regval = rd32(E1000_TSYNCRXCTL); + + if (!(regval E1000_TSYNCRXCTL_ENABLED)) + config.rx_filter = HWTSTAMP_FILTER_NONE; + else if (E1000_TSYNCRXCTL_TYPE_ALL == + (regval E1000_TSYNCRXCTL_TYPE_MASK)) + config.rx_filter = HWTSTAMP_FILTER_ALL; + else + return -ERANGE; + + goto end; + } else + return -EINVAL; + +set_policy: switch (config.tx_type) { case HWTSTAMP_TX_OFF: tsync_tx_ctl = 0; @@ -707,6 +735,7 @@ int igb_ptp_hwtstamp_ioctl(struct net_device *netdev, regval = rd32(E1000_RXSTMPL); regval = rd32(E1000_RXSTMPH); +end: return copy_to_user(ifr-ifr_data, config, sizeof(config)) ? -EFAULT : 0; } diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h index ae5df12..77147da 100644 --- a/include/uapi/linux/net_tstamp.h +++ b/include/uapi/linux/net_tstamp.h @@ -28,9 +28,10 @@ enum { /** * struct hwtstamp_config - %SIOCSHWTSTAMP parameter * + * @rw:0/1 represents set/get the hw time stamp policy * @flags: no flags defined right now, must be zero * @tx_type: one of HWTSTAMP_TX_* - * @rx_type: one of one of HWTSTAMP_FILTER_* + * @rx_filter: one of one of HWTSTAMP_FILTER_* * * %SIOCSHWTSTAMP expects a struct ifreq with a ifr_data pointer to * this structure. dev_ifsioc() in the kernel takes care of the @@ -39,6 +40,7 @@ enum { * 32 and 64 bit systems, don't break this! */ struct hwtstamp_config { + int rw; int flags; int tx_type; int rx_filter; -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] timekeeping: avoid adjust kernel time once hwclock kept in UTC time
On Tue, Dec 18, 2012 at 10:37:08PM -0800, John Stultz wrote: > On 12/18/2012 09:15 PM, Dong Zhu wrote: > >On Thu, Dec 06, 2012 at 10:03:34PM +0800, Dong Zhu wrote: > >> From c126376cf1837b0956e0268056db61870fbbc1d4 Mon Sep 17 00:00:00 2001 > >>From: Dong Zhu > >>Date: Thu, 6 Dec 2012 21:45:00 +0800 > >>Subject: [PATCH] timekeeping: avoid adjust kernel time once hwclock kept in > >> UTC time > >> > >>If the Hardware Clock kept in local time,kernel will adjust the time > >>to be UTC time.But if Hardware Clock kept in UTC time,system will make > >>a dummy settimeofday call first (sys_tz.tz_minuteswest = 0) to make sure > >>the time is not shifted,so at this point I think maybe it is not necessary > >>to set the kernel time once the sys_tz.tz_minuteswest is zero. > >> > >>Signed-off-by: Dong Zhu > >>--- > >> kernel/time.c | 8 +--- > >> 1 file changed, 5 insertions(+), 3 deletions(-) > >> > >>diff --git a/kernel/time.c b/kernel/time.c > >>index d226c6a..0b592ce 100644 > >>--- a/kernel/time.c > >>+++ b/kernel/time.c > >>@@ -134,9 +134,11 @@ static inline void warp_clock(void) > >> { > >>struct timespec adjust; > >>- adjust = current_kernel_time(); > >>- adjust.tv_sec += sys_tz.tz_minuteswest * 60; > >>- do_settimeofday(); > >>+ if (sys_tz.tz_minuteswest) { > >>+ adjust = current_kernel_time(); > >>+ adjust.tv_sec += sys_tz.tz_minuteswest * 60; > >>+ do_settimeofday(); > >>+ } > >> } > >> /* > >>-- > >>1.7.11.7 > >> > >Hi, > > > >Any comments ? > Sorry for the slow response, been a little busy. Honestly the > warp_clock() code path is always been a bit odd to me, so I've not > really been able to get my head around the implications of this > change. > > Really, I'm not sure if I'll get to this before the new year. > Its on my list, but don't be afraid to ping me early Jan if I > haven't queued it for 3.9 by then. > Hi, Distance my submit over a long time,whether my patch can be applied ? -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] timekeeping: avoid adjust kernel time once hwclock kept in UTC time
On Tue, Dec 18, 2012 at 10:37:08PM -0800, John Stultz wrote: On 12/18/2012 09:15 PM, Dong Zhu wrote: On Thu, Dec 06, 2012 at 10:03:34PM +0800, Dong Zhu wrote: From c126376cf1837b0956e0268056db61870fbbc1d4 Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Thu, 6 Dec 2012 21:45:00 +0800 Subject: [PATCH] timekeeping: avoid adjust kernel time once hwclock kept in UTC time If the Hardware Clock kept in local time,kernel will adjust the time to be UTC time.But if Hardware Clock kept in UTC time,system will make a dummy settimeofday call first (sys_tz.tz_minuteswest = 0) to make sure the time is not shifted,so at this point I think maybe it is not necessary to set the kernel time once the sys_tz.tz_minuteswest is zero. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- kernel/time.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/kernel/time.c b/kernel/time.c index d226c6a..0b592ce 100644 --- a/kernel/time.c +++ b/kernel/time.c @@ -134,9 +134,11 @@ static inline void warp_clock(void) { struct timespec adjust; - adjust = current_kernel_time(); - adjust.tv_sec += sys_tz.tz_minuteswest * 60; - do_settimeofday(adjust); + if (sys_tz.tz_minuteswest) { + adjust = current_kernel_time(); + adjust.tv_sec += sys_tz.tz_minuteswest * 60; + do_settimeofday(adjust); + } } /* -- 1.7.11.7 Hi, Any comments ? Sorry for the slow response, been a little busy. Honestly the warp_clock() code path is always been a bit odd to me, so I've not really been able to get my head around the implications of this change. Really, I'm not sure if I'll get to this before the new year. Its on my list, but don't be afraid to ping me early Jan if I haven't queued it for 3.9 by then. Hi, Distance my submit over a long time,whether my patch can be applied ? -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] timekeeping: avoid adjust kernel time once hwclock kept in UTC time
On Thu, Dec 06, 2012 at 10:03:34PM +0800, Dong Zhu wrote: > From c126376cf1837b0956e0268056db61870fbbc1d4 Mon Sep 17 00:00:00 2001 > From: Dong Zhu > Date: Thu, 6 Dec 2012 21:45:00 +0800 > Subject: [PATCH] timekeeping: avoid adjust kernel time once hwclock kept in > UTC time > > If the Hardware Clock kept in local time,kernel will adjust the time > to be UTC time.But if Hardware Clock kept in UTC time,system will make > a dummy settimeofday call first (sys_tz.tz_minuteswest = 0) to make sure > the time is not shifted,so at this point I think maybe it is not necessary > to set the kernel time once the sys_tz.tz_minuteswest is zero. > > Signed-off-by: Dong Zhu > --- > kernel/time.c | 8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/kernel/time.c b/kernel/time.c > index d226c6a..0b592ce 100644 > --- a/kernel/time.c > +++ b/kernel/time.c > @@ -134,9 +134,11 @@ static inline void warp_clock(void) > { > struct timespec adjust; > > - adjust = current_kernel_time(); > - adjust.tv_sec += sys_tz.tz_minuteswest * 60; > - do_settimeofday(); > + if (sys_tz.tz_minuteswest) { > + adjust = current_kernel_time(); > + adjust.tv_sec += sys_tz.tz_minuteswest * 60; > + do_settimeofday(); > + } > } > > /* > -- > 1.7.11.7 > Hi, Any comments ? -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] timekeeping: avoid adjust kernel time once hwclock kept in UTC time
On Thu, Dec 06, 2012 at 10:03:34PM +0800, Dong Zhu wrote: From c126376cf1837b0956e0268056db61870fbbc1d4 Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Thu, 6 Dec 2012 21:45:00 +0800 Subject: [PATCH] timekeeping: avoid adjust kernel time once hwclock kept in UTC time If the Hardware Clock kept in local time,kernel will adjust the time to be UTC time.But if Hardware Clock kept in UTC time,system will make a dummy settimeofday call first (sys_tz.tz_minuteswest = 0) to make sure the time is not shifted,so at this point I think maybe it is not necessary to set the kernel time once the sys_tz.tz_minuteswest is zero. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- kernel/time.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/kernel/time.c b/kernel/time.c index d226c6a..0b592ce 100644 --- a/kernel/time.c +++ b/kernel/time.c @@ -134,9 +134,11 @@ static inline void warp_clock(void) { struct timespec adjust; - adjust = current_kernel_time(); - adjust.tv_sec += sys_tz.tz_minuteswest * 60; - do_settimeofday(adjust); + if (sys_tz.tz_minuteswest) { + adjust = current_kernel_time(); + adjust.tv_sec += sys_tz.tz_minuteswest * 60; + do_settimeofday(adjust); + } } /* -- 1.7.11.7 Hi, Any comments ? -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] timekeeping: avoid adjust kernel time once hwclock kept in UTC time
>From c126376cf1837b0956e0268056db61870fbbc1d4 Mon Sep 17 00:00:00 2001 From: Dong Zhu Date: Thu, 6 Dec 2012 21:45:00 +0800 Subject: [PATCH] timekeeping: avoid adjust kernel time once hwclock kept in UTC time If the Hardware Clock kept in local time,kernel will adjust the time to be UTC time.But if Hardware Clock kept in UTC time,system will make a dummy settimeofday call first (sys_tz.tz_minuteswest = 0) to make sure the time is not shifted,so at this point I think maybe it is not necessary to set the kernel time once the sys_tz.tz_minuteswest is zero. Signed-off-by: Dong Zhu --- kernel/time.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/kernel/time.c b/kernel/time.c index d226c6a..0b592ce 100644 --- a/kernel/time.c +++ b/kernel/time.c @@ -134,9 +134,11 @@ static inline void warp_clock(void) { struct timespec adjust; - adjust = current_kernel_time(); - adjust.tv_sec += sys_tz.tz_minuteswest * 60; - do_settimeofday(); + if (sys_tz.tz_minuteswest) { + adjust = current_kernel_time(); + adjust.tv_sec += sys_tz.tz_minuteswest * 60; + do_settimeofday(); + } } /* -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] timekeeping: avoid adjust kernel time once hwclock kept in UTC time
From c126376cf1837b0956e0268056db61870fbbc1d4 Mon Sep 17 00:00:00 2001 From: Dong Zhu bluezhud...@gmail.com Date: Thu, 6 Dec 2012 21:45:00 +0800 Subject: [PATCH] timekeeping: avoid adjust kernel time once hwclock kept in UTC time If the Hardware Clock kept in local time,kernel will adjust the time to be UTC time.But if Hardware Clock kept in UTC time,system will make a dummy settimeofday call first (sys_tz.tz_minuteswest = 0) to make sure the time is not shifted,so at this point I think maybe it is not necessary to set the kernel time once the sys_tz.tz_minuteswest is zero. Signed-off-by: Dong Zhu bluezhud...@gmail.com --- kernel/time.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/kernel/time.c b/kernel/time.c index d226c6a..0b592ce 100644 --- a/kernel/time.c +++ b/kernel/time.c @@ -134,9 +134,11 @@ static inline void warp_clock(void) { struct timespec adjust; - adjust = current_kernel_time(); - adjust.tv_sec += sys_tz.tz_minuteswest * 60; - do_settimeofday(adjust); + if (sys_tz.tz_minuteswest) { + adjust = current_kernel_time(); + adjust.tv_sec += sys_tz.tz_minuteswest * 60; + do_settimeofday(adjust); + } } /* -- 1.7.11.7 -- Best Regards, Dong Zhu -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/