* [PATCH 1/3] f2fs: avoid memory allocation failure due to a long length
@ 2016-07-17 6:24 Jaegeuk Kim
2016-07-17 6:24 ` [PATCH 2/3] f2fs: support copy_file_range Jaegeuk Kim
2016-07-17 6:24 ` [PATCH 3/3] f2fs: support clone_file_range Jaegeuk Kim
0 siblings, 2 replies; 6+ messages in thread
From: Jaegeuk Kim @ 2016-07-17 6:24 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, linux-f2fs-devel; +Cc: Jaegeuk Kim
We need to avoid ENOMEM due to unexpected long length.
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/file.c | 46 ++++++++++++++++++++++++++++------------------
1 file changed, 28 insertions(+), 18 deletions(-)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 17b3059..3573b07 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1039,33 +1039,43 @@ static int __clone_blkaddrs(struct inode *src_inode, struct inode *dst_inode,
static int __exchange_data_block(struct inode *src_inode,
struct inode *dst_inode, pgoff_t src, pgoff_t dst,
- int len, bool full)
+ pgoff_t len, bool full)
{
block_t *src_blkaddr;
int *do_replace;
+ pgoff_t olen;
int ret;
- src_blkaddr = f2fs_kvzalloc(sizeof(block_t) * len, GFP_KERNEL);
- if (!src_blkaddr)
- return -ENOMEM;
+ while (len) {
+ olen = min((pgoff_t)4 * ADDRS_PER_BLOCK, len);
- do_replace = f2fs_kvzalloc(sizeof(int) * len, GFP_KERNEL);
- if (!do_replace) {
- kvfree(src_blkaddr);
- return -ENOMEM;
- }
+ src_blkaddr = f2fs_kvzalloc(sizeof(block_t) * olen, GFP_KERNEL);
+ if (!src_blkaddr)
+ return -ENOMEM;
- ret = __read_out_blkaddrs(src_inode, src_blkaddr, do_replace, src, len);
- if (ret)
- goto roll_back;
+ do_replace = f2fs_kvzalloc(sizeof(int) * olen, GFP_KERNEL);
+ if (!do_replace) {
+ kvfree(src_blkaddr);
+ return -ENOMEM;
+ }
- ret = __clone_blkaddrs(src_inode, dst_inode, src_blkaddr,
- do_replace, src, dst, len, full);
- if (ret)
- goto roll_back;
+ ret = __read_out_blkaddrs(src_inode, src_blkaddr,
+ do_replace, src, olen);
+ if (ret)
+ goto roll_back;
- kvfree(src_blkaddr);
- kvfree(do_replace);
+ ret = __clone_blkaddrs(src_inode, dst_inode, src_blkaddr,
+ do_replace, src, dst, olen, full);
+ if (ret)
+ goto roll_back;
+
+ src += olen;
+ dst += olen;
+ len -= olen;
+
+ kvfree(src_blkaddr);
+ kvfree(do_replace);
+ }
return 0;
roll_back:
--
2.8.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] f2fs: support copy_file_range
2016-07-17 6:24 [PATCH 1/3] f2fs: avoid memory allocation failure due to a long length Jaegeuk Kim
@ 2016-07-17 6:24 ` Jaegeuk Kim
2016-07-18 17:19 ` Jaegeuk Kim
2016-07-17 6:24 ` [PATCH 3/3] f2fs: support clone_file_range Jaegeuk Kim
1 sibling, 1 reply; 6+ messages in thread
From: Jaegeuk Kim @ 2016-07-17 6:24 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, linux-f2fs-devel; +Cc: Jaegeuk Kim
This patch implements copy_file_range in f2fs.
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/file.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 98 insertions(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 3573b07..c2b7e35 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2215,6 +2215,103 @@ static ssize_t f2fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
return ret;
}
+static int f2fs_clone_files(struct file *file_in, loff_t pos_in,
+ struct file *file_out, loff_t pos_out,
+ size_t len)
+{
+ struct inode *src = file_inode(file_in);
+ struct inode *dst = file_inode(file_out);
+ struct f2fs_sb_info *sbi = F2FS_I_SB(src);
+ size_t olen = len, dst_max_i_size = 0;
+ size_t dst_osize;
+ int ret;
+
+ if (file_in->f_path.mnt != file_out->f_path.mnt ||
+ src->i_sb != dst->i_sb)
+ return -EXDEV;
+
+ if (unlikely(f2fs_readonly(src->i_sb)))
+ return -EROFS;
+
+ if (S_ISDIR(src->i_mode) || S_ISDIR(dst->i_mode))
+ return -EISDIR;
+
+ inode_lock(src);
+ if (src != dst)
+ inode_lock(dst);
+
+ ret = -EINVAL;
+ if (pos_in + len > src->i_size || pos_in + len < pos_in)
+ goto out_unlock;
+ if (len == 0)
+ olen = len = src->i_size - pos_in;
+ if (pos_in + len == src->i_size)
+ len = ALIGN(src->i_size, F2FS_BLKSIZE) - pos_in;
+ if (len == 0) {
+ ret = 0;
+ goto out_unlock;
+ }
+
+ dst_osize = dst->i_size;
+ if (pos_out + olen > dst->i_size)
+ dst_max_i_size = pos_out + olen;
+
+ /* verify the end result is block aligned */
+ if (!IS_ALIGNED(pos_in, F2FS_BLKSIZE) ||
+ !IS_ALIGNED(pos_in + len, F2FS_BLKSIZE) ||
+ !IS_ALIGNED(pos_out, F2FS_BLKSIZE))
+ goto out_unlock;
+
+ ret = f2fs_convert_inline_inode(src);
+ if (ret)
+ goto out_unlock;
+
+ ret = f2fs_convert_inline_inode(dst);
+ if (ret)
+ goto out_unlock;
+
+ /* write out all dirty pages from offset */
+ ret = filemap_write_and_wait_range(src->i_mapping,
+ pos_in, pos_in + len);
+ if (ret)
+ goto out_unlock;
+
+ ret = filemap_write_and_wait_range(dst->i_mapping,
+ pos_out, pos_out + len);
+ if (ret)
+ goto out_unlock;
+
+ f2fs_balance_fs(sbi, true);
+ f2fs_lock_op(sbi);
+ ret = __exchange_data_block(src, dst, pos_in,
+ pos_out, len >> F2FS_BLKSIZE_BITS, false);
+
+ if (!ret) {
+ if (dst_max_i_size)
+ f2fs_i_size_write(dst, dst_max_i_size);
+ else if (dst_osize != dst->i_size)
+ f2fs_i_size_write(dst, dst_osize);
+ }
+ f2fs_unlock_op(sbi);
+out_unlock:
+ if (src != dst)
+ inode_unlock(dst);
+ inode_unlock(src);
+ return ret;
+}
+
+ssize_t f2fs_copy_file_range(struct file *file_in, loff_t pos_in,
+ struct file *file_out, loff_t pos_out,
+ size_t len, unsigned int flags)
+{
+ ssize_t ret;
+
+ ret = f2fs_clone_files(file_in, pos_in, file_out, pos_out, len);
+ if (!ret)
+ ret = len;
+ return ret;
+}
+
#ifdef CONFIG_COMPAT
long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
@@ -2261,6 +2358,7 @@ const struct file_operations f2fs_file_operations = {
#ifdef CONFIG_COMPAT
.compat_ioctl = f2fs_compat_ioctl,
#endif
+ .copy_file_range = f2fs_copy_file_range,
.splice_read = generic_file_splice_read,
.splice_write = iter_file_splice_write,
};
--
2.8.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] f2fs: support clone_file_range
2016-07-17 6:24 [PATCH 1/3] f2fs: avoid memory allocation failure due to a long length Jaegeuk Kim
2016-07-17 6:24 ` [PATCH 2/3] f2fs: support copy_file_range Jaegeuk Kim
@ 2016-07-17 6:24 ` Jaegeuk Kim
2016-07-19 3:47 ` Christoph Hellwig
1 sibling, 1 reply; 6+ messages in thread
From: Jaegeuk Kim @ 2016-07-17 6:24 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, linux-f2fs-devel; +Cc: Jaegeuk Kim
This patch implements clone_file_range in f2fs.
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/file.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index c2b7e35..37480f3 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2312,6 +2312,12 @@ ssize_t f2fs_copy_file_range(struct file *file_in, loff_t pos_in,
return ret;
}
+int f2fs_clone_file_range(struct file *file_in, loff_t pos_in,
+ struct file *file_out, loff_t pos_out, u64 len)
+{
+ return f2fs_clone_files(file_in, pos_in, file_out, pos_out, len);
+}
+
#ifdef CONFIG_COMPAT
long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
@@ -2359,6 +2365,7 @@ const struct file_operations f2fs_file_operations = {
.compat_ioctl = f2fs_compat_ioctl,
#endif
.copy_file_range = f2fs_copy_file_range,
+ .clone_file_range = f2fs_clone_file_range,
.splice_read = generic_file_splice_read,
.splice_write = iter_file_splice_write,
};
--
2.8.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] f2fs: support copy_file_range
2016-07-17 6:24 ` [PATCH 2/3] f2fs: support copy_file_range Jaegeuk Kim
@ 2016-07-18 17:19 ` Jaegeuk Kim
0 siblings, 0 replies; 6+ messages in thread
From: Jaegeuk Kim @ 2016-07-18 17:19 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, linux-f2fs-devel
Hi,
Please ignore copy/clone_file_range patches, which are wrong implementation.
Sorry for the noise.
Thanks,
On Sat, Jul 16, 2016 at 11:24:26PM -0700, Jaegeuk Kim wrote:
> This patch implements copy_file_range in f2fs.
>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
> fs/f2fs/file.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 98 insertions(+)
>
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 3573b07..c2b7e35 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -2215,6 +2215,103 @@ static ssize_t f2fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> return ret;
> }
>
> +static int f2fs_clone_files(struct file *file_in, loff_t pos_in,
> + struct file *file_out, loff_t pos_out,
> + size_t len)
> +{
> + struct inode *src = file_inode(file_in);
> + struct inode *dst = file_inode(file_out);
> + struct f2fs_sb_info *sbi = F2FS_I_SB(src);
> + size_t olen = len, dst_max_i_size = 0;
> + size_t dst_osize;
> + int ret;
> +
> + if (file_in->f_path.mnt != file_out->f_path.mnt ||
> + src->i_sb != dst->i_sb)
> + return -EXDEV;
> +
> + if (unlikely(f2fs_readonly(src->i_sb)))
> + return -EROFS;
> +
> + if (S_ISDIR(src->i_mode) || S_ISDIR(dst->i_mode))
> + return -EISDIR;
> +
> + inode_lock(src);
> + if (src != dst)
> + inode_lock(dst);
> +
> + ret = -EINVAL;
> + if (pos_in + len > src->i_size || pos_in + len < pos_in)
> + goto out_unlock;
> + if (len == 0)
> + olen = len = src->i_size - pos_in;
> + if (pos_in + len == src->i_size)
> + len = ALIGN(src->i_size, F2FS_BLKSIZE) - pos_in;
> + if (len == 0) {
> + ret = 0;
> + goto out_unlock;
> + }
> +
> + dst_osize = dst->i_size;
> + if (pos_out + olen > dst->i_size)
> + dst_max_i_size = pos_out + olen;
> +
> + /* verify the end result is block aligned */
> + if (!IS_ALIGNED(pos_in, F2FS_BLKSIZE) ||
> + !IS_ALIGNED(pos_in + len, F2FS_BLKSIZE) ||
> + !IS_ALIGNED(pos_out, F2FS_BLKSIZE))
> + goto out_unlock;
> +
> + ret = f2fs_convert_inline_inode(src);
> + if (ret)
> + goto out_unlock;
> +
> + ret = f2fs_convert_inline_inode(dst);
> + if (ret)
> + goto out_unlock;
> +
> + /* write out all dirty pages from offset */
> + ret = filemap_write_and_wait_range(src->i_mapping,
> + pos_in, pos_in + len);
> + if (ret)
> + goto out_unlock;
> +
> + ret = filemap_write_and_wait_range(dst->i_mapping,
> + pos_out, pos_out + len);
> + if (ret)
> + goto out_unlock;
> +
> + f2fs_balance_fs(sbi, true);
> + f2fs_lock_op(sbi);
> + ret = __exchange_data_block(src, dst, pos_in,
> + pos_out, len >> F2FS_BLKSIZE_BITS, false);
> +
> + if (!ret) {
> + if (dst_max_i_size)
> + f2fs_i_size_write(dst, dst_max_i_size);
> + else if (dst_osize != dst->i_size)
> + f2fs_i_size_write(dst, dst_osize);
> + }
> + f2fs_unlock_op(sbi);
> +out_unlock:
> + if (src != dst)
> + inode_unlock(dst);
> + inode_unlock(src);
> + return ret;
> +}
> +
> +ssize_t f2fs_copy_file_range(struct file *file_in, loff_t pos_in,
> + struct file *file_out, loff_t pos_out,
> + size_t len, unsigned int flags)
> +{
> + ssize_t ret;
> +
> + ret = f2fs_clone_files(file_in, pos_in, file_out, pos_out, len);
> + if (!ret)
> + ret = len;
> + return ret;
> +}
> +
> #ifdef CONFIG_COMPAT
> long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> {
> @@ -2261,6 +2358,7 @@ const struct file_operations f2fs_file_operations = {
> #ifdef CONFIG_COMPAT
> .compat_ioctl = f2fs_compat_ioctl,
> #endif
> + .copy_file_range = f2fs_copy_file_range,
> .splice_read = generic_file_splice_read,
> .splice_write = iter_file_splice_write,
> };
> --
> 2.8.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] f2fs: support clone_file_range
2016-07-17 6:24 ` [PATCH 3/3] f2fs: support clone_file_range Jaegeuk Kim
@ 2016-07-19 3:47 ` Christoph Hellwig
2016-07-19 4:48 ` Jaegeuk Kim
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2016-07-19 3:47 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-kernel, linux-fsdevel, linux-f2fs-devel, linux-api
On Sat, Jul 16, 2016 at 11:24:27PM -0700, Jaegeuk Kim wrote:
> This patch implements clone_file_range in f2fs.
[...]
> +int f2fs_clone_file_range(struct file *file_in, loff_t pos_in,
> + struct file *file_out, loff_t pos_out, u64 len)
> +{
> + return f2fs_clone_files(file_in, pos_in, file_out, pos_out, len);
> +}
> +
Falling back from copy to clone should be done in the VFS. I look into
implementing the fallback, and the code is trivial, but we don't seem
to have any useful coverage for copy_file_range yet, so I didn't dare
to add it yet. How did you test copy and clone in f2fs? And how did
you handle the difference in corner cases (e.g. the lacking 0 special
case in copy)?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] f2fs: support clone_file_range
2016-07-19 3:47 ` Christoph Hellwig
@ 2016-07-19 4:48 ` Jaegeuk Kim
0 siblings, 0 replies; 6+ messages in thread
From: Jaegeuk Kim @ 2016-07-19 4:48 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-kernel, linux-fsdevel, linux-f2fs-devel, linux-api
On Mon, Jul 18, 2016 at 08:47:36PM -0700, Christoph Hellwig wrote:
> On Sat, Jul 16, 2016 at 11:24:27PM -0700, Jaegeuk Kim wrote:
> > This patch implements clone_file_range in f2fs.
>
> [...]
>
> > +int f2fs_clone_file_range(struct file *file_in, loff_t pos_in,
> > + struct file *file_out, loff_t pos_out, u64 len)
> > +{
> > + return f2fs_clone_files(file_in, pos_in, file_out, pos_out, len);
> > +}
> > +
>
> Falling back from copy to clone should be done in the VFS. I look into
> implementing the fallback, and the code is trivial, but we don't seem
> to have any useful coverage for copy_file_range yet, so I didn't dare
> to add it yet. How did you test copy and clone in f2fs? And how did
> you handle the difference in corner cases (e.g. the lacking 0 special
> case in copy)?
Frankly speaking, I confused the behaviors without being aware of corner cases.
And thus, I dropped the patches already. Instead, I realized that what I did
was moving a range of blocks from one file to another file. So, I've been
testing a new ioctl, F2FS_IOC_MOVE_RANGE.
In terms of coverage, I just did some unit tests only.
Thanks,
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-19 4:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-17 6:24 [PATCH 1/3] f2fs: avoid memory allocation failure due to a long length Jaegeuk Kim
2016-07-17 6:24 ` [PATCH 2/3] f2fs: support copy_file_range Jaegeuk Kim
2016-07-18 17:19 ` Jaegeuk Kim
2016-07-17 6:24 ` [PATCH 3/3] f2fs: support clone_file_range Jaegeuk Kim
2016-07-19 3:47 ` Christoph Hellwig
2016-07-19 4:48 ` Jaegeuk Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).