Date: Thu, 21 May 2009 22:42:43 +0300 From: Kostik Belousov <kostikbel@gmail.com> To: Attilio Rao <attilio@freebsd.org> Cc: Scott Long <scottl@samsco.org>, src-committers@freebsd.org, John Baldwin <jhb@freebsd.org>, svn-src-all@freebsd.org, rwatson@freebsd.org, svn-src-head@freebsd.org, "M. Warner Losh" <imp@bsdimp.com> Subject: Re: svn commit: r192535 - head/sys/kern Message-ID: <20090521194243.GW1927@deviant.kiev.zoral.com.ua> In-Reply-To: <3bbf2fe10905211005m350dc4d1yed6dc1b79f1603d9@mail.gmail.com> References: <3bbf2fe10905210629p46c7a204v6863aaba77354462@mail.gmail.com> <20090521.094100.70797067.imp@bsdimp.com> <4A157919.7040103@samsco.org> <200905211211.00168.jhb@freebsd.org> <20090521161535.GQ1927@deviant.kiev.zoral.com.ua> <4A157FF3.8020408@samsco.org> <20090521163846.GT1927@deviant.kiev.zoral.com.ua> <3bbf2fe10905211005m350dc4d1yed6dc1b79f1603d9@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--UefMXxAhg/xpzpuf
Content-Type: text/plain; charset=koi8-r
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Thu, May 21, 2009 at 07:05:17PM +0200, Attilio Rao wrote:
> 2009/5/21 Kostik Belousov <kostikbel@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(). =9AI'd rather not have=
to
> >> roll my own decoupled version. =9AAnd I understand the argument about
> >> destroy_dev being a drain point for the API. =9AHowever, what about
> >> create_dev()? =9AMaking that non-blocking would help a lot.
> >
> > create_dev() can be made non-blocking, and this is the first argument p=
ro
> > 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)
> > =9A =9A =9A =9Astruct cdev *cdev;
> > =9A =9A =9A =9Astruct timespec ts;
> >
> > - =9A =9A =9A cdp =3D malloc(sizeof *cdp, M_CDEVP, M_USE_RESERVE | M_ZE=
RO | M_WAITOK);
> > + =9A =9A =9A cdp =3D malloc(sizeof *cdp, M_CDEVP, M_USE_RESERVE | M_ZE=
RO | M_NOWAIT);
> >
> > =9A =9A =9A =9Acdp->cdp_dirents =3D &cdp->cdp_dirent0;
> > =9A =9A =9A =9Acdp->cdp_dirent0 =3D 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)
> > =9A =9A =9A =9A =9A =9A =9A =9Areturn;
> > =9A =9A =9A =9Aif (devsw->d_flags & D_NEEDGIANT) {
> > =9A =9A =9A =9A =9A =9A =9A =9Adev_unlock();
> > - =9A =9A =9A =9A =9A =9A =9A dsw2 =3D malloc(sizeof *dsw2, M_DEVT, M_W=
AITOK);
> > + =9A =9A =9A =9A =9A =9A =9A dsw2 =3D malloc(sizeof *dsw2, M_DEVT, M_N=
OWAIT);
> > =9A =9A =9A =9A =9A =9A =9A =9Adev_lock();
> > =9A =9A =9A =9A} else
> > =9A =9A =9A =9A =9A =9A =9A =9Adsw2 =3D NULL;
>=20
> You need to check return values here if it returns NULL.
>=20
> 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.
diff --git a/share/man/man9/make_dev.9 b/share/man/man9/make_dev.9
index b39ca8b..0844e64 100644
--- a/share/man/man9/make_dev.9
+++ b/share/man/man9/make_dev.9
@@ -133,6 +133,7 @@ The following values are currently accepted:
.Pp
.Bd -literal -offset indent -compact
MAKEDEV_REF reference the created device
+MAKEDEV_NOWAIT do not sleep, may return NULL
.Ed
.Pp
The
diff --git a/sys/fs/devfs/devfs_devs.c b/sys/fs/devfs/devfs_devs.c
index 4041911..79f7893 100644
--- a/sys/fs/devfs/devfs_devs.c
+++ b/sys/fs/devfs/devfs_devs.c
@@ -114,17 +114,21 @@ SYSCTL_INT(_debug_sizeof, OID_AUTO, cdev_priv, CTLFLA=
G_RD,
0, sizeof(struct cdev_priv), "sizeof(struct cdev_priv)");
=20
struct cdev *
-devfs_alloc(void)
+devfs_alloc(int flags)
{
struct cdev_priv *cdp;
struct cdev *cdev;
struct timespec ts;
=20
- cdp =3D malloc(sizeof *cdp, M_CDEVP, M_USE_RESERVE | M_ZERO | M_WAITOK);
+ cdp =3D malloc(sizeof *cdp, M_CDEVP, M_USE_RESERVE | M_ZERO |
+ ((flags & MAKEDEV_NOWAIT) ? M_NOWAIT : M_WAITOK));
+ if (cdp =3D=3D NULL)
+ return (NULL);
=20
cdp->cdp_dirents =3D &cdp->cdp_dirent0;
cdp->cdp_dirent0 =3D NULL;
cdp->cdp_maxdirent =3D 0;
+ cdp->cdp_inode =3D 0;
=20
cdev =3D &cdp->cdp_c;
=20
@@ -132,6 +136,7 @@ devfs_alloc(void)
LIST_INIT(&cdev->si_children);
vfs_timestamp(&ts);
cdev->si_atime =3D cdev->si_mtime =3D cdev->si_ctime =3D ts;
+ cdev->si_cred =3D NULL;
=20
return (cdev);
}
diff --git a/sys/fs/devfs/devfs_int.h b/sys/fs/devfs/devfs_int.h
index 5a61dd4..f5612e1 100644
--- a/sys/fs/devfs/devfs_int.h
+++ b/sys/fs/devfs/devfs_int.h
@@ -70,7 +70,7 @@ struct cdev_priv {
=20
#define cdev2priv(c) member2struct(cdev_priv, cdp_c, c)
=20
-struct cdev *devfs_alloc(void);
+struct cdev *devfs_alloc(int);
void devfs_free(struct cdev *);
void devfs_create(struct cdev *dev);
void devfs_destroy(struct cdev *dev);
diff --git a/sys/kern/kern_conf.c b/sys/kern/kern_conf.c
index 284f482..082f5d3 100644
--- a/sys/kern/kern_conf.c
+++ b/sys/kern/kern_conf.c
@@ -482,7 +482,7 @@ giant_mmap(struct cdev *dev, vm_offset_t offset, vm_pad=
dr_t *paddr, int nprot)
=20
=20
static void
-notify(struct cdev *dev, const char *ev)
+notify(struct cdev *dev, const char *ev, int flags)
{
static const char prefix[] =3D "cdev=3D";
char *data;
@@ -491,7 +491,8 @@ notify(struct cdev *dev, const char *ev)
if (cold)
return;
namelen =3D strlen(dev->si_name);
- data =3D malloc(namelen + sizeof(prefix), M_TEMP, M_NOWAIT);
+ data =3D malloc(namelen + sizeof(prefix), M_TEMP,
+ (flags & MAKEDEV_NOWAIT) ? M_NOWAIT : M_WAITOK);
if (data =3D=3D NULL)
return;
memcpy(data, prefix, sizeof(prefix) - 1);
@@ -501,17 +502,17 @@ notify(struct cdev *dev, const char *ev)
}
=20
static void
-notify_create(struct cdev *dev)
+notify_create(struct cdev *dev, int flags)
{
=20
- notify(dev, "CREATE");
+ notify(dev, "CREATE", flags);
}
=20
static void
notify_destroy(struct cdev *dev)
{
=20
- notify(dev, "DESTROY");
+ notify(dev, "DESTROY", 0);
}
=20
static struct cdev *
@@ -549,24 +550,27 @@ fini_cdevsw(struct cdevsw *devsw)
devsw->d_flags &=3D ~D_INIT;
}
=20
-static void
-prep_cdevsw(struct cdevsw *devsw)
+static int
+prep_cdevsw(struct cdevsw *devsw, int flags)
{
struct cdevsw *dsw2;
=20
mtx_assert(&devmtx, MA_OWNED);
if (devsw->d_flags & D_INIT)
- return;
+ return (1);
if (devsw->d_flags & D_NEEDGIANT) {
dev_unlock();
- dsw2 =3D malloc(sizeof *dsw2, M_DEVT, M_WAITOK);
+ dsw2 =3D malloc(sizeof *dsw2, M_DEVT,
+ (flags & MAKEDEV_NOWAIT) ? M_NOWAIT : M_WAITOK);
dev_lock();
+ if (dsw2 =3D=3D NULL && !(devsw->d_flags & D_INIT))
+ return (0);
} else
dsw2 =3D NULL;
if (devsw->d_flags & D_INIT) {
if (dsw2 !=3D NULL)
cdevsw_free_devlocked(dsw2);
- return;
+ return (1);
}
=20
if (devsw->d_version !=3D D_VERSION_01) {
@@ -622,6 +626,7 @@ prep_cdevsw(struct cdevsw *devsw)
=20
if (dsw2 !=3D NULL)
cdevsw_free_devlocked(dsw2);
+ return (1);
}
=20
struct cdev *
@@ -632,9 +637,15 @@ make_dev_credv(int flags, struct cdevsw *devsw, int un=
it,
struct cdev *dev;
int i;
=20
- dev =3D devfs_alloc();
+ dev =3D devfs_alloc(flags);
+ if (dev =3D=3D NULL)
+ return (NULL);
dev_lock();
- prep_cdevsw(devsw);
+ if (!prep_cdevsw(devsw, flags)) {
+ dev_unlock();
+ devfs_free(dev);
+ return (NULL);
+ }
dev =3D newdev(devsw, unit, dev);
if (flags & MAKEDEV_REF)
dev_refl(dev);
@@ -661,8 +672,6 @@ make_dev_credv(int flags, struct cdevsw *devsw, int uni=
t,
dev->si_flags |=3D SI_NAMED;
if (cr !=3D NULL)
dev->si_cred =3D crhold(cr);
- else
- dev->si_cred =3D NULL;
dev->si_uid =3D uid;
dev->si_gid =3D gid;
dev->si_mode =3D mode;
@@ -671,7 +680,7 @@ make_dev_credv(int flags, struct cdevsw *devsw, int uni=
t,
clean_unrhdrl(devfs_inos);
dev_unlock_and_free();
=20
- notify_create(dev);
+ notify_create(dev, flags);
=20
return (dev);
}
@@ -746,7 +755,7 @@ make_dev_alias(struct cdev *pdev, const char *fmt, ...)
int i;
=20
KASSERT(pdev !=3D NULL, ("NULL pdev"));
- dev =3D devfs_alloc();
+ dev =3D devfs_alloc(0);
dev_lock();
dev->si_flags |=3D SI_ALIAS;
dev->si_flags |=3D SI_NAMED;
@@ -763,7 +772,7 @@ make_dev_alias(struct cdev *pdev, const char *fmt, ...)
clean_unrhdrl(devfs_inos);
dev_unlock();
=20
- notify_create(dev);
+ notify_create(dev, 0);
=20
return (dev);
}
@@ -947,9 +956,9 @@ clone_create(struct clonedevs **cdp, struct cdevsw *csw=
, int *up, struct cdev **
* the end of the list.
*/
unit =3D *up;
- ndev =3D devfs_alloc();
+ ndev =3D devfs_alloc(0);
dev_lock();
- prep_cdevsw(csw);
+ prep_cdevsw(csw, 0);
low =3D extra;
de =3D dl =3D NULL;
cd =3D *cdp;
diff --git a/sys/sys/conf.h b/sys/sys/conf.h
index 05c4bb5..7b65862 100644
--- a/sys/sys/conf.h
+++ b/sys/sys/conf.h
@@ -263,6 +263,7 @@ struct cdev *make_dev_cred(struct cdevsw *_devsw, int _=
unit,
const char *_fmt, ...) __printflike(7, 8);
#define MAKEDEV_REF 0x1
#define MAKEDEV_WHTOUT 0x2
+#define MAKEDEV_NOWAIT 0x4
struct cdev *make_dev_credf(int _flags,
struct cdevsw *_devsw, int _unit,
struct ucred *_cr, uid_t _uid, gid_t _gid, int _mode,
--UefMXxAhg/xpzpuf
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (FreeBSD)
iEYEARECAAYFAkoVrrMACgkQC3+MBN1Mb4hbtgCfSuOC9WSmOblI7zgHXjPpuu0n
WRUAnjLxpNr79a+EgvMQlMrzXWlTA4GQ
=HA2n
-----END PGP SIGNATURE-----
--UefMXxAhg/xpzpuf--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090521194243.GW1927>