[f2fs-dev] [PATCH 1/2] f2fs: fix remove page failed in invalidate compress pages

[f2fs-dev] [PATCH 1/2] f2fs: fix remove page failed in invalidate compress pages

[Fengnan Chang]

Since compress inode not a regular file, generic_error_remove_page in
f2fs_invalidate_compress_pages will always be failed, set compress
inode as a regular file to fix it.

Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
---
 fs/f2fs/inode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 0f8b2df3e1e0..7ea76784efcc 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -516,6 +516,7 @@ struct inode *f2fs_iget(struct super_block *sb, unsigned long ino)
 	} else if (ino == F2FS_COMPRESS_INO(sbi)) {
 #ifdef CONFIG_F2FS_FS_COMPRESSION
 		inode->i_mapping->a_ops = &f2fs_compress_aops;
+		inode->i_mode |= S_IFREG;
 #endif
 		mapping_set_gfp_mask(inode->i_mapping,
 			GFP_NOFS | __GFP_HIGHMEM | __GFP_MOVABLE);
-- 
2.32.0



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

[f2fs-dev] [PATCH 2/2] f2fs: support POSIX_FADV_DONTNEED drop compressed page cache

[Fengnan Chang]

Previously, compressed page cache drop when clean page cache, but
POSIX_FADV_DONTNEED can't clean compressed page cache, this commit
try to support it.

Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
---
 fs/f2fs/compress.c | 10 ++++++++--
 fs/f2fs/f2fs.h     |  7 ++++---
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index fb9e5149af5d..7ec5e3c2590b 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -842,7 +842,7 @@ void f2fs_end_read_compressed_page(struct page *page, bool failed,
 		WRITE_ONCE(dic->failed, true);
 	else if (blkaddr)
 		f2fs_cache_compressed_page(sbi, page,
-					dic->inode->i_ino, blkaddr);
+					dic, blkaddr);
 
 	if (atomic_dec_and_test(&dic->remaining_pages))
 		f2fs_decompress_cluster(dic);
@@ -1659,6 +1659,7 @@ static void f2fs_put_dic(struct decompress_io_ctx *dic)
 static void __f2fs_decompress_end_io(struct decompress_io_ctx *dic, bool failed)
 {
 	int i;
+	nid_t ino = dic->inode->i_ino;
 
 	for (i = 0; i < dic->cluster_size; i++) {
 		struct page *rpage = dic->rpages[i];
@@ -1666,6 +1667,9 @@ static void __f2fs_decompress_end_io(struct decompress_io_ctx *dic, bool failed)
 		if (!rpage)
 			continue;
 
+		if (dic->cpage_cached)
+			set_page_private_data(rpage, ino);
+
 		/* PG_error was set if verity failed. */
 		if (failed || PageError(rpage)) {
 			ClearPageUptodate(rpage);
@@ -1772,10 +1776,11 @@ void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi, block_t blkaddr)
 }
 
 void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
-						nid_t ino, block_t blkaddr)
+				struct decompress_io_ctx *dic, block_t blkaddr)
 {
 	struct page *cpage;
 	int ret;
+	nid_t ino = dic->inode->i_ino;
 
 	if (!test_opt(sbi, COMPRESS_CACHE))
 		return;
@@ -1804,6 +1809,7 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
 	}
 
 	set_page_private_data(cpage, ino);
+	dic->cpage_cached = true;
 
 	if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
 		goto out;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index ac6dda6c4c5a..128190b0c737 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1551,6 +1551,7 @@ struct decompress_io_ctx {
 	 */
 	refcount_t refcnt;
 
+	bool cpage_cached;		/* indicate cpages cached in compress mapping*/
 	bool failed;			/* IO error occurred before decompression? */
 	bool need_verity;		/* need fs-verity verification after decompression? */
 	void *private;			/* payload buffer for specified decompression algorithm */
@@ -4085,7 +4086,7 @@ void f2fs_destroy_compress_cache(void);
 struct address_space *COMPRESS_MAPPING(struct f2fs_sb_info *sbi);
 void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi, block_t blkaddr);
 void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
