svn commit: r243631 - in head/sys: kern sys

[Bruce Evans]

On Mon, 14 Jan 2013, John Baldwin wrote:

> On Monday, January 14, 2013 10:51:27 am Alexander Motin wrote:
>> As I've actually written, there are two different things:
>>  ncallout -- number of preallocated callout structures for purposes of
>> timeout() calls. That is a legacy API that is probably not very much
>> used now, so that value don't need to be too big. But that allocation is
>> static and if it will ever be exhausted system will panic. That is why
>> it was set quite high. The right way now would be to analyze where that
>> API is still used and estimate the really required number.
>
> FYI, I have slowly been working through the tree fixing users of timeout()
> to use callout_*() instead.

But timeout() is a better API, starting with its name, for the simple
cases that it handles.  It is easier to use since the storage allocation
for it is handled automatically.  Since it returns a handle, the
automatic storage allocation for it doesn't need to be static.  (The
allocation is not quite static, since ncallout is variable, but I will
describe it as static.)  The static allocation is just a minor
optimization for efficiency and simplicity (the callout API gives
further minor optimizations by allocating even more statically in
callers, so that no linked list management is needed and there is
better locality).  It can be replaced by a malloc() on every call to
timeout(), or maybe a buffer pool.  The static allocation is equivalent
to a buffer pool that is never expanded and has no fallback to malloc()
when it is too small.  Since there is no fallback, the pool has to be
very large (but not 512MB) to prevent panics.  Since use of timeout()
has rotted, there aren't many callers of it left, so if there was a
fallback then a very small buffer pool of size say 16 entries would
suffice.  Larger systems and newer ones that start using the better
timeout() API might need as many as 256 entries.  However, buffer pools
are what you use when the system's malloc() is too slow to use.  Since
malloc(9) or at least uma_zalloc(9) is not all that slow, it is probably
efficient enough to just use it, at least while timeout() is just a
compatibility wrapper.  There are already some minor efficiencies from
using the wrapper.  By using the system's malloc() and not using a
buffer pool, timeout() becomes even simpler than before: delete ncallout
and all associated code, and replace the linked list by malloc()'s
internal management:

% struct callout_handle
% timeout(ftn, arg, to_ticks)
% 	timeout_t *ftn;
% 	void *arg;
% 	int to_ticks;
% {
% 	struct callout_cpu *cc;
% 	struct callout *new;
% 	struct callout_handle handle;
% 
% 	cc = CC_CPU(timeout_cpu);
% 	CC_LOCK(cc);

All the locking becomes unnecessary too.  The locking might be just as
slow as dynamic allocation, especially if the lock is contended.

% 	/* Fill in the next free callout structure. */
% 	new = SLIST_FIRST(&cc->cc_callfree);

This would have to be malloc()ed.  There seems to be a problem with
waiting being impossible in some contexts, so malloc() might fail, but
this function can't fail.  So a large buffer pool might be needed after
all to handle cases where malloc() fails :-(.  Also, it must be checked
that there are no caller's of timeout() before malloc() is initialized,
or else the current early initialization of the buffer pool is still
needed for these early callers.

% 	if (new == NULL)
% 		/* XXX Attempt to malloc first */
% 		panic("timeout table full");

This code always knew that it shouldn't panic.

% 	SLIST_REMOVE_HEAD(&cc->cc_callfree, c_links.sle);
% 	callout_reset(new, to_ticks, ftn, arg);
% 	handle.callout = new;
% 	CC_UNLOCK(cc);
% 
% 	return (handle);
% }

Bruce