From 1cb26f06524bdddef24352a3d9ac90a402294f2b Mon Sep 17 00:00:00 2001 From: Brian Behlendorf <behlendorf1@llnl.gov> Date: Fri, 27 Oct 2017 15:49:14 -0700 Subject: [PATCH] Remove vn_rename and vn_remove Both vn_rename and vn_remove have been historically problematic to implement reliably. Rather than fixing them yet again they are being removed. Reviewed-by: Arkadiusz Bubala <arkadiusz.bubala@open-e.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes #648 Closes #661 --- config/spl-build.m4 | 101 ----------------- include/sys/vnode.h | 2 - module/spl/spl-vnode.c | 217 ------------------------------------- module/splat/splat-vnode.c | 96 ---------------- 4 files changed, 416 deletions(-) diff --git a/config/spl-build.m4 b/config/spl-build.m4 index 25013b5b..926abd5c 100644 --- a/config/spl-build.m4 +++ b/config/spl-build.m4 @@ -27,8 +27,6 @@ AC_DEFUN([SPL_AC_CONFIG_KERNEL], [ SPL_AC_CONFIG_TRIM_UNUSED_KSYMS SPL_AC_PDE_DATA SPL_AC_SET_FS_PWD_WITH_CONST - SPL_AC_2ARGS_VFS_UNLINK - SPL_AC_4ARGS_VFS_RENAME SPL_AC_2ARGS_VFS_FSYNC SPL_AC_INODE_TRUNCATE_RANGE SPL_AC_FS_STRUCT_SPINLOCK @@ -936,105 +934,6 @@ AC_DEFUN([SPL_AC_SET_FS_PWD_WITH_CONST], EXTRA_KCFLAGS="$tmp_flags" ]) -dnl # -dnl # 3.13 API change -dnl # vfs_unlink() updated to take a third delegated_inode argument. -dnl # -AC_DEFUN([SPL_AC_2ARGS_VFS_UNLINK], - [AC_MSG_CHECKING([whether vfs_unlink() wants 2 args]) - SPL_LINUX_TRY_COMPILE([ - #include <linux/fs.h> - ],[ - vfs_unlink((struct inode *) NULL, (struct dentry *) NULL); - ],[ - AC_MSG_RESULT(yes) - AC_DEFINE(HAVE_2ARGS_VFS_UNLINK, 1, - [vfs_unlink() wants 2 args]) - ],[ - AC_MSG_RESULT(no) - dnl # - dnl # Linux 3.13 API change - dnl # Added delegated inode - dnl # - AC_MSG_CHECKING([whether vfs_unlink() wants 3 args]) - SPL_LINUX_TRY_COMPILE([ - #include <linux/fs.h> - ],[ - vfs_unlink((struct inode *) NULL, - (struct dentry *) NULL, - (struct inode **) NULL); - ],[ - AC_MSG_RESULT(yes) - AC_DEFINE(HAVE_3ARGS_VFS_UNLINK, 1, - [vfs_unlink() wants 3 args]) - ],[ - AC_MSG_ERROR(no) - ]) - - ]) -]) - -dnl # -dnl # 3.13 and 3.15 API changes -dnl # Added delegated inode and flags argument. -dnl # -AC_DEFUN([SPL_AC_4ARGS_VFS_RENAME], - [AC_MSG_CHECKING([whether vfs_rename() wants 4 args]) - SPL_LINUX_TRY_COMPILE([ - #include <linux/fs.h> - ],[ - vfs_rename((struct inode *) NULL, (struct dentry *) NULL, - (struct inode *) NULL, (struct dentry *) NULL); - ],[ - AC_MSG_RESULT(yes) - AC_DEFINE(HAVE_4ARGS_VFS_RENAME, 1, - [vfs_rename() wants 4 args]) - ],[ - AC_MSG_RESULT(no) - dnl # - dnl # Linux 3.13 API change - dnl # Added delegated inode - dnl # - AC_MSG_CHECKING([whether vfs_rename() wants 5 args]) - SPL_LINUX_TRY_COMPILE([ - #include <linux/fs.h> - ],[ - vfs_rename((struct inode *) NULL, - (struct dentry *) NULL, - (struct inode *) NULL, - (struct dentry *) NULL, - (struct inode **) NULL); - ],[ - AC_MSG_RESULT(yes) - AC_DEFINE(HAVE_5ARGS_VFS_RENAME, 1, - [vfs_rename() wants 5 args]) - ],[ - AC_MSG_RESULT(no) - dnl # - dnl # Linux 3.15 API change - dnl # Added flags - dnl # - AC_MSG_CHECKING([whether vfs_rename() wants 6 args]) - SPL_LINUX_TRY_COMPILE([ - #include <linux/fs.h> - ],[ - vfs_rename((struct inode *) NULL, - (struct dentry *) NULL, - (struct inode *) NULL, - (struct dentry *) NULL, - (struct inode **) NULL, - (unsigned int) 0); - ],[ - AC_MSG_RESULT(yes) - AC_DEFINE(HAVE_6ARGS_VFS_RENAME, 1, - [vfs_rename() wants 6 args]) - ],[ - AC_MSG_ERROR(no) - ]) - ]) - ]) -]) - dnl # dnl # 2.6.36 API change, dnl # The 'struct fs_struct->lock' was changed from a rwlock_t to diff --git a/include/sys/vnode.h b/include/sys/vnode.h index 0b857d38..ad7f8f47 100644 --- a/include/sys/vnode.h +++ b/include/sys/vnode.h @@ -177,8 +177,6 @@ extern int vn_rdwr(uio_rw_t uio, vnode_t *vp, void *addr, ssize_t len, extern int vn_close(vnode_t *vp, int flags, int x1, int x2, void *x3, void *x4); extern int vn_seek(vnode_t *vp, offset_t o, offset_t *op, void *ct); -extern int vn_remove(const char *path, uio_seg_t seg, int flags); -extern int vn_rename(const char *path1, const char *path2, int x1); extern int vn_getattr(vnode_t *vp, vattr_t *vap, int flags, void *x3, void *x4); extern int vn_fsync(vnode_t *vp, int flags, void *x3, void *x4); extern int vn_space(vnode_t *vp, int cmd, struct flock *bfp, int flag, diff --git a/module/spl/spl-vnode.c b/module/spl/spl-vnode.c index ca19d734..77bfb459 100644 --- a/module/spl/spl-vnode.c +++ b/module/spl/spl-vnode.c @@ -269,223 +269,6 @@ vn_seek(vnode_t *vp, offset_t ooff, offset_t *noffp, void *ct) } EXPORT_SYMBOL(vn_seek); -/* - * spl_basename() takes a NULL-terminated string s as input containing a path. - * It returns a char pointer to a string and a length that describe the - * basename of the path. If the basename is not "." or "/", it will be an index - * into the string. While the string should be NULL terminated, the section - * referring to the basename is not. spl_basename is dual-licensed GPLv2+ and - * CC0. Anyone wishing to reuse it in another codebase may pick either license. - */ -static void -spl_basename(const char *s, const char **str, int *len) -{ - size_t i, end; - - ASSERT(str); - ASSERT(len); - - if (!s || !*s) { - *str = "."; - *len = 1; - return; - } - - i = strlen(s) - 1; - - while (i && s[i--] == '/'); - - if (i == 0) { - *str = "/"; - *len = 1; - return; - } - - end = i; - - for (end = i; i; i--) { - if (s[i] == '/') { - *str = &s[i+1]; - *len = end - i + 1; - return; - } - } - - *str = s; - *len = end + 1; -} - -static struct dentry * -spl_kern_path_locked(const char *name, struct path *path) -{ - struct path parent; - struct dentry *dentry; - const char *basename; - int len; - int rc; - - ASSERT(name); - ASSERT(path); - - spl_basename(name, &basename, &len); - - /* We do not accept "." or ".." */ - if (len <= 2 && basename[0] == '.') - if (len == 1 || basename[1] == '.') - return (ERR_PTR(-EACCES)); - - rc = kern_path(name, LOOKUP_PARENT, &parent); - if (rc) - return (ERR_PTR(rc)); - - /* use I_MUTEX_PARENT because vfs_unlink needs it */ - spl_inode_lock_nested(parent.dentry->d_inode, I_MUTEX_PARENT); - - dentry = lookup_one_len(basename, parent.dentry, len); - if (IS_ERR(dentry)) { - spl_inode_unlock(parent.dentry->d_inode); - path_put(&parent); - } else { - *path = parent; - } - - return (dentry); -} - -/* Based on do_unlinkat() from linux/fs/namei.c */ -int -vn_remove(const char *path, uio_seg_t seg, int flags) -{ - struct dentry *dentry; - struct path parent; - struct inode *inode = NULL; - int rc = 0; - - ASSERT(seg == UIO_SYSSPACE); - ASSERT(flags == RMFILE); - - dentry = spl_kern_path_locked(path, &parent); - rc = PTR_ERR(dentry); - if (!IS_ERR(dentry)) { - if (parent.dentry->d_name.name[parent.dentry->d_name.len]) { - rc = 0; - goto slashes; - } - - inode = dentry->d_inode; - if (inode) { - atomic_inc(&inode->i_count); - } else { - rc = 0; - goto slashes; - } - -#ifdef HAVE_2ARGS_VFS_UNLINK - rc = vfs_unlink(parent.dentry->d_inode, dentry); -#else - rc = vfs_unlink(parent.dentry->d_inode, dentry, NULL); -#endif /* HAVE_2ARGS_VFS_UNLINK */ -exit1: - dput(dentry); - } else { - return (-rc); - } - - spl_inode_unlock(parent.dentry->d_inode); - if (inode) - iput(inode); /* truncate the inode here */ - - path_put(&parent); - return (-rc); - -slashes: - rc = !dentry->d_inode ? -ENOENT : - S_ISDIR(dentry->d_inode->i_mode) ? -EISDIR : -ENOTDIR; - goto exit1; -} /* vn_remove() */ -EXPORT_SYMBOL(vn_remove); - -/* Based on do_rename() from linux/fs/namei.c */ -int -vn_rename(const char *oldname, const char *newname, int x1) -{ - struct dentry *old_dir, *new_dir; - struct dentry *old_dentry, *new_dentry; - struct dentry *trap; - struct path old_parent, new_parent; - int rc = 0; - - old_dentry = spl_kern_path_locked(oldname, &old_parent); - if (IS_ERR(old_dentry)) { - rc = PTR_ERR(old_dentry); - goto exit; - } - - spl_inode_unlock(old_parent.dentry->d_inode); - - new_dentry = spl_kern_path_locked(newname, &new_parent); - if (IS_ERR(new_dentry)) { - rc = PTR_ERR(new_dentry); - goto exit2; - } - - spl_inode_unlock(new_parent.dentry->d_inode); - - rc = -EXDEV; - if (old_parent.mnt != new_parent.mnt) - goto exit3; - - old_dir = old_parent.dentry; - new_dir = new_parent.dentry; - trap = lock_rename(new_dir, old_dir); - - /* source should not be ancestor of target */ - rc = -EINVAL; - if (old_dentry == trap) - goto exit4; - - /* target should not be an ancestor of source */ - rc = -ENOTEMPTY; - if (new_dentry == trap) - goto exit4; - - /* source must exist */ - rc = -ENOENT; - if (!old_dentry->d_inode) - goto exit4; - - /* unless the source is a directory trailing slashes give -ENOTDIR */ - if (!S_ISDIR(old_dentry->d_inode->i_mode)) { - rc = -ENOTDIR; - if (old_dentry->d_name.name[old_dentry->d_name.len]) - goto exit4; - if (new_dentry->d_name.name[new_dentry->d_name.len]) - goto exit4; - } - -#if defined(HAVE_4ARGS_VFS_RENAME) - rc = vfs_rename(old_dir->d_inode, old_dentry, - new_dir->d_inode, new_dentry); -#elif defined(HAVE_5ARGS_VFS_RENAME) - rc = vfs_rename(old_dir->d_inode, old_dentry, - new_dir->d_inode, new_dentry, NULL); -#else - rc = vfs_rename(old_dir->d_inode, old_dentry, - new_dir->d_inode, new_dentry, NULL, 0); -#endif -exit4: - unlock_rename(new_dir, old_dir); -exit3: - dput(new_dentry); - path_put(&new_parent); -exit2: - dput(old_dentry); - path_put(&old_parent); -exit: - return (-rc); -} -EXPORT_SYMBOL(vn_rename); - int vn_getattr(vnode_t *vp, vattr_t *vap, int flags, void *x3, void *x4) { diff --git a/module/splat/splat-vnode.c b/module/splat/splat-vnode.c index ad69cf64..91600037 100644 --- a/module/splat/splat-vnode.c +++ b/module/splat/splat-vnode.c @@ -42,10 +42,6 @@ #define SPLAT_VNODE_TEST3_NAME "vn_rdwr" #define SPLAT_VNODE_TEST3_DESC "Vn_rdwrt Test" -#define SPLAT_VNODE_TEST4_ID 0x0904 -#define SPLAT_VNODE_TEST4_NAME "vn_rename" -#define SPLAT_VNODE_TEST4_DESC "Vn_rename Test" - #define SPLAT_VNODE_TEST5_ID 0x0905 #define SPLAT_VNODE_TEST5_NAME "vn_getattr" #define SPLAT_VNODE_TEST5_DESC "Vn_getattr Test" @@ -218,94 +214,10 @@ splat_vnode_test3(struct file *file, void *arg) out: VOP_CLOSE(vp, 0, 0, 0, 0, 0); - vn_remove(SPLAT_VNODE_TEST_FILE_RW, UIO_SYSSPACE, RMFILE); return -rc; } /* splat_vnode_test3() */ -#if LINUX_VERSION_CODE <= KERNEL_VERSION(4,1,0) -static int -splat_vnode_test4(struct file *file, void *arg) -{ - vnode_t *vp; - char buf1[32] = "SPL VNode Interface Test File\n"; - char buf2[32] = ""; - int rc; - - if ((rc = splat_vnode_unlink_all(file, arg, SPLAT_VNODE_TEST4_NAME))) - return rc; - - if ((rc = vn_open(SPLAT_VNODE_TEST_FILE_RW1, UIO_SYSSPACE, - FWRITE | FREAD | FCREAT | FEXCL, 0644, &vp, 0, 0))) { - splat_vprint(file, SPLAT_VNODE_TEST4_NAME, - "Failed to vn_open test file: %s (%d)\n", - SPLAT_VNODE_TEST_FILE_RW1, rc); - goto out; - } - - rc = vn_rdwr(UIO_WRITE, vp, buf1, strlen(buf1), 0, - UIO_SYSSPACE, 0, RLIM64_INFINITY, 0, NULL); - if (rc) { - splat_vprint(file, SPLAT_VNODE_TEST4_NAME, - "Failed vn_rdwr write of test file: %s (%d)\n", - SPLAT_VNODE_TEST_FILE_RW1, rc); - goto out2; - } - - VOP_CLOSE(vp, 0, 0, 0, 0, 0); - - rc = vn_rename(SPLAT_VNODE_TEST_FILE_RW1,SPLAT_VNODE_TEST_FILE_RW2,0); - if (rc) { - splat_vprint(file, SPLAT_VNODE_TEST4_NAME, "Failed vn_rename " - "%s -> %s (%d)\n", - SPLAT_VNODE_TEST_FILE_RW1, - SPLAT_VNODE_TEST_FILE_RW2, rc); - goto out; - } - - if ((rc = vn_open(SPLAT_VNODE_TEST_FILE_RW2, UIO_SYSSPACE, - FREAD | FEXCL, 0644, &vp, 0, 0))) { - splat_vprint(file, SPLAT_VNODE_TEST4_NAME, - "Failed to vn_open test file: %s (%d)\n", - SPLAT_VNODE_TEST_FILE_RW2, rc); - goto out; - } - - rc = vn_rdwr(UIO_READ, vp, buf2, strlen(buf1), 0, - UIO_SYSSPACE, 0, RLIM64_INFINITY, 0, NULL); - if (rc) { - splat_vprint(file, SPLAT_VNODE_TEST4_NAME, - "Failed vn_rdwr read of test file: %s (%d)\n", - SPLAT_VNODE_TEST_FILE_RW2, rc); - goto out2; - } - - if (strncmp(buf1, buf2, strlen(buf1))) { - rc = EINVAL; - splat_vprint(file, SPLAT_VNODE_TEST4_NAME, - "Failed strncmp data written does not match " - "data read\nWrote: %sRead: %s\n", buf1, buf2); - goto out2; - } - - rc = 0; - splat_vprint(file, SPLAT_VNODE_TEST4_NAME, "Wrote to %s: %s", - SPLAT_VNODE_TEST_FILE_RW1, buf1); - splat_vprint(file, SPLAT_VNODE_TEST4_NAME, "Read from %s: %s", - SPLAT_VNODE_TEST_FILE_RW2, buf2); - splat_vprint(file, SPLAT_VNODE_TEST4_NAME, "Successfully renamed " - "test file %s -> %s and verified data pattern\n", - SPLAT_VNODE_TEST_FILE_RW1, SPLAT_VNODE_TEST_FILE_RW2); -out2: - VOP_CLOSE(vp, 0, 0, 0, 0, 0); -out: - vn_remove(SPLAT_VNODE_TEST_FILE_RW1, UIO_SYSSPACE, RMFILE); - vn_remove(SPLAT_VNODE_TEST_FILE_RW2, UIO_SYSSPACE, RMFILE); - - return -rc; -} /* splat_vnode_test4() */ -#endif - static int splat_vnode_test5(struct file *file, void *arg) { @@ -387,7 +299,6 @@ splat_vnode_test6(struct file *file, void *arg) "fsync'ed test file %s\n", SPLAT_VNODE_TEST_FILE_RW); out: VOP_CLOSE(vp, 0, 0, 0, 0, 0); - vn_remove(SPLAT_VNODE_TEST_FILE_RW, UIO_SYSSPACE, RMFILE); return -rc; } /* splat_vnode_test6() */ @@ -415,10 +326,6 @@ splat_vnode_init(void) SPLAT_VNODE_TEST2_ID, splat_vnode_test2); splat_test_init(sub, SPLAT_VNODE_TEST3_NAME, SPLAT_VNODE_TEST3_DESC, SPLAT_VNODE_TEST3_ID, splat_vnode_test3); -#if LINUX_VERSION_CODE <= KERNEL_VERSION(4,1,0) - splat_test_init(sub, SPLAT_VNODE_TEST4_NAME, SPLAT_VNODE_TEST4_DESC, - SPLAT_VNODE_TEST4_ID, splat_vnode_test4); -#endif splat_test_init(sub, SPLAT_VNODE_TEST5_NAME, SPLAT_VNODE_TEST5_DESC, SPLAT_VNODE_TEST5_ID, splat_vnode_test5); splat_test_init(sub, SPLAT_VNODE_TEST6_NAME, SPLAT_VNODE_TEST6_DESC, @@ -434,9 +341,6 @@ splat_vnode_fini(splat_subsystem_t *sub) splat_test_fini(sub, SPLAT_VNODE_TEST6_ID); splat_test_fini(sub, SPLAT_VNODE_TEST5_ID); -#if LINUX_VERSION_CODE <= KERNEL_VERSION(4,1,0) - splat_test_fini(sub, SPLAT_VNODE_TEST4_ID); -#endif splat_test_fini(sub, SPLAT_VNODE_TEST3_ID); splat_test_fini(sub, SPLAT_VNODE_TEST2_ID); splat_test_fini(sub, SPLAT_VNODE_TEST1_ID);