Linux-f2fs-devel Archive on lore.kernel.org
 help / color / Atom feed
* [f2fs-dev] [PATCH v2] f2fs: change the way of handling range.len in F2FS_IOC_SEC_TRIM_FILE
@ 2020-07-13  3:12 Daeho Jeong
  2020-07-13 18:11 ` Jaegeuk Kim
  0 siblings, 1 reply; 11+ messages in thread
From: Daeho Jeong @ 2020-07-13  3:12 UTC (permalink / raw)
  To: linux-kernel, linux-f2fs-devel, kernel-team; +Cc: Daeho Jeong

From: Daeho Jeong <daehojeong@google.com>

Changed the way of handling range.len of F2FS_IOC_SEC_TRIM_FILE.
 1. Added -1 value support for range.len to secure trim the whole blocks
    starting from range.start regardless of i_size.
 2. If the end of the range passes over the end of file, it means until
    the end of file (i_size).
 3. ignored the case of that range.len is zero to prevent the function
    from making end_addr zero and triggering different behaviour of
    the function.

Signed-off-by: Daeho Jeong <daehojeong@google.com>
---
Changes in v2:
 - Changed -1 range.len option to mean the whole blocks starting from
   range.start regardless of i_size
---
 fs/f2fs/file.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 368c80f8e2a1..2485841e3b2d 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3792,7 +3792,7 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
 	pgoff_t index, pg_end;
 	block_t prev_block = 0, len = 0;
 	loff_t end_addr;
-	bool to_end;
+	bool to_end = false;
 	int ret = 0;
 
 	if (!(filp->f_mode & FMODE_WRITE))
@@ -3813,23 +3813,23 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
 	file_start_write(filp);
 	inode_lock(inode);
 
-	if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode)) {
+	if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode) ||
+			range.start >= inode->i_size) {
 		ret = -EINVAL;
 		goto err;
 	}
 
-	if (range.start >= inode->i_size) {
-		ret = -EINVAL;
+	if (range.len == 0)
 		goto err;
-	}
 
-	if (inode->i_size - range.start < range.len) {
-		ret = -E2BIG;
-		goto err;
+	if (inode->i_size - range.start > range.len) {
+		end_addr = range.start + range.len;
+	} else {
+		end_addr = range.len == (u64)-1 ?
+			sbi->sb->s_maxbytes : inode->i_size;
+		to_end = true;
 	}
-	end_addr = range.start + range.len;
 
-	to_end = (end_addr == inode->i_size);
 	if (!IS_ALIGNED(range.start, F2FS_BLKSIZE) ||
 			(!to_end && !IS_ALIGNED(end_addr, F2FS_BLKSIZE))) {
 		ret = -EINVAL;
@@ -3846,7 +3846,8 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
 	down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
 	down_write(&F2FS_I(inode)->i_mmap_sem);
 
-	ret = filemap_write_and_wait_range(mapping, range.start, end_addr - 1);
+	ret = filemap_write_and_wait_range(mapping, range.start,
+			to_end ? LLONG_MAX : end_addr - 1);
 	if (ret)
 		goto out;
 
-- 
2.27.0.383.g050319c2ae-goog



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: change the way of handling range.len in F2FS_IOC_SEC_TRIM_FILE
  2020-07-13  3:12 [f2fs-dev] [PATCH v2] f2fs: change the way of handling range.len in F2FS_IOC_SEC_TRIM_FILE Daeho Jeong
@ 2020-07-13 18:11 ` Jaegeuk Kim
  2020-07-13 23:34   ` Daeho Jeong
  2020-07-14 12:36   ` Chao Yu
  0 siblings, 2 replies; 11+ messages in thread
From: Jaegeuk Kim @ 2020-07-13 18:11 UTC (permalink / raw)
  To: Daeho Jeong; +Cc: Daeho Jeong, kernel-team, linux-kernel, linux-f2fs-devel

Hi Daeho,

Please take a look at this.

https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=35245180459aebf6d70fde88a538f0400a794aa6

Thanks,

