[LinuxPPS] [PATCH 02b/12] PPS: Make pps_event capturing lockless.

[George Spelvin]

Rather than using spinlock mutual exclusion between pps_event
and PPS_FETCH, store captured timestamps into a circular buffer
and atomically update the sequence number to publish them to
PPS_FETCH.

This technically has a race if more than 5 pps_events of a single type
arrive during one PPS_FETCH; given a typical 1 Hz rate, is that really
likely?
---
 drivers/pps/kapi.c  |   46 ++++++++++++++++++++++++----------------------
 drivers/pps/pps.c   |   16 +++++++---------
 drivers/pps/sysfs.c |   14 ++++++++++----
 include/linux/pps.h |    4 ++--
 4 files changed, 43 insertions(+), 37 deletions(-)

diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index 4d6a2ed..77c2b96 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -258,7 +258,8 @@ EXPORT_SYMBOL(pps_unregister_source);
 void
 pps_event(struct pps_device *pps, struct pps_ktime *ts, int event, void *data)
 {
-	unsigned long flags;
+	int mode;
+	u32 seq;
 
 	if (!pps)
 		return;
@@ -273,42 +274,43 @@ pps_event(struct pps_device *pps, struct pps_ktime *ts, int event, void *data)
 	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(pps, 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, pps->id);
+			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, pps->id);
+			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);
+	if (event & mode) {
+		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..92fc76a 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) {
@@ -171,17 +172,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;
 
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 c8cacdc..bc025f2 100644
--- a/include/linux/pps.h
+++ b/include/linux/pps.h
@@ -153,8 +153,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? */
-- 
1.6.0.6