-						nid_t ino, block_t blkaddr);
+				struct decompress_io_ctx *dic, block_t blkaddr);
 bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
 								block_t blkaddr);
 void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino);
@@ -4137,8 +4138,8 @@ static inline int __init f2fs_init_compress_cache(void) { return 0; }
 static inline void f2fs_destroy_compress_cache(void) { }
 static inline void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi,
 				block_t blkaddr) { }
-static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi,
-				struct page *page, nid_t ino, block_t blkaddr) { }
+static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
+				struct decompress_io_ctx *dic, block_t blkaddr) { }
 static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
 				struct page *page, block_t blkaddr) { return false; }
 static inline void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi,
-- 
2.32.0



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

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix remove page failed in invalidate compress pages

[常凤楠]

Do we have some limit that we must use exported symbol to make f2fs must can load as a module? If no, maybe we can just use truncate_inode_page to replace generic_error_remove_page ?

> -----Original Message-----
> From: 常凤楠
> Sent: Wednesday, November 24, 2021 4:39 PM
> To: jaegeuk@kernel.org; chao@kernel.org
> Cc: linux-f2fs-devel@lists.sourceforge.net; 常凤楠 <changfengnan@vivo.com>
> Subject: [PATCH 1/2] f2fs: fix remove page failed in invalidate compress pages
> 
> Since compress inode not a regular file, generic_error_remove_page in
> f2fs_invalidate_compress_pages will always be failed, set compress inode as a
> regular file to fix it.
> 
> Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
> ---
>  fs/f2fs/inode.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c index 0f8b2df3e1e0..7ea76784efcc
> 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -516,6 +516,7 @@ struct inode *f2fs_iget(struct super_block *sb,
> unsigned long ino)
>  	} else if (ino == F2FS_COMPRESS_INO(sbi)) {  #ifdef
> CONFIG_F2FS_FS_COMPRESSION
>  		inode->i_mapping->a_ops = &f2fs_compress_aops;
> +		inode->i_mode |= S_IFREG;
>  #endif
>  		mapping_set_gfp_mask(inode->i_mapping,
>  			GFP_NOFS | __GFP_HIGHMEM | __GFP_MOVABLE);
> --
> 2.32.0


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

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix remove page failed in invalidate compress pages

[Chao Yu]

On 2021/11/24 16:39, Fengnan Chang wrote:
> Since compress inode not a regular file, generic_error_remove_page in
> f2fs_invalidate_compress_pages will always be failed, set compress
> inode as a regular file to fix it.
> 

Fixes: 6ce19aff0b8c ("f2fs: compress: add compress_inode to cache compressed blocks")

> Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
> ---
>   fs/f2fs/inode.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index 0f8b2df3e1e0..7ea76784efcc 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -516,6 +516,7 @@ struct inode *f2fs_iget(struct super_block *sb, unsigned long ino)
>   	} else if (ino == F2FS_COMPRESS_INO(sbi)) {
>   #ifdef CONFIG_F2FS_FS_COMPRESSION
>   		inode->i_mapping->a_ops = &f2fs_compress_aops;

/* generic_error_remove_page only truncates pages of regular inode */

Otherwise it looks good to me.

Thanks,

> +		inode->i_mode |= S_IFREG;
>   #endif
>   		mapping_set_gfp_mask(inode->i_mapping,
>   			GFP_NOFS | __GFP_HIGHMEM | __GFP_MOVABLE);
> 


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

Re: [f2fs-dev] [PATCH 2/2] f2fs: support POSIX_FADV_DONTNEED drop compressed page cache

[Chao Yu]

On 2021/11/24 16:39, Fengnan Chang wrote:
> Previously, compressed page cache drop when clean page cache, but
> POSIX_FADV_DONTNEED can't clean compressed page cache, this commit
> try to support it.
> 
> Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
> ---
>   fs/f2fs/compress.c | 10 ++++++++--
>   fs/f2fs/f2fs.h     |  7 ++++---
>   2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index fb9e5149af5d..7ec5e3c2590b 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -842,7 +842,7 @@ void f2fs_end_read_compressed_page(struct page *page, bool failed,
>   		WRITE_ONCE(dic->failed, true);
>   	else if (blkaddr)
>   		f2fs_cache_compressed_page(sbi, page,
> -					dic->inode->i_ino, blkaddr);
> +					dic, blkaddr);
>   
>   	if (atomic_dec_and_test(&dic->remaining_pages))
>   		f2fs_decompress_cluster(dic);
> @@ -1659,6 +1659,7 @@ static void f2fs_put_dic(struct decompress_io_ctx *dic)
>   static void __f2fs_decompress_end_io(struct decompress_io_ctx *dic, bool failed)
>   {
>   	int i;
> +	nid_t ino = dic->inode->i_ino;
>   
>   	for (i = 0; i < dic->cluster_size; i++) {
>   		struct page *rpage = dic->rpages[i];
> @@ -1666,6 +1667,9 @@ static void __f2fs_decompress_end_io(struct decompress_io_ctx *dic, bool failed)
>   		if (!rpage)
>   			continue;
>   
> +		if (dic->cpage_cached)
> +			set_page_private_data(rpage, ino);

I didn't get the point, why should we set ino into raw page's private field?

Thanks,

> +
>   		/* PG_error was set if verity failed. */
>   		if (failed || PageError(rpage)) {
>   			ClearPageUptodate(rpage);
> @@ -1772,10 +1776,11 @@ void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi, block_t blkaddr)
>   }
>   
>   void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> -						nid_t ino, block_t blkaddr)
> +				struct decompress_io_ctx *dic, block_t blkaddr)
>   {
>   	struct page *cpage;
>   	int ret;
> +	nid_t ino = dic->inode->i_ino;
>   
>   	if (!test_opt(sbi, COMPRESS_CACHE))
>   		return;
> @@ -1804,6 +1809,7 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>   	}
>   
>   	set_page_private_data(cpage, ino);
> +	dic->cpage_cached = true;
>   
>   	if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
>   		goto out;
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index ac6dda6c4c5a..128190b0c737 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1551,6 +1551,7 @@ struct decompress_io_ctx {
>   	 */
>   	refcount_t refcnt;
>   
> +	bool cpage_cached;		/* indicate cpages cached in compress mapping*/
>   	bool failed;			/* IO error occurred before decompression? */
>   	bool need_verity;		/* need fs-verity verification after decompression? */
>   	void *private;			/* payload buffer for specified decompression algorithm */
> @@ -4085,7 +4086,7 @@ void f2fs_destroy_compress_cache(void);
>   struct address_space *COMPRESS_MAPPING(struct f2fs_sb_info *sbi);
>   void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi, block_t blkaddr);
>   void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> -						nid_t ino, block_t blkaddr);
> +				struct decompress_io_ctx *dic, block_t blkaddr);
>   bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
>   								block_t blkaddr);
>   void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino);
> @@ -4137,8 +4138,8 @@ static inline int __init f2fs_init_compress_cache(void) { return 0; }
>   static inline void f2fs_destroy_compress_cache(void) { }
>   static inline void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi,
>   				block_t blkaddr) { }
> -static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi,
> -				struct page *page, nid_t ino, block_t blkaddr) { }
> +static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> +				struct decompress_io_ctx *dic, block_t blkaddr) { }
>   static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
>   				struct page *page, block_t blkaddr) { return false; }
>   static inline void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi,
> 


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

