Date: Sat, 4 Jan 2020 05:39:31 -0800 From: John Baldwin <jhb@FreeBSD.org> To: Chris Rees <crees@FreeBSD.org>, hackers@FreeBSD.org Subject: Re: Fixing audio/oss to use callout instead of timeouts Message-ID: <33a74368-73a7-fdc0-4655-cfce88d865e6@FreeBSD.org> In-Reply-To: <F3750395-7793-4121-8EF1-39884C63E30F@FreeBSD.org> References: <F3750395-7793-4121-8EF1-39884C63E30F@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 1/2/20 3:47 PM, Chris Rees wrote: > Hi Hackers (and perhaps John, as the author of r355732, sorry for the duplicate), > > I've attempted to use the callout functions instead of the now removed timeout functions for audio/oss, and I *think* that the code already stores and retrieves the list of handlers, so it should be a simple swap out. > > I've made this modification and run the module with mpg123 for a while and it hasn't killed my laptop, but I'd just like to check that I have the principle correct and haven't missed anything obvious. > > Please would you let me know if there is anything else I should have done? > > https://www.bayofrum.net/cgi-bin/fossil/oss/vinfo/ad269d7cbc02bf38?diff=2 > > (This is the change I've made to kernel/OS/FreeBSD/os_freebsd.c in oss) > > https://www.bayofrum.net/cgi-bin/fossil/oss/vinfo/ad269d7cbc02bf38?diff=2&dc=9999 A few suggestions: 1) You should do the callout_init() during a SYSINIT or MOD_LOAD event to initialize all the timers at once instead of doing it in oss_timeout(). 2) You should then add a SYSUNINIT or MOD_UNLOAD that uses callout_drain(). 3) You should add a mutex to protect the tmouts array and timeout_random, etc. and use callout_init_mtx with that lock, but probably use CALLOUT_RETURNUNLOCKED and drop the mutex right before calling the function (you can store the function pointer and void * argument in local variables before dropping the lock to be safe. 4) If possible, you should really alter the oss API so that drivers don't use a timeout()-like interface, but instead use a callout directly (or an interface that wraps callout). This would let drivers take advantage of callout_init_mtx with their own locks, etc. Does audio/oss contain all the code that makes use of oss_timeout? -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?33a74368-73a7-fdc0-4655-cfce88d865e6>