Thu Apr 23 22:58:36 2020 UTC ()

cache_lookup_linked(): We can't use the name to decide how to lock the dir,
since the name refers to the child (found object) not the parent (the thing
that's being locked).

Fix it by always doing rw_tryenter().  There's not much to be won by
optimising for the contended case, and were this routine doing lockless
lookups (the eventual goal) it wouldn't be hanging around waiting for
changes either.


(ad)

diff -r1.140 -r1.141 src/sys/kern/vfs_cache.c

--- src/sys/kern/vfs_cache.c 2020/04/22 21:35:52	1.140
+++ src/sys/kern/vfs_cache.c 2020/04/23 22:58:36	1.141

 @@ -1,14 +1,14 @@
-/*	$NetBSD: vfs_cache.c,v 1.140 2020/04/22 21:35:52 ad Exp $	*/
+/*	$NetBSD: vfs_cache.c,v 1.141 2020/04/23 22:58:36 ad Exp $	*/
 /*-
  * Copyright (c) 2008, 2019, 2020 The NetBSD Foundation, Inc.
  * All rights reserved.
+ *
  * This code is derived from software contributed to The NetBSD Foundation
  * by Andrew Doran.
+ *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
 @@ -162,27 +162,27 @@
+ *
  *	1) vi->vi_nc_lock	(tree or parent -> child direction,
  *				 used during forward lookup)
+ *
  *	2) vi->vi_nc_listlock	(list or child -> parent direction,
  *				 used during reverse lookup)
+ *
  *	3) cache_lru_lock	(LRU list direction, used during reclaim)
+ *
  *	4) vp->v_interlock	(what the cache entry points to)
  */
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_cache.c,v 1.140 2020/04/22 21:35:52 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_cache.c,v 1.141 2020/04/23 22:58:36 ad Exp $");
 #define __NAMECACHE_PRIVATE
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
 #include "opt_dtrace.h"
 #endif
 #include <sys/types.h>
 #include <sys/atomic.h>
 #include <sys/callout.h>
 #include <sys/cpu.h>
 #include <sys/errno.h>
 #include <sys/evcnt.h>
 @@ -658,32 +658,28 @@ cache_lookup_linked(struct vnode *dvp, c
 	 * here: the directory must be purged with cache_purge() before
 	 * being freed, and both parent & child's vi_nc_lock must be taken
 	 * before that point is passed.
+	 *
 	 * However if there's no previous lock, like at the root of the
 	 * chain, then "dvp" must be referenced to prevent dvp going away
 	 * before we get its lock.
+	 *
 	 * Note that the two locks can be the same if looking up a dot, for
 	 * example: /usr/bin/.  If looking up the parent (..) we can't wait
 	 * on the lock as child -> parent is the wrong direction.
 	 */
 	if (*plock != &dvi->vi_nc_lock) {
-		if (namelen == 2 && name[0] == '.' && name[1] == '.') {
+		if (!rw_tryenter(&dvi->vi_nc_lock, RW_READER)) {
-			if (!rw_tryenter(&dvi->vi_nc_lock, RW_READER)) {
+			return false;
 				return false;
 		} else {
 			rw_enter(&dvi->vi_nc_lock, RW_READER);
+		}
 		if (*plock != NULL) {
 			rw_exit(*plock);
+		}
 		*plock = &dvi->vi_nc_lock;
 	} else if (*plock == NULL) {
 		KASSERT(vrefcnt(dvp) > 0);
+	}
 	/*
 	 * First up check if the user is allowed to look up files in this
 	 * directory.
 	 */