diff mbox series

[1/2] vfs: move generic_remap_checks out of mm

Message ID 160272188127.913987.8729718777463390497.stgit@magnolia (mailing list archive)
State New, archived
Headers show
Series vfs: move the clone/dedupe/remap helpers to a single file | expand

Commit Message

Darrick J. Wong Oct. 15, 2020, 12:31 a.m. UTC 
From: Darrick J. Wong <darrick.wong@oracle.com>

I would like to move all the generic helpers for the vfs remap range
functionality (aka clonerange and dedupe) into a separate file so that
they won't be scattered across the vfs and the mm subsystems.  The
eventual goal is to be able to deselect remap_range.c if none of the
filesystems need that code, but the tricky part here is picking a
stable(ish) part of the merge window to rearrange code.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/Makefile        |    3 +-
 fs/remap_range.c   |  103 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h |    2 +
 mm/filemap.c       |   81 +----------------------------------------
 4 files changed, 108 insertions(+), 81 deletions(-)
 create mode 100644 fs/remap_range.c

Comments

Matthew Wilcox (Oracle) Oct. 15, 2020, 11:38 a.m. UTC | #1 
On Wed, Oct 14, 2020 at 05:31:21PM -0700, Darrick J. Wong wrote:
> I would like to move all the generic helpers for the vfs remap range
> functionality (aka clonerange and dedupe) into a separate file so that
> they won't be scattered across the vfs and the mm subsystems.  The
> eventual goal is to be able to deselect remap_range.c if none of the
> filesystems need that code, but the tricky part here is picking a
> stable(ish) part of the merge window to rearrange code.

This makes sense to me.  There's nothing page-cache about this function.

