From owner-freebsd-hackers@freebsd.org Sun Jan 5 10:26:54 2020 Return-Path: Delivered-To: freebsd-hackers@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 3DA761DBA4F for ; Sun, 5 Jan 2020 10:26:54 +0000 (UTC) (envelope-from crees@freebsd.org) Received: from mailman.nyi.freebsd.org (mailman.nyi.freebsd.org [IPv6:2610:1c1:1:606c::50:13]) by mx1.freebsd.org (Postfix) with ESMTP id 47rFDt0ybCz48ZZ for ; Sun, 5 Jan 2020 10:26:54 +0000 (UTC) (envelope-from crees@freebsd.org) Received: by mailman.nyi.freebsd.org (Postfix) id 20E981DBA4E; Sun, 5 Jan 2020 10:26:54 +0000 (UTC) Delivered-To: hackers@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 20A861DBA4D for ; Sun, 5 Jan 2020 10:26:54 +0000 (UTC) (envelope-from crees@freebsd.org) Received: from mail50c50.megamailservers.eu (mail166c50.megamailservers.eu [91.136.10.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 47rFDs33lkz48ZX; Sun, 5 Jan 2020 10:26:52 +0000 (UTC) (envelope-from crees@freebsd.org) X-Authenticated-User: crees@uwclub.net DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=megamailservers.eu; s=maildub; t=1578220010; bh=6rf6Mll5em0sqShE2WO8avFETpgjS7tqCLkbTy8qysE=; h=Date:In-Reply-To:References:Subject:To:From:From; b=otijVUCX8rqsmMJTIEBwl/+fwNslJ6aWLZk8HFLPjVhLBKWzZgeBCcVsh55kr5s09 d+p6jPwXozi8+86NNYYy4TxVPP+Hba9FFk7WeKsZBNe3IWyWcpJxDaqi01AB2ZYNRv 26d9CDQMEJOrIbgyxgzKBYRjmc9YSJwRCUvD2eyM= Feedback-ID: crees@freebsd.o Received: from pegasus.bayofrum.net (host-92-16-36-61.as13285.net [92.16.36.61]) (authenticated bits=0) by mail50c50.megamailservers.eu (8.14.9/8.13.1) with ESMTP id 005AQnKT013986; Sun, 5 Jan 2020 10:26:50 +0000 Received: from R.lan (176-35-83-100.xdsl.murphx.net [176.35.83.100]) by pegasus.bayofrum.net (Postfix) with ESMTPSA id BE9D1E5FF; Sun, 5 Jan 2020 10:26:46 +0000 (GMT) Date: Sun, 05 Jan 2020 10:26:50 +0000 User-Agent: K-9 Mail for Android In-Reply-To: <39ea5ba8-00d6-c5b2-4719-fba23bb53856@bayofrum.net> References: <33a74368-73a7-fdc0-4655-cfce88d865e6@FreeBSD.org> <39ea5ba8-00d6-c5b2-4719-fba23bb53856@bayofrum.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: Fixing audio/oss to use callout instead of timeouts To: Chris Rees , hackers@FreeBSD.org, jhb@freebsd.org From: Chris Rees Message-ID: <78949DD3-1152-429C-A685-F736B4B80E55@FreeBSD.org> X-bayofrum-MailScanner-Information: Please contact the ISP for more information X-bayofrum-MailScanner-ID: BE9D1E5FF.AC376 X-bayofrum-MailScanner: Found to be clean X-bayofrum-MailScanner-From: crees@freebsd.org X-Spam-Status: No X-CTCH-RefID: str=0001.0A0B0210.5E11B9EA.001C, ss=1, re=0.000, recu=0.000, reip=0.000, cl=1, cld=1, fgs=0 X-CTCH-VOD: Unknown X-CTCH-Spam: Unknown X-CTCH-Score: 0.000 X-CTCH-Rules: X-CTCH-Flags: 0 X-CTCH-ScoreCust: 0.000 X-CSC: 0 X-CHA: v=2.3 cv=N4FX6F1B c=1 sm=1 tr=0 a=ExehvxHEHCe6C2o3spqpZQ==:117 a=ExehvxHEHCe6C2o3spqpZQ==:17 a=jpOVt7BSZ2e4Z31A5e1TngXxSK0=:19 a=IkcTkHD0fZMA:10 a=Jdjhy38mL1oA:10 a=6I5d2MoRAAAA:8 a=ZB5LerlCAAAA:8 a=rBVGbDY6LXgkKXX8UWsA:9 a=QEXdDO2ut3YA:10 a=IjZwj45LgO3ly-622nXo:22 a=YKPTzOroS2oaEK2QgPcx:22 X-Rspamd-Queue-Id: 47rFDs33lkz48ZX X-Spamd-Bar: - Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-1.92 / 15.00]; local_wl_from(0.00)[freebsd.org]; NEURAL_HAM_MEDIUM(-0.94)[-0.944,0]; NEURAL_HAM_LONG(-0.98)[-0.978,0]; ASN(0.00)[asn:9115, ipnet:91.136.0.0/17, country:GB] X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 05 Jan 2020 10:26:54 -0000 On 4 January 2020 21:29:10 GMT, Chris Rees wrote: >[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=20 >>>> the duplicate), >>>> >>>> I've attempted to use the callout functions instead of the now=20 >>>> removed timeout functions for audio/oss, and I *think* that the >code=20 >>>> already stores and retrieves the list of handlers, so it should be >a=20 >>>> simple swap out. >>>> >>>> I've made this modification and run the module with mpg123 for a=20 >>>> while and it hasn't killed my laptop, but I'd just like to check=20 >>>> 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=20 >>>> done? >>>> >>>> >https://www.bayofrum.net/cgi-bin/fossil/oss/vinfo/ad269d7cbc02bf38?diff=3D2 > >>>> >>>> >>>> (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=3D= 2&dc=3D9999 > >>>> >>> A few suggestions: >>> >>> 1) You should do the callout_init() during a SYSINIT or MOD_LOAD=20 >>> event to initialize >>> =C2=A0=C2=A0=C2=A0 all the timers at once instead of doing it in oss_ti= meout(). >>> >>> 2) You should then add a SYSUNINIT or MOD_UNLOAD that uses=20 >>> callout_drain(). >>> >>> 3) You should add a mutex to protect the tmouts array and=20 >>> timeout_random, etc. >>> =C2=A0=C2=A0=C2=A0 and use callout_init_mtx with that lock, but probabl= y use=20 >>> CALLOUT_RETURNUNLOCKED >>> =C2=A0=C2=A0=C2=A0 and drop the mutex right before calling the function= (you can=20 >>> store the function >>> =C2=A0=C2=A0=C2=A0 pointer and void * argument in local variables befor= e dropping=20 >>> the lock to >>> =C2=A0=C2=A0=C2=A0 be safe. >>> >>> 4) If possible, you should really alter the oss API so that drivers=20 >>> don't use >>> =C2=A0=C2=A0=C2=A0 a timeout()-like interface, but instead use a callou= t directly=20 >>> (or an interface >>> =C2=A0=C2=A0=C2=A0 that wraps callout).=C2=A0 This would let drivers ta= ke advantage of=20 >>> callout_init_mtx >>> =C2=A0=C2=A0=C2=A0 with their own locks, etc. >>> >>> Does audio/oss contain all the code that makes use of oss_timeout? >>> >> Thanks so much for your feedback!=C2=A0 You've probably seen from the >naive=20 >> change my level of kernel hacking. >> >> 1-2, I think I've done here: >> >> >https://www.bayofrum.net/cgi-bin/fossil/oss/vdiff?from=3Dae2a86&to=3Db4376= 737b9d4a127 > >> >> >> (full context:=20 >> >https://www.bayofrum.net/cgi-bin/fossil/oss/vdiff?from=3Dae2a86&to=3Db4376= 737b9d4a127&dc=3D9999) >> >> 3. The function pointer is constant, and the argument is a local=20 >> variable I think. >> >> 4. I agree, but I think it'd kill portability, and to be honest, the=20 >> only reason I even use OSS is just for my CMI8788 soundcard. If I've=20 >> managed this, I'm looking at porting the driver to FreeBSD's native=20 >> sound driver.=C2=A0 I don't think anyone uses OSS for any other reason. >> >> (5.) Yes, it doesn't appear elsewhere. >> >> Have I correctly understood you? >> Ok, I see I slightly misunderstood where you told me to put the locks. https://www.bayofrum.net/cgi-bin/fossil/oss/vdiff?from=3Dae2a86&to=3Da10bd4= 8&dc=3D9999 I'm fairly confident this is what you meant and should be acceptable... I'= ve properly done what you said in (3) now. Chris --=20 This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean.