From owner-freebsd-current@FreeBSD.ORG Fri Dec 2 12:16:56 2011 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B0A011065678; Fri, 2 Dec 2011 12:16:56 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id 4E22C8FC16; Fri, 2 Dec 2011 12:16:55 +0000 (UTC) Received: from alf.home (alf.kiev.zoral.com.ua [10.1.1.177]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id pB2CGqav046935 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 2 Dec 2011 14:16:52 +0200 (EET) (envelope-from kostikbel@gmail.com) Received: from alf.home (kostik@localhost [127.0.0.1]) by alf.home (8.14.5/8.14.5) with ESMTP id pB2CGpKJ087380; Fri, 2 Dec 2011 14:16:51 +0200 (EET) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by alf.home (8.14.5/8.14.5/Submit) id pB2CGpKn087379; Fri, 2 Dec 2011 14:16:51 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: alf.home: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 2 Dec 2011 14:16:51 +0200 From: Kostik Belousov To: Baptiste Daroussin Message-ID: <20111202121651.GT50300@deviant.kiev.zoral.com.ua> References: <20111130124320.GA1449@azathoth.lan> <201111301005.11938.jhb@freebsd.org> <20111130154636.GX50300@deviant.kiev.zoral.com.ua> <20111130155521.GA52567@ci0.org> <20111130160450.GY50300@deviant.kiev.zoral.com.ua> <20111130162017.GA53362@ci0.org> <20111201231721.GA1669@azathoth.lan> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="g/Ej6EIvgQPzAS7S" Content-Disposition: inline In-Reply-To: <20111201231721.GA1669@azathoth.lan> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-3.9 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: freebsd-current@freebsd.org, imp@freebsd.org, Olivier Houchard Subject: Re: [patch] turning devctl into a "multiple openable" device X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 02 Dec 2011 12:16:56 -0000 --g/Ej6EIvgQPzAS7S Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Dec 02, 2011 at 12:17:21AM +0100, Baptiste Daroussin wrote: > On Wed, Nov 30, 2011 at 05:20:17PM +0100, Olivier Houchard wrote: > > On Wed, Nov 30, 2011 at 06:04:50PM +0200, Kostik Belousov wrote: > > > > > I wonder why the waiting_threads stuff is needed at all. The cv c= ould > > > > > be woken up unconditionally everytime. What is the reason for the= cv_wait > > > > > call in cdevpriv data destructor ? You cannot have a thread doing= e.g. > > > > > read on the file descriptor while destructor is run. > > > > >=20 > > > >=20 > > > > What will prevent you from having a thread stuck in read(), while a= n another=20 > > > > one close() the fd ? > > > >=20 > > > Nothing, but file reference count goes to zero only after the thread > > > stuck in read is unstuck. Cdevpriv destructor is run only when file > > > reference count becomes zero, i.e. there can be no any accessing thre= ads, > > > and new accesses are impossible since file descriptors also own refer= ences > > > on the file. > >=20 > > Right, I was a bit confused, this part can be removed. > >=20 > > Regards, > >=20 > > Olivier >=20 > Here is a new version of the patch mostly reworked by Olivier, > It doesn't duplicate anymore the devq, and fix all that have been > spotted here previously. >=20 > http://people.freebsd.org/~bapt/devctl_multi_open.diff >=20 > bonus, it removes the needless giant lock I do not see a need in the use of refcount KPI, since it seems that all manipulations of n->refcount happen under global_devctl.mtx. Am I missed the place ? Releasing refcount on dev_event_info before using it in devread() allows other thread to free the memory while current thread still uses it. Stylish notes: mtx member in struct global_devctl has weird indentation; please move declaration of n2 into the declaration block of devdtor(); devread():should_free is initialized at the declaration site; devctl_queue_data_f() has stray empty line ater the while { loop line, while need blank line at the start of devctl_queue_data() is removed. --g/Ej6EIvgQPzAS7S Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.18 (FreeBSD) iEYEARECAAYFAk7YwbMACgkQC3+MBN1Mb4jhcgCgusZFXAgV/4wimRsgTFNE2TPW NBMAnA/j975qxzQ2jOemnxsLe0RyEom/ =aMaX -----END PGP SIGNATURE----- --g/Ej6EIvgQPzAS7S--