Date: Sat, 4 Jan 2020 21:29:10 +0000 From: Chris Rees <crees@FreeBSD.org> To: Chris Rees <crees@FreeBSD.org>, hackers@FreeBSD.org Subject: Re: Fixing audio/oss to use callout instead of timeouts Message-ID: <39ea5ba8-00d6-c5b2-4719-fba23bb53856@bayofrum.net> In-Reply-To: <ce758663-7517-ad31-284b-ca9985a4f7b0@bayofrum.net> References: <F3750395-7793-4121-8EF1-39884C63E30F@FreeBSD.org> <33a74368-73a7-fdc0-4655-cfce88d865e6@FreeBSD.org> <ce758663-7517-ad31-284b-ca9985a4f7b0@bayofrum.net>
next in thread | previous in thread | raw e-mail | index | archive | help
[Keeping -hackers CC'd as again I've failed with from address :/] On 2020-01-04 21:27, Chris Rees wrote: > Hi John, > > On 2020-01-04 13:39, John Baldwin wrote: >> 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? >> > Thanks so much for your feedback! You've probably seen from the naive > change my level of kernel hacking. > > 1-2, I think I've done here: > > https://www.bayofrum.net/cgi-bin/fossil/oss/vdiff?from=ae2a86&to=b4376737b9d4a127 > > > (full context: > https://www.bayofrum.net/cgi-bin/fossil/oss/vdiff?from=ae2a86&to=b4376737b9d4a127&dc=9999) > > 3. The function pointer is constant, and the argument is a local > variable I think. > > 4. I agree, but I think it'd kill portability, and to be honest, the > only reason I even use OSS is just for my CMI8788 soundcard. If I've > managed this, I'm looking at porting the driver to FreeBSD's native > sound driver. I don't think anyone uses OSS for any other reason. > > (5.) Yes, it doesn't appear elsewhere. > > Have I correctly understood you? > > Chris > > -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?39ea5ba8-00d6-c5b2-4719-fba23bb53856>