The CS_DRAIN flag cannot be set at the same time like the async-drain function
pointer is set. These are orthogonal features. Assert this at the beginning
of the function.
Before:
if (flags & CS_DRAIN) {
/* FALLTHROUGH */
} else if (xxx) {
return yyy;
}
if (drain) {
zzz = drain;
}
After:
if (flags & CS_DRAIN) {
/* FALLTHROUGH */
} else if (xxx) {
return yyy;
} else {
if (drain) {
zzz = drain;
}
}
Reviewed by: markj@
Tested by: callout_test
Differential Revision: https://reviews.freebsd.org/D26285
MFC after: 1 week
Sponsored by: Mellanox Technologies // NVIDIA Networking
The intent seems to be zeroing all of the cc_cpu array, or its singleton on
such platforms. The assumption made is that the BSP is always zero. The
code smell was introduced in r326218, which changed the prior explicit zero
to 'curcpu'. The change is only valid if curcpu continues to be zero,
contrary to the aim expressed in that commit message.
So, more succinctly, the expression could be: memset(cc_cpu,0,sizeof(cc_cpu)).
However, there's no point. cc_cpu lives in the data section and has a zero
initial value already. So this revision just removes the problematic
statement.
No functional change. Appeases a (false positive, ish) Coverity CID.
CID: 1383567
Reported by: Puneeth Jothaiah <puneethkumar.jothaia AT dell.com>
Reviewed by: kib
Sponsored by: Dell EMC Isilon
Differential Revision: https://reviews.freebsd.org/D24089
This typedef is the same as timeout_t except that it is in the callout
namespace and header.
Use this typedef in various places of the callout implementation that
were either using the raw type or timeout_t.
While here, add <sys/callout.h> to the manpage.
Reviewed by: kib, imp
MFC after: 1 month
Differential Revision: https://reviews.freebsd.org/D22751
Save the last callout function pointer (and its argument) executed
on each CPU for inspection by a debugger. Add a ddb `show callout_last`
command to show these pointers. Add a kernel module that I used
for testing that command.
Relocate `ce_migration_cpu` to reduce padding and therefore preserve
the size of `struct callout_cpu` (320 bytes on amd64) despite the
added members.
This should help diagnose reference-after-free bugs where the
callout's mutex has already been freed when `softclock_call_cc`
tries to unlock it.
You might hope that the pointer would still be available, but it
isn't. The argument to that function is on the stack (because
`softclock_call_cc` uses it later), and that might be enough in
some cases, but even then, it's very laborious. A pointer to the
callout is saved right before these newly added fields, but that
callout might have been freed. We still have the pointer to its
associated mutex, and the name within might be enough, but it might
also have been freed.
Reviewed by: markj jhb
MFC after: 2 weeks
Sponsored by: Dell EMC Isilon
Differential Revision: https://reviews.freebsd.org/D20794
Mainly focus on files that use BSD 3-Clause license.
The Software Package Data Exchange (SPDX) group provides a specification
to make it easier for automated tools to detect and summarize well known
opensource licenses. We are gradually adopting the specification, noting
that the tags are considered only advisory and do not, in any way,
superceed or replace the license texts.
Special thanks to Wind River for providing access to "The Duke of
Highlander" tool: an older (2014) run over FreeBSD tree was useful as a
starting point.
A long long time ago the register keyword told the compiler to store
the corresponding variable in a CPU register, but it is not relevant
for any compiler used in the FreeBSD world today.
ANSIfy related prototypes while here.
Reviewed by: cem, jhb
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D10193
The callout may reschedule itself and execute again before callout_drain()
returns, but we should not clear CALLOUT_ACTIVE until the callout is
stopped.
Tested by: pho
MFC after: 2 weeks
Sponsored by: Dell EMC Isilon
The callout subsystem already handles early callouts and schedules
the first clock interrupt appropriately based on the currently pending
callouts. The one nit to fix was that callouts scheduled via C_HARDCLOCK
during early boot could fire too early once timers were enabled as the
per-CPU base time is always zero until timers are initialized. The change
in callout_when() handles this case by using the current uptime as the
base time of the callout during bootup if the per-CPU base time is zero.
Reviewed by: kib
MFC after: 2 weeks
Sponsored by: Netflix
it (either async or sync drain).
At this moment the only user of drain is TCP, but TCP wouldn't reschedule a
callout after it has drained it, since it drains only when a tcpcb is closed.
This for now the problem isn't observed.
Submitted by: rrs
callout_when(9). See the man page update for the description of the
intended use.
Tested by: pho
Reviewed by: jhb, bjk (man page updates)
Sponsored by: The FreeBSD Foundation
MFC after: 1 month
X-Differential revision: https://reviews.freebsd.org/D7137
fixes that I think closes up the races Gleb was
looking for. This is running quite nicely in Netflix and
now no longer causes TCP-tcb leaks.
Differential Revision: 7135
not scheduled -> scheduled -> running -> not scheduled. The API and the
manual page assume that, some comments in the code assume that, and looks
like some contributors to the code also did. The problem is that this
paradigm isn't true. A callout can be scheduled and running at the same
time, which makes API description ambigouous. In such case callout_stop()
family of functions/macros should return 1 and 0 at the same time, since it
successfully unscheduled future callout but the current one is running.
Before this change we returned 1 in such a case, with an exception that
if running callout was migrating we returned 0, unless CS_MIGRBLOCK was
specified.
With this change, we now return 0 in case if future callout was unscheduled,
but another one is still in action, indicating to API users that resources
are not yet safe to be freed.
However, the sleepqueue code relies on getting 1 return code in that case,
and there already was CS_MIGRBLOCK flag, that covered one of the edge cases.
In the new return path we will also use this flag, to keep sleepqueue safe.
Since the flag CS_MIGRBLOCK doesn't block migration and now isn't limited to
migration edge case, rename it to CS_EXECUTING.
This change fixes panics on a high loaded TCP server.
Reviewed by: jch, hselasky, rrs, kib
Approved by: re (gjb)
Differential Revision: https://reviews.freebsd.org/D7042
panic string again if set, in case it scrolled out of the active
window. This avoids having to remember the symbol name.
Also add a show callout <addr> command to DDB in order to inspect
some struct callout fields in case of panics in the callout code.
This may help to see if there was memory corruption or to further
ease debugging problems.
Obtained from: projects/vnet
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Reviewed by: jhb (comment only on the show panic initally)
Differential Revision: https://reviews.freebsd.org/D4527
but next invocation is cancelled while migrating,
sleepq_check_timeout() needs to be informed that the callout is
stopped. Otherwise the thread switches off CPU and never become
runnable, since running callout could have already raced with us,
while the migrating and cancelled callout could be one which is
expected to set TDP_TIMOFAIL flag for us. This contradicts with the
expected behaviour of callout_stop() for other callers, which
e.g. decrement references from the callout callbacks.
Add a new flag CS_MIGRBLOCK requesting report of the situation as
'successfully stopped'.
Reviewed by: jhb (previous version)
Tested by: cognet, pho
PR: 200992
Sponsored by: The FreeBSD Foundation
MFC after: 2 weeks
Differential revision: https://reviews.freebsd.org/D5221
- Use SDT_PROBE<N>() instead of SDT_PROBE(). This has no functional effect
at the moment, but will be needed for some future changes.
- Don't hardcode the module component of the probe identifier. This is
set automatically by the SDT framework.
MFC after: 1 week
should be used by TCP for sure in its cleanup of the IN-PCB (will be coming shortly).
Sponsored by: Netflix Inc.
Differential Revision: https://reviews.freebsd.org/D4076
SDT_PROBE requires 5 parameters whereas SDT_PROBE<n> requires n parameters
where n is typically smaller than 5.
Perhaps SDT_PROBE should be made a private implementation detail.
MFC after: 20 days
branch.
This function is used to drain a callout via a callback instead of
blocking the caller until the drain is complete. Refer to the
callout_drain_async() manual page for a detailed description.
Limitation: If a lock is used with the callout, the callout can only
be drained asynchronously one time unless the callout_init_mtx()
function is called again. This limitation is not present in
projects/hps_head and will require more invasive changes to the
timeout code, which was not in the scope of this patch.
Differential Revision: https://reviews.freebsd.org/D3521
Reviewed by: wblock
MFC after: 1 month
The typo was introduced in r278469 / 344ecf88af.
As a result of the bug there was a timing window where callout_reset()
would fail to cancel a concurrent execution of a callout that is about
to start and would schedule the callout again.
The callout would fire more times than it is scheduled.
That would happen even if the callout is initialized with a lock.
For example, the bug triggered the "Stray timeout" assertion in
taskqueue_timeout_func().
MFC after: 5 days
it helps only the TCP timers callout(9) usage. As the benefit for
others callout(9) usages did not reach a consensus the historical
usage should prevail.
Differential Revision: https://reviews.freebsd.org/D3078
being serviced return 0 (fail) but it is applicable only
mpsafe callouts. Thanks to hselasky for finding this.
Differential Revision: https://reviews.freebsd.org/D3078 (Updated)
Submitted by: hselasky
Reviewed by: jch
being serviced and indeed unstoppable.
A scenario to reproduce this case is:
- the callout is being serviced and at same time,
- callout_reset() is called on this callout that sets
the CALLOUT_PENDING flag and at same time,
- callout_stop() is called on this callout and returns 1 (success)
even if the callout is indeed currently running and unstoppable.
This issue was caught up while making r284245 (D2763) workaround, and
was discussed at BSDCan 2015. Once applied the r284245 workaround
is not needed anymore and will be reverted.
Differential Revision: https://reviews.freebsd.org/D3078
Reviewed by: jhb
Sponsored by: Verisign, Inc.
CPU, also add protection against invalid CPU's as well as
split c_flags and c_iflags so that if a user plays with the active
flag (the one expected to be played with by callers in MPSAFE) without
a lock, it won't adversely affect the callout system by causing a corrupt
list. This also means that all callers need to use the macros and *not*
play with the falgs directly (like netgraph used to).
Differential Revision: htts://reviews.freebsd.org/D1894
Reviewed by: .. timed out but looked at by jhb, imp, adrian hselasky
tested by hiren and netflix.
Sponsored by: Netflix Inc.
code in my last commit. The cc_exec_next is used to track the next
when a direct call is being made from callout. It is *never* used
in the in-direct method. When macro-izing I made it so that it
would separate out direct/vs/non-direct. This is incorrect and can
cause panics as Peter Holm has found for me (Thanks so much Peter for
all your help in this). What this change does is restore that behavior
but also get rid of the cc_next from the array and instead make it
be part of the base callout structure. This way no one else will get
confused since we will never use it for non-direct.
Reviewed by: Peter Holm and more importantly tested by him ;-)
MFC after: 3 days.
Sponsored by: Netflix Inc.
is being done in the callout code and harmonizes the macro
use.:
1) The callout_active() will lie. Basically if a migration
is occuring and the callout is about to expire and the
migration has been deferred, the callout_active will no
longer return true until after the migration. This confuses
and breaks callers that are doing callout_init(&c, 1); such
as TCP.
2) The migration code had a bug in it where when migrating, if
a two calls to callout_reset came in and they both collided with
the callout on the wheel about to run, then the second call to
callout_reset would corrupt the list the callout wheel uses
putting the callout thread into a endless loop.
3) Per imp, I have fixed all the macro occurance in the code that
were for the most part being ignored.
Phabricator D1711 and looked at by lstewart and jhb and sbruno.
Reviewed by: kostikbel, imp, adrian, hselasky
MFC after: 3 days
Sponsored by: Netflix Inc.
being held before sleeping.
This has bitten me (in ath(4)) once before and I'd like to see this
not bite anyone else.
Differential Revision: D1638
Reviewed by: jhb, hselasky
MFC after: 1 week
