Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1653570 > unrolled thread

[patch 05/26] posix-clocks: Remove interval timer facility and mmap/fasync callbacks

Started byThomas Gleixner <tglx@linutronix.de>
First post2017-05-30 23:50 +0200
Last post2017-06-05 10:20 +0200
Articles 3 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [patch 05/26] posix-clocks: Remove interval timer facility and  mmap/fasync callbacks Thomas Gleixner <tglx@linutronix.de> - 2017-05-30 23:50 +0200
    Re: [patch 05/26] posix-clocks: Remove interval timer facility and  mmap/fasync callbacks Richard Cochran <richardcochran@gmail.com> - 2017-05-31 11:10 +0200
    [tip:timers/core] posix-clocks: Remove interval timer facility and  mmap/fasync callbacks tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2017-06-05 10:20 +0200

#1653570 — [patch 05/26] posix-clocks: Remove interval timer facility and mmap/fasync callbacks

FromThomas Gleixner <tglx@linutronix.de>
Date2017-05-30 23:50 +0200
Subject[patch 05/26] posix-clocks: Remove interval timer facility and mmap/fasync callbacks
Message-ID<tMXbz-4e6-3@gated-at.bofh.it>
The only user of this facility is ptp_clock, which does not implement any of
those functions.

Remove them to prevent accidental users. Especially the interval timer
interfaces are now more or less impossible to implement because the
necessary infrastructure has been confined to the core code. Aside of that
it's really complex to make these callbacks implemented according to spec
as the alarm timer implementation demonstrates. If at all then a nanosleep
callback might be a reasonable extension. For now keep just what ptp_clock
needs.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/posix-clock.h |   22 --------
 kernel/time/posix-clock.c   |  113 --------------------------------------------
 2 files changed, 135 deletions(-)

--- a/include/linux/posix-clock.h
+++ b/include/linux/posix-clock.h
@@ -42,12 +42,6 @@ struct posix_clock;
  * @clock_gettime:  Read the current time
  * @clock_getres:   Get the clock resolution
  * @clock_settime:  Set the current time value
- * @timer_create:   Create a new timer
- * @timer_delete:   Remove a previously created timer
- * @timer_gettime:  Get remaining time and interval of a timer
- * @timer_settime: Set a timer's initial expiration and interval
- * @fasync:         Optional character device fasync method
- * @mmap:           Optional character device mmap method
  * @open:           Optional character device open method
  * @release:        Optional character device release method
  * @ioctl:          Optional character device ioctl method
