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 Mar 29, 2020
1 parent ba7d1ec commit b91c840
Show file tree
Hide file tree
Showing 10 changed files with 201 additions and 86 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,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
31 changes: 26 additions & 5 deletions config/kernel-rename.m4
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
])
])
])

4 changes: 2 additions & 2 deletions config/kernel.m4
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,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
Expand Down Expand Up @@ -206,7 +206,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
Expand Down
7 changes: 7 additions & 0 deletions include/sys/zfs_znode.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
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 @@ -1023,7 +1023,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
107 changes: 58 additions & 49 deletions module/os/linux/zfs/zfs_vnops.c
Original file line number Diff line number Diff line change
Expand Up @@ -3488,24 +3488,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 @@ -3702,7 +3696,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 @@ -3720,15 +3714,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 @@ -3752,7 +3745,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 @@ -3762,7 +3755,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 @@ -3815,7 +3808,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 @@ -3827,7 +3820,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 @@ -3861,51 +3854,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);
Expand All @@ -3915,11 +3914,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);

Expand Down
6 changes: 4 additions & 2 deletions module/os/linux/zfs/zpl_inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
Loading

0 comments on commit b91c840

Please sign in to comment.