From owner-svn-src-all@FreeBSD.ORG Sat Jul 12 16:53:52 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id B5BB77CC; Sat, 12 Jul 2014 16:53:52 +0000 (UTC) Received: from mail-wi0-x236.google.com (mail-wi0-x236.google.com [IPv6:2a00:1450:400c:c05::236]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id D818A21DD; Sat, 12 Jul 2014 16:53:51 +0000 (UTC) Received: by mail-wi0-f182.google.com with SMTP id d1so683970wiv.3 for ; Sat, 12 Jul 2014 09:53:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=s98RoL5u9pAX6WSYKIMjEcsaUQo/QRbWiK5mKy3q+ys=; b=gAAHKJ8jgNfYjlTpZWcIwcGf+Z4Q22bCXIoMQ8E4XUiAYov+8hp1jjueBR+VYEha8C lbItTdT5/Mgk24jY/LwMU8F+/FjpZcONis/i7H4xF2tvOAvTmJscOnQZGBOISmvQnNvO eo3c1LiPoyAs2/SumRgGJsKOemeRS3ttEUKUJpGnO+AmMM/tVMGIaynDxuj4wFp+OxUU MRzs7dmLQTBQz0bt+eGgmZPvyDVeOQOMRD+/8MlznoQhsMwCrTA4ahHuUEOgRLL/1NQg VyrJ+PsldM90/X03VggR0fwVVBdwFYGCVPumG+QfI+leUK6RVVZ6KeoFs7UgmYiwurci rCrQ== X-Received: by 10.194.87.97 with SMTP id w1mr7701063wjz.42.1405184030028; Sat, 12 Jul 2014 09:53:50 -0700 (PDT) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by mx.google.com with ESMTPSA id w5sm8594662wif.3.2014.07.12.09.53.48 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Sat, 12 Jul 2014 09:53:49 -0700 (PDT) Date: Sat, 12 Jul 2014 18:53:47 +0200 From: Mateusz Guzik To: Konstantin Belousov Subject: Re: svn commit: r268570 - head/sys/kern Message-ID: <20140712165346.GA16884@dft-labs.eu> References: <201407121535.s6CFZ42f063120@svn.freebsd.org> <20140712161801.GS93733@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20140712161801.GS93733@kib.kiev.ua> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Mateusz Guzik X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 12 Jul 2014 16:53:52 -0000 On Sat, Jul 12, 2014 at 07:18:01PM +0300, Konstantin Belousov wrote: > On Sat, Jul 12, 2014 at 03:35:04PM +0000, Mateusz Guzik wrote: > > Author: mjg > > Date: Sat Jul 12 15:35:04 2014 > > New Revision: 268570 > > URL: http://svnweb.freebsd.org/changeset/base/268570 > > > > Log: > > Clear nonblock and async on devctl close instaed of open. > > > > This is a purely cosmetic change. > > > > Modified: > > head/sys/kern/subr_bus.c > > > > Modified: head/sys/kern/subr_bus.c > > ============================================================================== > > --- head/sys/kern/subr_bus.c Sat Jul 12 15:19:30 2014 (r268569) > > +++ head/sys/kern/subr_bus.c Sat Jul 12 15:35:04 2014 (r268570) > > @@ -438,8 +438,6 @@ devopen(struct cdev *dev, int oflags, in > > } > > /* move to init */ > > devsoftc.inuse = 1; > > - devsoftc.nonblock = 0; > > - devsoftc.async = 0; > > mtx_unlock(&devsoftc.mtx); > > return (0); > > } > > @@ -450,6 +448,8 @@ devclose(struct cdev *dev, int fflag, in > > > > mtx_lock(&devsoftc.mtx); > > devsoftc.inuse = 0; > > + devsoftc.nonblock = 0; > > + devsoftc.async = 0; > > cv_broadcast(&devsoftc.cv); > > funsetown(&devsoftc.sigio); > > mtx_unlock(&devsoftc.mtx); > This is not pure cosmetic. devctl does not track closes, so opens are > not matched with close calls. Now any close clears nonblock and async, > which is not how it was done before. Tracking close calls does not > work reliably. > > FWIW, I think that both old and new behaviour are bugs, and I am not > sure why changing one for another (instead of fix). I don't see that, but maybe I'm misreading the code. There can be only one 'struct file' for devctl and devclose is only called when it is about to be destroyed. fd = open("/dev/devctl"); close(dup(fd)); does not result in calling devclose. If devclose is indeed reachable whlie fds are active this code needs serious help since devsoftc.inuse is of no use whatsoever. There is no support for multiple readers in the sense that each event can be read only once, hence the restriction on open. On the other hand it is indeed possible to obtain multiple fds for devctl which is harmless as far as consistency in the kernel goes. Concurrent reads are serialized with a mutex and closes are invisible to the device, except for the last one which destroys fp. -- Mateusz Guzik