| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CACOAw_y-nnM3ms-TAiHMxGRw9n+Ad5qdf+40yzQ-jVYg4uR-TA@mail.gmail.com>
Date: Fri, 12 Jan 2024 14:15:06 -0800
From: Daeho Jeong <daeho43@...il.com>
To: Chao Yu <chao@...nel.org>
Cc: jaegeuk@...nel.org, linux-kernel@...r.kernel.org,
linux-f2fs-devel@...ts.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH v3 1/6] f2fs: compress: fix to guarantee
persisting compressed blocks by CP
Reviewed-by: Daeho Jeong <daehojeong@...gle.com>
On Thu, Jan 11, 2024 at 9:17 AM Daeho Jeong <daeho43@...il.com> wrote:
>
> On Wed, Jan 10, 2024 at 5:26 PM Chao Yu <chao@...nel.org> wrote:
> >
> > On 2024/1/11 8:55, Daeho Jeong wrote:
> > > On Thu, Dec 28, 2023 at 6:33 AM Chao Yu <chao@...nel.org> wrote:
> > >>
> > >> If data block in compressed cluster is not persisted with metadata
> > >> during checkpoint, after SPOR, the data may be corrupted, let's
> > >> guarantee to write compressed page by checkpoint.
> > >>
> > >> Fixes: 4c8ff7095bef ("f2fs: support data compression")
> > >> Signed-off-by: Chao Yu <chao@...nel.org>
> > >> ---
> > >> v3:
> > >> - treat compressed page as CP guaranteed data explictly.
> > >> fs/f2fs/compress.c | 4 +++-
> > >> fs/f2fs/data.c | 17 +++++++++--------
> > >> fs/f2fs/f2fs.h | 4 +++-
> > >> 3 files changed, 15 insertions(+), 10 deletions(-)
> > >>
> > >> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > >> index c5a4364c4482..9940b7886e5d 100644
> > >> --- a/fs/f2fs/compress.c
> > >> +++ b/fs/f2fs/compress.c
> > >> @@ -1418,6 +1418,8 @@ void f2fs_compress_write_end_io(struct bio *bio, struct page *page)
> > >> struct f2fs_sb_info *sbi = bio->bi_private;
> > >> struct compress_io_ctx *cic =
> > >> (struct compress_io_ctx *)page_private(page);
> > >> + enum count_type type = WB_DATA_TYPE(page,
> > >> + f2fs_is_compressed_page(page));
> > >> int i;
> > >>
> > >> if (unlikely(bio->bi_status))
> > >> @@ -1425,7 +1427,7 @@ void f2fs_compress_write_end_io(struct bio *bio, struct page *page)
> > >>
> > >> f2fs_compress_free_page(page);
> > >>
> > >> - dec_page_count(sbi, F2FS_WB_DATA);
> > >> + dec_page_count(sbi, type);
> > >>
> > >> if (atomic_dec_return(&cic->pending_pages))
> > >> return;
> > >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > >> index dce8defdf4c7..81f9e2cc49e2 100644
> > >> --- a/fs/f2fs/data.c
> > >> +++ b/fs/f2fs/data.c
> > >> @@ -48,7 +48,7 @@ void f2fs_destroy_bioset(void)
> > >> bioset_exit(&f2fs_bioset);
> > >> }
> > >>
> > >> -static bool __is_cp_guaranteed(struct page *page)
> > >> +bool f2fs_is_cp_guaranteed(struct page *page)
> > >> {
> > >> struct address_space *mapping = page->mapping;
> > >> struct inode *inode;
> > >> @@ -65,8 +65,6 @@ static bool __is_cp_guaranteed(struct page *page)
> > >> S_ISDIR(inode->i_mode))
> > >> return true;
> > >>
> > >> - if (f2fs_is_compressed_page(page))
> > >> - return false;
> > >
> > > Out of curiosity, why don't we simply change the above to "return true"?
> >
> > Daeho,
> >
> > I used the implementation, please check v1 and related comments
> > from Jaegeuk and me, let me know if that was not clear enough. :)
> >
> > https://lore.kernel.org/linux-f2fs-devel/aae654e7-8a7e-478d-9f5a-65807a0e0343@kernel.org/
>
> Oh, I missed it. Sorry~
>
> >
> > >
> > >> if ((S_ISREG(inode->i_mode) && IS_NOQUOTA(inode)) ||
> > >> page_private_gcing(page))
> > >> return true;
> > >> @@ -338,7 +336,7 @@ static void f2fs_write_end_io(struct bio *bio)
> > >>
> > >> bio_for_each_segment_all(bvec, bio, iter_all) {
> > >> struct page *page = bvec->bv_page;
> > >> - enum count_type type = WB_DATA_TYPE(page);
> > >> + enum count_type type = WB_DATA_TYPE(page, false);
> > >>
> > >> if (page_private_dummy(page)) {
> > >> clear_page_private_dummy(page);
> > >> @@ -762,7 +760,7 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
> > >> wbc_account_cgroup_owner(fio->io_wbc, fio->page, PAGE_SIZE);
> > >>
> > >> inc_page_count(fio->sbi, is_read_io(fio->op) ?
> > >> - __read_io_type(page) : WB_DATA_TYPE(fio->page));
> > >> + __read_io_type(page) : WB_DATA_TYPE(fio->page, false));
> > >>
> > >> if (is_read_io(bio_op(bio)))
> > >> f2fs_submit_read_bio(fio->sbi, bio, fio->type);
> > >> @@ -973,7 +971,7 @@ int f2fs_merge_page_bio(struct f2fs_io_info *fio)
> > >> if (fio->io_wbc)
> > >> wbc_account_cgroup_owner(fio->io_wbc, fio->page, PAGE_SIZE);
> > >>
> > >> - inc_page_count(fio->sbi, WB_DATA_TYPE(page));
> > >> + inc_page_count(fio->sbi, WB_DATA_TYPE(page, false));
> > >>
> > >> *fio->last_block = fio->new_blkaddr;
> > >> *fio->bio = bio;
> > >> @@ -1007,6 +1005,7 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
> > >> enum page_type btype = PAGE_TYPE_OF_BIO(fio->type);
> > >> struct f2fs_bio_info *io = sbi->write_io[btype] + fio->temp;
> > >> struct page *bio_page;
> > >> + enum count_type type;
> > >>
> > >> f2fs_bug_on(sbi, is_read_io(fio->op));
> > >>
> > >> @@ -1046,7 +1045,8 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
> > >> /* set submitted = true as a return value */
> > >> fio->submitted = 1;
> > >>
> > >> - inc_page_count(sbi, WB_DATA_TYPE(bio_page));
> > >> + type = WB_DATA_TYPE(bio_page, fio->compressed_page);
> > >> + inc_page_count(sbi, type);
> > >>
> > >> if (io->bio &&
> > >> (!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio,
> > >> @@ -1059,7 +1059,8 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
> > >> if (F2FS_IO_ALIGNED(sbi) &&
> > >> (fio->type == DATA || fio->type == NODE) &&
> > >> fio->new_blkaddr & F2FS_IO_SIZE_MASK(sbi)) {
> > >> - dec_page_count(sbi, WB_DATA_TYPE(bio_page));
> > >> + dec_page_count(sbi, WB_DATA_TYPE(bio_page,
> > >> + fio->compressed_page));
> > >> fio->retry = 1;
> > >> goto skip;
> > >> }
> > >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > >> index 65294e3b0bef..50f3d546ded8 100644
> > >> --- a/fs/f2fs/f2fs.h
> > >> +++ b/fs/f2fs/f2fs.h
> > >> @@ -1080,7 +1080,8 @@ struct f2fs_sm_info {
> > >> * f2fs monitors the number of several block types such as on-writeback,
> > >> * dirty dentry blocks, dirty node blocks, and dirty meta blocks.
> > >> */
> > >> -#define WB_DATA_TYPE(p) (__is_cp_guaranteed(p) ? F2FS_WB_CP_DATA : F2FS_WB_DATA)
> > >> +#define WB_DATA_TYPE(p, f) \
> > >> + (f || f2fs_is_cp_guaranteed(p) ? F2FS_WB_CP_DATA : F2FS_WB_DATA)
> > >> enum count_type {
> > >> F2FS_DIRTY_DENTS,
> > >> F2FS_DIRTY_DATA,
> > >> @@ -3804,6 +3805,7 @@ void f2fs_init_ckpt_req_control(struct f2fs_sb_info *sbi);
> > >> */
> > >> int __init f2fs_init_bioset(void);
> > >> void f2fs_destroy_bioset(void);
> > >> +bool f2fs_is_cp_guaranteed(struct page *page);
> > >> int f2fs_init_bio_entry_cache(void);
> > >> void f2fs_destroy_bio_entry_cache(void);
> > >> void f2fs_submit_read_bio(struct f2fs_sb_info *sbi, struct bio *bio,
> > >> --
> > >> 2.40.1
> > >>
> > >>
> > >>
> > >> _______________________________________________
> > >> Linux-f2fs-devel mailing list
> > >> Linux-f2fs-devel@...ts.sourceforge.net
> > >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Powered by blists - more mailing lists