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]>

AUTHORS

@@ -234,6 +234,7 @@ CONTRIBUTORS:
     Paul Dagnelie <[email protected]>
     Paul Zuchowski <[email protected]>
     Pavel Boldin <[email protected]>
+    Pavel Snajdr <[email protected]>
     Pavel Zakharov <[email protected]>
     Pawel Jakub Dawidek <[email protected]>
     Pedro Giffuni <[email protected]>

config/kernel-rename.m4

@@ -3,8 +3,8 @@ dnl # 4.9 API change,
 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], [
+AC_DEFUN([ZFS_AC_KERNEL_SRC_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,
@@ -15,15 +15,36 @@ AC_DEFUN([ZFS_AC_KERNEL_SRC_RENAME_WANTS_FLAGS], [
 			.rename = rename_fn,
 		};
 	],[])
+
+	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_RENAME_WANTS_FLAGS], [
+AC_DEFUN([ZFS_AC_KERNEL_RENAME], [
 	AC_MSG_CHECKING([whether iops->rename() wants flags])
 	ZFS_LINUX_TEST_RESULT([inode_operations_rename], [
 		AC_MSG_RESULT(yes)
-		AC_DEFINE(HAVE_RENAME_WANTS_FLAGS, 1,
-		    [iops->rename() wants flags])
+		AC_DEFINE(HAVE_RENAME2, 1, [iops->rename2() exists])
 	],[
 		AC_MSG_RESULT(no)
+
+		AC_MSG_CHECKING([whether iops->rename() wants flags])
+		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)
+		])
 	])
 ])
+

config/kernel.m4

@@ -113,7 +113,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_RENAME_WANTS_FLAGS
+	ZFS_AC_KERNEL_SRC_RENAME
 	ZFS_AC_KERNEL_SRC_CURRENT_TIME
 	ZFS_AC_KERNEL_SRC_USERNS_CAPABILITIES
 	ZFS_AC_KERNEL_SRC_IN_COMPAT_SYSCALL
@@ -208,7 +208,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_RENAME_WANTS_FLAGS
+	ZFS_AC_KERNEL_RENAME
 	ZFS_AC_KERNEL_CURRENT_TIME
 	ZFS_AC_KERNEL_USERNS_CAPABILITIES
 	ZFS_AC_KERNEL_IN_COMPAT_SYSCALL

include/sys/zfs_znode.h

@@ -273,6 +273,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,

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

module/os/linux/zfs/zfs_dir.c

@@ -1034,7 +1034,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);

module/os/linux/zfs/zfs_vnops.c

@@ -3503,24 +3503,18 @@ zfs_rename(znode_t *sdzp, char *snm, znode_t *tdzp, 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;
 
@@ -3718,7 +3712,7 @@ zfs_rename(znode_t *sdzp, char *snm, znode_t *tdzp, 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;
 
@@ -3736,15 +3730,14 @@ zfs_rename(znode_t *sdzp, char *snm, znode_t *tdzp, 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;
@@ -3768,7 +3761,7 @@ zfs_rename(znode_t *sdzp, char *snm, znode_t *tdzp, 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);
@@ -3778,7 +3771,7 @@ zfs_rename(znode_t *sdzp, char *snm, znode_t *tdzp, 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);
 
@@ -3831,7 +3824,7 @@ zfs_rename(znode_t *sdzp, char *snm, znode_t *tdzp, char *tnm,
 
 	error = sa_update(szp->z_sa_hdl, SA_ZPL_FLAGS(zfsvfs),
 	    (void *)&szp->z_pflags, sizeof (uint64_t), tx);
-	ASSERT0(error);
+	VERIFY0(error);
 
 	error = zfs_link_destroy(sdl, szp, tx, ZRENAMING, NULL);
 	if (error)
@@ -3843,7 +3836,7 @@ zfs_rename(znode_t *sdzp, char *snm, znode_t *tdzp, char *tnm,
 	if (tzp) {
 		int tzflg = zflg;
 
-		if (txtype == TX_EXCHANGE) {
+		if (flags & RENAME_EXCHANGE) {
 			/* This inode will be re-linked soon. */
 			tzflg |= ZRENAMING;
 
@@ -3877,51 +3870,57 @@ zfs_rename(znode_t *sdzp, char *snm, znode_t *tdzp, 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);
+		VERIFY0(error);
+
+		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);
@@ -3931,11 +3930,21 @@ zfs_rename(znode_t *sdzp, char *snm, znode_t *tdzp, char *tnm,
 
 	zfs_inode_update(szp);
 	zrele(szp);
+	if (wzp) {
+		zfs_inode_update(wzp);
+		zrele(wzp);
+	}
 	if (tzp) {
 		zfs_inode_update(tzp);
 		zrele(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);
 

module/os/linux/zfs/zpl_inode.c

@@ -420,7 +420,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)
@@ -680,7 +680,9 @@ const struct inode_operations zpl_dir_inode_operations = {
 	.mkdir		= zpl_mkdir,
 	.rmdir		= zpl_rmdir,
 	.mknod		= zpl_mknod,
-#ifdef HAVE_RENAME_WANTS_FLAGS
+#ifdef HAVE_RENAME2
+	.rename2	= zpl_rename2,
+#elif defined(HAVE_RENAME_WANTS_FLAGS)
 	.rename		= zpl_rename2,
 #else
 	.rename		= zpl_rename,