Skip site navigation (1)Skip section navigation (2) 
Date:      Sun, 17 Jan 2016 08:40:51 +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: r294205 - in head/sys: kern sys
Message-ID:  <201601170840.u0H8epU9015236@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: kib
Date: Sun Jan 17 08:40:51 2016
New Revision: 294205
URL: https://svnweb.freebsd.org/changeset/base/294205

Log:
  When cleaning up from failed adv locking and checking for write, do
  not call VOP_CLOSE() manually.  Instead, delegate the close to
  fo_close() performed as part of the fdrop() on the file failed to
  open.  For this, finish constructing file on error, in particular, set
  f_vnode and f_ops.
  
  Forcibly resetting f_ops to badfileops disabled additional cleanups
  performed by fo_close() for some file types, in this case it was noted
  that cdevpriv data was corrupted.  Since fo_close() call must be
  enabled for some file types, it makes more sense to enable it for all
  files opened through vn_open_cred().
  
  In collaboration with:	pho
  Sponsored by:	The FreeBSD Foundation
  MFC after:	2 weeks

Modified:
  head/sys/kern/vfs_vnops.c
  head/sys/sys/fcntl.h

Modified: head/sys/kern/vfs_vnops.c
==============================================================================
--- head/sys/kern/vfs_vnops.c	Sun Jan 17 08:34:35 2016	(r294204)
+++ head/sys/kern/vfs_vnops.c	Sun Jan 17 08:40:51 2016	(r294205)
@@ -299,10 +299,9 @@ int
 vn_open_vnode(struct vnode *vp, int fmode, struct ucred *cred,
     struct thread *td, struct file *fp)
 {
-	struct mount *mp;
 	accmode_t accmode;
 	struct flock lf;
-	int error, have_flock, lock_flags, type;
+	int error, lock_flags, type;
 
 	if (vp->v_type == VLNK)
 		return (EMLINK);
@@ -365,10 +364,12 @@ vn_open_vnode(struct vnode *vp, int fmod
 		if ((fmode & FNONBLOCK) == 0)
 			type |= F_WAIT;
 		error = VOP_ADVLOCK(vp, (caddr_t)fp, F_SETLK, &lf, type);
-		have_flock = (error == 0);
+		if (error == 0)
+			fp->f_flag |= FHASLOCK;
 		vn_lock(vp, lock_flags | LK_RETRY);
 		if (error == 0 && vp->v_iflag & VI_DOOMED)
 			error = ENOENT;
+
 		/*
 		 * Another thread might have used this vnode as an
 		 * executable while the vnode lock was dropped.
@@ -377,34 +378,24 @@ vn_open_vnode(struct vnode *vp, int fmod
 		 */
 		if (error == 0 && accmode & VWRITE)
 			error = vn_writechk(vp);
-		if (error) {
-			VOP_UNLOCK(vp, 0);
-			if (have_flock) {
-				lf.l_whence = SEEK_SET;
-				lf.l_start = 0;
-				lf.l_len = 0;
-				lf.l_type = F_UNLCK;
-				(void) VOP_ADVLOCK(vp, fp, F_UNLCK, &lf,
-				    F_FLOCK);
+
+		if (error != 0) {
+			fp->f_flag |= FOPENFAILED;
+			fp->f_vnode = vp;
+			if (fp->f_ops == &badfileops) {
+				fp->f_type = DTYPE_VNODE;
+				fp->f_ops = &vnops;
 			}
-			vn_start_write(vp, &mp, V_WAIT);
-			vn_lock(vp, lock_flags | LK_RETRY);
-			(void)VOP_CLOSE(vp, fmode, cred, td);
-			vn_finished_write(mp);
-			/* Prevent second close from fdrop()->vn_close(). */
-			if (fp != NULL)
-				fp->f_ops= &badfileops;
-			return (error);
+			vref(vp);
 		}
-		fp->f_flag |= FHASLOCK;
 	}
-	if (fmode & FWRITE) {
+	if (error == 0 && fmode & FWRITE) {
 		VOP_ADD_WRITECOUNT(vp, 1);
 		CTR3(KTR_VFS, "%s: vp %p v_writecount increased to %d",
 		    __func__, vp, vp->v_writecount);
 	}
 	ASSERT_VOP_LOCKED(vp, "vn_open_vnode");
-	return (0);
+	return (error);
 }
 
 /*
@@ -449,7 +440,7 @@ vn_close(vp, flags, file_cred, td)
 
 	vn_start_write(vp, &mp, V_WAIT);
 	vn_lock(vp, lock_flags | LK_RETRY);
-	if (flags & FWRITE) {
+	if ((flags & (FWRITE | FOPENFAILED)) == FWRITE) {
 		VNASSERT(vp->v_writecount > 0, vp, 
 		    ("vn_close: negative writecount"));
 		VOP_ADD_WRITECOUNT(vp, -1);

Modified: head/sys/sys/fcntl.h
==============================================================================
--- head/sys/sys/fcntl.h	Sun Jan 17 08:34:35 2016	(r294204)
+++ head/sys/sys/fcntl.h	Sun Jan 17 08:40:51 2016	(r294205)
@@ -142,6 +142,8 @@ typedef	__pid_t		pid_t;
 /* Only for devfs d_close() flags. */
 #define	FLASTCLOSE	O_DIRECTORY
 #define	FREVOKE		O_VERIFY
+/* Only for fo_close() from half-succeeded open */
+#define	FOPENFAILED	O_TTY_INIT
 
 /* convert from open() flags to/from fflags; convert O_RD/WR to FREAD/FWRITE */
 #define	FFLAGS(oflags)	((oflags) & O_EXEC ? (oflags) : (oflags) + 1)

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