Date: Mon, 8 May 2017 13:48:42 -0600 From: Warner Losh <imp@bsdimp.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Vladimir Kondratyev <vladimir@kondratyev.su>, owner-freebsd-arch@freebsd.org, "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org> Subject: Re: psm(4) & atkbdc(4) locking Message-ID: <CANCZdfr-B_EBLPb2mbeNd12u6PduVrzghDTbMiN4eGAhTZEhbw@mail.gmail.com> In-Reply-To: <20170508193935.GE1622@kib.kiev.ua> References: <a50871663d281c388e16b146496ed035@kondratyev.su> <CANCZdfrvZ0OuQhRhcpMhxFphMjBuR22HJQTNyL=NDLb1VihJRQ@mail.gmail.com> <80600e8d3da7725a66e2a1e24cbfd916@kondratyev.su> <20170508193935.GE1622@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, May 8, 2017 at 1:39 PM, Konstantin Belousov <kostikbel@gmail.com> wrote: > 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. Yes. This is the snag I ran into... > I do not think that atkbd code correctly handles this situation now, > and simply adding a lock there probably make things worse. It did for me, since breaking to keyboard was totally broken by the changes I made to try to lock things since it was quite likely to interrupt things when locks were held... I had a very naive implementation, which wasn't up to the task, so some care is needed. But this was in the 5.x or 6.x time frame, and the locking situation wrt GIANT is much better today than then... I don't think it's a huge problem, but just one that the implementor needs to be aware of... Since I was unaware of all the details up front, I came up with a solution that couldn't possibly work so I abandoned it. Warner >> > 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?CANCZdfr-B_EBLPb2mbeNd12u6PduVrzghDTbMiN4eGAhTZEhbw>