Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 17 Sep 2018 19:38:43 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r338733 - head/sys/ufs/ufs
Message-ID:  <201809171938.w8HJchX9069081@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: kib
Date: Mon Sep 17 19:38:43 2018
New Revision: 338733
URL: https://svnweb.freebsd.org/changeset/base/338733

Log:
  Do not upgrade the vnode lock to call getinoquota().
  
  Doing so can deadlock when the thread already owns another vnode lock,
  e.g. during a rename, as was demonstrated by the reporter.  In fact,
  there seems to be no need to force the call to getinoquota() always,
  because vn_open() locks vnode exclusively, and this is the most
  important case.  To add to the point, directories where the dirent is
  added or removed, are locked exclusively as well.
  
  Reported by:	bwidawsk
  Tested by:	bwidawsk, pho (as part of the larger patch)
  Sponsored by:	The FreeBSD Foundation
  Approved by:	re (gjb)
  MFC after:	1 week

Modified:
  head/sys/ufs/ufs/ufs_vnops.c

Modified: head/sys/ufs/ufs/ufs_vnops.c
==============================================================================
--- head/sys/ufs/ufs/ufs_vnops.c	Mon Sep 17 19:20:50 2018	(r338732)
+++ head/sys/ufs/ufs/ufs_vnops.c	Mon Sep 17 19:38:43 2018	(r338733)
@@ -325,9 +325,6 @@ ufs_accessx(ap)
 	struct inode *ip = VTOI(vp);
 	accmode_t accmode = ap->a_accmode;
 	int error;
-#ifdef QUOTA
-	int relocked;
-#endif
 #ifdef UFS_ACL
 	struct acl *acl;
 	acl_type_t type;
@@ -350,32 +347,14 @@ ufs_accessx(ap)
 			 * Inode is accounted in the quotas only if struct
 			 * dquot is attached to it. VOP_ACCESS() is called
 			 * from vn_open_cred() and provides a convenient
-			 * point to call getinoquota().
+			 * point to call getinoquota().  The lock mode is
+			 * exclusive when the file is opening for write.
 			 */
-			if (VOP_ISLOCKED(vp) != LK_EXCLUSIVE) {
-
-				/*
-				 * Upgrade vnode lock, since getinoquota()
-				 * requires exclusive lock to modify inode.
-				 */
-				relocked = 1;
-				vhold(vp);
-				vn_lock(vp, LK_UPGRADE | LK_RETRY);
-				VI_LOCK(vp);
-				if (vp->v_iflag & VI_DOOMED) {
-					vdropl(vp);
-					error = ENOENT;
-					goto relock;
-				}
-				vdropl(vp);
-			} else
-				relocked = 0;
-			error = getinoquota(ip);
-relock:
-			if (relocked)
-				vn_lock(vp, LK_DOWNGRADE | LK_RETRY);
-			if (error != 0)
-				return (error);
+			if (VOP_ISLOCKED(vp) == LK_EXCLUSIVE) {
+				error = getinoquota(ip);
+				if (error != 0)
+					return (error);
+			}
 #endif
 			break;
 		default:

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201809171938.w8HJchX9069081>