From owner-freebsd-arch@freebsd.org Mon May 8 21:03:37 2017 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 641E7D63BF8; Mon, 8 May 2017 21:03:37 +0000 (UTC) (envelope-from vladimir@kondratyev.su) Received: from corp.infotel.ru (corp.infotel.ru [195.170.219.3]) by mx1.freebsd.org (Postfix) with ESMTP id D499312F6; Mon, 8 May 2017 21:03:36 +0000 (UTC) (envelope-from vladimir@kondratyev.su) Received: from corp (corp.infotel.ru [195.170.219.3]) by corp.infotel.ru (Postfix) with ESMTP id F15841BF70; Tue, 9 May 2017 00:03:29 +0300 (MSK) X-Virus-Scanned: amavisd-new at corp.infotel.ru Received: from corp.infotel.ru ([195.170.219.3]) by corp (corp.infotel.ru [195.170.219.3]) (amavisd-new, port 10024) with ESMTP id 6SF-dpfxiDyJ; Tue, 9 May 2017 00:03:17 +0300 (MSK) Received: from mail.cicgroup.ru (unknown [195.170.219.74]) by corp.infotel.ru (Postfix) with ESMTP id 0A0C61BF69; Tue, 9 May 2017 00:03:17 +0300 (MSK) Received: from mail.cicgroup.ru (localhost [127.0.0.1]) by mail.cicgroup.ru (Postfix) with ESMTP id EAB8A574A91; Tue, 9 May 2017 00:03:12 +0300 (MSK) X-Virus-Scanned: amavisd-new at cicgroup.ru Received: from mail.cicgroup.ru ([127.0.0.1]) by mail.cicgroup.ru (mail.cicgroup.ru [127.0.0.1]) (amavisd-new, port 10024) with SMTP id Wh6Mb0FnBZ9n; Tue, 9 May 2017 00:03:10 +0300 (MSK) Received: from localhost (localhost [127.0.0.1]) by mail.cicgroup.ru (Postfix) with ESMTPA id 64D78574ABD; Tue, 9 May 2017 00:03:10 +0300 (MSK) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Tue, 09 May 2017 00:03:10 +0300 From: Vladimir Kondratyev To: Konstantin Belousov Cc: freebsd-arch@freebsd.org, owner-freebsd-arch@freebsd.org Subject: Re: psm(4) & atkbdc(4) locking In-Reply-To: <20170508193935.GE1622@kib.kiev.ua> References: <80600e8d3da7725a66e2a1e24cbfd916@kondratyev.su> <20170508193935.GE1622@kib.kiev.ua> Message-ID: <295ff62aa24ca6d9fd1c47ea4544ccd3@kondratyev.su> X-Sender: vladimir@kondratyev.su User-Agent: Roundcube Webmail/1.2.2 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 May 2017 21:03:37 -0000 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 >> > 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