psm(4) & atkbdc(4) locking

[Vladimir Kondratyev]

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 at 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