Allow caller of callout_stop_safe() to interpret one case of stop failure.
ClosedPublic
Actions

Authored by kib on Feb 6 2016, 7:14 PM.

Details

Reviewers

jhb
rrs

Commits

rS296320: If callout_stop_safe() noted that the callout is currently executing,

Summary

So from rrs feedback on D3115, some uses of callout_stop_safe() do want the return value of 0 when we prevented the migration, while other cases, in particular, sleepq use, require it to report that the callout was running. I do not see any other way around than to allow caller to explicitely request the desired behaviour (assuming the statement of the tcp code wanting the return (0) is true).

The patch below does just that, and from cognet@ testing, it solves the issue for sleepqueue.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kib updated this revision to Diff 13097.Feb 6 2016, 7:14 PM

kib retitled this revision from to Allow caller of callout_stop_safe() to interpret one case of stop failure..

kib updated this object.

kib edited the test plan for this revision. (Show Details)

kib added reviewers: jhb, rrs.

kib set the repository for this revision to rS FreeBSD src repository - subversion.

Herald added a subscriber: imp. · View Herald TranscriptFeb 6 2016, 7:14 PM

kib mentioned this in D3115: Fix return value for callout_stop_safe(9)..Feb 6 2016, 7:14 PM

cognet added a subscriber: cognet.Feb 6 2016, 7:37 PM

In essence, the situation shoul be clear: there is a previous instance of callback running, and there is scheduled new instance. The new instance is cancelled, the previous is not. For sleepqueue it is critical to know that the new callback would be never run, otherwise sleepq code waits for its action and the thread never becomes runnable.

emaste added a subscriber: emaste.Feb 24 2016, 2:15 PM

jhb accepted this revision.Mar 1 2016, 6:29 PM

jhb edited edge metadata.

jhb added inline comments.

sys/kern/kern_timeout.c
1165 ↗	(On Diff #13097)	Possibly rename 'safe' to 'flags' now that it is a flags field rather than a boolean.
sys/sys/callout.h
65 ↗	(On Diff #13097)	It might be nice to document these flags here.