Page MenuHomeFreeBSD

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

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

Details

Summary

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

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

Event Timeline

jeff created this revision.Feb 2 2020, 12:25 AM
jeff updated this revision to Diff 67649.Feb 2 2020, 12:39 AM

Fixes for invariants

jeff edited the summary of this revision. (Show Details)Feb 2 2020, 12:41 AM
jeff added reviewers: rlibby, markj.
jeff set the repository for this revision to rS FreeBSD src repository.
rlibby accepted this revision.Feb 2 2020, 8:37 PM

Looks good.

sys/kern/subr_smr.c
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
jkim added a subscriber: jkim.Feb 6 2020, 9:39 PM

It seems you forgot to define SMR_SEQ_DELTA() macro.

rlibby added a comment.Feb 6 2020, 9:46 PM
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.

jkim added a comment.EditedFeb 6 2020, 9:51 PM

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)