NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

kern/60957: Make the specdev hash buckets into SLISTs



>Number:         60957
>Category:       kern
>Synopsis:       Make the specdev hash buckets into SLISTs
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Tue Nov 03 03:30:00 +0000 2026
>Originator:     Sarah Purohit
>Release:        CVS trunk
>Organization:
>Environment:
>Description:
While going through some unrelated vnode code, I noticed that specdev
vnops use manual list manipulation operations rather than using
queue(3). 

There *should* hopefully be no functional change here other than making
the code nicer to read. The change in spec_init() could likely be
dropped, but is there for completeness (c.f: SLIST_HEAD_INITIALIZER).

I verified the patch by building a GENERIC kernel with DIAGNOSTIC and
DEBUG enabled and the installer ramdisk seems to boot okay. Still need
to run ATF though.

If one wanted to be super picky, the ((SPECHSZ&(SPECHSZ-1)) == 0)
could be replaced with powerof2, but that's for another day.
>How-To-Repeat:

>Fix:
diff --git a/sys/miscfs/specfs/spec_vnops.c b/sys/miscfs/specfs/spec_vnops.c
index 802376cdf876..10c85f0ebc08 100644
--- a/sys/miscfs/specfs/spec_vnops.c
+++ b/sys/miscfs/specfs/spec_vnops.c
@@ -117,8 +117,9 @@ const char	devcls[] = "devcls";
 #else
 #define	SPECHASH(rdev)	(((unsigned)((rdev>>5)+(rdev)))%SPECHSZ)
 #endif
+#define	SPECHASH_HEAD(rdev)	(&specfs_hash[SPECHASH(rdev)])
 
-static vnode_t	*specfs_hash[SPECHSZ];
+static struct vnodechain specfs_hash[SPECHSZ];
 extern struct mount *dead_rootmount;
 
 /*
@@ -227,7 +228,10 @@ rawio_listener_cb(kauth_cred_t cred, kauth_action_t action, void *cookie,
 void
 spec_init(void)
 {
+	int i;
 
+	for (i = 0; i < SPECHSZ; i++)
+		SLIST_INIT(&specfs_hash[i]);
 	rawio_listener = kauth_listen_scope(KAUTH_SCOPE_DEVICE,
 	    rawio_listener_cb, NULL);
 	cv_init(&specfs_iocv, "specio");
@@ -376,7 +380,7 @@ spec_node_init(vnode_t *vp, dev_t rdev)
 	specnode_t *sn;
 	specdev_t *sd;
 	vnode_t *vp2;
-	vnode_t **vpp;
+	struct vnodechain *vch;
 
 	KASSERT(vp->v_type == VBLK || vp->v_type == VCHR);
 	KASSERT(vp->v_specnode == NULL);
@@ -390,8 +394,8 @@ spec_node_init(vnode_t *vp, dev_t rdev)
 	sn = kmem_alloc(sizeof(*sn), KM_SLEEP);
 	sd = kmem_alloc(sizeof(*sd), KM_SLEEP);
 	mutex_enter(&device_lock);
-	vpp = &specfs_hash[SPECHASH(rdev)];
-	for (vp2 = *vpp; vp2 != NULL; vp2 = vp2->v_specnext) {
+	vch = SPECHASH_HEAD(rdev);
+	SLIST_FOREACH(vp2, vch, v_specnext) {
 		KASSERT(vp2->v_specnode != NULL);
 		if (rdev == vp2->v_rdev && vp->v_type == vp2->v_type) {
 			break;
@@ -419,8 +423,7 @@ spec_node_init(vnode_t *vp, dev_t rdev)
 	sn->sn_rdev = rdev;
 	sn->sn_gone = false;
 	vp->v_specnode = sn;
-	vp->v_specnext = *vpp;
-	*vpp = vp;
+	SLIST_INSERT_HEAD(vch, vp, v_specnext);
 	mutex_exit(&device_lock);
 
 	/* Free the record we allocated if unused. */
@@ -439,7 +442,7 @@ spec_node_lookup_by_dev(enum vtype type, dev_t dev, int flags, vnode_t **vpp)
 	vnode_t *vp;
 
 top:	mutex_enter(&device_lock);
-	for (vp = specfs_hash[SPECHASH(dev)]; vp; vp = vp->v_specnext) {
+	SLIST_FOREACH(vp, SPECHASH_HEAD(dev), v_specnext) {
 		if (type == vp->v_type && dev == vp->v_rdev) {
 			mutex_enter(vp->v_interlock);
 			/* If clean or being cleaned, then ignore it. */
@@ -498,7 +501,7 @@ spec_node_lookup_by_mount(struct mount *mp, vnode_t **vpp)
 
 	mutex_enter(&device_lock);
 	for (i = 0, vq = NULL; i < SPECHSZ && vq == NULL; i++) {
-		for (vp = specfs_hash[i]; vp; vp = vp->v_specnext) {
+		SLIST_FOREACH(vp, SPECHASH_HEAD(i), v_specnext) {
 			if (vp->v_type != VBLK)
 				continue;
 			vq = vp->v_specnode->sn_dev->sd_bdevvp;
@@ -600,7 +603,7 @@ spec_node_revoke(vnode_t *vp)
 {
 	specnode_t *sn;
 	specdev_t *sd;
-	struct vnode **vpp;
+	struct vnodechain *vch;
 
 	KASSERT(VOP_ISLOCKED(vp) == LK_EXCLUSIVE);
 
@@ -644,14 +647,9 @@ spec_node_revoke(vnode_t *vp)
 	 */
 	KASSERT(sn->sn_gone);
 	KASSERT(sn->sn_opencnt == 0);
-	for (vpp = &specfs_hash[SPECHASH(vp->v_rdev)];;
-	     vpp = &(*vpp)->v_specnext) {
-		if (*vpp == vp) {
-			*vpp = vp->v_specnext;
-			vp->v_specnext = NULL;
-			break;
-		}
-	}
+	vch = SPECHASH_HEAD(vp->v_rdev);
+	SLIST_REMOVE(vch, vp, vnode, v_specnext);
+	SLIST_NEXT(vp, v_specnext) = NULL;
 	mutex_exit(&device_lock);
 }
 
diff --git a/sys/miscfs/specfs/specdev.h b/sys/miscfs/specfs/specdev.h
index b31489505af3..ae249271f1d5 100644
--- a/sys/miscfs/specfs/specdev.h
+++ b/sys/miscfs/specfs/specdev.h
@@ -63,8 +63,10 @@
 #include <sys/mutex.h>
 #include <sys/vnode.h>
 
+SLIST_HEAD(vnodechain, vnode);
+
 typedef struct specnode {
-	vnode_t		*sn_next;
+	SLIST_ENTRY(vnode) sn_next;
 	struct specdev	*sn_dev;
 	dev_t		sn_rdev;
 	u_int		sn_opencnt;	/* # of opens, share of sd_opencnt */




Home | Main Index | Thread Index | Old Index