@@ -66,28 +60,12 @@ struct posix_clock_operations {
 	int  (*clock_settime)(struct posix_clock *pc,
 			      const struct timespec64 *ts);
 
-	int  (*timer_create) (struct posix_clock *pc, struct k_itimer *kit);
-
-	int  (*timer_delete) (struct posix_clock *pc, struct k_itimer *kit);
-
-	void (*timer_gettime)(struct posix_clock *pc,
-			      struct k_itimer *kit, struct itimerspec64 *tsp);
-
-	int  (*timer_settime)(struct posix_clock *pc,
-			      struct k_itimer *kit, int flags,
-			      struct itimerspec64 *tsp, struct itimerspec64 *old);
 	/*
 	 * Optional character device methods:
 	 */
-	int     (*fasync)  (struct posix_clock *pc,
-			    int fd, struct file *file, int on);
-
 	long    (*ioctl)   (struct posix_clock *pc,
 			    unsigned int cmd, unsigned long arg);
 
-	int     (*mmap)    (struct posix_clock *pc,
-			    struct vm_area_struct *vma);
-
 	int     (*open)    (struct posix_clock *pc, fmode_t f_mode);
 
 	uint    (*poll)    (struct posix_clock *pc,
--- a/kernel/time/posix-clock.c
+++ b/kernel/time/posix-clock.c
@@ -82,38 +82,6 @@ static unsigned int posix_clock_poll(str
 	return result;
 }
 
-static int posix_clock_fasync(int fd, struct file *fp, int on)
-{
-	struct posix_clock *clk = get_posix_clock(fp);
-	int err = 0;
-
-	if (!clk)
-		return -ENODEV;
-
-	if (clk->ops.fasync)
-		err = clk->ops.fasync(clk, fd, fp, on);
-
-	put_posix_clock(clk);
-
-	return err;
-}
-
-static int posix_clock_mmap(struct file *fp, struct vm_area_struct *vma)
-{
-	struct posix_clock *clk = get_posix_clock(fp);
-	int err = -ENODEV;
-
-	if (!clk)
-		return -ENODEV;
-
-	if (clk->ops.mmap)
-		err = clk->ops.mmap(clk, vma);
-
-	put_posix_clock(clk);
-
-	return err;
-}
-
 static long posix_clock_ioctl(struct file *fp,
 			      unsigned int cmd, unsigned long arg)
 {
@@ -199,8 +167,6 @@ static const struct file_operations posi
 	.unlocked_ioctl	= posix_clock_ioctl,
 	.open		= posix_clock_open,
 	.release	= posix_clock_release,
-	.fasync		= posix_clock_fasync,
-	.mmap		= posix_clock_mmap,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl	= posix_clock_compat_ioctl,
 #endif
@@ -359,88 +325,9 @@ static int pc_clock_settime(clockid_t id
 	return err;
 }
 
-static int pc_timer_create(struct k_itimer *kit)
-{
-	clockid_t id = kit->it_clock;
-	struct posix_clock_desc cd;
-	int err;
-
-	err = get_clock_desc(id, &cd);
-	if (err)
-		return err;
-
-	if (cd.clk->ops.timer_create)
-		err = cd.clk->ops.timer_create(cd.clk, kit);
-	else
-		err = -EOPNOTSUPP;
-
-	put_clock_desc(&cd);
-
-	return err;
-}
-
-static int pc_timer_delete(struct k_itimer *kit)
-{
-	clockid_t id = kit->it_clock;
-	struct posix_clock_desc cd;
-	int err;
-
-	err = get_clock_desc(id, &cd);
-	if (err)
-		return err;
-
-	if (cd.clk->ops.timer_delete)
-		err = cd.clk->ops.timer_delete(cd.clk, kit);
-	else
-		err = -EOPNOTSUPP;
-
-	put_clock_desc(&cd);
-
-	return err;
-}
-
-static void pc_timer_gettime(struct k_itimer *kit, struct itimerspec64 *ts)
-{
-	clockid_t id = kit->it_clock;
-	struct posix_clock_desc cd;
-
-	if (get_clock_desc(id, &cd))
-		return;
-
-	if (cd.clk->ops.timer_gettime)
-		cd.clk->ops.timer_gettime(cd.clk, kit, ts);
-
-	put_clock_desc(&cd);
-}
-
-static int pc_timer_settime(struct k_itimer *kit, int flags,
-			    struct itimerspec64 *ts, struct itimerspec64 *old)
-{
-	clockid_t id = kit->it_clock;
-	struct posix_clock_desc cd;
-	int err;
-
-	err = get_clock_desc(id, &cd);
-	if (err)
-		return err;
-
-	if (cd.clk->ops.timer_settime)
-		err = cd.clk->ops.timer_settime(cd.clk, kit, flags, ts, old);
-	else
-		err = -EOPNOTSUPP;
-
-	put_clock_desc(&cd);
-
-	return err;
-}
-
 const struct k_clock clock_posix_dynamic = {
 	.clock_getres	= pc_clock_getres,
 	.clock_set	= pc_clock_settime,
 	.clock_get	= pc_clock_gettime,
 	.clock_adj	= pc_clock_adjtime,
-	.timer_create	= pc_timer_create,
-	.timer_set	= pc_timer_settime,
-	.timer_del	= pc_timer_delete,
-	.timer_get	= pc_timer_gettime,
 };

[toc] | [next] | [standalone]

#1654026

FromRichard Cochran <richardcochran@gmail.com>
Date2017-05-31 11:10 +0200
Message-ID<tN7NE-2Od-17@gated-at.bofh.it>
In reply to#1653570
On Tue, May 30, 2017 at 11:15:38PM +0200, Thomas Gleixner wrote:
> The only user of this facility is ptp_clock, which does not implement any of
> those functions.
> 
> Remove them to prevent accidental users. Especially the interval timer
> interfaces are now more or less impossible to implement because the
> necessary infrastructure has been confined to the core code. Aside of that
> it's really complex to make these callbacks implemented according to spec
> as the alarm timer implementation demonstrates. If at all then a nanosleep
> callback might be a reasonable extension. For now keep just what ptp_clock
> needs.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Acked-by: Richard Cochran <richardcochran@gmail.com>

For the record, recently someone did try to implement this interface
for the i210 card.  Although there were issues in the implementation,
still I was curious enough to test the performance.

   https://www.mail-archive.com/linuxptp-devel@lists.sourceforge.net/msg01738.html

The result confirmed my expectation that using the normal system clock
sychronized to the PHC using the phc2sys utility yields better
performance than the direct timer_settime() implementation.

The nanosleep() idea might be worthwhile, but only for devices with
directly mapped registers (like in some SoCs) and not for the common
PCIe devices.  In any case, I am happy to see the timer interface
removed, as it is basically useless and gives people false
impressions.

Could you please include this documentation fix, too?

Thanks,
Richard

---8<---
diff --git a/Documentation/ptp/ptp.txt b/Documentation/ptp/ptp.txt
index ae8fef8..11e904e 100644
--- a/Documentation/ptp/ptp.txt
+++ b/Documentation/ptp/ptp.txt
@@ -18,7 +18,6 @@
     - Adjust clock frequency
 
   + Ancillary clock features
-    - One short or periodic alarms, with signal delivery to user program
     - Time stamp external events
     - Period output signals configurable from user space
     - Synchronization of the Linux system time via the PPS subsystem
@@ -48,9 +47,7 @@
    User space programs may control the clock using standardized
    ioctls. A program may query, enable, configure, and disable the
    ancillary clock features. User space can receive time stamped
-   events via blocking read() and poll(). One shot and periodic
-   signals may be configured via the POSIX timer_settime() system
-   call.
+   events via blocking read() and poll().
 
 ** Writing clock drivers

[toc] | [prev] | [next] | [standalone]

#1657342 — [tip:timers/core] posix-clocks: Remove interval timer facility and mmap/fasync callbacks

Fromtip-bot for Thomas Gleixner <tipbot@zytor.com>
Date2017-06-05 10:20 +0200
Subject[tip:timers/core] posix-clocks: Remove interval timer facility and mmap/fasync callbacks
Message-ID<tOVoZ-nU-3@gated-at.bofh.it>
In reply to#1653570
Commit-ID:  3a06c7ac24f9f24ec059cd77c2dbdf7fbfd0aaaf
Gitweb:     http://git.kernel.org/tip/3a06c7ac24f9f24ec059cd77c2dbdf7fbfd0aaaf
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 30 May 2017 23:15:38 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 4 Jun 2017 15:40:22 +0200

posix-clocks: Remove interval timer facility and mmap/fasync callbacks

The only user of this facility is ptp_clock, which does not implement any of
those functions.

Remove them to prevent accidental users. Especially the interval timer
interfaces are now more or less impossible to implement because the
necessary infrastructure has been confined to the core code. Aside of that
it's really complex to make these callbacks implemented according to spec
as the alarm timer implementation demonstrates. If at all then a nanosleep
callback might be a reasonable extension. For now keep just what ptp_clock
needs.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: John Stultz <john.stultz@linaro.org>
Link: http://lkml.kernel.org/r/20170530211656.145036286@linutronix.de

---
 include/linux/posix-clock.h |  22 ---------
 kernel/time/posix-clock.c   | 113 --------------------------------------------
 2 files changed, 135 deletions(-)

diff --git a/include/linux/posix-clock.h b/include/linux/posix-clock.h
index 83b22ae..38d8225 100644
--- a/include/linux/posix-clock.h
+++ b/include/linux/posix-clock.h
@@ -42,12 +42,6 @@ struct posix_clock;
  * @clock_gettime:  Read the current time
  * @clock_getres:   Get the clock resolution
  * @clock_settime:  Set the current time value
- * @timer_create:   Create a new timer
- * @timer_delete:   Remove a previously created timer
- * @timer_gettime:  Get remaining time and interval of a timer
- * @timer_settime: Set a timer's initial expiration and interval
- * @fasync:         Optional character device fasync method
- * @mmap:           Optional character device mmap method
  * @open:           Optional character device open method
  * @release:        Optional character device release method
  * @ioctl:          Optional character device ioctl method
@@ -66,28 +60,12 @@ struct posix_clock_operations {
 	int  (*clock_settime)(struct posix_clock *pc,
 			      const struct timespec64 *ts);
 
-	int  (*timer_create) (struct posix_clock *pc, struct k_itimer *kit);
-
-	int  (*timer_delete) (struct posix_clock *pc, struct k_itimer *kit);
-
-	void (*timer_gettime)(struct posix_clock *pc,
-			      struct k_itimer *kit, struct itimerspec64 *tsp);
-
-	int  (*timer_settime)(struct posix_clock *pc,
-			      struct k_itimer *kit, int flags,
-			      struct itimerspec64 *tsp, struct itimerspec64 *old);
 	/*
 	 * Optional character device methods:
 	 */
-	int     (*fasync)  (struct posix_clock *pc,
-			    int fd, struct file *file, int on);
-
 	long    (*ioctl)   (struct posix_clock *pc,
 			    unsigned int cmd, unsigned long arg);
 
-	int     (*mmap)    (struct posix_clock *pc,
-			    struct vm_area_struct *vma);
-
 	int     (*open)    (struct posix_clock *pc, fmode_t f_mode);
 
 	uint    (*poll)    (struct posix_clock *pc,
diff --git a/kernel/time/posix-clock.c b/kernel/time/posix-clock.c
index 7e45300..bd4fb78 100644
--- a/kernel/time/posix-clock.c
+++ b/kernel/time/posix-clock.c
@@ -82,38 +82,6 @@ static unsigned int posix_clock_poll(struct file *fp, poll_table *wait)
 	return result;
 }
 
-static int posix_clock_fasync(int fd, struct file *fp, int on)
-{
-	struct posix_clock *clk = get_posix_clock(fp);
-	int err = 0;
-
-	if (!clk)
-		return -ENODEV;
-
-	if (clk->ops.fasync)
-		err = clk->ops.fasync(clk, fd, fp, on);
-
-	put_posix_clock(clk);
-
-	return err;
-}
-
-static int posix_clock_mmap(struct file *fp, struct vm_area_struct *vma)
-{
-	struct posix_clock *clk = get_posix_clock(fp);
-	int err = -ENODEV;
-
-	if (!clk)
-		return -ENODEV;
-
-	if (clk->ops.mmap)
-		err = clk->ops.mmap(clk, vma);
-
-	put_posix_clock(clk);
-
-	return err;
-}
-
 static long posix_clock_ioctl(struct file *fp,
 			      unsigned int cmd, unsigned long arg)
 {
@@ -199,8 +167,6 @@ static const struct file_operations posix_clock_file_operations = {
 	.unlocked_ioctl	= posix_clock_ioctl,
 	.open		= posix_clock_open,
 	.release	= posix_clock_release,
-	.fasync		= posix_clock_fasync,
-	.mmap		= posix_clock_mmap,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl	= posix_clock_compat_ioctl,
 #endif
@@ -359,88 +325,9 @@ out:
 	return err;
 }
 
-static int pc_timer_create(struct k_itimer *kit)
-{
-	clockid_t id = kit->it_clock;
-	struct posix_clock_desc cd;
-	int err;
-
-	err = get_clock_desc(id, &cd);
-	if (err)
-		return err;
-
-	if (cd.clk->ops.timer_create)
-		err = cd.clk->ops.timer_create(cd.clk, kit);
-	else
-		err = -EOPNOTSUPP;
-
-	put_clock_desc(&cd);
-
-	return err;
-}
-
-static int pc_timer_delete(struct k_itimer *kit)
-{
-	clockid_t id = kit->it_clock;
-	struct posix_clock_desc cd;
-	int err;
-
-	err = get_clock_desc(id, &cd);
-	if (err)
-		return err;
-
-	if (cd.clk->ops.timer_delete)
-		err = cd.clk->ops.timer_delete(cd.clk, kit);
-	else
-		err = -EOPNOTSUPP;
-
-	put_clock_desc(&cd);
-
-	return err;
-}
-
-static void pc_timer_gettime(struct k_itimer *kit, struct itimerspec64 *ts)
-{
-	clockid_t id = kit->it_clock;
-	struct posix_clock_desc cd;
-
-	if (get_clock_desc(id, &cd))
-		return;
-
-	if (cd.clk->ops.timer_gettime)
-		cd.clk->ops.timer_gettime(cd.clk, kit, ts);
-
-	put_clock_desc(&cd);
-}
-
-static int pc_timer_settime(struct k_itimer *kit, int flags,
-			    struct itimerspec64 *ts, struct itimerspec64 *old)
-{
-	clockid_t id = kit->it_clock;
-	struct posix_clock_desc cd;
-	int err;
-
-	err = get_clock_desc(id, &cd);
-	if (err)
-		return err;
-
-	if (cd.clk->ops.timer_settime)
-		err = cd.clk->ops.timer_settime(cd.clk, kit, flags, ts, old);
-	else
-		err = -EOPNOTSUPP;
-
-	put_clock_desc(&cd);
-
-	return err;
-}
-
 const struct k_clock clock_posix_dynamic = {
 	.clock_getres	= pc_clock_getres,
 	.clock_set	= pc_clock_settime,
 	.clock_get	= pc_clock_gettime,
 	.clock_adj	= pc_clock_adjtime,
-	.timer_create	= pc_timer_create,
-	.timer_set	= pc_timer_settime,
-	.timer_del	= pc_timer_delete,
-	.timer_get	= pc_timer_gettime,
 };

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web