On 07/13, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@google.com>
> 
> Changed the way of handling range.len of F2FS_IOC_SEC_TRIM_FILE.
>  1. Added -1 value support for range.len to secure trim the whole blocks
>     starting from range.start regardless of i_size.
>  2. If the end of the range passes over the end of file, it means until
>     the end of file (i_size).
>  3. ignored the case of that range.len is zero to prevent the function
>     from making end_addr zero and triggering different behaviour of
>     the function.
> 
> Signed-off-by: Daeho Jeong <daehojeong@google.com>
> ---
> Changes in v2:
>  - Changed -1 range.len option to mean the whole blocks starting from
>    range.start regardless of i_size
> ---
>  fs/f2fs/file.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 368c80f8e2a1..2485841e3b2d 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -3792,7 +3792,7 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
>  	pgoff_t index, pg_end;
>  	block_t prev_block = 0, len = 0;
>  	loff_t end_addr;
> -	bool to_end;
> +	bool to_end = false;
>  	int ret = 0;
>  
>  	if (!(filp->f_mode & FMODE_WRITE))
> @@ -3813,23 +3813,23 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
>  	file_start_write(filp);
>  	inode_lock(inode);
>  
> -	if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode)) {
> +	if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode) ||
> +			range.start >= inode->i_size) {
>  		ret = -EINVAL;
>  		goto err;
>  	}
>  
> -	if (range.start >= inode->i_size) {
> -		ret = -EINVAL;
> +	if (range.len == 0)
>  		goto err;
> -	}
>  
> -	if (inode->i_size - range.start < range.len) {
> -		ret = -E2BIG;
> -		goto err;
> +	if (inode->i_size - range.start > range.len) {
> +		end_addr = range.start + range.len;
> +	} else {
> +		end_addr = range.len == (u64)-1 ?
> +			sbi->sb->s_maxbytes : inode->i_size;
> +		to_end = true;
>  	}
> -	end_addr = range.start + range.len;
>  
> -	to_end = (end_addr == inode->i_size);
>  	if (!IS_ALIGNED(range.start, F2FS_BLKSIZE) ||
>  			(!to_end && !IS_ALIGNED(end_addr, F2FS_BLKSIZE))) {
>  		ret = -EINVAL;
> @@ -3846,7 +3846,8 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
>  	down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>  	down_write(&F2FS_I(inode)->i_mmap_sem);
>  
> -	ret = filemap_write_and_wait_range(mapping, range.start, end_addr - 1);
> +	ret = filemap_write_and_wait_range(mapping, range.start,
> +			to_end ? LLONG_MAX : end_addr - 1);
>  	if (ret)
>  		goto out;
>  
> -- 
> 2.27.0.383.g050319c2ae-goog


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: change the way of handling range.len in F2FS_IOC_SEC_TRIM_FILE
  2020-07-13 18:11 ` Jaegeuk Kim
@ 2020-07-13 23:34   ` Daeho Jeong
  2020-07-14 12:36   ` Chao Yu
  1 sibling, 0 replies; 11+ messages in thread
From: Daeho Jeong @ 2020-07-13 23:34 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: Daeho Jeong, kernel-team, linux-kernel, linux-f2fs-devel

It's correct~ :)

2020년 7월 14일 (화) 오전 3:11, Jaegeuk Kim <jaegeuk@kernel.org>님이 작성:
>
> Hi Daeho,
>
> Please take a look at this.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=35245180459aebf6d70fde88a538f0400a794aa6
>
> Thanks,
>
> On 07/13, Daeho Jeong wrote:
> > From: Daeho Jeong <daehojeong@google.com>
> >
> > Changed the way of handling range.len of F2FS_IOC_SEC_TRIM_FILE.
> >  1. Added -1 value support for range.len to secure trim the whole blocks
> >     starting from range.start regardless of i_size.
> >  2. If the end of the range passes over the end of file, it means until
> >     the end of file (i_size).
> >  3. ignored the case of that range.len is zero to prevent the function
> >     from making end_addr zero and triggering different behaviour of
> >     the function.
> >
> > Signed-off-by: Daeho Jeong <daehojeong@google.com>
> > ---
> > Changes in v2:
> >  - Changed -1 range.len option to mean the whole blocks starting from
> >    range.start regardless of i_size
> > ---
> >  fs/f2fs/file.c | 23 ++++++++++++-----------
> >  1 file changed, 12 insertions(+), 11 deletions(-)
> >
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index 368c80f8e2a1..2485841e3b2d 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -3792,7 +3792,7 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
> >       pgoff_t index, pg_end;
> >       block_t prev_block = 0, len = 0;
> >       loff_t end_addr;
> > -     bool to_end;
> > +     bool to_end = false;
> >       int ret = 0;
> >
> >       if (!(filp->f_mode & FMODE_WRITE))
> > @@ -3813,23 +3813,23 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
> >       file_start_write(filp);
> >       inode_lock(inode);
> >
> > -     if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode)) {
> > +     if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode) ||
> > +                     range.start >= inode->i_size) {
> >               ret = -EINVAL;
> >               goto err;
> >       }
> >
> > -     if (range.start >= inode->i_size) {
> > -             ret = -EINVAL;
> > +     if (range.len == 0)
> >               goto err;
> > -     }
> >
> > -     if (inode->i_size - range.start < range.len) {
> > -             ret = -E2BIG;
> > -             goto err;
> > +     if (inode->i_size - range.start > range.len) {
> > +             end_addr = range.start + range.len;
> > +     } else {
> > +             end_addr = range.len == (u64)-1 ?
> > +                     sbi->sb->s_maxbytes : inode->i_size;
> > +             to_end = true;
> >       }
> > -     end_addr = range.start + range.len;
> >
> > -     to_end = (end_addr == inode->i_size);
> >       if (!IS_ALIGNED(range.start, F2FS_BLKSIZE) ||
> >                       (!to_end && !IS_ALIGNED(end_addr, F2FS_BLKSIZE))) {
> >               ret = -EINVAL;
> > @@ -3846,7 +3846,8 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
> >       down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> >       down_write(&F2FS_I(inode)->i_mmap_sem);
> >
> > -     ret = filemap_write_and_wait_range(mapping, range.start, end_addr - 1);
> > +     ret = filemap_write_and_wait_range(mapping, range.start,
> > +                     to_end ? LLONG_MAX : end_addr - 1);
> >       if (ret)
> >               goto out;
> >
> > --
> > 2.27.0.383.g050319c2ae-goog


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: change the way of handling range.len in F2FS_IOC_SEC_TRIM_FILE
  2020-07-13 18:11 ` Jaegeuk Kim
  2020-07-13 23:34   ` Daeho Jeong
@ 2020-07-14 12:36   ` Chao Yu
  2020-07-15  4:06     ` Daeho Jeong
  1 sibling, 1 reply; 11+ messages in thread
From: Chao Yu @ 2020-07-14 12:36 UTC (permalink / raw)
  To: Jaegeuk Kim, Daeho Jeong
  Cc: linux-f2fs-devel, kernel-team, Daeho Jeong, linux-kernel

On 2020/7/14 2:11, Jaegeuk Kim wrote:
> Hi Daeho,
> 
> Please take a look at this.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=35245180459aebf6d70fde88a538f0400a794aa6

I'm curious about what will happen if we call
sec_trim_file(F2FS_TRIM_FILE_ZEROOUT) on an encrypted file, will
it use zero bits covering encrypted data on disk?

Thanks,

> 
> Thanks,
> 
> On 07/13, Daeho Jeong wrote:
>> From: Daeho Jeong <daehojeong@google.com>
>>
>> Changed the way of handling range.len of F2FS_IOC_SEC_TRIM_FILE.
>>  1. Added -1 value support for range.len to secure trim the whole blocks
>>     starting from range.start regardless of i_size.
>>  2. If the end of the range passes over the end of file, it means until
>>     the end of file (i_size).
>>  3. ignored the case of that range.len is zero to prevent the function
>>     from making end_addr zero and triggering different behaviour of
>>     the function.
>>
>> Signed-off-by: Daeho Jeong <daehojeong@google.com>
>> ---
>> Changes in v2:
>>  - Changed -1 range.len option to mean the whole blocks starting from
>>    range.start regardless of i_size
>> ---
>>  fs/f2fs/file.c | 23 ++++++++++++-----------
>>  1 file changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index 368c80f8e2a1..2485841e3b2d 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -3792,7 +3792,7 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
>>  	pgoff_t index, pg_end;
>>  	block_t prev_block = 0, len = 0;
>>  	loff_t end_addr;
>> -	bool to_end;
>> +	bool to_end = false;
>>  	int ret = 0;
>>  
>>  	if (!(filp->f_mode & FMODE_WRITE))
>> @@ -3813,23 +3813,23 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
>>  	file_start_write(filp);
>>  	inode_lock(inode);
>>  
>> -	if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode)) {
>> +	if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode) ||
>> +			range.start >= inode->i_size) {
>>  		ret = -EINVAL;
>>  		goto err;
>>  	}
>>  
>> -	if (range.start >= inode->i_size) {
>> -		ret = -EINVAL;
>> +	if (range.len == 0)
>>  		goto err;
>> -	}
>>  
>> -	if (inode->i_size - range.start < range.len) {
>> -		ret = -E2BIG;
>> -		goto err;
>> +	if (inode->i_size - range.start > range.len) {
>> +		end_addr = range.start + range.len;
>> +	} else {
>> +		end_addr = range.len == (u64)-1 ?
>> +			sbi->sb->s_maxbytes : inode->i_size;
>> +		to_end = true;
>>  	}
>> -	end_addr = range.start + range.len;
>>  
>> -	to_end = (end_addr == inode->i_size);
>>  	if (!IS_ALIGNED(range.start, F2FS_BLKSIZE) ||
>>  			(!to_end && !IS_ALIGNED(end_addr, F2FS_BLKSIZE))) {
>>  		ret = -EINVAL;
>> @@ -3846,7 +3846,8 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
>>  	down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>>  	down_write(&F2FS_I(inode)->i_mmap_sem);
>>  
>> -	ret = filemap_write_and_wait_range(mapping, range.start, end_addr - 1);
>> +	ret = filemap_write_and_wait_range(mapping, range.start,
>> +			to_end ? LLONG_MAX : end_addr - 1);
>>  	if (ret)
>>  		goto out;
>>  
>> -- 
>> 2.27.0.383.g050319c2ae-goog
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: change the way of handling range.len in F2FS_IOC_SEC_TRIM_FILE
  2020-07-14 12:36   ` Chao Yu
@ 2020-07-15  4:06     ` Daeho Jeong
  2020-07-15  6:16       ` Chao Yu
  0 siblings, 1 reply; 11+ messages in thread
From: Daeho Jeong @ 2020-07-15  4:06 UTC (permalink / raw)
  To: Chao Yu
  Cc: Jaegeuk Kim, linux-f2fs-devel, kernel-team, Daeho Jeong, linux-kernel

We could use fscrypt_zeroout_range() for an encrypted file.
But, one problem is fscrypt_zeroout_range() assumes that filesystems
only use a single block device.
It means it doesn't receive bdev as a parameter.
How about changing the interface of fscrypt_zeroout_range() first and using it?

2020년 7월 14일 (화) 오후 9:36, Chao Yu <yuchao0@huawei.com>님이 작성:
>
> On 2020/7/14 2:11, Jaegeuk Kim wrote:
> > Hi Daeho,
> >
> > Please take a look at this.
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=35245180459aebf6d70fde88a538f0400a794aa6
>
> I'm curious about what will happen if we call
> sec_trim_file(F2FS_TRIM_FILE_ZEROOUT) on an encrypted file, will
> it use zero bits covering encrypted data on disk?
>
> Thanks,
>
> >
> > Thanks,
> >
> > On 07/13, Daeho Jeong wrote:
> >> From: Daeho Jeong <daehojeong@google.com>
> >>
> >> Changed the way of handling range.len of F2FS_IOC_SEC_TRIM_FILE.
> >>  1. Added -1 value support for range.len to secure trim the whole blocks
> >>     starting from range.start regardless of i_size.
> >>  2. If the end of the range passes over the end of file, it means until
> >>     the end of file (i_size).
> >>  3. ignored the case of that range.len is zero to prevent the function
> >>     from making end_addr zero and triggering different behaviour of
> >>     the function.
> >>
> >> Signed-off-by: Daeho Jeong <daehojeong@google.com>
> >> ---
> >> Changes in v2:
> >>  - Changed -1 range.len option to mean the whole blocks starting from
> >>    range.start regardless of i_size
> >> ---
> >>  fs/f2fs/file.c | 23 ++++++++++++-----------
> >>  1 file changed, 12 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> >> index 368c80f8e2a1..2485841e3b2d 100644
> >> --- a/fs/f2fs/file.c
> >> +++ b/fs/f2fs/file.c
> >> @@ -3792,7 +3792,7 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
> >>      pgoff_t index, pg_end;
> >>      block_t prev_block = 0, len = 0;
> >>      loff_t end_addr;
> >> -    bool to_end;
> >> +    bool to_end = false;
> >>      int ret = 0;
> >>
> >>      if (!(filp->f_mode & FMODE_WRITE))
> >> @@ -3813,23 +3813,23 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
> >>      file_start_write(filp);
> >>      inode_lock(inode);
> >>
> >> -    if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode)) {
> >> +    if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode) ||
> >> +                    range.start >= inode->i_size) {
> >>              ret = -EINVAL;
> >>              goto err;
> >>      }
> >>
> >> -    if (range.start >= inode->i_size) {
> >> -            ret = -EINVAL;
> >> +    if (range.len == 0)
> >>              goto err;
> >> -    }
> >>
> >> -    if (inode->i_size - range.start < range.len) {
> >> -            ret = -E2BIG;
> >> -            goto err;
> >> +    if (inode->i_size - range.start > range.len) {
> >> +            end_addr = range.start + range.len;
> >> +    } else {
> >> +            end_addr = range.len == (u64)-1 ?
> >> +                    sbi->sb->s_maxbytes : inode->i_size;
> >> +            to_end = true;
> >>      }
> >> -    end_addr = range.start + range.len;
> >>
> >> -    to_end = (end_addr == inode->i_size);
> >>      if (!IS_ALIGNED(range.start, F2FS_BLKSIZE) ||
> >>                      (!to_end && !IS_ALIGNED(end_addr, F2FS_BLKSIZE))) {
> >>              ret = -EINVAL;
> >> @@ -3846,7 +3846,8 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
> >>      down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> >>      down_write(&F2FS_I(inode)->i_mmap_sem);
> >>
> >> -    ret = filemap_write_and_wait_range(mapping, range.start, end_addr - 1);
> >> +    ret = filemap_write_and_wait_range(mapping, range.start,
> >> +                    to_end ? LLONG_MAX : end_addr - 1);
> >>      if (ret)
> >>              goto out;
> >>
> >> --
> >> 2.27.0.383.g050319c2ae-goog
> >
> >
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > .
> >


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: change the way of handling range.len in F2FS_IOC_SEC_TRIM_FILE
  2020-07-15  4:06     ` Daeho Jeong
@ 2020-07-15  6:16       ` Chao Yu
  2020-07-15  6:54         ` Daeho Jeong
  0 siblings, 1 reply; 11+ messages in thread
From: Chao Yu @ 2020-07-15  6:16 UTC (permalink / raw)
  To: Daeho Jeong
  Cc: Jaegeuk Kim, linux-f2fs-devel, kernel-team, Daeho Jeong, linux-kernel

On 2020/7/15 12:06, Daeho Jeong wrote:
> We could use fscrypt_zeroout_range() for an encrypted file.
> But, one problem is fscrypt_zeroout_range() assumes that filesystems
> only use a single block device.
> It means it doesn't receive bdev as a parameter.
> How about changing the interface of fscrypt_zeroout_range() first and using it?

Yes, please limited to use fscrypt_zeroout_range() on non-multidevice f2fs image
first, we can add a condition to check that in the beginning of ioctl interface,
once fscrypt_zeroout_range() accepts bdev as parameter, we can remove that limitation.

Thanks,

> 
> 2020년 7월 14일 (화) 오후 9:36, Chao Yu <yuchao0@huawei.com>님이 작성:
>>
>> On 2020/7/14 2:11, Jaegeuk Kim wrote:
>>> Hi Daeho,
>>>
>>> Please take a look at this.
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=35245180459aebf6d70fde88a538f0400a794aa6
>>
>> I'm curious about what will happen if we call
>> sec_trim_file(F2FS_TRIM_FILE_ZEROOUT) on an encrypted file, will
>> it use zero bits covering encrypted data on disk?
>>
>> Thanks,
>>
>>>
>>> Thanks,
>>>
>>> On 07/13, Daeho Jeong wrote:
>>>> From: Daeho Jeong <daehojeong@google.com>
>>>>
>>>> Changed the way of handling range.len of F2FS_IOC_SEC_TRIM_FILE.
>>>>  1. Added -1 value support for range.len to secure trim the whole blocks
>>>>     starting from range.start regardless of i_size.
>>>>  2. If the end of the range passes over the end of file, it means until
>>>>     the end of file (i_size).
>>>>  3. ignored the case of that range.len is zero to prevent the function
>>>>     from making end_addr zero and triggering different behaviour of
>>>>     the function.
>>>>
>>>> Signed-off-by: Daeho Jeong <daehojeong@google.com>
>>>> ---
>>>> Changes in v2:
>>>>  - Changed -1 range.len option to mean the whole blocks starting from
>>>>    range.start regardless of i_size
>>>> ---
>>>>  fs/f2fs/file.c | 23 ++++++++++++-----------
>>>>  1 file changed, 12 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>> index 368c80f8e2a1..2485841e3b2d 100644
>>>> --- a/fs/f2fs/file.c
>>>> +++ b/fs/f2fs/file.c
>>>> @@ -3792,7 +3792,7 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
>>>>      pgoff_t index, pg_end;
>>>>      block_t prev_block = 0, len = 0;
>>>>      loff_t end_addr;
>>>> -    bool to_end;
>>>> +    bool to_end = false;
>>>>      int ret = 0;
>>>>
>>>>      if (!(filp->f_mode & FMODE_WRITE))
>>>> @@ -3813,23 +3813,23 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
>>>>      file_start_write(filp);
>>>>      inode_lock(inode);
>>>>
>>>> -    if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode)) {
>>>> +    if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode) ||
>>>> +                    range.start >= inode->i_size) {
>>>>              ret = -EINVAL;
>>>>              goto err;
>>>>      }
>>>>
>>>> -    if (range.start >= inode->i_size) {
>>>> -            ret = -EINVAL;
>>>> +    if (range.len == 0)
>>>>              goto err;
>>>> -    }
>>>>
>>>> -    if (inode->i_size - range.start < range.len) {
>>>> -            ret = -E2BIG;
>>>> -            goto err;
>>>> +    if (inode->i_size - range.start > range.len) {
>>>> +            end_addr = range.start + range.len;
>>>> +    } else {
>>>> +            end_addr = range.len == (u64)-1 ?
>>>> +                    sbi->sb->s_maxbytes : inode->i_size;
>>>> +            to_end = true;
>>>>      }
>>>> -    end_addr = range.start + range.len;
>>>>
>>>> -    to_end = (end_addr == inode->i_size);
>>>>      if (!IS_ALIGNED(range.start, F2FS_BLKSIZE) ||
>>>>                      (!to_end && !IS_ALIGNED(end_addr, F2FS_BLKSIZE))) {
>>>>              ret = -EINVAL;
>>>> @@ -3846,7 +3846,8 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
>>>>      down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>>>>      down_write(&F2FS_I(inode)->i_mmap_sem);
>>>>
>>>> -    ret = filemap_write_and_wait_range(mapping, range.start, end_addr - 1);
>>>> +    ret = filemap_write_and_wait_range(mapping, range.start,
>>>> +                    to_end ? LLONG_MAX : end_addr - 1);
>>>>      if (ret)
>>>>              goto out;
>>>>
>>>> --
>>>> 2.27.0.383.g050319c2ae-goog
>>>
>>>
>>> _______________________________________________
>>> Linux-f2fs-devel mailing list
>>> Linux-f2fs-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>> .
>>>
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: change the way of handling range.len in F2FS_IOC_SEC_TRIM_FILE
  2020-07-15  6:16       ` Chao Yu
@ 2020-07-15  6:54         ` Daeho Jeong
  2020-07-15  7:17           ` Chao Yu
  0 siblings, 1 reply; 11+ messages in thread
From: Daeho Jeong @ 2020-07-15  6:54 UTC (permalink / raw)
  To: Chao Yu
  Cc: Jaegeuk Kim, linux-f2fs-devel, kernel-team, Daeho Jeong, linux-kernel

You mean we can support ZEROOUT option only for encrypted files of
non-multidevice f2fs,
and return -EOPNOTSUPP in the multidevice case, right now?

2020년 7월 15일 (수) 오후 3:17, Chao Yu <yuchao0@huawei.com>님이 작성:
>
> On 2020/7/15 12:06, Daeho Jeong wrote:
> > We could use fscrypt_zeroout_range() for an encrypted file.
> > But, one problem is fscrypt_zeroout_range() assumes that filesystems
> > only use a single block device.
> > It means it doesn't receive bdev as a parameter.
> > How about changing the interface of fscrypt_zeroout_range() first and using it?
>
> Yes, please limited to use fscrypt_zeroout_range() on non-multidevice f2fs image
> first, we can add a condition to check that in the beginning of ioctl interface,
> once fscrypt_zeroout_range() accepts bdev as parameter, we can remove that limitation.
>
> Thanks,
>
> >
> > 2020년 7월 14일 (화) 오후 9:36, Chao Yu <yuchao0@huawei.com>님이 작성:
> >>
> >> On 2020/7/14 2:11, Jaegeuk Kim wrote:
> >>> Hi Daeho,
> >>>
> >>> Please take a look at this.
> >>>
> >>> https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=35245180459aebf6d70fde88a538f0400a794aa6
> >>
> >> I'm curious about what will happen if we call
> >> sec_trim_file(F2FS_TRIM_FILE_ZEROOUT) on an encrypted file, will
> >> it use zero bits covering encrypted data on disk?
> >>
> >> Thanks,
> >>
> >>>
> >>> Thanks,
> >>>
> >>> On 07/13, Daeho Jeong wrote:
> >>>> From: Daeho Jeong <daehojeong@google.com>
> >>>>
> >>>> Changed the way of handling range.len of F2FS_IOC_SEC_TRIM_FILE.
> >>>>  1. Added -1 value support for range.len to secure trim the whole blocks
> >>>>     starting from range.start regardless of i_size.
> >>>>  2. If the end of the range passes over the end of file, it means until
> >>>>     the end of file (i_size).
> >>>>  3. ignored the case of that range.len is zero to prevent the function
> >>>>     from making end_addr zero and triggering different behaviour of
> >>>>     the function.
> >>>>
> >>>> Signed-off-by: Daeho Jeong <daehojeong@google.com>
> >>>> ---
> >>>> Changes in v2:
> >>>>  - Changed -1 range.len option to mean the whole blocks starting from
> >>>>    range.start regardless of i_size
> >>>> ---
> >>>>  fs/f2fs/file.c | 23 ++++++++++++-----------
> >>>>  1 file changed, 12 insertions(+), 11 deletions(-)
> >>>>
> >>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> >>>> index 368c80f8e2a1..2485841e3b2d 100644
> >>>> --- a/fs/f2fs/file.c
> >>>> +++ b/fs/f2fs/file.c
> >>>> @@ -3792,7 +3792,7 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
> >>>>      pgoff_t index, pg_end;
> >>>>      block_t prev_block = 0, len = 0;
> >>>>      loff_t end_addr;
> >>>> -    bool to_end;
> >>>> +    bool to_end = false;
> >>>>      int ret = 0;
> >>>>
> >>>>      if (!(filp->f_mode & FMODE_WRITE))
> >>>> @@ -3813,23 +3813,23 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
> >>>>      file_start_write(filp);
> >>>>      inode_lock(inode);
> >>>>
> >>>> -    if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode)) {
> >>>> +    if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode) ||
> >>>> +                    range.start >= inode->i_size) {
> >>>>              ret = -EINVAL;
> >>>>              goto err;
> >>>>      }
> >>>>
> >>>> -    if (range.start >= inode->i_size) {
> >>>> -            ret = -EINVAL;
> >>>> +    if (range.len == 0)
> >>>>              goto err;
> >>>> -    }
> >>>>
> >>>> -    if (inode->i_size - range.start < range.len) {
> >>>> -            ret = -E2BIG;
> >>>> -            goto err;
> >>>> +    if (inode->i_size - range.start > range.len) {
> >>>> +            end_addr = range.start + range.len;
> >>>> +    } else {
> >>>> +            end_addr = range.len == (u64)-1 ?
> >>>> +                    sbi->sb->s_maxbytes : inode->i_size;
> >>>> +            to_end = true;
> >>>>      }
> >>>> -    end_addr = range.start + range.len;
> >>>>
> >>>> -    to_end = (end_addr == inode->i_size);
> >>>>      if (!IS_ALIGNED(range.start, F2FS_BLKSIZE) ||
> >>>>                      (!to_end && !IS_ALIGNED(end_addr, F2FS_BLKSIZE))) {
> >>>>              ret = -EINVAL;
> >>>> @@ -3846,7 +3846,8 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
> >>>>      down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> >>>>      down_write(&F2FS_I(inode)->i_mmap_sem);
> >>>>
> >>>> -    ret = filemap_write_and_wait_range(mapping, range.start, end_addr - 1);
> >>>> +    ret = filemap_write_and_wait_range(mapping, range.start,
> >>>> +                    to_end ? LLONG_MAX : end_addr - 1);
> >>>>      if (ret)
> >>>>              goto out;
> >>>>
> >>>> --
> >>>> 2.27.0.383.g050319c2ae-goog
> >>>
> >>>
> >>> _______________________________________________
> >>> Linux-f2fs-devel mailing list
> >>> Linux-f2fs-devel@lists.sourceforge.net
> >>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> >>> .
> >>>
> > .
> >


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: change the way of handling range.len in F2FS_IOC_SEC_TRIM_FILE
  2020-07-15  6:54         ` Daeho Jeong
@ 2020-07-15  7:17           ` Chao Yu
  2020-07-15 10:25             ` Daeho Jeong
  0 siblings, 1 reply; 11+ messages in thread
From: Chao Yu @ 2020-07-15  7:17 UTC (permalink / raw)
  To: Daeho Jeong
  Cc: Jaegeuk Kim, linux-f2fs-devel, kernel-team, Daeho Jeong, linux-kernel

On 2020/7/15 14:54, Daeho Jeong wrote:
> You mean we can support ZEROOUT option only for encrypted files of
> non-multidevice f2fs,
> and return -EOPNOTSUPP in the multidevice case, right now?

Yes, something like:

f2fs_sec_trim_file()

if ((range.flags & F2FS_TRIM_FILE_ZEROOUT) &&
	f2fs_encrypted_file() && f2fs_is_multi_device())
	return -EOPNOTSUPP;


f2fs_secure_erase()

if (!ret && (flags & F2FS_TRIM_FILE_ZEROOUT)) {
	if (f2fs_encrypted_file()) {
		if (fscrypt_inode_uses_fs_layer_crypto)
			ret = fscrypt_zeroout_range();
		else
			ret = fscrypt_zeroout_range_inline_crypt();
	} else {
		ret = blkdev_issue_zeroout();
	}
}

Thanks,

> 
> 2020년 7월 15일 (수) 오후 3:17, Chao Yu <yuchao0@huawei.com>님이 작성:
>>
>> On 2020/7/15 12:06, Daeho Jeong wrote:
>>> We could use fscrypt_zeroout_range() for an encrypted file.
>>> But, one problem is fscrypt_zeroout_range() assumes that filesystems
>>> only use a single block device.
>>> It means it doesn't receive bdev as a parameter.
>>> How about changing the interface of fscrypt_zeroout_range() first and using it?
>>
>> Yes, please limited to use fscrypt_zeroout_range() on non-multidevice f2fs image
>> first, we can add a condition to check that in the beginning of ioctl interface,
>> once fscrypt_zeroout_range() accepts bdev as parameter, we can remove that limitation.
>>
>> Thanks,
>>
>>>
>>> 2020년 7월 14일 (화) 오후 9:36, Chao Yu <yuchao0@huawei.com>님이 작성:
>>>>
>>>> On 2020/7/14 2:11, Jaegeuk Kim wrote:
>>>>> Hi Daeho,
>>>>>
>>>>> Please take a look at this.
>>>>>
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=35245180459aebf6d70fde88a538f0400a794aa6
>>>>
>>>> I'm curious about what will happen if we call
>>>> sec_trim_file(F2FS_TRIM_FILE_ZEROOUT) on an encrypted file, will
>>>> it use zero bits covering encrypted data on disk?
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> On 07/13, Daeho Jeong wrote:
>>>>>> From: Daeho Jeong <daehojeong@google.com>
>>>>>>
>>>>>> Changed the way of handling range.len of F2FS_IOC_SEC_TRIM_FILE.
>>>>>>  1. Added -1 value support for range.len to secure trim the whole blocks
>>>>>>     starting from range.start regardless of i_size.
>>>>>>  2. If the end of the range passes over the end of file, it means until
>>>>>>     the end of file (i_size).
>>>>>>  3. ignored the case of that range.len is zero to prevent the function
>>>>>>     from making end_addr zero and triggering different behaviour of
>>>>>>     the function.
>>>>>>
>>>>>> Signed-off-by: Daeho Jeong <daehojeong@google.com>
>>>>>> ---
>>>>>> Changes in v2:
>>>>>>  - Changed -1 range.len option to mean the whole blocks starting from
>>>>>>    range.start regardless of i_size
>>>>>> ---
>>>>>>  fs/f2fs/file.c | 23 ++++++++++++-----------
>>>>>>  1 file changed, 12 insertions(+), 11 deletions(-)
>>>>>>
>>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>>>> index 368c80f8e2a1..2485841e3b2d 100644
>>>>>> --- a/fs/f2fs/file.c
>>>>>> +++ b/fs/f2fs/file.c
>>>>>> @@ -3792,7 +3792,7 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
>>>>>>      pgoff_t index, pg_end;
>>>>>>      block_t prev_block = 0, len = 0;
>>>>>>      loff_t end_addr;
>>>>>> -    bool to_end;
>>>>>> +    bool to_end = false;
>>>>>>      int ret = 0;
>>>>>>
>>>>>>      if (!(filp->f_mode & FMODE_WRITE))
>>>>>> @@ -3813,23 +3813,23 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
>>>>>>      file_start_write(filp);
>>>>>>      inode_lock(inode);
>>>>>>
>>>>>> -    if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode)) {
>>>>>> +    if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode) ||
>>>>>> +                    range.start >= inode->i_size) {
>>>>>>              ret = -EINVAL;
>>>>>>              goto err;
>>>>>>      }
>>>>>>
>>>>>> -    if (range.start >= inode->i_size) {
>>>>>> -            ret = -EINVAL;
>>>>>> +    if (range.len == 0)
>>>>>>              goto err;
>>>>>> -    }
>>>>>>
>>>>>> -    if (inode->i_size - range.start < range.len) {
>>>>>> -            ret = -E2BIG;
>>>>>> -            goto err;
>>>>>> +    if (inode->i_size - range.start > range.len) {
>>>>>> +            end_addr = range.start + range.len;
>>>>>> +    } else {
>>>>>> +            end_addr = range.len == (u64)-1 ?
>>>>>> +                    sbi->sb->s_maxbytes : inode->i_size;
>>>>>> +            to_end = true;
>>>>>>      }
>>>>>> -    end_addr = range.start + range.len;
>>>>>>
>>>>>> -    to_end = (end_addr == inode->i_size);
>>>>>>      if (!IS_ALIGNED(range.start, F2FS_BLKSIZE) ||
>>>>>>                      (!to_end && !IS_ALIGNED(end_addr, F2FS_BLKSIZE))) {
>>>>>>              ret = -EINVAL;
>>>>>> @@ -3846,7 +3846,8 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
>>>>>>      down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>>>>>>      down_write(&F2FS_I(inode)->i_mmap_sem);
>>>>>>
>>>>>> -    ret = filemap_write_and_wait_range(mapping, range.start, end_addr - 1);
>>>>>> +    ret = filemap_write_and_wait_range(mapping, range.start,
>>>>>> +                    to_end ? LLONG_MAX : end_addr - 1);
>>>>>>      if (ret)
>>>>>>              goto out;
>>>>>>
>>>>>> --
>>>>>> 2.27.0.383.g050319c2ae-goog
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Linux-f2fs-devel mailing list
>>>>> Linux-f2fs-devel@lists.sourceforge.net
>>>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>>>> .
>>>>>
>>> .
>>>
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: change the way of handling range.len in F2FS_IOC_SEC_TRIM_FILE
  2020-07-15  7:17           ` Chao Yu
@ 2020-07-15 10:25             ` Daeho Jeong
  2020-07-15 16:42               ` Eric Biggers
  0 siblings, 1 reply; 11+ messages in thread
From: Daeho Jeong @ 2020-07-15 10:25 UTC (permalink / raw)
  To: Chao Yu
  Cc: Jaegeuk Kim, linux-f2fs-devel, kernel-team, Daeho Jeong, linux-kernel

Chao,

I can't find fscrypt_zeroout_range_inline_crypt() function. Do you
mean we need to implement this one for inline encryption?

2020년 7월 15일 (수) 오후 4:17, Chao Yu <yuchao0@huawei.com>님이 작성:
>
> On 2020/7/15 14:54, Daeho Jeong wrote:
> > You mean we can support ZEROOUT option only for encrypted files of
> > non-multidevice f2fs,
> > and return -EOPNOTSUPP in the multidevice case, right now?
>
> Yes, something like:
>
> f2fs_sec_trim_file()
>
> if ((range.flags & F2FS_TRIM_FILE_ZEROOUT) &&
>         f2fs_encrypted_file() && f2fs_is_multi_device())
>         return -EOPNOTSUPP;
>
>
> f2fs_secure_erase()
>
> if (!ret && (flags & F2FS_TRIM_FILE_ZEROOUT)) {
>         if (f2fs_encrypted_file()) {
>                 if (fscrypt_inode_uses_fs_layer_crypto)
>                         ret = fscrypt_zeroout_range();
>                 else
>                         ret = fscrypt_zeroout_range_inline_crypt();
>         } else {
>                 ret = blkdev_issue_zeroout();
>         }
> }
>
> Thanks,
>
> >
> > 2020년 7월 15일 (수) 오후 3:17, Chao Yu <yuchao0@huawei.com>님이 작성:
> >>
> >> On 2020/7/15 12:06, Daeho Jeong wrote:
> >>> We could use fscrypt_zeroout_range() for an encrypted file.
> >>> But, one problem is fscrypt_zeroout_range() assumes that filesystems
> >>> only use a single block device.
> >>> It means it doesn't receive bdev as a parameter.
> >>> How about changing the interface of fscrypt_zeroout_range() first and using it?
> >>
> >> Yes, please limited to use fscrypt_zeroout_range() on non-multidevice f2fs image
> >> first, we can add a condition to check that in the beginning of ioctl interface,
> >> once fscrypt_zeroout_range() accepts bdev as parameter, we can remove that limitation.
> >>
> >> Thanks,
> >>
> >>>
> >>> 2020년 7월 14일 (화) 오후 9:36, Chao Yu <yuchao0@huawei.com>님이 작성:
> >>>>
> >>>> On 2020/7/14 2:11, Jaegeuk Kim wrote:
> >>>>> Hi Daeho,
> >>>>>
> >>>>> Please take a look at this.
> >>>>>
> >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev&id=35245180459aebf6d70fde88a538f0400a794aa6
> >>>>
> >>>> I'm curious about what will happen if we call
> >>>> sec_trim_file(F2FS_TRIM_FILE_ZEROOUT) on an encrypted file, will
> >>>> it use zero bits covering encrypted data on disk?
> >>>>
> >>>> Thanks,
> >>>>
> >>>>>
> >>>>> Thanks,
> >>>>>
> >>>>> On 07/13, Daeho Jeong wrote:
> >>>>>> From: Daeho Jeong <daehojeong@google.com>
> >>>>>>
> >>>>>> Changed the way of handling range.len of F2FS_IOC_SEC_TRIM_FILE.
> >>>>>>  1. Added -1 value support for range.len to secure trim the whole blocks
> >>>>>>     starting from range.start regardless of i_size.
> >>>>>>  2. If the end of the range passes over the end of file, it means until
> >>>>>>     the end of file (i_size).
> >>>>>>  3. ignored the case of that range.len is zero to prevent the function
> >>>>>>     from making end_addr zero and triggering different behaviour of
> >>>>>>     the function.
> >>>>>>
> >>>>>> Signed-off-by: Daeho Jeong <daehojeong@google.com>
> >>>>>> ---
> >>>>>> Changes in v2:
> >>>>>>  - Changed -1 range.len option to mean the whole blocks starting from
> >>>>>>    range.start regardless of i_size
> >>>>>> ---
> >>>>>>  fs/f2fs/file.c | 23 ++++++++++++-----------
> >>>>>>  1 file changed, 12 insertions(+), 11 deletions(-)
> >>>>>>
> >>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> >>>>>> index 368c80f8e2a1..2485841e3b2d 100644
> >>>>>> --- a/fs/f2fs/file.c
> >>>>>> +++ b/fs/f2fs/file.c
> >>>>>> @@ -3792,7 +3792,7 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
> >>>>>>      pgoff_t index, pg_end;
> >>>>>>      block_t prev_block = 0, len = 0;
> >>>>>>      loff_t end_addr;
> >>>>>> -    bool to_end;
> >>>>>> +    bool to_end = false;
> >>>>>>      int ret = 0;
> >>>>>>
> >>>>>>      if (!(filp->f_mode & FMODE_WRITE))
> >>>>>> @@ -3813,23 +3813,23 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
> >>>>>>      file_start_write(filp);
> >>>>>>      inode_lock(inode);
> >>>>>>
> >>>>>> -    if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode)) {
> >>>>>> +    if (f2fs_is_atomic_file(inode) || f2fs_compressed_file(inode) ||
> >>>>>> +                    range.start >= inode->i_size) {
> >>>>>>              ret = -EINVAL;
> >>>>>>              goto err;
> >>>>>>      }
> >>>>>>
> >>>>>> -    if (range.start >= inode->i_size) {
> >>>>>> -            ret = -EINVAL;
> >>>>>> +    if (range.len == 0)
> >>>>>>              goto err;
> >>>>>> -    }
> >>>>>>
> >>>>>> -    if (inode->i_size - range.start < range.len) {
> >>>>>> -            ret = -E2BIG;
> >>>>>> -            goto err;
> >>>>>> +    if (inode->i_size - range.start > range.len) {
> >>>>>> +            end_addr = range.start + range.len;
> >>>>>> +    } else {
> >>>>>> +            end_addr = range.len == (u64)-1 ?
> >>>>>> +                    sbi->sb->s_maxbytes : inode->i_size;
> >>>>>> +            to_end = true;
> >>>>>>      }
> >>>>>> -    end_addr = range.start + range.len;
> >>>>>>
> >>>>>> -    to_end = (end_addr == inode->i_size);
> >>>>>>      if (!IS_ALIGNED(range.start, F2FS_BLKSIZE) ||
> >>>>>>                      (!to_end && !IS_ALIGNED(end_addr, F2FS_BLKSIZE))) {
> >>>>>>              ret = -EINVAL;
> >>>>>> @@ -3846,7 +3846,8 @@ static int f2fs_sec_trim_file(struct file *filp, unsigned long arg)
> >>>>>>      down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> >>>>>>      down_write(&F2FS_I(inode)->i_mmap_sem);
> >>>>>>
> >>>>>> -    ret = filemap_write_and_wait_range(mapping, range.start, end_addr - 1);
> >>>>>> +    ret = filemap_write_and_wait_range(mapping, range.start,
> >>>>>> +                    to_end ? LLONG_MAX : end_addr - 1);
> >>>>>>      if (ret)
> >>>>>>              goto out;
> >>>>>>
> >>>>>> --
> >>>>>> 2.27.0.383.g050319c2ae-goog
> >>>>>
> >>>>>
> >>>>> _______________________________________________
> >>>>> Linux-f2fs-devel mailing list
> >>>>> Linux-f2fs-devel@lists.sourceforge.net
> >>>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> >>>>> .
> >>>>>
> >>> .
> >>>
> > .
> >


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: change the way of handling range.len in F2FS_IOC_SEC_TRIM_FILE
  2020-07-15 10:25             ` Daeho Jeong
@ 2020-07-15 16:42               ` Eric Biggers
  2020-07-16  1:04                 ` Chao Yu
  0 siblings, 1 reply; 11+ messages in thread
From: Eric Biggers @ 2020-07-15 16:42 UTC (permalink / raw)
  To: Daeho Jeong
  Cc: Daeho Jeong, linux-kernel, linux-f2fs-devel, Jaegeuk Kim, kernel-team

On Wed, Jul 15, 2020 at 07:25:13PM +0900, Daeho Jeong wrote:
> Chao,
> 
> I can't find fscrypt_zeroout_range_inline_crypt() function. Do you
> mean we need to implement this one for inline encryption?
> 
> 2020년 7월 15일 (수) 오후 4:17, Chao Yu <yuchao0@huawei.com>님이 작성:
> >
> > On 2020/7/15 14:54, Daeho Jeong wrote:
> > > You mean we can support ZEROOUT option only for encrypted files of
> > > non-multidevice f2fs,
> > > and return -EOPNOTSUPP in the multidevice case, right now?
> >
> > Yes, something like:
> >
> > f2fs_sec_trim_file()
> >
> > if ((range.flags & F2FS_TRIM_FILE_ZEROOUT) &&
> >         f2fs_encrypted_file() && f2fs_is_multi_device())
> >         return -EOPNOTSUPP;
> >
> >
> > f2fs_secure_erase()
> >
> > if (!ret && (flags & F2FS_TRIM_FILE_ZEROOUT)) {
> >         if (f2fs_encrypted_file()) {
> >                 if (fscrypt_inode_uses_fs_layer_crypto)
> >                         ret = fscrypt_zeroout_range();
> >                 else
> >                         ret = fscrypt_zeroout_range_inline_crypt();
> >         } else {
> >                 ret = blkdev_issue_zeroout();
> >         }
> > }

fscrypt_zeroout_range_inline_crypt() is being added by
"fscrypt: add inline encryption support", which is queued in the fscrypt tree
(the master branch of https://git.kernel.org/pub/scm/fs/fscrypt/fscrypt.git).

But that's not actually relevant here because fscrypt_zeroout_range() calls
fscrypt_zeroout_range_inline_crypt() when needed.

Just use fscrypt_zeroout_range().

- Eric


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [f2fs-dev] [PATCH v2] f2fs: change the way of handling range.len in F2FS_IOC_SEC_TRIM_FILE
  2020-07-15 16:42               ` Eric Biggers
@ 2020-07-16  1:04                 ` Chao Yu
  0 siblings, 0 replies; 11+ messages in thread
From: Chao Yu @ 2020-07-16  1:04 UTC (permalink / raw)
  To: Eric Biggers, Daeho Jeong
  Cc: Jaegeuk Kim, kernel-team, Daeho Jeong, linux-kernel, linux-f2fs-devel

On 2020/7/16 0:42, Eric Biggers wrote:
> On Wed, Jul 15, 2020 at 07:25:13PM +0900, Daeho Jeong wrote:
>> Chao,
>>
>> I can't find fscrypt_zeroout_range_inline_crypt() function. Do you
>> mean we need to implement this one for inline encryption?
>>
>> 2020년 7월 15일 (수) 오후 4:17, Chao Yu <yuchao0@huawei.com>님이 작성:
>>>
>>> On 2020/7/15 14:54, Daeho Jeong wrote:
>>>> You mean we can support ZEROOUT option only for encrypted files of
>>>> non-multidevice f2fs,
>>>> and return -EOPNOTSUPP in the multidevice case, right now?
>>>
>>> Yes, something like:
>>>
>>> f2fs_sec_trim_file()
>>>
>>> if ((range.flags & F2FS_TRIM_FILE_ZEROOUT) &&
>>>         f2fs_encrypted_file() && f2fs_is_multi_device())
>>>         return -EOPNOTSUPP;
>>>
>>>
>>> f2fs_secure_erase()
>>>
>>> if (!ret && (flags & F2FS_TRIM_FILE_ZEROOUT)) {
>>>         if (f2fs_encrypted_file()) {
>>>                 if (fscrypt_inode_uses_fs_layer_crypto)
>>>                         ret = fscrypt_zeroout_range();
>>>                 else
>>>                         ret = fscrypt_zeroout_range_inline_crypt();
>>>         } else {
>>>                 ret = blkdev_issue_zeroout();
>>>         }
>>> }
> 
> fscrypt_zeroout_range_inline_crypt() is being added by
> "fscrypt: add inline encryption support", which is queued in the fscrypt tree
> (the master branch of https://git.kernel.org/pub/scm/fs/fscrypt/fscrypt.git).
> 
> But that's not actually relevant here because fscrypt_zeroout_range() calls
> fscrypt_zeroout_range_inline_crypt() when needed.

Oh, correct, thanks for pointing out.

Thanks,

> 
> Just use fscrypt_zeroout_range().
> 
> - Eric
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-13  3:12 [f2fs-dev] [PATCH v2] f2fs: change the way of handling range.len in F2FS_IOC_SEC_TRIM_FILE Daeho Jeong
2020-07-13 18:11 ` Jaegeuk Kim
2020-07-13 23:34   ` Daeho Jeong
2020-07-14 12:36   ` Chao Yu
2020-07-15  4:06     ` Daeho Jeong
2020-07-15  6:16       ` Chao Yu
2020-07-15  6:54         ` Daeho Jeong
2020-07-15  7:17           ` Chao Yu
2020-07-15 10:25             ` Daeho Jeong
2020-07-15 16:42               ` Eric Biggers
2020-07-16  1:04                 ` Chao Yu

Linux-f2fs-devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-f2fs-devel/0 linux-f2fs-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-f2fs-devel linux-f2fs-devel/ https://lore.kernel.org/linux-f2fs-devel \
		linux-f2fs-devel@lists.sourceforge.net
	public-inbox-index linux-f2fs-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/net.sourceforge.lists.linux-f2fs-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git