On 2022-04-02 20:15, Ola Liljedahl wrote:
On 4/1/22 17:07, Mattias Rönnblom wrote:
+
+/**
+ * End a read-side critical section.
+ *
+ * A call to this function marks the end of a read-side critical
+ * section, for @p seqlock. The application must supply the sequence
+ * number produced by the corresponding rte_seqlock_read_lock() (or,
+ * in case of a retry, the rte_seqlock_tryunlock()) call.
+ *
+ * After this function has been called, the caller should not access
+ * the protected data.
+ *
+ * In case this function returns true, the just-read data was
+ * consistent and the set of atomic and non-atomic load operations
+ * performed between rte_seqlock_read_lock() and
+ * rte_seqlock_read_tryunlock() were atomic, as a whole.
+ *
+ * In case rte_seqlock_read_tryunlock() returns false, the data was
+ * modified as it was being read and may be inconsistent, and thus
+ * should be discarded. The @p begin_sn is updated with the
+ * now-current sequence number.
+ *
+ * @param seqlock
+ * A pointer to the seqlock.
+ * @param begin_sn
+ * The seqlock sequence number returned by
+ * rte_seqlock_read_lock() (potentially updated in subsequent
+ * rte_seqlock_read_tryunlock() calls) for this critical section.
+ * @return
+ * true or false, if the just-read seqlock-protected data was
consistent
+ * or inconsistent, respectively, at the time it was read.
+ *
+ * @see rte_seqlock_read_lock()
+ */
+__rte_experimental
+static inline bool
+rte_seqlock_read_tryunlock(const rte_seqlock_t *seqlock, uint32_t
*begin_sn)
+{
+ uint32_t end_sn;
+
+ /* make sure the data loads happens before the sn load */
+ rte_atomic_thread_fence(__ATOMIC_ACQUIRE);
+
+ end_sn = __atomic_load_n(&seqlock->sn, __ATOMIC_RELAXED);
Since we are reading and potentially returning the sequence number here
(repeating the read of the protected data), we need to use load-acquire.
I assume it is not expected that the user will call
rte_seqlock_read_lock() again.
Good point.
Seeing this implementation, I might actually prefer the original
implementation, I think it is cleaner.
Me too.
But I would like for the begin
function also to wait for an even sequence number, the end function
would only have to check for same sequence number, this might improve
performance a little bit as readers won't perform one or several broken
reads while a write is in progress. The function names are a different
thing though.
The odd sn should be a rare case, if the seqlock is used for relatively
low frequency update scenarios, which is what I think it should be
designed for.
Waiting for an even sn in read_begin() would exclude the option for the
caller to defer reading the new data to same later time, in case it's
being written. That in turn would force even a single writer to make
sure its thread is not preempted, or risk blocking all lcore worker
cores attempting to read the protected data.
You could complete the above API with a read_trybegin() function to
address that issue, for those who care, but that would force some extra
complexity on the user.
The writer side behaves much more like a lock with mutual exclusion so
write_lock/write_unlock makes sense.
+
+ if (unlikely(end_sn & 1 || *begin_sn != end_sn)) {
+ *begin_sn = end_sn;
+ return false;
+ }
+
+ return true;
+}
+
