Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 7 Sep 2015 20:02:56 +0000 (UTC)
From:      Mateusz Guzik <mjg@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r287539 - in head/sys: kern sys
Message-ID:  <201509072002.t87K2uRv045187@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: mjg
Date: Mon Sep  7 20:02:56 2015
New Revision: 287539
URL: https://svnweb.freebsd.org/changeset/base/287539

Log:
  fd: make the common case in filecaps_copy work lockless
  
  The filedesc lock is only needed if ioctls caps are present, which is a
  rare situation. This is a step towards reducing the scope of the filedesc
  lock.

Modified:
  head/sys/kern/kern_descrip.c
  head/sys/kern/uipc_usrreq.c
  head/sys/sys/filedesc.h

Modified: head/sys/kern/kern_descrip.c
==============================================================================
--- head/sys/kern/kern_descrip.c	Mon Sep  7 17:56:49 2015	(r287538)
+++ head/sys/kern/kern_descrip.c	Mon Sep  7 20:02:56 2015	(r287539)
@@ -911,7 +911,7 @@ kern_dup(struct thread *td, u_int mode, 
 #endif
 	filecaps_free(&newfde->fde_caps);
 	memcpy(newfde, oldfde, fde_change_size);
-	filecaps_copy(&oldfde->fde_caps, &newfde->fde_caps);
+	filecaps_copy(&oldfde->fde_caps, &newfde->fde_caps, true);
 	if ((flags & FDDUP_FLAG_CLOEXEC) != 0)
 		newfde->fde_flags = oldfde->fde_flags | UF_EXCLOSE;
 	else
@@ -1433,21 +1433,31 @@ filecaps_init(struct filecaps *fcaps)
 
 /*
  * Copy filecaps structure allocating memory for ioctls array if needed.
+ *
+ * The last parameter indicates whether the fdtable is locked. If it is not and
+ * ioctls are encountered, copying fails and the caller must lock the table.
+ *
+ * Note that if the table was not locked, the caller has to check the relevant
+ * sequence counter to determine whether the operation was successful.
  */
-void
-filecaps_copy(const struct filecaps *src, struct filecaps *dst)
+int
+filecaps_copy(const struct filecaps *src, struct filecaps *dst, bool locked)
 {
 	size_t size;
 
 	*dst = *src;
-	if (src->fc_ioctls != NULL) {
-		KASSERT(src->fc_nioctls > 0,
-		    ("fc_ioctls != NULL, but fc_nioctls=%hd", src->fc_nioctls));
-
-		size = sizeof(src->fc_ioctls[0]) * src->fc_nioctls;
-		dst->fc_ioctls = malloc(size, M_FILECAPS, M_WAITOK);
-		bcopy(src->fc_ioctls, dst->fc_ioctls, size);
-	}
+	if (src->fc_ioctls == NULL)
+		return (0);
+	if (!locked)
+		return (1);
+
+	KASSERT(src->fc_nioctls > 0,
+	    ("fc_ioctls != NULL, but fc_nioctls=%hd", src->fc_nioctls));
+
+	size = sizeof(src->fc_ioctls[0]) * src->fc_nioctls;
+	dst->fc_ioctls = malloc(size, M_FILECAPS, M_WAITOK);
+	bcopy(src->fc_ioctls, dst->fc_ioctls, size);
+	return (0);
 }
 
 /*
@@ -1956,7 +1966,7 @@ fdcopy(struct filedesc *fdp)
 		}
 		nfde = &newfdp->fd_ofiles[i];
 		*nfde = *ofde;
-		filecaps_copy(&ofde->fde_caps, &nfde->fde_caps);
+		filecaps_copy(&ofde->fde_caps, &nfde->fde_caps, true);
 		fhold(nfde->fde_file);
 		fdused_init(newfdp, i);
 		newfdp->fd_lastfile = i;
@@ -2012,7 +2022,7 @@ fdcopy_remapped(struct filedesc *fdp, co
 		}
 		nfde = &newfdp->fd_ofiles[i];
 		*nfde = *ofde;
-		filecaps_copy(&ofde->fde_caps, &nfde->fde_caps);
+		filecaps_copy(&ofde->fde_caps, &nfde->fde_caps, true);
 		fhold(nfde->fde_file);
 		fdused_init(newfdp, i);
 		newfdp->fd_lastfile = i;
@@ -2723,7 +2733,7 @@ fgetvp_rights(struct thread *td, int fd,
 
 	*vpp = fp->f_vnode;
 	vref(*vpp);
-	filecaps_copy(&fdp->fd_ofiles[fd].fde_caps, havecaps);
+	filecaps_copy(&fdp->fd_ofiles[fd].fde_caps, havecaps, true);
 
 	return (0);
 }
@@ -2938,7 +2948,7 @@ dupfdopen(struct thread *td, struct file
 		seq_write_begin(&newfde->fde_seq);
 #endif
 		memcpy(newfde, oldfde, fde_change_size);
-		filecaps_copy(&oldfde->fde_caps, &newfde->fde_caps);
+		filecaps_copy(&oldfde->fde_caps, &newfde->fde_caps, true);
 #ifdef CAPABILITIES
 		seq_write_end(&newfde->fde_seq);
 #endif

Modified: head/sys/kern/uipc_usrreq.c
==============================================================================
--- head/sys/kern/uipc_usrreq.c	Mon Sep  7 17:56:49 2015	(r287538)
+++ head/sys/kern/uipc_usrreq.c	Mon Sep  7 20:02:56 2015	(r287539)
@@ -1972,7 +1972,7 @@ unp_internalize(struct mbuf **controlp, 
 				fdep[i] = fdev;
 				fdep[i]->fde_file = fde->fde_file;
 				filecaps_copy(&fde->fde_caps,
-				    &fdep[i]->fde_caps);
+				    &fdep[i]->fde_caps, true);
 				unp_internalize_fp(fdep[i]->fde_file);
 			}
 			FILEDESC_SUNLOCK(fdesc);

Modified: head/sys/sys/filedesc.h
==============================================================================
--- head/sys/sys/filedesc.h	Mon Sep  7 17:56:49 2015	(r287538)
+++ head/sys/sys/filedesc.h	Mon Sep  7 20:02:56 2015	(r287539)
@@ -153,7 +153,8 @@ enum {
 struct thread;
 
 void	filecaps_init(struct filecaps *fcaps);
-void	filecaps_copy(const struct filecaps *src, struct filecaps *dst);
+int	filecaps_copy(const struct filecaps *src, struct filecaps *dst,
+	    bool locked);
 void	filecaps_move(struct filecaps *src, struct filecaps *dst);
 void	filecaps_free(struct filecaps *fcaps);

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