zfs_rename: pick up and finish renameat2 flags support

Removing new txtypes in favor of compound ZIL operations, see comment in module/zfs/zfs_log.c. Other notable changes: - unlock after the inodes are updated - pass whiteout znode pointer to zfs_log_rename_whiteout - don't wrap code directly in ASSERT*(), it turn to noop on non-debug builds - update configure time tests for rename2 to support kernels from 3.5 to 4.8 Fixes openzfs#2256 Fixes openzfs#8648 Fixes openzfs#8774 Signed-off-by: Pavel Snajdr <[email protected]>
vpsfreecz · Oct 20, 2019 · 08955ab · 08955ab
1 parent 1179b25
commit 08955ab
Show file tree

Hide file tree

Showing 9 changed files with 199 additions and 81 deletions.
diff --git a/config/kernel-rename.m4 b/config/kernel-rename.m4
@@ -4,7 +4,7 @@ dnl # iops->rename2() merged into iops->rename(), and iops->rename() now wants
 dnl # flags.
 dnl #
 AC_DEFUN([ZFS_AC_KERNEL_SRC_RENAME_WANTS_FLAGS], [
-	ZFS_LINUX_TEST_SRC([inode_operations_rename], [
+	ZFS_LINUX_TEST_SRC([inode_operations_rename_flags], [
 		#include <linux/fs.h>
 		int rename_fn(struct inode *sip, struct dentry *sdp,
 			struct inode *tip, struct dentry *tdp,
@@ -19,11 +19,39 @@ AC_DEFUN([ZFS_AC_KERNEL_SRC_RENAME_WANTS_FLAGS], [
 
 AC_DEFUN([ZFS_AC_KERNEL_RENAME_WANTS_FLAGS], [
 	AC_MSG_CHECKING([whether iops->rename() wants flags])
-	ZFS_LINUX_TEST_RESULT([inode_operations_rename], [
+	ZFS_LINUX_TEST_RESULT([inode_operations_rename_flags], [
 		AC_MSG_RESULT(yes)
 		AC_DEFINE(HAVE_RENAME_WANTS_FLAGS, 1,
 		    [iops->rename() wants flags])
 	],[
 		AC_MSG_RESULT(no)
 	])
 ])
+
+dnl #
+dnl # iops->rename2() is supported since Linux 3.5
+dnl #
+AC_DEFUN([ZFS_AC_KERNEL_SRC_HAS_RENAME2], [
+	ZFS_LINUX_TEST_SRC([inode_operations_rename], [
+		#include <linux/fs.h>
+		int rename2_fn(struct inode *sip, struct dentry *sdp,
+			struct inode *tip, struct dentry *tdp,
+			unsigned int flags) { return 0; }
+
+		static const struct inode_operations
+		    iops __attribute__ ((unused)) = {
+			.rename2 = rename2_fn,
+		};
+	],[])
+])
+
+AC_DEFUN([ZFS_AC_KERNEL_HAS_RENAME2], [
+	AC_MSG_CHECKING([whether iops->rename() wants flags])
+	ZFS_LINUX_TEST_RESULT([inode_operations_rename], [
+		AC_MSG_RESULT(yes)
+		AC_DEFINE(HAVE_RENAME2, 1,
+		    [iops->rename2() exists])
+	],[
+		AC_MSG_RESULT(no)
+	])
+])
diff --git a/config/kernel.m4 b/config/kernel.m4
@@ -133,6 +133,7 @@ AC_DEFUN([ZFS_AC_KERNEL_TEST_SRC], [
 	ZFS_AC_KERNEL_SRC_KUIDGID_T
 	ZFS_AC_KERNEL_SRC_KUID_HELPERS
 	ZFS_AC_KERNEL_SRC_MODULE_PARAM_CALL_CONST
+	ZFS_AC_KERNEL_SRC_HAS_RENAME2
 	ZFS_AC_KERNEL_SRC_RENAME_WANTS_FLAGS
 	ZFS_AC_KERNEL_SRC_CURRENT_TIME
 	ZFS_AC_KERNEL_SRC_USERNS_CAPABILITIES
@@ -250,6 +251,7 @@ AC_DEFUN([ZFS_AC_KERNEL_TEST_RESULT], [
 	ZFS_AC_KERNEL_KUIDGID_T
 	ZFS_AC_KERNEL_KUID_HELPERS
 	ZFS_AC_KERNEL_MODULE_PARAM_CALL_CONST
+	ZFS_AC_KERNEL_HAS_RENAME2
 	ZFS_AC_KERNEL_RENAME_WANTS_FLAGS
 	ZFS_AC_KERNEL_CURRENT_TIME
 	ZFS_AC_KERNEL_USERNS_CAPABILITIES

diff --git a/include/sys/zfs_znode.h b/include/sys/zfs_znode.h
@@ -402,6 +402,13 @@ extern void zfs_log_link(zilog_t *zilog, dmu_tx_t *tx, uint64_t txtype,
     znode_t *dzp, znode_t *zp, char *name);
 extern void zfs_log_symlink(zilog_t *zilog, dmu_tx_t *tx, uint64_t txtype,
     znode_t *dzp, znode_t *zp, char *name, char *link);
+extern void zfs_log_rename_exchange(zilog_t *zilog, dmu_tx_t *tx,
+    uint64_t txtype, znode_t *sdzp, char *sname, znode_t *tdzp,
+    char *dname, znode_t *szp);
+extern void zfs_log_rename_whiteout(zilog_t *zilog, dmu_tx_t *tx,
+    uint64_t txtype, znode_t *sdzp, char *sname, znode_t *tdzp,
+    char *dname, znode_t *szp, znode_t *wzp, vsecattr_t *vsecp,
+    zfs_fuid_info_t *fuidp, vattr_t *vap);
 extern void zfs_log_rename(zilog_t *zilog, dmu_tx_t *tx, uint64_t txtype,
     znode_t *sdzp, char *sname, znode_t *tdzp, char *dname, znode_t *szp);
 extern void zfs_log_write(zilog_t *zilog, dmu_tx_t *tx, int txtype,

diff --git a/include/sys/zil.h b/include/sys/zil.h
@@ -162,9 +162,7 @@ typedef enum zil_create {
 #define	TX_MKDIR_ATTR		18	/* mkdir with attr */
 #define	TX_MKDIR_ACL_ATTR	19	/* mkdir with ACL + attrs */
 #define	TX_WRITE2		20	/* dmu_sync EALREADY write */
-#define	TX_EXCHANGE		21	/* Exchange two paths */
-#define	TX_WHITEOUT		22	/* Rename a file, leaving a whiteout */
-#define	TX_MAX_TYPE		23	/* Max transaction type */
+#define	TX_MAX_TYPE		21	/* Max transaction type */
 
 /*
  * The transactions for mkdir, symlink, remove, rmdir, link, and rename

diff --git a/module/os/linux/zfs/zfs_dir.c b/module/os/linux/zfs/zfs_dir.c
@@ -1015,7 +1015,8 @@ zfs_link_destroy(zfs_dirlock_t *dl, znode_t *zp, dmu_tx_t *tx, int flag,
 		}
 
 		/* The only error is !zfs_dirempty() and we checked earlier. */
-		ASSERT3U(zfs_drop_nlink_locked(zp, tx, &unlinked), ==, 0);
+		error = zfs_drop_nlink_locked(zp, tx, &unlinked);
+		ASSERT3U(error, ==, 0);
 		mutex_exit(&zp->z_lock);
 	} else {
 		error = zfs_dropname(dl, zp, dzp, tx, flag);

diff --git a/module/os/linux/zfs/zfs_vnops.c b/module/os/linux/zfs/zfs_vnops.c
@@ -3671,24 +3671,18 @@ zfs_rename(struct inode *sdip, char *snm, struct inode *tdip, char *tnm,
 	int		error = 0;
 	int		zflg = 0;
 	boolean_t	waited = B_FALSE;
-	uint64_t	txtype;
 	/* Needed for whiteout inode creation. */
 	vattr_t		wo_vap;
 	uint64_t	wo_projid;
 	boolean_t	fuid_dirtied;
 	zfs_acl_ids_t	acl_ids;
 	boolean_t	have_acl = B_FALSE;
+	znode_t		*wzp = NULL;
+
 
 	if (snm == NULL || tnm == NULL)
 		return (SET_ERROR(EINVAL));
 
-	if (flags & RENAME_EXCHANGE)
-		txtype = TX_EXCHANGE;
-	else if (flags & RENAME_WHITEOUT)
-		txtype = TX_WHITEOUT;
-	else
-		txtype = TX_RENAME;
-
 	ZFS_ENTER(zfsvfs);
 	zilog = zfsvfs->z_log;
 
@@ -3887,7 +3881,7 @@ zfs_rename(struct inode *sdip, char *snm, struct inode *tdip, char *tnm,
 		/*
 		 * Source and target must be the same type (unless exchanging).
 		 */
-		if (txtype != TX_EXCHANGE) {
+		if (!(flags & RENAME_EXCHANGE)) {
 			boolean_t s_is_dir = S_ISDIR(ZTOI(szp)->i_mode) != 0;
 			boolean_t t_is_dir = S_ISDIR(ZTOI(tzp)->i_mode) != 0;
 
@@ -3905,15 +3899,14 @@ zfs_rename(struct inode *sdip, char *snm, struct inode *tdip, char *tnm,
 			error = 0;
 			goto out;
 		}
-	}
-	/* Target must exist for RENAME_EXCHANGE. */
-	if (!tzp && txtype == TX_EXCHANGE) {
+	} else if (flags & RENAME_EXCHANGE) {
+		/* Target must exist for RENAME_EXCHANGE. */
 		error = SET_ERROR(ENOENT);
 		goto out;
 	}
 
 	/* Set up inode creation for RENAME_WHITEOUT. */
-	if (txtype == TX_WHITEOUT) {
+	if (flags & RENAME_WHITEOUT) {
 		error = zfs_zaccess(sdzp, ACE_ADD_FILE, 0, B_FALSE, cr);
 		if (error)
 			goto out;
@@ -3924,6 +3917,8 @@ zfs_rename(struct inode *sdip, char *snm, struct inode *tdip, char *tnm,
 
 		error = zfs_acl_ids_create(sdzp, 0, &wo_vap, cr, NULL,
 		    &acl_ids);
+		/* zpl_vap_init increases reference on cr */
+		crfree(cr);
 		if (error)
 			goto out;
 		have_acl = B_TRUE;
@@ -3937,7 +3932,7 @@ zfs_rename(struct inode *sdip, char *snm, struct inode *tdip, char *tnm,
 	tx = dmu_tx_create(zfsvfs->z_os);
 	dmu_tx_hold_sa(tx, szp->z_sa_hdl, B_FALSE);
 	dmu_tx_hold_sa(tx, sdzp->z_sa_hdl, B_FALSE);
-	dmu_tx_hold_zap(tx, sdzp->z_id, txtype == TX_EXCHANGE, snm);
+	dmu_tx_hold_zap(tx, sdzp->z_id, !!(flags & RENAME_EXCHANGE), snm);
 	dmu_tx_hold_zap(tx, tdzp->z_id, TRUE, tnm);
 	if (sdzp != tdzp) {
 		dmu_tx_hold_sa(tx, tdzp->z_sa_hdl, B_FALSE);
@@ -3947,7 +3942,7 @@ zfs_rename(struct inode *sdip, char *snm, struct inode *tdip, char *tnm,
 		dmu_tx_hold_sa(tx, tzp->z_sa_hdl, B_FALSE);
 		zfs_sa_upgrade_txholds(tx, tzp);
 	}
-	if (txtype == TX_WHITEOUT) {
+	if (flags & RENAME_WHITEOUT) {
 		dmu_tx_hold_sa_create(tx, acl_ids.z_aclp->z_acl_bytes +
 		    ZFS_SA_BASE_ATTR_SIZE);
 
@@ -4012,7 +4007,7 @@ zfs_rename(struct inode *sdip, char *snm, struct inode *tdip, char *tnm,
 	if (tzp) {
 		int tzflg = zflg;
 
-		if (txtype == TX_EXCHANGE) {
+		if (flags & RENAME_EXCHANGE) {
 			/* This inode will be re-linked soon. */
 			tzflg |= ZRENAMING;
 
@@ -4046,51 +4041,56 @@ zfs_rename(struct inode *sdip, char *snm, struct inode *tdip, char *tnm,
 		goto commit_link_tzp;
 	}
 
-	switch (txtype) {
-		case TX_EXCHANGE:
-			error = zfs_link_create(sdl, tzp, tx, ZRENAMING);
-			/*
-			 * The same argument as zfs_link_create() failing for
-			 * szp applies here, since the source directory must
-			 * have had an entry we are replacing.
-			 */
-			ASSERT3U(error, ==, 0);
-			if (error)
-				goto commit_unlink_td_szp;
-			break;
-		case TX_WHITEOUT: {
-			znode_t		*wzp;
-
-			zfs_mknode(sdzp, &wo_vap, tx, cr, 0, &wzp, &acl_ids);
-			error = zfs_link_create(sdl, wzp, tx, ZNEW);
-			if (error) {
-				zfs_znode_delete(wzp, tx);
-				remove_inode_hash(ZTOI(wzp));
-				goto commit_unlink_td_szp;
-			}
-			/* No need to zfs_log_create_txtype here. */
+	if (flags & RENAME_EXCHANGE) {
+		error = zfs_link_create(sdl, tzp, tx, ZRENAMING);
+		/*
+		 * The same argument as zfs_link_create() failing for
+		 * szp applies here, since the source directory must
+		 * have had an entry we are replacing.
+		 */
+		ASSERT3U(error, ==, 0);
+		if (error)
+			goto commit_unlink_td_szp;
+	} else if (flags & RENAME_WHITEOUT) {
+		zfs_mknode(sdzp, &wo_vap, tx, cr, 0, &wzp, &acl_ids);
+		error = zfs_link_create(sdl, wzp, tx, ZNEW);
+		if (error) {
+			zfs_znode_delete(wzp, tx);
+			remove_inode_hash(ZTOI(wzp));
+			goto commit_unlink_td_szp;
 		}
+		/* No need to zfs_log_create_txtype here. */
 	}
 
 	if (fuid_dirtied)
 		zfs_fuid_sync(zfsvfs, tx);
 
-	zfs_log_rename(zilog, tx, txtype |
-	    (flags & FIGNORECASE ? TX_CI : 0), sdzp,
-	    sdl->dl_name, tdzp, tdl->dl_name, szp);
+	if (flags & RENAME_EXCHANGE) {
+		zfs_log_rename_exchange(zilog, tx,
+		    (flags & FIGNORECASE ? TX_CI : 0), sdzp,
+		    sdl->dl_name, tdzp, tdl->dl_name, szp);
+	} else if (flags & RENAME_WHITEOUT) {
+		vsecattr_t vsecp;
+
+		vsecp.vsa_mask |= VSA_ACE_ALLTYPES;
+		error = zfs_getacl(szp, &vsecp, B_TRUE, cr);
+
+		zfs_log_rename_whiteout(zilog, tx,
+		    (flags & FIGNORECASE ? TX_CI : 0), sdzp,
+		    sdl->dl_name, tdzp, tdl->dl_name, szp, wzp,
+		    &vsecp, acl_ids.z_fuidp, &wo_vap);
+	} else {
+		zfs_log_rename(zilog, tx,
+		    (flags & FIGNORECASE ? TX_CI : 0), sdzp,
+		    sdl->dl_name, tdzp, tdl->dl_name, szp);
+	}
 
 commit:
 	dmu_tx_commit(tx);
 out:
 	if (have_acl)
 		zfs_acl_ids_free(&acl_ids);
 
-	if (zl != NULL)
-		zfs_rename_unlock(&zl);
-
-	zfs_dirent_unlock(sdl);
-	zfs_dirent_unlock(tdl);
-
 	zfs_inode_update(sdzp);
 	if (sdzp == tdzp)
 		rw_exit(&sdzp->z_name_lock);
@@ -4100,11 +4100,21 @@ zfs_rename(struct inode *sdip, char *snm, struct inode *tdip, char *tnm,
 
 	zfs_inode_update(szp);
 	iput(ZTOI(szp));
+	if (wzp) {
+		zfs_inode_update(wzp);
+		iput(ZTOI(wzp));
+	}
 	if (tzp) {
 		zfs_inode_update(tzp);
 		iput(ZTOI(tzp));
 	}
 
+	if (zl != NULL)
+		zfs_rename_unlock(&zl);
+
+	zfs_dirent_unlock(sdl);
+	zfs_dirent_unlock(tdl);
+
 	if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
 		zil_commit(zilog, 0);
 

diff --git a/module/os/linux/zfs/zpl_inode.c b/module/os/linux/zfs/zpl_inode.c
@@ -421,7 +421,7 @@ zpl_rename2(struct inode *sdip, struct dentry *sdentry,
 	return (error);
 }
 
-#ifndef HAVE_RENAME_WANTS_FLAGS
+#if defined(HAVE_RENAME_WANTS_FLAGS) && !defined(HAVE_RENAME2)
 static int
 zpl_rename(struct inode *sdip, struct dentry *sdentry,
     struct inode *tdip, struct dentry *tdentry)
@@ -690,11 +690,15 @@ const struct inode_operations zpl_dir_inode_operations = {
 	.mkdir		= zpl_mkdir,
 	.rmdir		= zpl_rmdir,
 	.mknod		= zpl_mknod,
+#ifdef HAVE_RENAME2
+	.rename2	= zpl_rename2,
+#else /* not HAVE_RENAME2, still might accept flags */
 #ifdef HAVE_RENAME_WANTS_FLAGS
 	.rename		= zpl_rename2,
-#else
+#else /* not HAVE_RENAME_WANTS_FLAGS, doesn't accept flags at all */
 	.rename		= zpl_rename,
-#endif
+#endif /* HAVE_RENAME_WANTS_FLAGS */
+#endif /* HAVE_RENAME2 */
 #ifdef HAVE_TMPFILE
 	.tmpfile	= zpl_tmpfile,
 #endif