From owner-freebsd-current@FreeBSD.ORG Fri Aug 5 16:05:03 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 8E771106566C for ; Fri, 5 Aug 2011 16:05:03 +0000 (UTC) (envelope-from jh@FreeBSD.org) Received: from gw02.mail.saunalahti.fi (gw02.mail.saunalahti.fi [195.197.172.116]) by mx1.freebsd.org (Postfix) with ESMTP id 0B2868FC0C for ; Fri, 5 Aug 2011 16:05:02 +0000 (UTC) Received: from a91-153-123-205.elisa-laajakaista.fi (a91-153-123-205.elisa-laajakaista.fi [91.153.123.205]) by gw02.mail.saunalahti.fi (Postfix) with SMTP id 03A3E13960E; Fri, 5 Aug 2011 18:45:22 +0300 (EEST) Date: Fri, 5 Aug 2011 18:45:22 +0300 From: Jaakko Heinonen To: Kostik Belousov Message-ID: <20110805154522.GA2054@a91-153-123-205.elisa-laajakaista.fi> References: <20110803.141636.145758949453810779.okuno.kohji@jp.panasonic.com> <20110803.144423.2018247186416313018.okuno.kohji@jp.panasonic.com> <20110803135044.GM17489@deviant.kiev.zoral.com.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20110803135044.GM17489@deviant.kiev.zoral.com.ua> User-Agent: Mutt/1.5.21 (2010-09-15) 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 16:05:03 -0000 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(). > > > > > > On the other hand, "devfs_generation" is incremented in devfs_create() > > > and devfs_destroy() the context holds only "devmtx" in devfs_create() > > > and devfs_destroy(). > > > > > > 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 ENOENT). > > I think the problem you described is real, and suggested change is right. > 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. > > 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. 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? > @@ -630,13 +630,15 @@ devfs_populate_loop(struct devfs_mount *dm, int cleanup) > void > devfs_populate(struct devfs_mount *dm) > { > + unsigned gen; > > sx_assert(&dm->dm_lock, SX_XLOCKED); > - if (dm->dm_generation == devfs_generation) > + gen = devfs_generation; > + if (dm->dm_generation == gen) > return; > while (devfs_populate_loop(dm, 0)) > continue; > - dm->dm_generation = devfs_generation; > + dm->dm_generation = gen; > } After this change dm->dm_generation may be stale although the mount point is up to date? This is probably harmless, though. -- Jaakko