From owner-freebsd-current@FreeBSD.ORG Fri Aug 5 17:14:34 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 63353106567B; Fri, 5 Aug 2011 17:14:34 +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 039C18FC08; Fri, 5 Aug 2011 17:14:33 +0000 (UTC) Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id p75HEUVQ014090 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 5 Aug 2011 20:14:30 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4) with ESMTP id p75HEUno086264; Fri, 5 Aug 2011 20:14:30 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4/Submit) id p75HEUb4086263; Fri, 5 Aug 2011 20:14:30 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 5 Aug 2011 20:14:30 +0300 From: Kostik Belousov To: Jaakko Heinonen Message-ID: <20110805171430.GW17489@deviant.kiev.zoral.com.ua> References: <20110803.141636.145758949453810779.okuno.kohji@jp.panasonic.com> <20110803.144423.2018247186416313018.okuno.kohji@jp.panasonic.com> <20110803135044.GM17489@deviant.kiev.zoral.com.ua> <20110805154522.GA2054@a91-153-123-205.elisa-laajakaista.fi> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="T/mHeVm7rU1pEax+" Content-Disposition: inline In-Reply-To: <20110805154522.GA2054@a91-153-123-205.elisa-laajakaista.fi> 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.3 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00, DNS_FROM_OPENWHOIS autolearn=no 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, Kohji Okuno Subject: Re: Bug: devfs is sure to have the bug. 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, 05 Aug 2011 17:14:34 -0000 --T/mHeVm7rU1pEax+ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Aug 05, 2011 at 06:45:22PM +0300, Jaakko Heinonen wrote: > On 2011-08-03, Kostik Belousov wrote: > > On Wed, Aug 03, 2011 at 02:44:23PM +0900, Kohji Okuno wrote: > > > > devfs_populate(), and the context holds only "dm->dm_lock" in > > > > devfs_populate(). > > > >=20 > > > > On the other hand, "devfs_generation" is incremented in devfs_creat= e() > > > > and devfs_destroy() the context holds only "devmtx" in devfs_create= () > > > > and devfs_destroy(). > > > >=20 > > > > If a context executes devfs_create() when other context is executing > > > > (***), then "dm->dm_generation" is updated incorrect value. > > > > As a result, we can not open the last detected device (we receive E= NOENT). > >=20 > > I think the problem you described is real, and suggested change is righ= t. > > Initially, I thought that we should work with devfs_generation as with > > the atomic type due to unlocked access in the devfs_populate(), but then > > convinced myself that this is not needed. > >=20 > > But also, I think there is another half of the problem. Namely, > > devfs_lookup() calls devfs_populate_vp(), and then does lookup with the > > help of devfs_lookupx(). We will miss the generation update > > happen after the drop of the dm_lock in devfs_populate_vp() to reacquire > > the directory vnode lock. >=20 > I don't understand this. devfs_generation is not protected with dm_lock > in devfs_create() and devfs_destroy(). On the other hand if you mean > that another thread calls devfs_populate() while we drop dm_lock in > devfs_populate_vp(), isn't the mount point up to date when we re-lock > dm_lock? Yes, I was not quite exact in describing what I mean, and the reference to dm_lock drop is both vague and not correct. I am after the fact that we do allow the situation where it is externally visible that new cdev node was successfully created before the lookup returns ENOENT for the path of the node. >=20 > > @@ -630,13 +630,15 @@ devfs_populate_loop(struct devfs_mount *dm, int c= leanup) > > void > > devfs_populate(struct devfs_mount *dm) > > { > > + unsigned gen; > > =20 > > sx_assert(&dm->dm_lock, SX_XLOCKED); > > - if (dm->dm_generation =3D=3D devfs_generation) > > + gen =3D devfs_generation; > > + if (dm->dm_generation =3D=3D gen) > > return; > > while (devfs_populate_loop(dm, 0)) > > continue; > > - dm->dm_generation =3D devfs_generation; > > + dm->dm_generation =3D gen; > > } >=20 > After this change dm->dm_generation may be stale although the mount > point is up to date? This is probably harmless, though. This must be harmless, in the worst case it will cause more calls to the populate. In fact, this even allows the dm_generation to roll backward, which is again harmless. --T/mHeVm7rU1pEax+ Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (FreeBSD) iEYEARECAAYFAk48JPYACgkQC3+MBN1Mb4hBKACeIeXpOd36YNL9gX20iBprVDl7 os4An0+4QuRROZYb2mLjzE0kRVaPcl7G =QS/7 -----END PGP SIGNATURE----- --T/mHeVm7rU1pEax+--