From owner-svn-src-head@freebsd.org Tue Jul 14 21:15:00 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 1CF8B36F776; Tue, 14 Jul 2020 21:15:00 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4B5tZW6hFWz3f75; Tue, 14 Jul 2020 21:14:59 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id B8551C6DA; Tue, 14 Jul 2020 21:14:59 +0000 (UTC) (envelope-from mjg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 06ELExrm021866; Tue, 14 Jul 2020 21:14:59 GMT (envelope-from mjg@FreeBSD.org) Received: (from mjg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 06ELExFf021865; Tue, 14 Jul 2020 21:14:59 GMT (envelope-from mjg@FreeBSD.org) Message-Id: <202007142114.06ELExFf021865@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mjg set sender to mjg@FreeBSD.org using -f From: Mateusz Guzik Date: Tue, 14 Jul 2020 21:14:59 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r363196 - head/sys/kern X-SVN-Group: head X-SVN-Commit-Author: mjg X-SVN-Commit-Paths: head/sys/kern X-SVN-Commit-Revision: 363196 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Jul 2020 21:15:00 -0000 Author: mjg Date: Tue Jul 14 21:14:59 2020 New Revision: 363196 URL: https://svnweb.freebsd.org/changeset/base/363196 Log: cache: create a dedicate struct for negative entries .. and stuff if into the unused target vnode field This gets rid of concurrent nc_flag modifications racing with the shrinker and consequently fixes a bug where such a change could have been missed when cache_ncp_invalidate was being issued.. Reported by: zeising Tested by: pho, zeising Fixes: r362828 ("cache: lockless forward lookup with smr") Modified: head/sys/kern/vfs_cache.c Modified: head/sys/kern/vfs_cache.c ============================================================================== --- head/sys/kern/vfs_cache.c Tue Jul 14 20:37:50 2020 (r363195) +++ head/sys/kern/vfs_cache.c Tue Jul 14 21:14:59 2020 (r363196) @@ -104,6 +104,11 @@ SDT_PROBE_DEFINE2(vfs, namecache, shrink_negative, don * This structure describes the elements in the cache of recent * names looked up by namei. */ +struct negstate { + u_char neg_flag; +}; +_Static_assert(sizeof(struct negstate) <= sizeof(struct vnode *), + "the state must fit in a union with a pointer without growing it"); struct namecache { CK_LIST_ENTRY(namecache) nc_hash;/* hash chain */ @@ -112,6 +117,7 @@ struct namecache { struct vnode *nc_dvp; /* vnode of parent of name */ union { struct vnode *nu_vp; /* vnode the name refers to */ + struct negstate nu_neg;/* negative entry state */ } n_un; u_char nc_flag; /* flag bits */ u_char nc_nlen; /* length of name */ @@ -134,6 +140,7 @@ struct namecache_ts { }; #define nc_vp n_un.nu_vp +#define nc_neg n_un.nu_neg /* * Flags in namecache.nc_flag @@ -144,10 +151,14 @@ struct namecache_ts { #define NCF_DTS 0x08 #define NCF_DVDROP 0x10 #define NCF_NEGATIVE 0x20 -#define NCF_HOTNEGATIVE 0x40 -#define NCF_INVALID 0x80 +#define NCF_INVALID 0x40 /* + * Flags in negstate.neg_flag + */ +#define NEG_HOT 0x01 + +/* * Mark an entry as invalid. * * This is called before it starts getting deconstructed. @@ -271,6 +282,14 @@ NCP2NEGLIST(struct namecache *ncp) return (&neglists[(((uintptr_t)(ncp) >> 8) & ncneghash)]); } +static inline struct negstate * +NCP2NEGSTATE(struct namecache *ncp) +{ + + MPASS(ncp->nc_flag & NCF_NEGATIVE); + return (&ncp->nc_neg); +} + #define numbucketlocks (ncbuckethash + 1) static u_int __read_mostly ncbuckethash; static struct rwlock_padalign __read_mostly *bucketlocks; @@ -713,21 +732,32 @@ SYSCTL_PROC(_debug_hashstat, OID_AUTO, nchash, CTLTYPE * round-robin manner. */ static void +cache_negative_init(struct namecache *ncp) +{ + struct negstate *negstate; + + ncp->nc_flag |= NCF_NEGATIVE; + negstate = NCP2NEGSTATE(ncp); + negstate->neg_flag = 0; +} + +static void cache_negative_hit(struct namecache *ncp) { struct neglist *neglist; + struct negstate *negstate; - MPASS(ncp->nc_flag & NCF_NEGATIVE); - if (ncp->nc_flag & NCF_HOTNEGATIVE) + negstate = NCP2NEGSTATE(ncp); + if ((negstate->neg_flag & NEG_HOT) != 0) return; neglist = NCP2NEGLIST(ncp); mtx_lock(&ncneg_hot.nl_lock); mtx_lock(&neglist->nl_lock); - if (!(ncp->nc_flag & NCF_HOTNEGATIVE)) { + if ((negstate->neg_flag & NEG_HOT) == 0) { numhotneg++; TAILQ_REMOVE(&neglist->nl_list, ncp, nc_dst); TAILQ_INSERT_TAIL(&ncneg_hot.nl_list, ncp, nc_dst); - ncp->nc_flag |= NCF_HOTNEGATIVE; + negstate->neg_flag |= NEG_HOT; } mtx_unlock(&neglist->nl_lock); mtx_unlock(&ncneg_hot.nl_lock); @@ -756,17 +786,18 @@ static void cache_negative_remove(struct namecache *ncp, bool neg_locked) { struct neglist *neglist; + struct negstate *negstate; bool hot_locked = false; bool list_locked = false; - MPASS(ncp->nc_flag & NCF_NEGATIVE); cache_assert_bucket_locked(ncp, RA_WLOCKED); neglist = NCP2NEGLIST(ncp); + negstate = NCP2NEGSTATE(ncp); if (!neg_locked) { - if (ncp->nc_flag & NCF_HOTNEGATIVE) { + if ((negstate->neg_flag & NEG_HOT) != 0) { hot_locked = true; mtx_lock(&ncneg_hot.nl_lock); - if (!(ncp->nc_flag & NCF_HOTNEGATIVE)) { + if ((negstate->neg_flag & NEG_HOT) == 0) { list_locked = true; mtx_lock(&neglist->nl_lock); } @@ -775,7 +806,7 @@ cache_negative_remove(struct namecache *ncp, bool neg_ mtx_lock(&neglist->nl_lock); } } - if (ncp->nc_flag & NCF_HOTNEGATIVE) { + if ((negstate->neg_flag & NEG_HOT) != 0) { mtx_assert(&ncneg_hot.nl_lock, MA_OWNED); TAILQ_REMOVE(&ncneg_hot.nl_list, ncp, nc_dst); numhotneg--; @@ -821,6 +852,7 @@ cache_negative_zap_one(void) { struct namecache *ncp, *ncp2; struct neglist *neglist; + struct negstate *negstate; struct mtx *dvlp; struct rwlock *blp; @@ -834,10 +866,12 @@ cache_negative_zap_one(void) ncp = TAILQ_FIRST(&ncneg_hot.nl_list); if (ncp != NULL) { neglist = NCP2NEGLIST(ncp); + negstate = NCP2NEGSTATE(ncp); mtx_lock(&neglist->nl_lock); + MPASS((negstate->neg_flag & NEG_HOT) != 0); TAILQ_REMOVE(&ncneg_hot.nl_list, ncp, nc_dst); TAILQ_INSERT_TAIL(&neglist->nl_list, ncp, nc_dst); - ncp->nc_flag &= ~NCF_HOTNEGATIVE; + negstate->neg_flag &= ~NEG_HOT; numhotneg--; mtx_unlock(&neglist->nl_lock); } @@ -1337,6 +1371,7 @@ cache_lookup(struct vnode *dvp, struct vnode **vpp, st { struct namecache_ts *ncp_ts; struct namecache *ncp; + struct negstate *negstate; struct rwlock *blp; struct mtx *dvlp; uint32_t hash; @@ -1507,7 +1542,8 @@ negative_success: /* * We need to take locks to promote an entry. */ - if ((ncp->nc_flag & NCF_HOTNEGATIVE) == 0 || + negstate = NCP2NEGSTATE(ncp); + if ((negstate->neg_flag & NEG_HOT) == 0 || cache_ncp_invalid(ncp)) { vfs_smr_exit(); doing_smr = false; @@ -1834,7 +1870,7 @@ cache_enter_time(struct vnode *dvp, struct vnode *vp, ncp->nc_flag = flag; ncp->nc_vp = vp; if (vp == NULL) - ncp->nc_flag |= NCF_NEGATIVE; + cache_negative_init(ncp); ncp->nc_dvp = dvp; if (tsp != NULL) { ncp_ts = __containerof(ncp, struct namecache_ts, nc_nc); @@ -1869,11 +1905,7 @@ cache_enter_time(struct vnode *dvp, struct vnode *vp, n2_ts->nc_ticks = ncp_ts->nc_ticks; if (dtsp != NULL) { n2_ts->nc_dotdottime = ncp_ts->nc_dotdottime; - if (ncp->nc_flag & NCF_NEGATIVE) - mtx_lock(&ncneg_hot.nl_lock); n2_ts->nc_nc.nc_flag |= NCF_DTS; - if (ncp->nc_flag & NCF_NEGATIVE) - mtx_unlock(&ncneg_hot.nl_lock); } } goto out_unlock_free;