Date: Fri, 5 Aug 2011 18:45:22 +0300 From: Jaakko Heinonen <jh@FreeBSD.org> To: Kostik Belousov <kostikbel@gmail.com> Cc: freebsd-current@freebsd.org, Kohji Okuno <okuno.kohji@jp.panasonic.com> Subject: Re: Bug: devfs is sure to have the bug. Message-ID: <20110805154522.GA2054@a91-153-123-205.elisa-laajakaista.fi> In-Reply-To: <20110803135044.GM17489@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>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110805154522.GA2054>