Date: Wed, 03 Aug 2011 14:44:23 +0900 (JST) From: Kohji Okuno <okuno.kohji@jp.panasonic.com> To: freebsd-current@freebsd.org Cc: okuno.kohji@jp.panasonic.com Subject: Re: Bug: devfs is sure to have the bug. Message-ID: <20110803.144423.2018247186416313018.okuno.kohji@jp.panasonic.com> In-Reply-To: <20110803.141636.145758949453810779.okuno.kohji@jp.panasonic.com> References: <20110803.141636.145758949453810779.okuno.kohji@jp.panasonic.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Hello,
> Hello,
>
> I think that devfs is sure to have the bug.
> I found that I couldn't open "/dev/XXX" though the kernel detected XXX
> device.
>
>
> "dm->dm_generation" is updated with "devfs_generation" in
> 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 that we should change the lock method.
> May I have advice?
>
> void
> devfs_populate(struct devfs_mount *dm)
> {
>
> sx_assert(&dm->dm_lock, SX_XLOCKED);
> if (dm->dm_generation == devfs_generation)
> return;
> while (devfs_populate_loop(dm, 0))
> continue;
> (***)
> dm->dm_generation = devfs_generation;
> }
>
> void
> devfs_create(struct cdev *dev)
> {
> struct cdev_priv *cdp;
>
> mtx_assert(&devmtx, MA_OWNED);
> cdp = cdev2priv(dev);
> cdp->cdp_flags |= CDP_ACTIVE;
> cdp->cdp_inode = alloc_unrl(devfs_inos);
> dev_refl(dev);
> TAILQ_INSERT_TAIL(&cdevp_list, cdp, cdp_list);
> devfs_generation++;
> }
>
> void
> devfs_destroy(struct cdev *dev)
> {
> struct cdev_priv *cdp;
>
> mtx_assert(&devmtx, MA_OWNED);
> cdp = cdev2priv(dev);
> cdp->cdp_flags &= ~CDP_ACTIVE;
> devfs_generation++;
> }
>
> Thanks.
I propose the solution. May I have advice?
void
devfs_populate(struct devfs_mount *dm)
{
sx_assert(&dm->dm_lock, SX_XLOCKED);
#if 1 /* I propose... */
int tmp_generation;
retry:
tmp_generation = devfs_generation;
if (dm->dm_generation == tmp_generation)
return;
while (devfs_populate_loop(dm, 0))
continue;
if (tmp_generation != devfs_generation)
goto retry;
dm->dm_generation = tmp_generation;
#else /* Original */
if (dm->dm_generation == devfs_generation)
return;
while (devfs_populate_loop(dm, 0))
continue;
dm->dm_generation = devfs_generation;
#endif
}
Thanks,
Kohji Okuno (okuno.kohji@jp.panasonic.com)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110803.144423.2018247186416313018.okuno.kohji>