Subject: callout facility/old style timeout() for NetBSD NDIS
To: None <thorpej@netbsd.org>
From: Alan Ritter <ritter.alan@gmail.com>
List: tech-kern
Date: 01/02/2006 12:50:40
Hi, I ported the FreeBSD NDIS code to NetBSD as part of Google's
"Summer of Code", and now I'm trying to get it up to snuff to
hopefully include in NetBSD-current.

One problem that needs to be fixed has to do with the callout(9)
functions, and I noticed in the "History" section of the manpage that
you adapted it to NetBSD so I thought I'd write to solicit your
advice.

The problem I'm having is that Windows drivers use a "struct ktimer"
structure for timers which is emulated in FreeBSD using
timeout()/untimeout().  As NetBSD doesn't have these functions I
simply re-implemented everything using the newer callout facility.=20
Following is the definition of the ktimer structure I use from
sys/compat/ndis/ntoskrnl_var.h

/*
  * We need to use the timeout()/untimeout() API for ktimers
 * since timers can be initialized, but not destroyed (so
  * malloc()ing our own callout structures would mean a leak,
  * since there'd be no way to free() them). This means we
  * need to use struct callout_handle, which is really just a
 * pointer. To make it easier to deal with, we use a union
 * to overlay the callout_handle over the k_timerlistentry.
 * The latter is a list_entry, which is two pointers, so
 * there's enough space available to hide a callout_handle
 * there.
 */

struct ktimer {
        nt_dispatch_header      k_header;
        uint64_t                k_duetime;
        union {
                list_entry              k_timerlistentry;
#ifdef __FreeBSD__
                struct callout_handle   k_handle;
#else /* __NetBSD__ */
                struct callout                  *k_handle;
#endif
        } u;
        void                    *k_dpc;
        uint32_t                k_period;
};

As you can see I'm just using a pointer to a callout structure which I
malloc() every time a timer is created (thus causing a memory leak).=20
In practice however (for the Windows drivers I've tested so far) only
about 20 or so of these structures get allocated, and only when the
device interface is first set up, so these drivers run fine in NetBSD
even though there is a memory leak.

Of course getting rid of this memory leak is one of my highest
priorities right now, so I'm trying to figure out how to go about
this.  I'm thinking essentially what I'll need to do is re-implement
the old-style timeout()/untimeout() using the newer callout facility.=20
From looking at the FreeBSD code in src/sys/kern/kern_timeout.c:

struct callout_handle
timeout(ftn, arg, to_ticks)
        timeout_t *ftn;
        void *arg;
        int to_ticks;
{
        struct callout *new;
        struct callout_handle handle;

        mtx_lock_spin(&callout_lock);

        /* Fill in the next free callout structure. */
        new =3D SLIST_FIRST(&callfree);
        if (new =3D=3D NULL)
                /* XXX Attempt to malloc first */
                panic("timeout table full");
        SLIST_REMOVE_HEAD(&callfree, c_links.sle);

        callout_reset(new, to_ticks, ftn, arg);

        handle.callout =3D new;
        mtx_unlock_spin(&callout_lock);
        return (handle);
}

It appears that they allocate from a pool of callout structures, and
then schedule the requested function to be called with it's argument.=20
I'm not seeing how the allocated callout structure pointed to by "new"
ends up getting returned to the callfree queue however (I haven't
looked into this in that much detail yet however).

Anyway, I'm just writing to see if you think that re-implementing the
old-style callout facility like they do in FreeBSD is the right thing
to do here, or if I would be better off with a simpler solution just
for NDIS.

Thanks!