Skip to content

Commit

Permalink
zfs_rename: pick up and finish renameat2 flags support
Browse files Browse the repository at this point in the history
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 <snajpa@snajpa.net>
  • Loading branch information
snajpa committed Apr 27, 2021
1 parent f09cbf0 commit b8ead7e
Show file tree
Hide file tree
Showing 9 changed files with 193 additions and 78 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ CONTRIBUTORS:
Paul Dagnelie <pcd@delphix.com>
Paul Zuchowski <pzuchowski@datto.com>
Pavel Boldin <boldin.pavel@gmail.com>
Pavel Snajdr <snajpa@snajpa.net>
Pavel Zakharov <pavel.zakharov@delphix.com>
Pawel Jakub Dawidek <pjd@FreeBSD.org>
Pedro Giffuni <pfg@freebsd.org>
Expand Down
18 changes: 18 additions & 0 deletions config/kernel-rename.m4
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,18 @@ AC_DEFUN([ZFS_AC_KERNEL_SRC_RENAME], [
.rename = rename_fn,
};
],[])
ZFS_LINUX_TEST_SRC([inode_operations_rename2], [
#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], [
Expand All @@ -49,8 +61,14 @@ AC_DEFUN([ZFS_AC_KERNEL_RENAME], [
AC_MSG_RESULT(yes)
AC_DEFINE(HAVE_RENAME_WANTS_FLAGS, 1,
[iops->rename() wants flags])
],[
AC_MSG_CHECKING([whether iops->rename2() exists])
ZFS_LINUX_TEST_RESULT([inode_operations_rename2], [
AC_MSG_RESULT(yes)
AC_DEFINE(HAVE_RENAME2, 1, [iops->rename2() exists])
],[
AC_MSG_RESULT(no)
])
])
])
7 changes: 7 additions & 0 deletions include/sys/zfs_znode.h
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,13 @@ extern void zfs_log_link(zilog_t *zilog, dmu_tx_t *tx, uint64_t txtype,
znode_t *dzp, znode_t *zp, const char *name);
extern void zfs_log_symlink(zilog_t *zilog, dmu_tx_t *tx, uint64_t txtype,
znode_t *dzp, znode_t *zp, const char *name, const 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, const char *sname, znode_t *tdzp, const char *dname,
znode_t *szp);
Expand Down
4 changes: 1 addition & 3 deletions include/sys/zil.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion module/os/linux/zfs/zfs_dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -1035,7 +1035,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);
Expand Down
109 changes: 60 additions & 49 deletions module/os/linux/zfs/zfs_vnops_os.c
Original file line number Diff line number Diff line change
Expand Up @@ -2686,24 +2686,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;

Expand Down Expand Up @@ -2900,7 +2894,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;

Expand All @@ -2918,15 +2912,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;
Expand All @@ -2950,7 +2943,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);
Expand All @@ -2960,7 +2953,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);

Expand Down Expand Up @@ -3013,7 +3006,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)
Expand All @@ -3025,7 +3018,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;

Expand Down Expand Up @@ -3059,52 +3052,59 @@ 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_znode_update_vfs(sdzp);

if (sdzp == tdzp)
rw_exit(&sdzp->z_name_lock);

Expand All @@ -3113,11 +3113,22 @@ zfs_rename(znode_t *sdzp, char *snm, znode_t *tdzp, char *tnm,

zfs_znode_update_vfs(szp);
zrele(szp);

if (wzp) {
zfs_znode_update_vfs(wzp);
zrele(wzp);
}
if (tzp) {
zfs_znode_update_vfs(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);

Expand Down
5 changes: 4 additions & 1 deletion module/os/linux/zfs/zpl_inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,8 @@ zpl_rename2(struct inode *sdip, struct dentry *sdentry,
return (error);
}

#if !defined(HAVE_RENAME_WANTS_FLAGS) && !defined(HAVE_IOPS_RENAME_USERNS)
#if !defined(HAVE_RENAME_WANTS_FLAGS) && !defined(HAVE_IOPS_RENAME_USERNS) && \
!defined(HAVE_RENAME2)
static int
zpl_rename(struct inode *sdip, struct dentry *sdentry,
struct inode *tdip, struct dentry *tdentry)
Expand Down Expand Up @@ -692,6 +693,8 @@ const struct inode_operations zpl_dir_inode_operations = {
.mknod = zpl_mknod,
#if defined(HAVE_RENAME_WANTS_FLAGS) || defined(HAVE_IOPS_RENAME_USERNS)
.rename = zpl_rename2,
#elif defined(HAVE_RENAME2)
.rename2 = zpl_rename2,
#else
.rename = zpl_rename,
#endif
Expand Down
Loading

0 comments on commit b8ead7e

Please sign in to comment.