Page MenuHomeFreeBSD

Fix a race in smr_advance() that could result in unnecessary poll calls.

Authored by jeff on Feb 2 2020, 12:25 AM.



If we load rd_seq after goal rd_seq may have already advanced beyond goal making the comparison incorrect.

While here fix up the limits on invariants kernels because it was polling way too frequently.

Diff Detail

rS FreeBSD src repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jeff added reviewers: rlibby, markj.
jeff set the repository for this revision to rS FreeBSD src repository.

Looks good.

199–210 ↗(On Diff #67649)

Is there any difference to the cost of a release barrier followed by an acquire barrier vs a full barrier?

This revision is now accepted and ready to land.Feb 2 2020, 8:37 PM

It seems you forgot to define SMR_SEQ_DELTA() macro.

In D23464#516865, @jkim wrote:

It seems you forgot to define SMR_SEQ_DELTA() macro.

It was in the review, but it looks like sys/sys/smr.h was left out of commit r357641. Thanks for reporting.

How about something like this?

Index: sys/sys/smr.h
--- sys/sys/smr.h       (revision 357642)
+++ sys/sys/smr.h       (working copy)
@@ -45,10 +45,11 @@
  * Modular arithmetic for comparing sequence numbers that have
  * potentially wrapped.  Copied from tcp_seq.h.
-#define        SMR_SEQ_LT(a, b)        ((int32_t)((a)-(b)) < 0)
-#define        SMR_SEQ_LEQ(a, b)       ((int32_t)((a)-(b)) <= 0)
-#define        SMR_SEQ_GT(a, b)        ((int32_t)((a)-(b)) > 0)
-#define        SMR_SEQ_GEQ(a, b)       ((int32_t)((a)-(b)) >= 0)
+#define        SMR_SEQ_DELTA(a, b)     (int32_t)((a) - (b))
+#define        SMR_SEQ_LT(a, b)        (SMR_SEQ_DELTA(a, b) < 0)
+#define        SMR_SEQ_LEQ(a, b)       (SMR_SEQ_DELTA(a, b) <= 0)
+#define        SMR_SEQ_GT(a, b)        (SMR_SEQ_DELTA(a, b) > 0)
+#define        SMR_SEQ_GEQ(a, b)       (SMR_SEQ_DELTA(a, b) >= 0)