* [PATCH] f2fs: compress: disallow disabling compress on non-empty compressed file
[not found] <CGME20211025140526epcas1p3b4a59df0935297572c6417c841a4c514@epcas1p3.samsung.com>
@ 2021-10-25 14:05 ` Hyeong-Jun Kim
0 siblings, 0 replies; 4+ messages in thread
From: Hyeong-Jun Kim @ 2021-10-25 14:05 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu
Cc: Hyeong-Jun Kim, Sungjong Seo, linux-f2fs-devel, linux-kernel
Compresse file and normal file has differ in i_addr addressing,
specifically addrs per inode/block. So, we will face data loss, if we
disable the compression flag on non-empty files. Therefore we should
disallow not only enabling but disabling the compression flag on
non-empty files.
Signed-off-by: Sungjong Seo <sj1557.seo@samsung.com>
Signed-off-by: Hyeong-Jun Kim <hj514.kim@samsung.com>
---
fs/f2fs/f2fs.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index b8e8f8c716b0..19146c834abd 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4177,8 +4177,7 @@ static inline bool f2fs_disable_compressed_file(struct inode *inode)
if (!f2fs_compressed_file(inode))
return true;
- if (S_ISREG(inode->i_mode) &&
- (get_dirty_pages(inode) || atomic_read(&fi->i_compr_blocks)))
+ if (S_ISREG(inode->i_mode) && inode->i_size)
return false;
fi->i_flags &= ~F2FS_COMPR_FL;
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [f2fs-dev] [PATCH] f2fs: compress: disallow disabling compress on non-empty compressed file
@ 2021-10-25 14:05 ` Hyeong-Jun Kim
0 siblings, 0 replies; 4+ messages in thread
From: Hyeong-Jun Kim @ 2021-10-25 14:05 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu; +Cc: linux-kernel, linux-f2fs-devel, Sungjong Seo
Compresse file and normal file has differ in i_addr addressing,
specifically addrs per inode/block. So, we will face data loss, if we
disable the compression flag on non-empty files. Therefore we should
disallow not only enabling but disabling the compression flag on
non-empty files.
Signed-off-by: Sungjong Seo <sj1557.seo@samsung.com>
Signed-off-by: Hyeong-Jun Kim <hj514.kim@samsung.com>
---
fs/f2fs/f2fs.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index b8e8f8c716b0..19146c834abd 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4177,8 +4177,7 @@ static inline bool f2fs_disable_compressed_file(struct inode *inode)
if (!f2fs_compressed_file(inode))
return true;
- if (S_ISREG(inode->i_mode) &&
- (get_dirty_pages(inode) || atomic_read(&fi->i_compr_blocks)))
+ if (S_ISREG(inode->i_mode) && inode->i_size)
return false;
fi->i_flags &= ~F2FS_COMPR_FL;
--
2.25.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] f2fs: compress: disallow disabling compress on non-empty compressed file
2021-10-25 14:05 ` [f2fs-dev] " Hyeong-Jun Kim
@ 2021-10-27 2:30 ` Chao Yu
-1 siblings, 0 replies; 4+ messages in thread
From: Chao Yu @ 2021-10-27 2:30 UTC (permalink / raw)
To: Hyeong-Jun Kim, Jaegeuk Kim; +Cc: Sungjong Seo, linux-f2fs-devel, linux-kernel
On 2021/10/25 22:05, Hyeong-Jun Kim wrote:
> Compresse file and normal file has differ in i_addr addressing,
> specifically addrs per inode/block. So, we will face data loss, if we
> disable the compression flag on non-empty files. Therefore we should
> disallow not only enabling but disabling the compression flag on
> non-empty files. >
Hi Hyeong-Jun,
I think this makes sense.
But one more case may bypass below condition, could you please take a look
at this case:
touch file
chattr +c file
f2fs_io pinfile set file
fallocate file -o 0 -l 16384 -n
chattr -c file
So it's better to check F2FS_HAS_BLOCKS() as well in the condition?
Fixes: 4c8ff7095bef ("f2fs: support data compression")
Thanks,
> Signed-off-by: Sungjong Seo <sj1557.seo@samsung.com>
> Signed-off-by: Hyeong-Jun Kim <hj514.kim@samsung.com>
> ---
> fs/f2fs/f2fs.h | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index b8e8f8c716b0..19146c834abd 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -4177,8 +4177,7 @@ static inline bool f2fs_disable_compressed_file(struct inode *inode)
>
> if (!f2fs_compressed_file(inode))
> return true;
> - if (S_ISREG(inode->i_mode) &&
> - (get_dirty_pages(inode) || atomic_read(&fi->i_compr_blocks)))
> + if (S_ISREG(inode->i_mode) && inode->i_size)
> return false;
>
> fi->i_flags &= ~F2FS_COMPR_FL;
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: compress: disallow disabling compress on non-empty compressed file
@ 2021-10-27 2:30 ` Chao Yu
0 siblings, 0 replies; 4+ messages in thread
From: Chao Yu @ 2021-10-27 2:30 UTC (permalink / raw)
To: Hyeong-Jun Kim, Jaegeuk Kim; +Cc: Sungjong Seo, linux-kernel, linux-f2fs-devel
On 2021/10/25 22:05, Hyeong-Jun Kim wrote:
> Compresse file and normal file has differ in i_addr addressing,
> specifically addrs per inode/block. So, we will face data loss, if we
> disable the compression flag on non-empty files. Therefore we should
> disallow not only enabling but disabling the compression flag on
> non-empty files. >
Hi Hyeong-Jun,
I think this makes sense.
But one more case may bypass below condition, could you please take a look
at this case:
touch file
chattr +c file
f2fs_io pinfile set file
fallocate file -o 0 -l 16384 -n
chattr -c file
So it's better to check F2FS_HAS_BLOCKS() as well in the condition?
Fixes: 4c8ff7095bef ("f2fs: support data compression")
Thanks,
> Signed-off-by: Sungjong Seo <sj1557.seo@samsung.com>
> Signed-off-by: Hyeong-Jun Kim <hj514.kim@samsung.com>
> ---
> fs/f2fs/f2fs.h | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index b8e8f8c716b0..19146c834abd 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -4177,8 +4177,7 @@ static inline bool f2fs_disable_compressed_file(struct inode *inode)
>
> if (!f2fs_compressed_file(inode))
> return true;
> - if (S_ISREG(inode->i_mode) &&
> - (get_dirty_pages(inode) || atomic_read(&fi->i_compr_blocks)))
> + if (S_ISREG(inode->i_mode) && inode->i_size)
> return false;
>
> fi->i_flags &= ~F2FS_COMPR_FL;
>
_______________________________________________
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] 4+ messages in thread
end of thread, other threads:[~2021-10-27 2:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20211025140526epcas1p3b4a59df0935297572c6417c841a4c514@epcas1p3.samsung.com>
2021-10-25 14:05 ` [PATCH] f2fs: compress: disallow disabling compress on non-empty compressed file Hyeong-Jun Kim
2021-10-25 14:05 ` [f2fs-dev] " Hyeong-Jun Kim
2021-10-27 2:30 ` Chao Yu
2021-10-27 2:30 ` [f2fs-dev] " Chao Yu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.