svn commit: r192535 - head/sys/kern

[Kostik Belousov]

On Fri, May 22, 2009 at 12:11:02AM +0200, Attilio Rao wrote:
> 2009/5/21 Kostik Belousov <kostikbel at gmail.com>:
> > On Thu, May 21, 2009 at 07:05:17PM +0200, Attilio Rao wrote:
> >> 2009/5/21 Kostik Belousov <kostikbel at gmail.com>:
> >> > On Thu, May 21, 2009 at 09:23:15AM -0700, Scott Long wrote:
> >> >> Kostik Belousov wrote:
> >> >> >We do have the KPI for the callers that cannot drop the locks and need
> >> >> >to do destroy_dev, destroy_dev_sched(9).
> >> >>
> >> >> Good to know, I'll look at destroy_dev_sched().  I'd rather not have to
> >> >> roll my own decoupled version.  And I understand the argument about
> >> >> destroy_dev being a drain point for the API.  However, what about
> >> >> create_dev()?  Making that non-blocking would help a lot.
> >> >
> >> > create_dev() can be made non-blocking, and this is the first argument pro
> >> > Attilio patch.
> >> >
> >> > From the quick look, all that is needed is to replace M_WAITOK with
> >> > M_NOWAIT inside prep_cdevsw() and devfs_alloc(). Untested patch below.
> >> >
> >> > diff --git a/sys/fs/devfs/devfs_devs.c b/sys/fs/devfs/devfs_devs.c
> >> > index 4041911..f470ee8 100644
> >> > --- a/sys/fs/devfs/devfs_devs.c
> >> > +++ b/sys/fs/devfs/devfs_devs.c
> >> > @@ -120,7 +120,7 @@ devfs_alloc(void)
> >> >        struct cdev *cdev;
> >> >        struct timespec ts;
> >> >
> >> > -       cdp = malloc(sizeof *cdp, M_CDEVP, M_USE_RESERVE | M_ZERO | M_WAITOK);
> >> > +       cdp = malloc(sizeof *cdp, M_CDEVP, M_USE_RESERVE | M_ZERO | M_NOWAIT);
> >> >
> >> >        cdp->cdp_dirents = &cdp->cdp_dirent0;
> >> >        cdp->cdp_dirent0 = NULL;
> >> > diff --git a/sys/kern/kern_conf.c b/sys/kern/kern_conf.c
> >> > index 284f482..acdd44a 100644
> >> > --- a/sys/kern/kern_conf.c
> >> > +++ b/sys/kern/kern_conf.c
> >> > @@ -559,7 +559,7 @@ prep_cdevsw(struct cdevsw *devsw)
> >> >                return;
> >> >        if (devsw->d_flags & D_NEEDGIANT) {
> >> >                dev_unlock();
> >> > -               dsw2 = malloc(sizeof *dsw2, M_DEVT, M_WAITOK);
> >> > +               dsw2 = malloc(sizeof *dsw2, M_DEVT, M_NOWAIT);
> >> >                dev_lock();
> >> >        } else
> >> >                dsw2 = NULL;
> >>
> >> You need to check return values here if it returns NULL.
> >>
> >> IMHO, having a non-sleepable version of destroy_dev(), create_dev()
> >> and such would be ideal.
> >> Ideally, we should resolve all the sleeping point and do the conversion.
> >> I'm unable to check the code right now.
> >
> > Sure. Something like this.
> >
> 
> At this point I wonder what's the purpose of maintaining the sleeping
> version for such functions?
They cannot fail, thus behaving in the way that all present consumers
of the function expect.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-head/attachments/20090522/9c7e6c16/attachment.pgp