From owner-svn-src-head@FreeBSD.ORG Sun Jul 13 08:01:22 2014 Return-Path: Delivered-To: svn-src-head@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 9F5F3894; Sun, 13 Jul 2014 08:01:22 +0000 (UTC) Received: from mail-we0-x233.google.com (mail-we0-x233.google.com [IPv6:2a00:1450:400c:c03::233]) (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 BAC262F0F; Sun, 13 Jul 2014 08:01:21 +0000 (UTC) Received: by mail-we0-f179.google.com with SMTP id u57so617919wes.38 for ; Sun, 13 Jul 2014 01:01:19 -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=FgIayXPpRzwuJLfDL2JAyBBn1KLk2tl/xAVYxHocIVQ=; b=l5nGR/qG+0cNXuC4fX4zIxVdMRZcsGqdyvM0M4l5HiMK/RZlrz92glTZ8ohBGlXSya 1jrrhnxyrsoWHHwMgdQnRI7rb1JI/FqT127HhxhCOspOvhiuV5GXH4CH+8fsPHZvOf77 Yk7ylFlF/gKqxary5sIEOVJ4XXC6d9Xj+AFWhe1F9gi9xePjyM/SfCp3bFfqDRRjF2ah 8Y5L+IqCBgU3nZ2GEbu+YE5g9HOCKvjybao9ZLl2kpo0Ix0knW4wVmRFWFQo5qUY+Gsy nkpOI/MeGDMrlPlJw8WVa6zsLDWgJM+pCCZMOhmeQw89UbTxP1QXtkPNUuymY1ryY0Na 9Pgg== X-Received: by 10.180.36.225 with SMTP id t1mr17084755wij.38.1405238479148; Sun, 13 Jul 2014 01:01:19 -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 i4sm15985783wib.21.2014.07.13.01.01.17 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Sun, 13 Jul 2014 01:01:18 -0700 (PDT) Date: Sun, 13 Jul 2014 10:01:16 +0200 From: Mateusz Guzik To: Konstantin Belousov Subject: Re: svn commit: r268570 - head/sys/kern Message-ID: <20140713080115.GD16884@dft-labs.eu> References: <201407121535.s6CFZ42f063120@svn.freebsd.org> <20140712161801.GS93733@kib.kiev.ua> <20140712165346.GA16884@dft-labs.eu> <20140712171015.GT93733@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20140712171015.GT93733@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-head@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 13 Jul 2014 08:01:22 -0000 On Sat, Jul 12, 2014 at 08:10:15PM +0300, Konstantin Belousov wrote: > On Sat, Jul 12, 2014 at 06:53:47PM +0200, Mateusz Guzik wrote: > > 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. > Well, I argue that devsoftc.inuse is broken too. It was introduced in > time when the only way of tracking the shared use of cdev was cloning. > Note that it does not prevent multiple threads from simultaneously > fall into the cdevsw methods; e.g. cv_wait_sig() in devread() drops > devsoftc.mtx etc. > But this is again harmless as far as kernel consistency goes. > IMO the right thing to do is to allow multiple opens and to keep > non-blocking attribute and async bindings in the per-file structure. > Then your change would be real nop. > This would be an option, but then what to do with events? Whoever happens to read one consumes it? Current behaviour of denying further opens seems safer since the process which opened can be sure nobody suddenly steals any. If it decides to 'share' device fd, well, it is its own problem. Assignments in devclose are not going to be executed as long as there is an active fp, thus this is a nop from perspective of devctl users. > BTW, another, this time really big, user of the private (yes) cloning > implementation is snd(4). The conversion of it to devfs_cdevpriv(9) > would be also highly desirable. This was meant to be a cosmetic change, I'm not interested in working on this, sorry. I can revert the change if you want. I plan to merge the following to stable/10: - r264114 Fix SIGIO delivery. Use fsetown() to handle file descriptor owner ioctl and use pgsigio() to send SIGIO. - r264310 Add kqueue support for devctl. Do you have any objections? If you don't want a revert of this patch, I'll MFC it as well. -- Mateusz Guzik