Page MenuHomeFreeBSD

callout: provide CALLOUT_TRYLOCK flag
ClosedPublic

Authored by glebius on Jun 26 2024, 3:56 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Oct 28, 5:11 AM
Unknown Object (File)
Thu, Oct 24, 5:23 PM
Unknown Object (File)
Thu, Oct 17, 12:49 AM
Unknown Object (File)
Sun, Oct 13, 9:42 AM
Unknown Object (File)
Fri, Oct 11, 3:46 PM
Unknown Object (File)
Fri, Oct 11, 3:46 PM
Unknown Object (File)
Thu, Oct 10, 5:58 AM
Unknown Object (File)
Oct 7 2024, 7:40 PM
Subscribers

Details

Summary

If a callout was initialized with the flag, then the callout(9) system
will not drop the callwheel lock in softclock_call_cc() to obtain the
callout lock. Instead it will use try-lock semantic to obtain the
callout's lock. In case of a failure the callout will be rescheduled to
the nearest cycle reusing the original precision value. The main benefit
of such behavior is not the avoidance of the lock contention in the
callout thread, but the fact that callout with such flag can be actually
stopped in a safe manner, because the race window in the beginning of
softclock_call_cc() is closed.

Call of callout_stop() on such a callout would guarantee that nothing will
be executed after callout_stop() returns, neither callout lock will be
dereferenced. A callout marked as CALLOUT_TRYLOCK |
CALLOUT_RETURNUNLOCKED can call callout_stop() from the callout function
itself (0, a failure to stop, will be returned), then unlock the lock and
then free the memory containing the callout structure.

Caveat: when calling callout_stop() from outside the callout function, the
return value from callout_stop() is still inconsistent. A race window at
the end of softclock_call_cc() still exists, so callout_stop() may report
failure to stop, which would not be a true.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 58361
Build 55249: arc lint + arc unit

Event Timeline

jtl added a subscriber: jtl.

Overall, I think this approach has value. See my in-line comments for suggestions on things to review further.

sys/kern/kern_timeout.c
685

Just a note to say that it looks like this is going to change the way callout_sbt_reset_on() works. Previously, that function could cancel a callout which was waiting on the INP_LOCK. Now, it will work differently and I'm not sure it still makes sense to have the cancellation concept there for callouts with CALLOUT_TRYLOCK defined.

699

I wonder if this should be moved into the portion where we hold the CC_LOCK for the new TRYLOCK cases where we know we aren't going to wait on a lock. That might (or might not?) help address my comment about the interactions with callout_reset_sbt_on().

This revision is now accepted and ready to land.Jun 26 2024, 5:25 PM
sys/kern/kern_timeout.c
680

Just a quick note that you may want to check for (and avoid) the case which would produce a tight loop here, which is possible if direct is true and cc->cc_lastscan is the current bucket. I would almost go so far as to suggest that you should add (1 << (32 - CC_HASH_SHIFT)) to cc->cc_lastscan when direct is true.

sys/kern/kern_timeout.c
680

This is what I dislike in this design, and what I proposed to replace by a function call. The user function would define the policy what to do with the failed trylock, instead of the voluntarily reschedule in the current or next slot.

682

Reschedule forward with 50% of precision.

This revision now requires review to proceed.Aug 22 2024, 3:17 PM

Man page should be updated

This revision is now accepted and ready to land.Aug 23 2024, 9:33 AM
This revision was automatically updated to reflect the committed changes.