From 3ecd65d3375f0a4fa4c28f9b59cdf6a1f6fd51b8 Mon Sep 17 00:00:00 2001 From: Rainer Toebbicke Date: Mon, 8 Nov 2010 21:59:09 -0500 Subject: [PATCH] Lockless path through afs_linux_dentry_revalidate Permit a popular path through afs_linux_dentry_revalidate to pass without taking a lock which it actually does not need. This affects multi-core software-build nodes in particular, where serialization and high stat() counts restricts useful processing to a single core. Change-Id: I6151a1240519d9f91f6e258af71b797ce276f4e1 Reviewed-on: http://gerrit.openafs.org/3298 Tested-by: BuildBot Reviewed-by: Derrick Brashear Tested-by: Derrick Brashear --- src/afs/LINUX/osi_vnodeops.c | 99 ++++++++++++++++++++---------------- 1 file changed, 56 insertions(+), 43 deletions(-) diff --git a/src/afs/LINUX/osi_vnodeops.c b/src/afs/LINUX/osi_vnodeops.c index 780c8efd3..c9d67097e 100644 --- a/src/afs/LINUX/osi_vnodeops.c +++ b/src/afs/LINUX/osi_vnodeops.c @@ -699,13 +699,15 @@ struct file_operations afs_file_fops = { * AFS Linux dentry operations **********************************************************************/ -/* check_bad_parent() : Checks if this dentry's vcache is a root vcache +/* fix_bad_parent() : called if this dentry's vcache is a root vcache * that has its mvid (parent dir's fid) pointer set to the wrong directory - * due to being mounted in multiple points at once. If so, check_bad_parent() + * due to being mounted in multiple points at once. fix_bad_parent() * calls afs_lookup() to correct the vcache's mvid, as well as the volume's * dotdotfid and mtpoint fid members. * Parameters: * dp - dentry to be checked. + * credp - credentials + * vcp, pvc - item's and parent's vcache pointer * Return Values: * None. * Sideeffects: @@ -716,33 +718,20 @@ struct file_operations afs_file_fops = { */ static inline void -check_bad_parent(struct dentry *dp) +fix_bad_parent(struct dentry *dp, cred_t *credp, struct vcache *vcp, struct vcache *pvc) { - cred_t *credp; - struct dentry *parent; - struct vcache *vcp, *pvc, *avc = NULL; - - vcp = VTOAFS(dp->d_inode); - parent = dget_parent(dp); - pvc = VTOAFS(parent->d_inode); - - if (vcp->mvid->Fid.Volume != pvc->f.fid.Fid.Volume) { /* bad parent */ - credp = crref(); - - /* force a lookup, so vcp->mvid is fixed up */ - afs_lookup(pvc, (char *)dp->d_name.name, &avc, credp); - if (!avc || vcp != avc) { /* bad, very bad.. */ - afs_Trace4(afs_iclSetp, CM_TRACE_TMP_1S3L, ICL_TYPE_STRING, - "check_bad_parent: bad pointer returned from afs_lookup origvc newvc dentry", - ICL_TYPE_POINTER, vcp, ICL_TYPE_POINTER, avc, - ICL_TYPE_POINTER, dp); - } - if (avc) - AFS_RELE(AFSTOV(avc)); - crfree(credp); + struct vcache *avc = NULL; + + /* force a lookup, so vcp->mvid is fixed up */ + afs_lookup(pvc, (char *)dp->d_name.name, &avc, credp); + if (!avc || vcp != avc) { /* bad, very bad.. */ + afs_Trace4(afs_iclSetp, CM_TRACE_TMP_1S3L, ICL_TYPE_STRING, + "check_bad_parent: bad pointer returned from afs_lookup origvc newvc dentry", + ICL_TYPE_POINTER, vcp, ICL_TYPE_POINTER, avc, + ICL_TYPE_POINTER, dp); } - - dput(parent); + if (avc) + AFS_RELE(AFSTOV(avc)); return; } @@ -765,12 +754,18 @@ afs_linux_revalidate(struct dentry *dp) #ifdef notyet /* Make this a fast path (no crref), since it's called so often. */ - if (vcp->f.states & CStatd) { + if (vcp->states & CStatd) { + struct vcache *pvc = VTOAFS(dp->d_parent->d_inode); - if (*dp->d_name.name != '/' && vcp->mvstat == 2) /* root vnode */ - check_bad_parent(dp); /* check and correct mvid */ - - AFS_GUNLOCK(); + if (*dp->d_name.name != '/' && vcp->mvstat == 2) { /* root vnode */ + if (vcp->mvid->Fid.Volume != pvc->fid.Fid.Volume) { /* bad parent */ + credp = crref(); + AFS_GLOCK(); + fix_bad_parent(dp); /* check and correct mvid */ + AFS_GUNLOCK(); + crfree(credp); + } + } return 0; } #endif @@ -788,6 +783,7 @@ afs_linux_revalidate(struct dentry *dp) code = afs_getattr(vcp, &vattr, credp); crfree(credp); } + if (!code) afs_fill_inode(AFSTOV(vcp), &vattr); @@ -812,6 +808,8 @@ afs_linux_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *sta * we are advised to follow the entry if it is a link or to make sure that * it is a directory. But since the kernel itself checks these possibilities * later on, we shouldn't have to do it until later. Perhaps in the future.. + * + * The code here assumes that on entry the global lock is not held */ static int #ifdef DOP_REVALIDATE_TAKES_NAMEIDATA @@ -826,23 +824,30 @@ afs_linux_dentry_revalidate(struct dentry *dp, int flags) struct dentry *parent; int valid; struct afs_fakestat_state fakestate; + int locked = 0; - AFS_GLOCK(); afs_InitFakeStat(&fakestate); if (dp->d_inode) { + parent = dget_parent(dp); + pvcp = VTOAFS(parent->d_inode); vcp = VTOAFS(dp->d_inode); if (vcp == afs_globalVp) goto good_dentry; - if (vcp->mvstat == 1) { /* mount point */ + if ((vcp->mvstat == 1) || (vcp->mvstat == 2)) { /* need to lock */ + credp = crref(); + AFS_GLOCK(); + locked = 1; + } + + if (locked && vcp->mvstat == 1) { /* mount point */ if (vcp->mvid && (vcp->f.states & CMValid)) { int tryEvalOnly = 0; int code = 0; struct vrequest treq; - credp = crref(); code = afs_InitReq(&treq, credp); if ( (strcmp(dp->d_name.name, ".directory") == 0)) { @@ -858,8 +863,11 @@ afs_linux_dentry_revalidate(struct dentry *dp, int flags) } } } else - if (*dp->d_name.name != '/' && vcp->mvstat == 2) /* root vnode */ - check_bad_parent(dp); /* check and correct mvid */ + if (locked && *dp->d_name.name != '/' && vcp->mvstat == 2) { /* root vnode */ + if (vcp->mvid->Fid.Volume != pvcp->f.fid.Fid.Volume) { /* bad parent */ + fix_bad_parent(dp, credp, vcp, pvcp); /* check and correct mvid */ + } + } #ifdef notdef /* If the last looker changes, we should make sure the current @@ -874,17 +882,19 @@ afs_linux_dentry_revalidate(struct dentry *dp, int flags) } #endif - parent = dget_parent(dp); - pvcp = VTOAFS(parent->d_inode); /* If the parent's DataVersion has changed or the vnode * is longer valid, we need to do a full lookup. VerifyVCache * isn't enough since the vnode may have been renamed. */ - if (hgetlo(pvcp->f.m.DataVersion) > dp->d_time || !(vcp->f.states & CStatd)) { - + if ((!locked) && (hgetlo(pvcp->f.m.DataVersion) > dp->d_time || !(vcp->f.states & CStatd)) ) { credp = crref(); + AFS_GLOCK(); + locked = 1; + } + + if (locked && (hgetlo(pvcp->f.m.DataVersion) > dp->d_time || !(vcp->f.states & CStatd))) { afs_lookup(pvcp, (char *)dp->d_name.name, &tvc, credp); if (!tvc || tvc != vcp) { dput(parent); @@ -930,8 +940,11 @@ afs_linux_dentry_revalidate(struct dentry *dp, int flags) /* Clean up */ if (tvc) afs_PutVCache(tvc); - afs_PutFakeStat(&fakestate); - AFS_GUNLOCK(); + afs_PutFakeStat(&fakestate); /* from here on vcp may be no longer valid */ + if (locked) { + /* we hold the global lock if we evaluated a mount point */ + AFS_GUNLOCK(); + } if (credp) crfree(credp); -- 2.39.5