On Thu, Mar 31, 2016 at 06:02:07AM -0600, Todd C. Miller wrote:
> On Thu, 31 Mar 2016 09:34:32 +0200, Martin Natano wrote:
>
> > Thank you all for the input. Allocatig the bitmap via malloc() really
> > seems like the way to go, so we don't waste space for non-cloning
> > devices. See updated patch below.
> >
> > Would it make sense to move the (rdev == VCHR && ...) condition to a
> > macro in <sys/specdev.h>? I figured it's only used twice, so I inlined
> > it.
>
> I think it is fine to inline that but please put a set of parens
> around the bitwise and part for readability.
Sure; see the updated patch below. Ok?
natano
Index: sys/specdev.h
===================================================================
RCS file: /cvs/src/sys/sys/specdev.h,v
retrieving revision 1.34
diff -u -p -r1.34 specdev.h
--- sys/specdev.h 2 Nov 2013 00:16:31 -0000 1.34
+++ sys/specdev.h 31 Mar 2016 12:32:53 -0000
@@ -46,7 +46,7 @@ struct specinfo {
daddr_t si_lastr;
union {
struct vnode *ci_parent; /* pointer back to parent device */
- u_int8_t ci_bitmap[8]; /* bitmap of devices cloned off us */
+ u_int8_t *ci_bitmap; /* bitmap of devices cloned off us */
} si_ci;
};
@@ -71,6 +71,7 @@ struct cloneinfo {
* This gives us 8 bits for encoding the real minor number.
*/
#define CLONE_SHIFT 8
+#define CLONE_MAP_SZ 128
/*
* Special device management
Index: kern/vfs_subr.c
===================================================================
RCS file: /cvs/src/sys/kern/vfs_subr.c,v
retrieving revision 1.240
diff -u -p -r1.240 vfs_subr.c
--- kern/vfs_subr.c 19 Mar 2016 12:04:15 -0000 1.240
+++ kern/vfs_subr.c 31 Mar 2016 12:32:54 -0000
@@ -555,7 +555,13 @@ loop:
nvp->v_specnext = *vpp;
nvp->v_specmountpoint = NULL;
nvp->v_speclockf = NULL;
- memset(nvp->v_specbitmap, 0, sizeof(nvp->v_specbitmap));
+ nvp->v_specbitmap = NULL;
+ if (nvp_rdev == VCHR &&
+ (cdevsw[major(nvp_rdev)].d_flags & D_CLONE) &&
+ minor(nvp_rdev) >> CLONE_SHIFT == 0) {
+ nvp->v_specbitmap = malloc(CLONE_MAP_SZ, M_VNODE,
+ M_WAITOK | M_ZERO);
+ }
*vpp = nvp;
if (vp != NULLVP) {
nvp->v_flag |= VALIASED;
@@ -1092,6 +1098,11 @@ vgonel(struct vnode *vp, struct proc *p)
if (vq == NULL)
vx->v_flag &= ~VALIASED;
vp->v_flag &= ~VALIASED;
+ }
+ if (vp->v_rdev == VCHR &&
+ (cdevsw[major(vp->v_rdev)].d_flags & D_CLONE) &&
+ minor(vp->v_rdev) >> CLONE_SHIFT == 0) {
+ free(vp->v_specbitmap, M_VNODE, CLONE_MAP_SZ);
}
free(vp->v_specinfo, M_VNODE, sizeof(struct specinfo));
vp->v_specinfo = NULL;
Index: kern/spec_vnops.c
===================================================================
RCS file: /cvs/src/sys/kern/spec_vnops.c,v
retrieving revision 1.86
diff -u -p -r1.86 spec_vnops.c
--- kern/spec_vnops.c 19 Mar 2016 12:04:15 -0000 1.86
+++ kern/spec_vnops.c 31 Mar 2016 12:32:54 -0000
@@ -707,13 +707,14 @@ spec_open_clone(struct vop_open_args *ap
if (minor(vp->v_rdev) >= (1 << CLONE_SHIFT))
return (ENXIO);
- for (i = 1; i < sizeof(vp->v_specbitmap) * NBBY; i++)
+ for (i = 1; i < CLONE_MAP_SZ * NBBY; i++) {
if (isclr(vp->v_specbitmap, i)) {
setbit(vp->v_specbitmap, i);
break;
}
+ }
- if (i == sizeof(vp->v_specbitmap) * NBBY)
+ if (i == CLONE_MAP_SZ * NBBY)
return (EBUSY); /* too many open instances */
error = cdevvp(makedev(major(vp->v_rdev),