Re: [f2fs-dev] [PATCH 2/2] f2fs: support POSIX_FADV_DONTNEED drop compressed page cache

[常凤楠]

> -----Original Message-----
> From: changfengnan@vivo.com <changfengnan@vivo.com> On Behalf Of Chao
> Yu
> Sent: Thursday, November 25, 2021 11:15 PM
> To: 常凤楠 <changfengnan@vivo.com>; jaegeuk@kernel.org
> Cc: linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [PATCH 2/2] f2fs: support POSIX_FADV_DONTNEED drop
> compressed page cache
> 
> On 2021/11/24 16:39, Fengnan Chang wrote:
> > Previously, compressed page cache drop when clean page cache, but
> > POSIX_FADV_DONTNEED can't clean compressed page cache, this commit try
> > to support it.
> >
> > Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
> > ---
> >   fs/f2fs/compress.c | 10 ++++++++--
> >   fs/f2fs/f2fs.h     |  7 ++++---
> >   2 files changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c index
> > fb9e5149af5d..7ec5e3c2590b 100644
> > --- a/fs/f2fs/compress.c
> > +++ b/fs/f2fs/compress.c
> > @@ -842,7 +842,7 @@ void f2fs_end_read_compressed_page(struct page
> *page, bool failed,
> >   		WRITE_ONCE(dic->failed, true);
> >   	else if (blkaddr)
> >   		f2fs_cache_compressed_page(sbi, page,
> > -					dic->inode->i_ino, blkaddr);
> > +					dic, blkaddr);
> >
> >   	if (atomic_dec_and_test(&dic->remaining_pages))
> >   		f2fs_decompress_cluster(dic);
> > @@ -1659,6 +1659,7 @@ static void f2fs_put_dic(struct decompress_io_ctx
> *dic)
> >   static void __f2fs_decompress_end_io(struct decompress_io_ctx *dic, bool
> failed)
> >   {
> >   	int i;
> > +	nid_t ino = dic->inode->i_ino;
> >
> >   	for (i = 0; i < dic->cluster_size; i++) {
> >   		struct page *rpage = dic->rpages[i]; @@ -1666,6 +1667,9 @@
> static
> > void __f2fs_decompress_end_io(struct decompress_io_ctx *dic, bool failed)
> >   		if (!rpage)
> >   			continue;
> >
> > +		if (dic->cpage_cached)
> > +			set_page_private_data(rpage, ino);
> 
> I didn't get the point, why should we set ino into raw page's private field?
Yes, because raw page will add into page cache, and 
POSIX_FADV_DONTNEED:
invalidate_mapping_pagevec
  ->__invalidate_mapping_pages
    ->invalidate_inode_page
      ->invalidate_complete_page  // call try_to_release_page when page has private data

So, if raw page don't have private data, it will not call f2fs_invalidate_compress_pages.
This commit try use private data to connect raw page which compressed page has been cached.
 
> 
> Thanks,
> 
> > +
> >   		/* PG_error was set if verity failed. */
> >   		if (failed || PageError(rpage)) {
> >   			ClearPageUptodate(rpage);
> > @@ -1772,10 +1776,11 @@ void f2fs_invalidate_compress_page(struct
> f2fs_sb_info *sbi, block_t blkaddr)
> >   }
> >
> >   void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page
> *page,
> > -						nid_t ino, block_t blkaddr)
> > +				struct decompress_io_ctx *dic, block_t blkaddr)
> >   {
> >   	struct page *cpage;
> >   	int ret;
> > +	nid_t ino = dic->inode->i_ino;
> >
> >   	if (!test_opt(sbi, COMPRESS_CACHE))
> >   		return;
> > @@ -1804,6 +1809,7 @@ void f2fs_cache_compressed_page(struct
> f2fs_sb_info *sbi, struct page *page,
> >   	}
> >
> >   	set_page_private_data(cpage, ino);
> > +	dic->cpage_cached = true;
> >
> >   	if (!f2fs_is_valid_blkaddr(sbi, blkaddr,
> DATA_GENERIC_ENHANCE_READ))
> >   		goto out;
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index
> > ac6dda6c4c5a..128190b0c737 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -1551,6 +1551,7 @@ struct decompress_io_ctx {
> >   	 */
> >   	refcount_t refcnt;
> >
> > +	bool cpage_cached;		/* indicate cpages cached in compress
> mapping*/
> >   	bool failed;			/* IO error occurred before decompression?
> */
> >   	bool need_verity;		/* need fs-verity verification after
> decompression? */
> >   	void *private;			/* payload buffer for specified
> decompression algorithm */
> > @@ -4085,7 +4086,7 @@ void f2fs_destroy_compress_cache(void);
> >   struct address_space *COMPRESS_MAPPING(struct f2fs_sb_info *sbi);
> >   void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi, block_t
> blkaddr);
> >   void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page
> *page,
> > -						nid_t ino, block_t blkaddr);
> > +				struct decompress_io_ctx *dic, block_t blkaddr);
> >   bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page
> *page,
> >   								block_t blkaddr);
> >   void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t
> > ino); @@ -4137,8 +4138,8 @@ static inline int __init
> f2fs_init_compress_cache(void) { return 0; }
> >   static inline void f2fs_destroy_compress_cache(void) { }
> >   static inline void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi,
> >   				block_t blkaddr) { }
> > -static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi,
> > -				struct page *page, nid_t ino, block_t blkaddr) { }
> > +static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi,
> struct page *page,
> > +				struct decompress_io_ctx *dic, block_t blkaddr) { }
> >   static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
> >   				struct page *page, block_t blkaddr) { return false; }
> >   static inline void f2fs_invalidate_compress_pages(struct
> > f2fs_sb_info *sbi,
> >

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

Re: [f2fs-dev] [PATCH 2/2] f2fs: support POSIX_FADV_DONTNEED drop compressed page cache

[Chao Yu]

On 2021/11/26 10:14, 常凤楠 wrote:
> 
> 
>> -----Original Message-----
>> From: changfengnan@vivo.com <changfengnan@vivo.com> On Behalf Of Chao
>> Yu
>> Sent: Thursday, November 25, 2021 11:15 PM
>> To: 常凤楠 <changfengnan@vivo.com>; jaegeuk@kernel.org
>> Cc: linux-f2fs-devel@lists.sourceforge.net
>> Subject: Re: [PATCH 2/2] f2fs: support POSIX_FADV_DONTNEED drop
>> compressed page cache
>>
>> On 2021/11/24 16:39, Fengnan Chang wrote:
>>> Previously, compressed page cache drop when clean page cache, but
>>> POSIX_FADV_DONTNEED can't clean compressed page cache, this commit try
>>> to support it.
>>>
>>> Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
>>> ---
>>>    fs/f2fs/compress.c | 10 ++++++++--
>>>    fs/f2fs/f2fs.h     |  7 ++++---
>>>    2 files changed, 12 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c index
>>> fb9e5149af5d..7ec5e3c2590b 100644
>>> --- a/fs/f2fs/compress.c
>>> +++ b/fs/f2fs/compress.c
>>> @@ -842,7 +842,7 @@ void f2fs_end_read_compressed_page(struct page
>> *page, bool failed,
>>>    		WRITE_ONCE(dic->failed, true);
>>>    	else if (blkaddr)
>>>    		f2fs_cache_compressed_page(sbi, page,
>>> -					dic->inode->i_ino, blkaddr);
>>> +					dic, blkaddr);
>>>
>>>    	if (atomic_dec_and_test(&dic->remaining_pages))
>>>    		f2fs_decompress_cluster(dic);
>>> @@ -1659,6 +1659,7 @@ static void f2fs_put_dic(struct decompress_io_ctx
>> *dic)
>>>    static void __f2fs_decompress_end_io(struct decompress_io_ctx *dic, bool
>> failed)
>>>    {
>>>    	int i;
>>> +	nid_t ino = dic->inode->i_ino;
>>>
>>>    	for (i = 0; i < dic->cluster_size; i++) {
>>>    		struct page *rpage = dic->rpages[i]; @@ -1666,6 +1667,9 @@
>> static
>>> void __f2fs_decompress_end_io(struct decompress_io_ctx *dic, bool failed)
>>>    		if (!rpage)
>>>    			continue;
>>>
>>> +		if (dic->cpage_cached)
>>> +			set_page_private_data(rpage, ino);
>>
>> I didn't get the point, why should we set ino into raw page's private field?
> Yes, because raw page will add into page cache, and
> POSIX_FADV_DONTNEED:
> invalidate_mapping_pagevec
>    ->__invalidate_mapping_pages
>      ->invalidate_inode_page
>        ->invalidate_complete_page  // call try_to_release_page when page has private data
> 
> So, if raw page don't have private data, it will not call f2fs_invalidate_compress_pages.
> This commit try use private data to connect raw page which compressed page has been cached.

Oh, yup, how about calling f2fs_invalidate_compress_pages() directly in f2fs_file_fadvise()
for POSIX_FADV_DONTNEED case? it can avoid unneeded use of private field space.

Thanks,

>   
>>
>> Thanks,
>>
>>> +
>>>    		/* PG_error was set if verity failed. */
>>>    		if (failed || PageError(rpage)) {
>>>    			ClearPageUptodate(rpage);
>>> @@ -1772,10 +1776,11 @@ void f2fs_invalidate_compress_page(struct
>> f2fs_sb_info *sbi, block_t blkaddr)
>>>    }
>>>
>>>    void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page
>> *page,
>>> -						nid_t ino, block_t blkaddr)
>>> +				struct decompress_io_ctx *dic, block_t blkaddr)
>>>    {
>>>    	struct page *cpage;
>>>    	int ret;
>>> +	nid_t ino = dic->inode->i_ino;
>>>
>>>    	if (!test_opt(sbi, COMPRESS_CACHE))
>>>    		return;
>>> @@ -1804,6 +1809,7 @@ void f2fs_cache_compressed_page(struct
>> f2fs_sb_info *sbi, struct page *page,
>>>    	}
>>>
>>>    	set_page_private_data(cpage, ino);
>>> +	dic->cpage_cached = true;
>>>
>>>    	if (!f2fs_is_valid_blkaddr(sbi, blkaddr,
>> DATA_GENERIC_ENHANCE_READ))
>>>    		goto out;
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index
>>> ac6dda6c4c5a..128190b0c737 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -1551,6 +1551,7 @@ struct decompress_io_ctx {
>>>    	 */
>>>    	refcount_t refcnt;
>>>
>>> +	bool cpage_cached;		/* indicate cpages cached in compress
>> mapping*/
>>>    	bool failed;			/* IO error occurred before decompression?
>> */
>>>    	bool need_verity;		/* need fs-verity verification after
>> decompression? */
>>>    	void *private;			/* payload buffer for specified
>> decompression algorithm */
>>> @@ -4085,7 +4086,7 @@ void f2fs_destroy_compress_cache(void);
>>>    struct address_space *COMPRESS_MAPPING(struct f2fs_sb_info *sbi);
>>>    void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi, block_t
>> blkaddr);
>>>    void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page
>> *page,
>>> -						nid_t ino, block_t blkaddr);
>>> +				struct decompress_io_ctx *dic, block_t blkaddr);
>>>    bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page
>> *page,
>>>    								block_t blkaddr);
>>>    void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t
>>> ino); @@ -4137,8 +4138,8 @@ static inline int __init
>> f2fs_init_compress_cache(void) { return 0; }
>>>    static inline void f2fs_destroy_compress_cache(void) { }
>>>    static inline void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi,
>>>    				block_t blkaddr) { }
>>> -static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi,
>>> -				struct page *page, nid_t ino, block_t blkaddr) { }
>>> +static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi,
>> struct page *page,
>>> +				struct decompress_io_ctx *dic, block_t blkaddr) { }
>>>    static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
>>>    				struct page *page, block_t blkaddr) { return false; }
>>>    static inline void f2fs_invalidate_compress_pages(struct
>>> f2fs_sb_info *sbi,
>>>


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

Re: [f2fs-dev] [PATCH 2/2] f2fs: support POSIX_FADV_DONTNEED drop compressed page cache

[常凤楠]

> -----Original Message-----
> From: changfengnan@vivo.com <changfengnan@vivo.com> On Behalf Of Chao
> Yu
> Sent: Friday, November 26, 2021 10:58 AM
> To: 常凤楠 <changfengnan@vivo.com>; jaegeuk@kernel.org
> Cc: linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [PATCH 2/2] f2fs: support POSIX_FADV_DONTNEED drop
> compressed page cache
> 
> On 2021/11/26 10:14, 常凤楠 wrote:
> >
> >
> >> -----Original Message-----
> >> From: changfengnan@vivo.com <changfengnan@vivo.com> On Behalf Of
> Chao
> >> Yu
> >> Sent: Thursday, November 25, 2021 11:15 PM
> >> To: 常凤楠 <changfengnan@vivo.com>; jaegeuk@kernel.org
> >> Cc: linux-f2fs-devel@lists.sourceforge.net
> >> Subject: Re: [PATCH 2/2] f2fs: support POSIX_FADV_DONTNEED drop
> >> compressed page cache
> >>
> >> On 2021/11/24 16:39, Fengnan Chang wrote:
> >>> Previously, compressed page cache drop when clean page cache, but
> >>> POSIX_FADV_DONTNEED can't clean compressed page cache, this commit
> >>> try to support it.
> >>>
> >>> Signed-off-by: Fengnan Chang <changfengnan@vivo.com>
> >>> ---
> >>>    fs/f2fs/compress.c | 10 ++++++++--
> >>>    fs/f2fs/f2fs.h     |  7 ++++---
> >>>    2 files changed, 12 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c index
> >>> fb9e5149af5d..7ec5e3c2590b 100644
> >>> --- a/fs/f2fs/compress.c
> >>> +++ b/fs/f2fs/compress.c
> >>> @@ -842,7 +842,7 @@ void f2fs_end_read_compressed_page(struct page
> >> *page, bool failed,
> >>>    		WRITE_ONCE(dic->failed, true);
> >>>    	else if (blkaddr)
> >>>    		f2fs_cache_compressed_page(sbi, page,
> >>> -					dic->inode->i_ino, blkaddr);
> >>> +					dic, blkaddr);
> >>>
> >>>    	if (atomic_dec_and_test(&dic->remaining_pages))
> >>>    		f2fs_decompress_cluster(dic);
> >>> @@ -1659,6 +1659,7 @@ static void f2fs_put_dic(struct
> >>> decompress_io_ctx
> >> *dic)
> >>>    static void __f2fs_decompress_end_io(struct decompress_io_ctx
> >>> *dic, bool
> >> failed)
> >>>    {
> >>>    	int i;
> >>> +	nid_t ino = dic->inode->i_ino;
> >>>
> >>>    	for (i = 0; i < dic->cluster_size; i++) {
> >>>    		struct page *rpage = dic->rpages[i]; @@ -1666,6 +1667,9 @@
> >> static
> >>> void __f2fs_decompress_end_io(struct decompress_io_ctx *dic, bool
> failed)
> >>>    		if (!rpage)
> >>>    			continue;
> >>>
> >>> +		if (dic->cpage_cached)
> >>> +			set_page_private_data(rpage, ino);
> >>
> >> I didn't get the point, why should we set ino into raw page's private field?
> > Yes, because raw page will add into page cache, and
> > POSIX_FADV_DONTNEED:
> > invalidate_mapping_pagevec
> >    ->__invalidate_mapping_pages
> >      ->invalidate_inode_page
> >        ->invalidate_complete_page  // call try_to_release_page when
> > page has private data
> >
> > So, if raw page don't have private data, it will not call
> f2fs_invalidate_compress_pages.
> > This commit try use private data to connect raw page which compressed page
> has been cached.
> 
> Oh, yup, how about calling f2fs_invalidate_compress_pages() directly in
> f2fs_file_fadvise() for POSIX_FADV_DONTNEED case? it can avoid unneeded
> use of private field space.


Good idea, I will modify like this.

Thanks.
> 
> Thanks,
> 
> >
> >>
> >> Thanks,
> >>
> >>> +
> >>>    		/* PG_error was set if verity failed. */
> >>>    		if (failed || PageError(rpage)) {
> >>>    			ClearPageUptodate(rpage);
> >>> @@ -1772,10 +1776,11 @@ void f2fs_invalidate_compress_page(struct
> >> f2fs_sb_info *sbi, block_t blkaddr)
> >>>    }
> >>>
> >>>    void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct
> >>> page
> >> *page,
> >>> -						nid_t ino, block_t blkaddr)
> >>> +				struct decompress_io_ctx *dic, block_t blkaddr)
> >>>    {
> >>>    	struct page *cpage;
> >>>    	int ret;
> >>> +	nid_t ino = dic->inode->i_ino;
> >>>
> >>>    	if (!test_opt(sbi, COMPRESS_CACHE))
> >>>    		return;
> >>> @@ -1804,6 +1809,7 @@ void f2fs_cache_compressed_page(struct
> >> f2fs_sb_info *sbi, struct page *page,
> >>>    	}
> >>>
> >>>    	set_page_private_data(cpage, ino);
> >>> +	dic->cpage_cached = true;
> >>>
> >>>    	if (!f2fs_is_valid_blkaddr(sbi, blkaddr,
> >> DATA_GENERIC_ENHANCE_READ))
> >>>    		goto out;
> >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index
> >>> ac6dda6c4c5a..128190b0c737 100644
> >>> --- a/fs/f2fs/f2fs.h
> >>> +++ b/fs/f2fs/f2fs.h
> >>> @@ -1551,6 +1551,7 @@ struct decompress_io_ctx {
> >>>    	 */
> >>>    	refcount_t refcnt;
> >>>
> >>> +	bool cpage_cached;		/* indicate cpages cached in compress
> >> mapping*/
> >>>    	bool failed;			/* IO error occurred before decompression?
> >> */
> >>>    	bool need_verity;		/* need fs-verity verification after
> >> decompression? */
> >>>    	void *private;			/* payload buffer for specified
> >> decompression algorithm */
> >>> @@ -4085,7 +4086,7 @@ void f2fs_destroy_compress_cache(void);
> >>>    struct address_space *COMPRESS_MAPPING(struct f2fs_sb_info *sbi);
> >>>    void f2fs_invalidate_compress_page(struct f2fs_sb_info *sbi,
> >>> block_t
> >> blkaddr);
> >>>    void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct
> >>> page
> >> *page,
> >>> -						nid_t ino, block_t blkaddr);
> >>> +				struct decompress_io_ctx *dic, block_t blkaddr);
> >>>    bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct
> >>> page
> >> *page,
> >>>    								block_t blkaddr);
> >>>    void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi,
> >>> nid_t ino); @@ -4137,8 +4138,8 @@ static inline int __init
> >> f2fs_init_compress_cache(void) { return 0; }
> >>>    static inline void f2fs_destroy_compress_cache(void) { }
> >>>    static inline void f2fs_invalidate_compress_page(struct f2fs_sb_info
> *sbi,
> >>>    				block_t blkaddr) { }
> >>> -static inline void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi,
> >>> -				struct page *page, nid_t ino, block_t blkaddr) { }
> >>> +static inline void f2fs_cache_compressed_page(struct f2fs_sb_info
> >>> +*sbi,
> >> struct page *page,
> >>> +				struct decompress_io_ctx *dic, block_t blkaddr) { }
> >>>    static inline bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi,
> >>>    				struct page *page, block_t blkaddr) { return false; }
> >>>    static inline void f2fs_invalidate_compress_pages(struct
> >>> f2fs_sb_info *sbi,
> >>>

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