[LinuxPPS] [PATCH 02/11] Streamline interrupt handling; get rid of a lock and idr_find()
George Spelvin
linux at horizon.com
Fri Feb 6 14:28:25 CET 2009
This speeds up some code by passing and returninf a struct pps_device *, rather than a
numeric ID that has to be looked up.
---
Documentation/pps/pps.txt | 8 ++-
drivers/pps/clients/ktimer.c | 19 ++++----
drivers/pps/clients/pps-ldisc.c | 26 ++++++-----
drivers/pps/kapi.c | 91 +++++++++++++++++++--------------------
drivers/pps/pps.c | 48 +++++++++++----------
drivers/pps/sysfs.c | 14 ++++--
include/linux/pps.h | 11 +++--
7 files changed, 115 insertions(+), 102 deletions(-)
diff --git a/Documentation/pps/pps.txt b/Documentation/pps/pps.txt
index 125f4ab..8470a69 100644
--- a/Documentation/pps/pps.txt
+++ b/Documentation/pps/pps.txt
@@ -85,12 +85,12 @@ pps_source_info_s as follows:
and then calling the function pps_register_source() in your
intialization routine as follows:
- source = pps_register_source(&pps_ktimer_info,
+ pps = pps_register_source(&pps_ktimer_info,
PPS_CAPTUREASSERT | PPS_OFFSETASSERT);
The pps_register_source() prototype is:
- int pps_register_source(struct pps_source_info_s *info, int default_params)
+ struct pps_source *pps_register_source(struct pps_source_info_s *info, int default_params)
where "info" is a pointer to a structure that describes a particular
PPS source, "default_params" tells the system what the initial default
@@ -98,11 +98,13 @@ parameters for the device should be (it is obvious that these parameters
must be a subset of ones defined in the struct
pps_source_info_s which describe the capabilities of the driver).
+The return value is either a pointer to a pps struct or an ERR_PTR.
+
Once you have registered a new PPS source into the system you can
signal an assert event (for example in the interrupt handler routine)
just using:
- pps_event(source, &ts, PPS_CAPTUREASSERT, ptr)
+ pps_event(pps, &ts, PPS_CAPTUREASSERT, ptr)
where "ts" is the event's timestamp.
diff --git a/drivers/pps/clients/ktimer.c b/drivers/pps/clients/ktimer.c
index 259baa7..64719e0 100644
--- a/drivers/pps/clients/ktimer.c
+++ b/drivers/pps/clients/ktimer.c
@@ -25,6 +25,7 @@
#include <linux/init.h>
#include <linux/time.h>
#include <linux/timer.h>
+#include <linux/err.h>
#include <linux/pps.h>
@@ -32,7 +33,7 @@
* Global variables
*/
-static int source;
+static struct pps_device *source;
static struct timer_list ktimer;
/*
@@ -62,12 +63,12 @@ static void pps_ktimer_event(unsigned long ptr)
* The echo function
*/
-static void pps_ktimer_echo(int source, int event, void *data)
+static void pps_ktimer_echo(int id, int event, void *data)
{
pr_info("echo %s %s for source %d\n",
event & PPS_CAPTUREASSERT ? "assert" : "",
event & PPS_CAPTURECLEAR ? "clear" : "",
- source);
+ id);
}
/*
@@ -98,20 +99,20 @@ static void __exit pps_ktimer_exit(void)
static int __init pps_ktimer_init(void)
{
- int ret;
+ struct pps_device *pps;
- ret = pps_register_source(&pps_ktimer_info,
+ pps = pps_register_source(&pps_ktimer_info,
PPS_CAPTUREASSERT | PPS_OFFSETASSERT);
- if (ret < 0) {
+ if (IS_ERR(pps)) {
printk(KERN_ERR "cannot register ktimer source\n");
- return ret;
+ return PTR_ERR(pps);
}
- source = ret;
+ source = pps;
setup_timer(&ktimer, pps_ktimer_event, 0);
mod_timer(&ktimer, jiffies + HZ);
- pr_info("ktimer PPS source registered at %d\n", source);
+ pr_info("ktimer PPS source registered at %d\n", pps->id);
return 0;
}
diff --git a/drivers/pps/clients/pps-ldisc.c b/drivers/pps/clients/pps-ldisc.c
index 0406f40..bc8b56a 100644
--- a/drivers/pps/clients/pps-ldisc.c
+++ b/drivers/pps/clients/pps-ldisc.c
@@ -29,7 +29,7 @@
static void pps_tty_dcd_change(struct tty_struct *tty, unsigned int status,
struct timespec *ts)
{
- long id = (long) tty->disc_data;
+ struct pps_device *pps = tty->disc_data;
struct timespec __ts;
struct pps_ktime pps_ts;
@@ -42,11 +42,11 @@ static void pps_tty_dcd_change(struct tty_struct *tty, unsigned int status,
pps_ts.nsec = ts->tv_nsec;
/* Now do the PPS event report */
- pps_event(id, &pps_ts, status ? PPS_CAPTUREASSERT : PPS_CAPTURECLEAR,
+ pps_event(pps, &pps_ts, status ? PPS_CAPTUREASSERT : PPS_CAPTURECLEAR,
NULL);
pr_debug("PPS %s at %lu on source #%d\n",
- status ? "assert" : "clear", jiffies, (int) id);
+ status ? "assert" : "clear", jiffies, pps->id);
}
static int pps_tty_open(struct tty_struct *tty)
@@ -54,7 +54,8 @@ static int pps_tty_open(struct tty_struct *tty)
struct pps_source_info info;
struct tty_driver *drv = tty->driver;
int index = tty->index + drv->name_base;
- long ret;
+ struct pps_device *pps;
+ int ret;
info.owner = THIS_MODULE;
info.dev = NULL;
@@ -64,30 +65,31 @@ static int pps_tty_open(struct tty_struct *tty)
PPS_OFFSETASSERT | PPS_OFFSETCLEAR | \
PPS_CANWAIT | PPS_TSFMT_TSPEC;
- ret = pps_register_source(&info, PPS_CAPTUREBOTH | \
+ pps = pps_register_source(&info, PPS_CAPTUREBOTH | \
PPS_OFFSETASSERT | PPS_OFFSETCLEAR);
- if (ret < 0) {
+ if (IS_ERR(pps)) {
pr_err("cannot register PPS source \"%s\"\n", info.path);
- return ret;
+ return PTR_ERR(pps);
}
- tty->disc_data = (void *) ret;
+ tty->disc_data = pps;
/* Should open N_TTY ldisc too */
ret = n_tty_open(tty);
if (ret < 0)
- pps_unregister_source((long) tty->disc_data);
+ pps_unregister_source(tty->disc_data);
- pr_info("PPS source #%d \"%s\" added\n", (int) ret, info.path);
+ pr_info("PPS source #%d \"%s\" added\n", pps->id, info.path);
return 0;
}
static void pps_tty_close(struct tty_struct *tty)
{
- long id = (long) tty->disc_data;
+ struct pps_device *pps = tty->disc_data;
+ int id = pps->id;
- pps_unregister_source(id);
n_tty_close(tty);
+ pps_unregister_source(pps);
pr_info("PPS source #%d removed\n", (int) id);
}
diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index 917b379..c1129f5 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -124,7 +124,8 @@ exit:
* The function returns, in case of success, the PPS source ID.
*/
-int pps_register_source(struct pps_source_info *info, int default_params)
+struct pps_device *
+pps_register_source(struct pps_source_info *info, int default_params)
{
struct pps_device *pps;
int id;
@@ -198,7 +199,7 @@ int pps_register_source(struct pps_source_info *info, int default_params)
pr_info("new PPS source %s at ID %d\n", info->name, id);
- return id;
+ return pps;
free_idr:
spin_lock_irq(&pps_idr_lock);
@@ -211,7 +212,7 @@ kfree_pps:
pps_register_source_exit:
printk(KERN_ERR "pps: %s: unable to register source\n", info->name);
- return err;
+ return ERR_PTR(err);
}
EXPORT_SYMBOL(pps_register_source);
@@ -222,27 +223,19 @@ EXPORT_SYMBOL(pps_register_source);
* the system.
*/
-void pps_unregister_source(int source)
+void pps_unregister_source(struct pps_device *pps)
{
- struct pps_device *pps;
-
- spin_lock_irq(&pps_idr_lock);
- pps = idr_find(&pps_idr, source);
-
if (!pps) {
BUG();
- spin_unlock_irq(&pps_idr_lock);
return;
}
- spin_unlock_irq(&pps_idr_lock);
-
pps_unregister_cdev(pps);
pps_put_source(pps);
}
EXPORT_SYMBOL(pps_unregister_source);
/* pps_event - register a PPS event into the system
- * @source: the PPS source ID
+ * @pps: the pps_device structure
* @ts: the event timestamp
* @event: the event type
* @data: userdef pointer
@@ -252,66 +245,72 @@ EXPORT_SYMBOL(pps_unregister_source);
*
* If an echo function is associated with the PPS source it will be called
* as:
- * pps->info.echo(source, event, data);
+ * pps->info.echo(pps->id, event, data);
+ *
+ * NOTE: This function does no locking. The caller must ensure that
+ * a) calls are serialized, and
+ * b) do not occur after pps_deregister_source is called.
+ * For current PPS clients, this is taken care of by:
+ * serial: a) struct uart_port's lock field, b) ldisc refcount
+ * ktimer: a) struct tvec_base's lock, b) del_timer_sync()
*/
-void pps_event(int source, struct pps_ktime *ts, int event, void *data)
+void
+pps_event(struct pps_device *pps, struct pps_ktime *ts, int event, void *data)
{
- struct pps_device *pps;
- unsigned long flags;
+ int mode;
+ u32 seq;
+
+ if (!pps)
+ return;
if ((event & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR)) == 0) {
printk(KERN_ERR "pps: unknown event (%x) for source %d\n",
- event, source);
+ event, pps->id);
return;
}
-
- pps = pps_get_source(source);
- if (!pps)
- return;
+ event &= PPS_CAPTUREASSERT | PPS_CAPTURECLEAR;
pr_debug("PPS event on source %d at %llu.%06u\n",
pps->id, (unsigned long long) ts->sec, ts->nsec);
- spin_lock_irqsave(&pps->lock, flags);
-
- /* Must call the echo function? */
- if ((pps->params.mode & (PPS_ECHOASSERT | PPS_ECHOCLEAR)))
- pps->info.echo(source, event, data);
-
/* Check the event */
- pps->current_mode = pps->params.mode;
+ mode = pps->params.mode;
if (event & PPS_CAPTUREASSERT) {
+ if (mode & PPS_ECHOASSERT)
+ pps->info.echo(pps->id, PPS_CAPTUREASSERT, data);
/* We have to add an offset? */
- if (pps->params.mode & PPS_OFFSETASSERT)
+ if (mode & PPS_OFFSETASSERT)
pps_add_offset(ts, &pps->params.assert_off_tu);
/* Save the time stamp */
- pps->assert_tu = *ts;
- pps->assert_sequence++;
+ seq = pps->assert_sequence + 1;
+ pps->assert_tu[seq % 4] = *ts;
+ wmb();
+ pps->assert_sequence = seq;
pr_debug("capture assert seq #%u for source %d\n",
- pps->assert_sequence, source);
+ seq, pps->id);
}
if (event & PPS_CAPTURECLEAR) {
+ if (mode & PPS_ECHOCLEAR)
+ pps->info.echo(pps->id, PPS_CAPTURECLEAR, data);
/* We have to add an offset? */
- if (pps->params.mode & PPS_OFFSETCLEAR)
+ if (mode & PPS_OFFSETCLEAR)
pps_add_offset(ts, &pps->params.clear_off_tu);
/* Save the time stamp */
- pps->clear_tu = *ts;
- pps->clear_sequence++;
+ seq = pps->clear_sequence + 1;
+ pps->clear_tu[seq % 4] = *ts;
+ wmb();
+ pps->clear_sequence = seq;
pr_debug("capture clear seq #%u for source %d\n",
- pps->clear_sequence, source);
+ seq, pps->id);
}
- pps->go = ~0;
- wake_up_interruptible(&pps->queue);
-
- kill_fasync(&pps->async_queue, SIGIO, POLL_IN);
-
- spin_unlock_irqrestore(&pps->lock, flags);
-
- /* Now we can release the PPS source for (possible) deregistration */
- pps_put_source(pps);
+ if (event & mode & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR)) {
+ pps->go = ~0;
+ wake_up_interruptible(&pps->queue);
+ kill_fasync(&pps->async_queue, SIGIO, POLL_IN);
+ }
}
EXPORT_SYMBOL(pps_event);
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index aeaa52e..9f6fa02 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -65,6 +65,7 @@ static long pps_cdev_ioctl(struct file *file,
unsigned long ticks;
void __user *uarg = (void __user *) arg;
int __user *iuarg = (int __user *) arg;
+ u32 seq1, seq2;
int err;
switch (cmd) {
@@ -78,8 +79,9 @@ static long pps_cdev_ioctl(struct file *file,
pr_debug("PPS_GETPARAMS: source %d\n", pps->id);
/* Return current parameters */
- err = copy_to_user(uarg, &pps->params,
- sizeof(struct pps_kparams));
+ spin_lock(&pps->lock);
+ err = copy_to_user(uarg, &pps->params, sizeof pps->params);
+ spin_unlock(&pps->lock);
if (err)
return -EFAULT;
@@ -92,7 +94,10 @@ static long pps_cdev_ioctl(struct file *file,
if (!capable(CAP_SYS_TIME))
return -EPERM;
- err = copy_from_user(¶ms, uarg, sizeof(struct pps_kparams));
+ /* Sanity checks */
+ if (!uarg)
+ return -EINVAL;
+ err = copy_from_user(¶ms, uarg, sizeof params);
if (err)
return -EFAULT;
if (!(params.mode & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR))) {
@@ -108,23 +113,21 @@ static long pps_cdev_ioctl(struct file *file,
return -EINVAL;
}
- spin_lock_irq(&pps->lock);
-
- /* Save the new parameters */
- pps->params = params;
-
/* Restore the read only parameters */
if ((params.mode & (PPS_TSFMT_TSPEC | PPS_TSFMT_NTPFP)) == 0) {
/* section 3.3 of RFC 2783 interpreted */
pr_debug("time format unspecified (%x)\n",
params.mode);
- pps->params.mode |= PPS_TSFMT_TSPEC;
+ params.mode |= PPS_TSFMT_TSPEC;
}
if (pps->info.mode & PPS_CANWAIT)
- pps->params.mode |= PPS_CANWAIT;
- pps->params.api_version = PPS_API_VERS;
+ params.mode |= PPS_CANWAIT;
+ params.api_version = PPS_API_VERS;
- spin_unlock_irq(&pps->lock);
+ /* Save the new parameters */
+ spin_lock(&pps->lock);
+ pps->params = params;
+ spin_unlock(&pps->lock);
break;
@@ -140,7 +143,9 @@ static long pps_cdev_ioctl(struct file *file,
case PPS_FETCH:
pr_debug("PPS_FETCH: source %d\n", pps->id);
- err = copy_from_user(&fdata, uarg, sizeof(struct pps_fdata));
+ if (!uarg)
+ return -EINVAL;
+ err = copy_from_user(&fdata, uarg, sizeof fdata);
if (err)
return -EFAULT;
@@ -171,17 +176,14 @@ static long pps_cdev_ioctl(struct file *file,
}
/* Return the fetched timestamp */
- spin_lock_irq(&pps->lock);
-
- fdata.info.assert_sequence = pps->assert_sequence;
- fdata.info.clear_sequence = pps->clear_sequence;
- fdata.info.assert_tu = pps->assert_tu;
- fdata.info.clear_tu = pps->clear_tu;
+ fdata.info.assert_sequence = seq1 = pps->assert_sequence;
+ fdata.info.clear_sequence = seq2 = pps->clear_sequence;
+ read_barrier_depends();
+ fdata.info.assert_tu = pps->assert_tu[seq1 % 4];
+ fdata.info.clear_tu = pps->clear_tu[seq2 % 4];
fdata.info.current_mode = pps->current_mode;
- spin_unlock_irq(&pps->lock);
-
- err = copy_to_user(uarg, &fdata, sizeof(struct pps_fdata));
+ err = copy_to_user(uarg, &fdata, sizeof fdata);
if (err)
return -EFAULT;
@@ -201,7 +203,7 @@ static int pps_cdev_open(struct inode *inode, struct file *file)
struct pps_device, cdev);
int found;
- found = pps_get_source(pps->id) != 0;
+ found = pps_get_source(pps->id) != 0; /* Bump refcount */
if (!found)
return -ENODEV;
diff --git a/drivers/pps/sysfs.c b/drivers/pps/sysfs.c
index b4cc850..84a5634 100644
--- a/drivers/pps/sysfs.c
+++ b/drivers/pps/sysfs.c
@@ -33,26 +33,32 @@ static ssize_t pps_show_assert(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct pps_device *pps = dev_get_drvdata(dev);
+ u32 seq;
if (!(pps->info.mode & PPS_CAPTUREASSERT))
return 0;
+ seq = pps->assert_sequence;
+ rmb();
return sprintf(buf, "%lld.%09d#%d\n",
- (long long) pps->assert_tu.sec, pps->assert_tu.nsec,
- pps->assert_sequence);
+ (long long) pps->assert_tu[seq % 4].sec,
+ pps->assert_tu[seq % 4].nsec, (unsigned)seq);
}
static ssize_t pps_show_clear(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct pps_device *pps = dev_get_drvdata(dev);
+ u32 seq;
if (!(pps->info.mode & PPS_CAPTURECLEAR))
return 0;
+ seq = pps->clear_sequence;
+ rmb();
return sprintf(buf, "%lld.%09d#%d\n",
- (long long) pps->clear_tu.sec, pps->clear_tu.nsec,
- pps->clear_sequence);
+ (long long) pps->clear_tu[seq % 4].sec,
+ pps->clear_tu[seq % 4].nsec, (unsigned)seq);
}
static ssize_t pps_show_mode(struct device *dev,
diff --git a/include/linux/pps.h b/include/linux/pps.h
index d7719b3..2609708 100644
--- a/include/linux/pps.h
+++ b/include/linux/pps.h
@@ -151,8 +151,8 @@ struct pps_device {
__u32 assert_sequence; /* PPS' assert event seq # */
__u32 clear_sequence; /* PPS' clear event seq # */
- struct pps_ktime assert_tu;
- struct pps_ktime clear_tu;
+ struct pps_ktime assert_tu[4];
+ struct pps_ktime clear_tu[4];
int current_mode; /* PPS mode at event time */
int go; /* PPS event is arrived? */
@@ -184,12 +184,13 @@ extern struct device_attribute pps_attrs[];
struct pps_device *pps_get_source(int source);
extern void pps_put_source(struct pps_device *pps);
-extern int pps_register_source(struct pps_source_info *info,
+extern struct pps_device *pps_register_source(struct pps_source_info *info,
int default_params);
-extern void pps_unregister_source(int source);
+extern void pps_unregister_source(struct pps_device *pps);
extern int pps_register_cdev(struct pps_device *pps);
extern void pps_unregister_cdev(struct pps_device *pps);
-extern void pps_event(int source, struct pps_ktime *ts, int event, void *data);
+extern void pps_event(struct pps_device *pps, struct pps_ktime *ts,
+ int event, void *data);
#endif /* __KERNEL__ */
--
1.6.0.6
More information about the LinuxPPS
mailing list