Date: Mon, 8 May 2017 22:39:35 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Vladimir Kondratyev <vladimir@kondratyev.su> Cc: Warner Losh <imp@bsdimp.com>, owner-freebsd-arch@freebsd.org, freebsd-arch@freebsd.org Subject: Re: psm(4) & atkbdc(4) locking Message-ID: <20170508193935.GE1622@kib.kiev.ua> In-Reply-To: <80600e8d3da7725a66e2a1e24cbfd916@kondratyev.su> References: <a50871663d281c388e16b146496ed035@kondratyev.su> <CANCZdfrvZ0OuQhRhcpMhxFphMjBuR22HJQTNyL=NDLb1VihJRQ@mail.gmail.com> <80600e8d3da7725a66e2a1e24cbfd916@kondratyev.su>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, May 08, 2017 at 09:58:27PM +0300, Vladimir Kondratyev wrote: > On 2017-05-08 21:39, Warner Losh wrote: > > On Mon, May 8, 2017 at 12:33 PM, Vladimir Kondratyev > > <vladimir@kondratyev.su> wrote: > >> Hi All > >> > >> In order to implement evdev support in psm(4) driver I`m going to add > >> mutexes to psm and atkbdc drivers and mark psm interrupt and cdev > >> handlers > >> MPSAFE. > >> Atkbd(4) is depending on Giant like before. > >> > >> Locking of these drivers seems to be low-hanging fruit as spl() calls > >> are > >> still in place but it has not occurred. > >> > >> Does someone know why it is not done yet? For reasons other than > >> "Nobody > >> took care of it"? > >> Any hidden traps? > > > > Working rock-solid reliably in ddb(4) is what tripped me up when I > > started down this path 10 years ago. > > I tried to avoid atkbd changes as much as possible. Patch adds atkbdc > mutex acquisition > before each hardware access and nothing more. I think mutexes should be > ignored in ddb mode. Locks are not ignored in kdb_active mode, and this is reasonable. Instead, kdb should not acquire locks at all. Consider a situation where you need to send a composite command to the hardware, which consists of several registers accesses, and the whole sequence of accesses is covered by a lock to ensure atomicity. Kdb may be entered at arbitrary moment, including the middle of the lock-protected section. This means that kdb might be entered with the lock owned by some thread, not neccessary the thread which was executing when entrance occured. More, the hardware state is some intermediate state of being commanded the composite sequence, not yet finished. I do not think that atkbd code correctly handles this situation now, and simply adding a lock there probably make things worse. > > > Understanding all the rules for > > that was a bridge too far for me given the time I had for the project > > then. If you have that covered (I haven't looked at the diffs yet), > > you're golden. > > I`m not familiar with ddb internals, that is why i wrote first mail. > > > > > Warner > > > >> Patches can be found here: > >> https://reviews.freebsd.org/D10263 (atkbdc, serialize hw registers > >> access) > >> https://reviews.freebsd.org/D10264 (psm, serialize softc access, mark > >> interrupt and cdev handlers MPSAFE) > >> > > -- > WBR > Vladimir Kondratyev > _______________________________________________ > freebsd-arch@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-arch > To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170508193935.GE1622>