Date: Tue, 09 May 2017 00:03:10 +0300 From: Vladimir Kondratyev <vladimir@kondratyev.su> To: Konstantin Belousov <kostikbel@gmail.com> Cc: freebsd-arch@freebsd.org, owner-freebsd-arch@freebsd.org Subject: Re: psm(4) & atkbdc(4) locking Message-ID: <295ff62aa24ca6d9fd1c47ea4544ccd3@kondratyev.su> 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 2017-05-08 22:39, Konstantin Belousov 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. > Sending a composite command does not happen on regular operation. It can happen on initialization, errors, led state changing and some ioctls only, so i can change patch to protect them with giant as before. But receive path is looking more complex as there are 2 data queues that can be filled in both interrupt and polling ways Nevertheless, there is simple fallback way: just add giant lock support to evdev > I do not think that atkbd code correctly handles this situation now, > and simply adding a lock there probably make things worse. > -- WBR Vladimir Kondratyev
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?295ff62aa24ca6d9fd1c47ea4544ccd3>