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

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

Details

Reviewers

rlibby
markj

Commits

rS357644: smr.h: fix build after r357641
rS357641: Fix a race in smr_advance() that could result in unnecessary poll calls.

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 - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jeff created this revision.Feb 2 2020, 12:25 AM

Harbormaster completed remote builds in B29103: Diff 67647.Feb 2 2020, 12:25 AM

Fixes for invariants

Harbormaster completed remote builds in B29104: Diff 67649.Feb 2 2020, 12:39 AM

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 - subversion.

Herald added a subscriber: imp. · View Herald TranscriptFeb 2 2020, 12:41 AM

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

Closed by commit rS357641: Fix a race in smr_advance() that could result in unnecessary poll calls. (authored by jeff). · Explain WhyFeb 6 2020, 8:51 PM

This revision was automatically updated to reflect the committed changes.

jeff added a commit: rS357641: Fix a race in smr_advance() that could result in unnecessary poll calls..

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)