From owner-svn-src-all@freebsd.org Fri May 26 11:39:36 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 2954AD82B33; Fri, 26 May 2017 11:39:36 +0000 (UTC) (envelope-from avg@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 mx1.freebsd.org (Postfix) with ESMTPS id 00DE310D0; Fri, 26 May 2017 11:39:35 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v4QBdYr4080469; Fri, 26 May 2017 11:39:34 GMT (envelope-from avg@FreeBSD.org) Received: (from avg@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v4QBdY1w080462; Fri, 26 May 2017 11:39:34 GMT (envelope-from avg@FreeBSD.org) Message-Id: <201705261139.v4QBdY1w080462@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: avg set sender to avg@FreeBSD.org using -f From: Andriy Gapon Date: Fri, 26 May 2017 11:39:34 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-vendor@freebsd.org Subject: svn commit: r318933 - in vendor-sys/illumos/dist/uts/common: fs fs/zfs sys X-SVN-Group: vendor-sys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 May 2017 11:39:36 -0000 Author: avg Date: Fri May 26 11:39:34 2017 New Revision: 318933 URL: https://svnweb.freebsd.org/changeset/base/318933 Log: 8064 need a static DTrace probe in VN_HOLD illumos/illumos-gate@ade42b557a6e29c3d17a61b1535d99af10e379be https://github.com/illumos/illumos-gate/commit/ade42b557a6e29c3d17a61b1535d99af10e379be https://www.illumos.org/issues/8064 It's currently nearly impossible to trace what process places a hold on a vnode, as the only ways holds are place is via the `VN_HOLD()` and `VN_HOLD_CALLER()` macros, which inline the bumping of `v_count`. Adding static DTrace probes to these macros would enable tracing of where specific vnode references come from. For completeness and symmetry, a similar static probe should be added to `vn_rele()` and `vn_rele_dnlc()`. Reviewed by: Pavel Zakharov Reviewed by: Prakash Surya Reviewed by: Prashanth Sreenivasa Reviewed by: Matthew Ahrens Approved by: Robert Mustacchi Author: Sebastien Roy Modified: vendor-sys/illumos/dist/uts/common/fs/gfs.c vendor-sys/illumos/dist/uts/common/fs/vnode.c vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_ctldir.c vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_vnops.c vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_znode.c vendor-sys/illumos/dist/uts/common/sys/vnode.h Modified: vendor-sys/illumos/dist/uts/common/fs/gfs.c ============================================================================== --- vendor-sys/illumos/dist/uts/common/fs/gfs.c Fri May 26 11:37:11 2017 (r318932) +++ vendor-sys/illumos/dist/uts/common/fs/gfs.c Fri May 26 11:39:34 2017 (r318933) @@ -23,8 +23,9 @@ * Copyright 2008 Sun Microsystems, Inc. All rights reserved. * Use is subject to license terms. */ - -#pragma ident "%Z%%M% %I% %E% SMI" +/* + * Copyright (c) 2017 by Delphix. All rights reserved. + */ #include #include @@ -671,7 +672,7 @@ found: } vn_free(vp); } else { - vp->v_count--; + VN_RELE_LOCKED(vp); data = NULL; mutex_exit(&vp->v_lock); if (vp->v_flag & V_XATTRDIR) { Modified: vendor-sys/illumos/dist/uts/common/fs/vnode.c ============================================================================== --- vendor-sys/illumos/dist/uts/common/fs/vnode.c Fri May 26 11:37:11 2017 (r318932) +++ vendor-sys/illumos/dist/uts/common/fs/vnode.c Fri May 26 11:39:34 2017 (r318933) @@ -22,6 +22,8 @@ /* * Copyright (c) 1988, 2010, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2013, Joyent, Inc. All rights reserved. + * Copyright 2016 Nexenta Systems, Inc. All rights reserved. + * Copyright (c) 2011, 2017 by Delphix. All rights reserved. */ /* Copyright (c) 1983, 1984, 1985, 1986, 1987, 1988, 1989 AT&T */ @@ -836,7 +838,7 @@ vn_rele(vnode_t *vp) VOP_INACTIVE(vp, CRED(), NULL); return; } - vp->v_count--; + VN_RELE_LOCKED(vp); mutex_exit(&vp->v_lock); } @@ -857,15 +859,15 @@ vn_rele_dnlc(vnode_t *vp) VOP_INACTIVE(vp, CRED(), NULL); return; } - vp->v_count--; + VN_RELE_LOCKED(vp); } mutex_exit(&vp->v_lock); } /* * Like vn_rele() except that it clears v_stream under v_lock. - * This is used by sockfs when it dismantels the association between - * the sockfs node and the vnode in the underlaying file system. + * This is used by sockfs when it dismantles the association between + * the sockfs node and the vnode in the underlying file system. * v_lock has to be held to prevent a thread coming through the lookupname * path from accessing a stream head that is going away. */ @@ -880,7 +882,7 @@ vn_rele_stream(vnode_t *vp) VOP_INACTIVE(vp, CRED(), NULL); return; } - vp->v_count--; + VN_RELE_LOCKED(vp); mutex_exit(&vp->v_lock); } @@ -911,7 +913,7 @@ vn_rele_async(vnode_t *vp, taskq_t *task vp, TQ_SLEEP) != NULL); return; } - vp->v_count--; + VN_RELE_LOCKED(vp); mutex_exit(&vp->v_lock); } Modified: vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_ctldir.c ============================================================================== --- vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_ctldir.c Fri May 26 11:37:11 2017 (r318932) +++ vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_ctldir.c Fri May 26 11:39:34 2017 (r318933) @@ -20,7 +20,7 @@ */ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2012, 2015 by Delphix. All rights reserved. + * Copyright (c) 2012, 2017 by Delphix. All rights reserved. * Copyright 2015, OmniTI Computer Consulting, Inc. All rights reserved. */ @@ -1210,7 +1210,7 @@ zfsctl_snapshot_inactive(vnode_t *vp, cr mutex_enter(&vp->v_lock); if (vp->v_count > 1) { - vp->v_count--; + VN_RELE_LOCKED(vp); mutex_exit(&vp->v_lock); mutex_exit(&sdp->sd_lock); VN_RELE(dvp); Modified: vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_vnops.c ============================================================================== --- vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_vnops.c Fri May 26 11:37:11 2017 (r318932) +++ vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_vnops.c Fri May 26 11:39:34 2017 (r318933) @@ -21,7 +21,7 @@ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2012, 2015 by Delphix. All rights reserved. + * Copyright (c) 2012, 2017 by Delphix. All rights reserved. * Copyright (c) 2014 Integros [integros.com] * Copyright 2015 Joyent, Inc. * Copyright 2017 Nexenta Systems, Inc. @@ -1819,7 +1819,7 @@ top: ASSERT0(error); } mutex_enter(&vp->v_lock); - vp->v_count--; + VN_RELE_LOCKED(vp); ASSERT0(vp->v_count); mutex_exit(&vp->v_lock); mutex_exit(&zp->z_lock); @@ -4411,7 +4411,7 @@ zfs_inactive(vnode_t *vp, cred_t *cr, ca mutex_enter(&zp->z_lock); mutex_enter(&vp->v_lock); ASSERT(vp->v_count == 1); - vp->v_count = 0; + VN_RELE_LOCKED(vp); mutex_exit(&vp->v_lock); mutex_exit(&zp->z_lock); rw_exit(&zfsvfs->z_teardown_inactive_lock); Modified: vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_znode.c ============================================================================== --- vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_znode.c Fri May 26 11:37:11 2017 (r318932) +++ vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_znode.c Fri May 26 11:39:34 2017 (r318933) @@ -20,7 +20,7 @@ */ /* * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. - * Copyright (c) 2012, 2014 by Delphix. All rights reserved. + * Copyright (c) 2012, 2017 by Delphix. All rights reserved. * Copyright (c) 2014 Integros [integros.com] */ @@ -1293,7 +1293,7 @@ zfs_zinactive(znode_t *zp) mutex_enter(&zp->z_lock); mutex_enter(&vp->v_lock); - vp->v_count--; + VN_RELE_LOCKED(vp); if (vp->v_count > 0 || vn_has_cached_data(vp)) { /* * If the hold count is greater than zero, somebody has Modified: vendor-sys/illumos/dist/uts/common/sys/vnode.h ============================================================================== --- vendor-sys/illumos/dist/uts/common/sys/vnode.h Fri May 26 11:37:11 2017 (r318932) +++ vendor-sys/illumos/dist/uts/common/sys/vnode.h Fri May 26 11:39:34 2017 (r318933) @@ -22,6 +22,7 @@ /* * Copyright (c) 1988, 2010, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2013, Joyent, Inc. All rights reserved. + * Copyright (c) 2011, 2017 by Delphix. All rights reserved. */ /* Copyright (c) 1983, 1984, 1985, 1986, 1987, 1988, 1989 AT&T */ @@ -53,6 +54,7 @@ #include #ifdef _KERNEL #include +#include #endif /* _KERNEL */ #ifdef __cplusplus @@ -1342,10 +1344,32 @@ int vn_vmpss_usepageio(vnode_t *); */ extern uint_t pvn_vmodsort_supported; -#define VN_HOLD(vp) { \ - mutex_enter(&(vp)->v_lock); \ - (vp)->v_count++; \ - mutex_exit(&(vp)->v_lock); \ +/* + * All changes to v_count should be done through VN_HOLD() or VN_RELE(), or + * one of their variants. This makes it possible to ensure proper locking, + * and to guarantee that all modifications are accompanied by a firing of + * the vn-hold or vn-rele SDT DTrace probe. + * + * Example DTrace command for tracing vnode references using these probes: + * + * dtrace -q -n 'sdt:::vn-hold,sdt:::vn-rele + * { + * this->vp = (vnode_t *)arg0; + * printf("%s %s(%p[%s]) %d\n", execname, probename, this->vp, + * this->vp->v_path == NULL ? "NULL" : stringof(this->vp->v_path), + * this->vp->v_count) + * }' + */ +#define VN_HOLD_LOCKED(vp) { \ + ASSERT(mutex_owned(&(vp)->v_lock)); \ + (vp)->v_count++; \ + DTRACE_PROBE1(vn__hold, vnode_t *, vp); \ +} + +#define VN_HOLD(vp) { \ + mutex_enter(&(vp)->v_lock); \ + VN_HOLD_LOCKED(vp); \ + mutex_exit(&(vp)->v_lock); \ } #define VN_RELE(vp) { \ @@ -1356,6 +1380,13 @@ extern uint_t pvn_vmodsort_supported; vn_rele_async(vp, taskq); \ } +#define VN_RELE_LOCKED(vp) { \ + ASSERT(mutex_owned(&(vp)->v_lock)); \ + ASSERT((vp)->v_count >= 1); \ + (vp)->v_count--; \ + DTRACE_PROBE1(vn__rele, vnode_t *, vp); \ +} + #define VN_SET_VFS_TYPE_DEV(vp, vfsp, type, dev) { \ (vp)->v_vfsp = (vfsp); \ (vp)->v_type = (type); \