Skip to content
Permalink
Browse files

zfs_rename: restructure to have cleaner fallbacks

This is in preparation for RENAME_EXCHANGE and RENAME_WHITEOUT support
for ZoL, but the changes here allow for far nicer fallbacks than the
previous implementation (the source and target are re-linked in case of
the final link failing).

In addition, a small cleanup was done for the "target exists but is a
different type" codepath so that it's more understandable.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Signed-off-by: Pavel Snajdr <snajpa@snajpa.net>
  • Loading branch information...
cyphar authored and snajpa committed Apr 26, 2019
1 parent 286d2eb commit 977570e12a9a8b37409846020f01ca93f876f435
Showing with 141 additions and 78 deletions.
  1. +1 −0 include/os/linux/zfs/sys/zfs_dir.h
  2. +70 −25 module/os/linux/zfs/zfs_dir.c
  3. +70 −53 module/os/linux/zfs/zfs_vnops.c
@@ -52,6 +52,7 @@ extern "C" {
extern int zfs_dirent_lock(zfs_dirlock_t **, znode_t *, char *, znode_t **,
int, int *, pathname_t *);
extern void zfs_dirent_unlock(zfs_dirlock_t *);
extern int zfs_drop_nlink(znode_t *, dmu_tx_t *, boolean_t *);
extern int zfs_link_create(zfs_dirlock_t *, znode_t *, dmu_tx_t *, int);
extern int zfs_link_destroy(zfs_dirlock_t *, znode_t *, dmu_tx_t *, int,
boolean_t *);
@@ -906,6 +906,74 @@ zfs_dropname(zfs_dirlock_t *dl, znode_t *zp, znode_t *dzp, dmu_tx_t *tx,
return (error);
}

static int
zfs_drop_nlink_locked(znode_t *zp, dmu_tx_t *tx, boolean_t *unlinkedp)
{
zfsvfs_t *zfsvfs = ZTOZSB(zp);
int zp_is_dir = S_ISDIR(ZTOI(zp)->i_mode);
boolean_t unlinked = B_FALSE;
sa_bulk_attr_t bulk[3];
uint64_t mtime[2], ctime[2];
uint64_t links;
int count = 0;
int error;

if (zp_is_dir && !zfs_dirempty(zp))
return (SET_ERROR(ENOTEMPTY));

if (ZTOI(zp)->i_nlink <= zp_is_dir) {
zfs_panic_recover("zfs: link count on %lu is %u, "
"should be at least %u", zp->z_id,
(int)ZTOI(zp)->i_nlink, zp_is_dir + 1);
set_nlink(ZTOI(zp), zp_is_dir + 1);
}
drop_nlink(ZTOI(zp));
if (ZTOI(zp)->i_nlink == zp_is_dir) {
zp->z_unlinked = B_TRUE;
clear_nlink(ZTOI(zp));
unlinked = B_TRUE;
} else {
SA_ADD_BULK_ATTR(bulk, count, SA_ZPL_CTIME(zfsvfs),
NULL, &ctime, sizeof (ctime));
SA_ADD_BULK_ATTR(bulk, count, SA_ZPL_FLAGS(zfsvfs),
NULL, &zp->z_pflags, sizeof (zp->z_pflags));
zfs_tstamp_update_setup(zp, STATE_CHANGED, mtime,
ctime);
}
links = ZTOI(zp)->i_nlink;
SA_ADD_BULK_ATTR(bulk, count, SA_ZPL_LINKS(zfsvfs),
NULL, &links, sizeof (links));
error = sa_bulk_update(zp->z_sa_hdl, bulk, count, tx);
ASSERT3U(error, ==, 0);

if (unlinkedp != NULL)
*unlinkedp = unlinked;
else if (unlinked)
zfs_unlinked_add(zp, tx);

return (0);
}

/*
* Forcefully drop an nlink reference from (zp) and mark it for deletion if it
* was the last link. This *must* only be done to znodes which have already
* been zfs_link_destroy()'d with ZRENAMING. This is explicitly only used in
* the error path of zfs_rename(), where we have to correct the nlink count if
* we failed to link the target as well as failing to re-link the original
* znodes.
*/
int
zfs_drop_nlink(znode_t *zp, dmu_tx_t *tx, boolean_t *unlinkedp)
{
int error;

mutex_enter(&zp->z_lock);
error = zfs_drop_nlink_locked(zp, tx, unlinkedp);
mutex_exit(&zp->z_lock);

return (error);
}

/*
* Unlink zp from dl, and mark zp for deletion if this was the last link. Can
* fail if zp is a mount point (EBUSY) or a non-empty directory (ENOTEMPTY).
@@ -946,31 +1014,8 @@ zfs_link_destroy(zfs_dirlock_t *dl, znode_t *zp, dmu_tx_t *tx, int flag,
return (error);
}

if (ZTOI(zp)->i_nlink <= zp_is_dir) {
zfs_panic_recover("zfs: link count on %lu is %u, "
"should be at least %u", zp->z_id,
(int)ZTOI(zp)->i_nlink, zp_is_dir + 1);
set_nlink(ZTOI(zp), zp_is_dir + 1);
}
drop_nlink(ZTOI(zp));
if (ZTOI(zp)->i_nlink == zp_is_dir) {
zp->z_unlinked = B_TRUE;
clear_nlink(ZTOI(zp));
unlinked = B_TRUE;
} else {
SA_ADD_BULK_ATTR(bulk, count, SA_ZPL_CTIME(zfsvfs),
NULL, &ctime, sizeof (ctime));
SA_ADD_BULK_ATTR(bulk, count, SA_ZPL_FLAGS(zfsvfs),
NULL, &zp->z_pflags, sizeof (zp->z_pflags));
zfs_tstamp_update_setup(zp, STATE_CHANGED, mtime,
ctime);
}
links = ZTOI(zp)->i_nlink;
SA_ADD_BULK_ATTR(bulk, count, SA_ZPL_LINKS(zfsvfs),
NULL, &links, sizeof (links));
error = sa_bulk_update(zp->z_sa_hdl, bulk, count, tx);
count = 0;
ASSERT(error == 0);
/* The only error is !zfs_dirempty() and we checked earlier. */
ASSERT3U(zfs_drop_nlink_locked(zp, tx, &unlinked), ==, 0);
mutex_exit(&zp->z_lock);
} else {
error = zfs_dropname(dl, zp, dzp, tx, flag);
@@ -3662,8 +3662,7 @@ int
zfs_rename(struct inode *sdip, char *snm, struct inode *tdip, char *tnm,
cred_t *cr, int flags)
{
znode_t *tdzp, *szp, *tzp;
znode_t *sdzp = ITOZ(sdip);
znode_t *sdzp, *tdzp, *szp, *tzp;
zfsvfs_t *zfsvfs = ITOZSB(sdip);
zilog_t *zilog;
zfs_dirlock_t *sdl, *tdl;
@@ -3678,9 +3677,10 @@ zfs_rename(struct inode *sdip, char *snm, struct inode *tdip, char *tnm,
return (SET_ERROR(EINVAL));

ZFS_ENTER(zfsvfs);
ZFS_VERIFY_ZP(sdzp);
zilog = zfsvfs->z_log;

sdzp = ITOZ(sdip);
ZFS_VERIFY_ZP(sdzp);
tdzp = ITOZ(tdip);
ZFS_VERIFY_ZP(tdzp);

@@ -3870,16 +3870,12 @@ zfs_rename(struct inode *sdip, char *snm, struct inode *tdip, char *tnm,
/*
* Source and target must be the same type.
*/
if (S_ISDIR(ZTOI(szp)->i_mode)) {
if (!S_ISDIR(ZTOI(tzp)->i_mode)) {
error = SET_ERROR(ENOTDIR);
goto out;
}
} else {
if (S_ISDIR(ZTOI(tzp)->i_mode)) {
error = SET_ERROR(EISDIR);
goto out;
}
boolean_t s_is_dir = S_ISDIR(ZTOI(szp)->i_mode) != 0;
boolean_t t_is_dir = S_ISDIR(ZTOI(tzp)->i_mode) != 0;

if (s_is_dir != t_is_dir) {
error = SET_ERROR(s_is_dir ? ENOTDIR : EISDIR);
goto out;
}
/*
* POSIX dictates that when the source and target
@@ -3935,51 +3931,49 @@ zfs_rename(struct inode *sdip, char *snm, struct inode *tdip, char *tnm,
return (error);
}

if (tzp) /* Attempt to remove the existing target */
error = zfs_link_destroy(tdl, tzp, tx, zflg, NULL);
/*
* Unlink the source.
*/
szp->z_pflags |= ZFS_AV_MODIFIED;
if (tdzp->z_pflags & ZFS_PROJINHERIT)
szp->z_pflags |= ZFS_PROJINHERIT;

if (error == 0) {
error = zfs_link_create(tdl, szp, tx, ZRENAMING);
if (error == 0) {
szp->z_pflags |= ZFS_AV_MODIFIED;
if (tdzp->z_pflags & ZFS_PROJINHERIT)
szp->z_pflags |= ZFS_PROJINHERIT;
error = sa_update(szp->z_sa_hdl, SA_ZPL_FLAGS(zfsvfs),
(void *)&szp->z_pflags, sizeof (uint64_t), tx);
ASSERT0(error);

error = sa_update(szp->z_sa_hdl, SA_ZPL_FLAGS(zfsvfs),
(void *)&szp->z_pflags, sizeof (uint64_t), tx);
ASSERT0(error);
error = zfs_link_destroy(sdl, szp, tx, ZRENAMING, NULL);
if (error)
goto commit;

error = zfs_link_destroy(sdl, szp, tx, ZRENAMING, NULL);
if (error == 0) {
zfs_log_rename(zilog, tx, TX_RENAME |
(flags & FIGNORECASE ? TX_CI : 0), sdzp,
sdl->dl_name, tdzp, tdl->dl_name, szp);
} else {
/*
* At this point, we have successfully created
* the target name, but have failed to remove
* the source name. Since the create was done
* with the ZRENAMING flag, there are
* complications; for one, the link count is
* wrong. The easiest way to deal with this
* is to remove the newly created target, and
* return the original error. This must
* succeed; fortunately, it is very unlikely to
* fail, since we just created it.
*/
VERIFY3U(zfs_link_destroy(tdl, szp, tx,
ZRENAMING, NULL), ==, 0);
}
} else {
/*
* If we had removed the existing target, subsequent
* call to zfs_link_create() to add back the same entry
* but, the new dnode (szp) should not fail.
*/
ASSERT(tzp == NULL);
}
/*
* Unlink the target.
*/
if (tzp) {
error = zfs_link_destroy(tdl, tzp, tx, zflg, NULL);
if (error)
goto commit_link_szp;
}

/*
* Create a new link at the target.
*/
error = zfs_link_create(tdl, szp, tx, ZRENAMING);
if (error) {
/*
* If we have removed the existing target, a subsequent call to
* zfs_link_create() to add back the same entry, but with a new
* dnode (szp), should not fail.
*/
ASSERT3P(tzp, ==, NULL);
goto commit_link_tzp;
}

zfs_log_rename(zilog, tx, TX_RENAME |
(flags & FIGNORECASE ? TX_CI : 0), sdzp,
sdl->dl_name, tdzp, tdl->dl_name, szp);

commit:
dmu_tx_commit(tx);
out:
if (zl != NULL)
@@ -4007,6 +4001,29 @@ zfs_rename(struct inode *sdip, char *snm, struct inode *tdip, char *tnm,

ZFS_EXIT(zfsvfs);
return (error);

/*
* Clean-up path for broken link state.
*
* At this point we are in a (very) bad state, so we need to do our
* best to correct the state. In particular, the nlink of szp is wrong
* because we were destroying and creating links with ZRENAMING.
*
* link_create()s are allowed to fail (though they shouldn't because we
* only just unlinked them and are putting the entries back during
* clean-up). But if they fail, we can just forcefully drop the nlink
* value to (at the very least) avoid broken nlink values -- though in
* the case of non-empty directories we will have to panic.
*/
commit_link_tzp:
if (tzp) {
if (zfs_link_create(tdl, tzp, tx, ZRENAMING))
VERIFY3U(zfs_drop_nlink(tzp, tx, NULL), ==, 0);
}
commit_link_szp:
if (zfs_link_create(sdl, szp, tx, ZRENAMING))
VERIFY3U(zfs_drop_nlink(szp, tx, NULL), ==, 0);
goto commit;
}

/*

0 comments on commit 977570e

Please sign in to comment.
You can’t perform that action at this time.