> -----Original Message-----
> From: [email protected] <[email protected]> On Behalf Of Chao
> Yu
> Sent: Friday, November 26, 2021 10:58 AM
> To: 常凤楠 <[email protected]>; [email protected]
> Cc: [email protected]
> Subject: Re: [PATCH 2/2] f2fs: support POSIX_FADV_DONTNEED drop
> compressed page cache
>
> On 2021/11/26 10:14, 常凤楠 wrote:
> >
> >
> >> -----Original Message-----
> >> From: [email protected] <[email protected]> On Behalf Of
> Chao
> >> Yu
> >> Sent: Thursday, November 25, 2021 11:15 PM
> >> To: 常凤楠 <[email protected]>; [email protected]
> >> Cc: [email protected]
> >> 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 <[email protected]>
> >>> ---
> >>> 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