> diff --git a/mm/filemap.c b/mm/filemap.c
> index 99c49eeae71b..cf20e5aeb11b 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -3098,8 +3098,7 @@ EXPORT_SYMBOL(read_cache_page_gfp);
>   * LFS limits.  If pos is under the limit it becomes a short access.  If it
>   * exceeds the limit we return -EFBIG.
>   */
> -static int generic_write_check_limits(struct file *file, loff_t pos,
> -				      loff_t *count)
> +int generic_write_check_limits(struct file *file, loff_t pos, loff_t *count)
>  {
>  	struct inode *inode = file->f_mapping->host;
>  	loff_t max_size = inode->i_sb->s_maxbytes;

I wonder if generic_write_check_limits should be in fs/read_write.c --
it has nothing to do with the pagecache either.
Darrick J. Wong Oct. 15, 2020, 4:39 p.m. UTC | #2 
On Thu, Oct 15, 2020 at 12:38:26PM +0100, Matthew Wilcox wrote:
> On Wed, Oct 14, 2020 at 05:31:21PM -0700, Darrick J. Wong wrote:
> > I would like to move all the generic helpers for the vfs remap range
> > functionality (aka clonerange and dedupe) into a separate file so that
> > they won't be scattered across the vfs and the mm subsystems.  The
> > eventual goal is to be able to deselect remap_range.c if none of the
> > filesystems need that code, but the tricky part here is picking a
> > stable(ish) part of the merge window to rearrange code.
> 
> This makes sense to me.  There's nothing page-cache about this function.
> 
> > diff --git a/mm/filemap.c b/mm/filemap.c
> > index 99c49eeae71b..cf20e5aeb11b 100644
> > --- a/mm/filemap.c
> > +++ b/mm/filemap.c
> > @@ -3098,8 +3098,7 @@ EXPORT_SYMBOL(read_cache_page_gfp);
> >   * LFS limits.  If pos is under the limit it becomes a short access.  If it
> >   * exceeds the limit we return -EFBIG.
> >   */
> > -static int generic_write_check_limits(struct file *file, loff_t pos,
> > -				      loff_t *count)
> > +int generic_write_check_limits(struct file *file, loff_t pos, loff_t *count)
> >  {
> >  	struct inode *inode = file->f_mapping->host;
> >  	loff_t max_size = inode->i_sb->s_maxbytes;
> 
> I wonder if generic_write_check_limits should be in fs/read_write.c --
> it has nothing to do with the pagecache either.

Yeah, probably.

--D
diff mbox series

Patch

diff --git a/fs/Makefile b/fs/Makefile
index 1c7b0e3f6daa..7173350739c5 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -13,7 +13,8 @@  obj-y :=	open.o read_write.o file_table.o super.o \
 		seq_file.o xattr.o libfs.o fs-writeback.o \
 		pnode.o splice.o sync.o utimes.o d_path.o \
 		stack.o fs_struct.o statfs.o fs_pin.o nsfs.o \
-		fs_types.o fs_context.o fs_parser.o fsopen.o init.o
+		fs_types.o fs_context.o fs_parser.o fsopen.o init.o \
+		remap_range.o
 
 ifeq ($(CONFIG_BLOCK),y)
 obj-y +=	buffer.o block_dev.o direct-io.o mpage.o
diff --git a/fs/remap_range.c b/fs/remap_range.c
new file mode 100644
index 000000000000..e66d8c131b69
--- /dev/null
+++ b/fs/remap_range.c
@@ -0,0 +1,103 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * linux/fs/remap_range.c
+ *
+ * Copyright (C) 1994-1999  Linus Torvalds
+ */
+
+#include <linux/slab.h>
+#include <linux/stat.h>
+#include <linux/sched/xacct.h>
+#include <linux/fcntl.h>
+#include <linux/file.h>
+#include <linux/uio.h>
+#include <linux/fsnotify.h>
+#include <linux/security.h>
+#include <linux/export.h>
+#include <linux/syscalls.h>
+#include <linux/pagemap.h>
+#include <linux/splice.h>
+#include <linux/compat.h>
+#include <linux/mount.h>
+#include <linux/fs.h>
+#include "internal.h"
+
+#include <linux/uaccess.h>
+#include <asm/unistd.h>
+
+/*
+ * Performs necessary checks before doing a clone.
+ *
+ * Can adjust amount of bytes to clone via @req_count argument.
+ * Returns appropriate error code that caller should return or
+ * zero in case the clone should be allowed.
+ */
+int generic_remap_checks(struct file *file_in, loff_t pos_in,
+			 struct file *file_out, loff_t pos_out,
+			 loff_t *req_count, unsigned int remap_flags)
+{
+	struct inode *inode_in = file_in->f_mapping->host;
+	struct inode *inode_out = file_out->f_mapping->host;
+	uint64_t count = *req_count;
+	uint64_t bcount;
+	loff_t size_in, size_out;
+	loff_t bs = inode_out->i_sb->s_blocksize;
+	int ret;
+
+	/* The start of both ranges must be aligned to an fs block. */
+	if (!IS_ALIGNED(pos_in, bs) || !IS_ALIGNED(pos_out, bs))
+		return -EINVAL;
+
+	/* Ensure offsets don't wrap. */
+	if (pos_in + count < pos_in || pos_out + count < pos_out)
+		return -EINVAL;
+
+	size_in = i_size_read(inode_in);
+	size_out = i_size_read(inode_out);
+
+	/* Dedupe requires both ranges to be within EOF. */
+	if ((remap_flags & REMAP_FILE_DEDUP) &&
+	    (pos_in >= size_in || pos_in + count > size_in ||
+	     pos_out >= size_out || pos_out + count > size_out))
+		return -EINVAL;
+
+	/* Ensure the infile range is within the infile. */
+	if (pos_in >= size_in)
+		return -EINVAL;
+	count = min(count, size_in - (uint64_t)pos_in);
+
+	ret = generic_write_check_limits(file_out, pos_out, &count);
+	if (ret)
+		return ret;
+
+	/*
+	 * If the user wanted us to link to the infile's EOF, round up to the
+	 * next block boundary for this check.
+	 *
+	 * Otherwise, make sure the count is also block-aligned, having
+	 * already confirmed the starting offsets' block alignment.
+	 */
+	if (pos_in + count == size_in) {
+		bcount = ALIGN(size_in, bs) - pos_in;
+	} else {
+		if (!IS_ALIGNED(count, bs))
+			count = ALIGN_DOWN(count, bs);
+		bcount = count;
+	}
+
+	/* Don't allow overlapped cloning within the same file. */
+	if (inode_in == inode_out &&
+	    pos_out + bcount > pos_in &&
+	    pos_out < pos_in + bcount)
+		return -EINVAL;
+
+	/*
+	 * We shortened the request but the caller can't deal with that, so
+	 * bounce the request back to userspace.
+	 */
+	if (*req_count != count && !(remap_flags & REMAP_FILE_CAN_SHORTEN))
+		return -EINVAL;
+
+	*req_count = count;
+	return 0;
+}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 7519ae003a08..eea754a8dd67 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3012,6 +3012,8 @@  extern ssize_t generic_write_checks(struct kiocb *, struct iov_iter *);
 extern int generic_remap_checks(struct file *file_in, loff_t pos_in,
 				struct file *file_out, loff_t pos_out,
 				loff_t *count, unsigned int remap_flags);
+extern int generic_write_check_limits(struct file *file, loff_t pos,
+		loff_t *count);
 extern int generic_file_rw_checks(struct file *file_in, struct file *file_out);
 extern int generic_copy_file_checks(struct file *file_in, loff_t pos_in,
 				    struct file *file_out, loff_t pos_out,
diff --git a/mm/filemap.c b/mm/filemap.c
index 99c49eeae71b..cf20e5aeb11b 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -3098,8 +3098,7 @@  EXPORT_SYMBOL(read_cache_page_gfp);
  * LFS limits.  If pos is under the limit it becomes a short access.  If it
  * exceeds the limit we return -EFBIG.
  */
-static int generic_write_check_limits(struct file *file, loff_t pos,
-				      loff_t *count)
+int generic_write_check_limits(struct file *file, loff_t pos, loff_t *count)
 {
 	struct inode *inode = file->f_mapping->host;
 	loff_t max_size = inode->i_sb->s_maxbytes;
@@ -3161,84 +3160,6 @@  inline ssize_t generic_write_checks(struct kiocb *iocb, struct iov_iter *from)
 }
 EXPORT_SYMBOL(generic_write_checks);
 
-/*
- * Performs necessary checks before doing a clone.
- *
- * Can adjust amount of bytes to clone via @req_count argument.
- * Returns appropriate error code that caller should return or
- * zero in case the clone should be allowed.
- */
-int generic_remap_checks(struct file *file_in, loff_t pos_in,
-			 struct file *file_out, loff_t pos_out,
-			 loff_t *req_count, unsigned int remap_flags)
-{
-	struct inode *inode_in = file_in->f_mapping->host;
-	struct inode *inode_out = file_out->f_mapping->host;
-	uint64_t count = *req_count;
-	uint64_t bcount;
-	loff_t size_in, size_out;
-	loff_t bs = inode_out->i_sb->s_blocksize;
-	int ret;
-
-	/* The start of both ranges must be aligned to an fs block. */
-	if (!IS_ALIGNED(pos_in, bs) || !IS_ALIGNED(pos_out, bs))
-		return -EINVAL;
-
-	/* Ensure offsets don't wrap. */
-	if (pos_in + count < pos_in || pos_out + count < pos_out)
-		return -EINVAL;
-
-	size_in = i_size_read(inode_in);
-	size_out = i_size_read(inode_out);
-
-	/* Dedupe requires both ranges to be within EOF. */
-	if ((remap_flags & REMAP_FILE_DEDUP) &&
-	    (pos_in >= size_in || pos_in + count > size_in ||
-	     pos_out >= size_out || pos_out + count > size_out))
-		return -EINVAL;
-
-	/* Ensure the infile range is within the infile. */
-	if (pos_in >= size_in)
-		return -EINVAL;
-	count = min(count, size_in - (uint64_t)pos_in);
-
-	ret = generic_write_check_limits(file_out, pos_out, &count);
-	if (ret)
-		return ret;
-
-	/*
-	 * If the user wanted us to link to the infile's EOF, round up to the
-	 * next block boundary for this check.
-	 *
-	 * Otherwise, make sure the count is also block-aligned, having
-	 * already confirmed the starting offsets' block alignment.
-	 */
-	if (pos_in + count == size_in) {
-		bcount = ALIGN(size_in, bs) - pos_in;
-	} else {
-		if (!IS_ALIGNED(count, bs))
-			count = ALIGN_DOWN(count, bs);
-		bcount = count;
-	}
-
-	/* Don't allow overlapped cloning within the same file. */
-	if (inode_in == inode_out &&
-	    pos_out + bcount > pos_in &&
-	    pos_out < pos_in + bcount)
-		return -EINVAL;
-
-	/*
-	 * We shortened the request but the caller can't deal with that, so
-	 * bounce the request back to userspace.
-	 */
-	if (*req_count != count && !(remap_flags & REMAP_FILE_CAN_SHORTEN))
-		return -EINVAL;
-
-	*req_count = count;
-	return 0;
-}
-
-
 /*
  * Performs common checks before doing a file copy/clone
  * from @file_in to @file_out.