From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <chao@kernel.org>
Cc: Chao Yu <yuchao0@huawei.com>,
linux-kernel@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks
Date: Tue, 19 Jan 2021 13:42:39 -0800 [thread overview]
Message-ID: <YAdSTzYF8Hvxdcqy@google.com> (raw)
In-Reply-To: <20a1dbd3-808e-e62a-53f3-7f1e2a316b3c@kernel.org>
On 01/16, Chao Yu wrote:
> On 2021/1/15 22:59, Jaegeuk Kim wrote:
> > On 01/15, Chao Yu wrote:
> > > On 2021/1/14 12:06, Jaegeuk Kim wrote:
> > > > On 01/14, Chao Yu wrote:
> > > > > On 2021/1/13 23:41, Jaegeuk Kim wrote:
> > > > > > [58690.961685] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of f2fs_quota_write+0x150/0x1f0 [f2fs]
> > > > > > [58691.071481] F2FS-fs (vdb): Inconsistent error blkaddr:31058, sit bitmap:0
> > > > > > [58691.077338] ------------[ cut here ]------------
> > > > > > [58691.081461] WARNING: CPU: 5 PID: 8308 at fs/f2fs/checkpoint.c:151 f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> > > > > > [58691.086734] Modules linked in: f2fs(O) quota_v2 quota_tree dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua ppdev intel_rapl_msr intel_rapl_common sb_edac kvm_intel kvm irqbypass joydev parport_pc parport input_leds serio_raw mac_hid qemu_fw_cfg sch_fq_codel ip_tables x_tables autofs4 btrfs blake2b_generic raid10 raid456 async_raid6_recov async_memcpy asy
> > > > > > [58691.120632] CPU: 5 PID: 8308 Comm: kworker/u17:5 Tainted: G D O 5.11.0-rc3-custom #1
> > > > > > [58691.125438] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> > > > > > [58691.129625] Workqueue: f2fs_post_read_wq f2fs_post_read_work [f2fs]
> > > > > > [58691.133142] RIP: 0010:f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> > > > > > [58691.136221] Code: 3c 07 b8 01 00 00 00 d3 e0 21 f8 75 57 83 fa 07 75 52 89 f2 31 c9 48 c7 c6 20 6a a7 c0 48 89 df e8 bc d6 03 00 f0 80 4b 48 04 <0f> 0b 31 c0 e9 5e fe ff ff 48 8b 57 10 8b 42 30 d3 e0 03 42 48 39
> > > > > > [58691.143142] RSP: 0018:ffffb429047afd40 EFLAGS: 00010206
> > > > > > [58691.145639] RAX: 0000000000000000 RBX: ffff9c3b84041000 RCX: 0000000000000000
> > > > > > [58691.148899] RDX: 0000000000000000 RSI: ffff9c3bbbd58940 RDI: ffff9c3bbbd58940
> > > > > > [58691.152130] RBP: ffffb429047afd48 R08: ffff9c3bbbd58940 R09: ffffb429047afaa8
> > > > > > [58691.155266] R10: 00000000001ba090 R11: 0000000000000003 R12: 0000000000007952
> > > > > > [58691.158304] R13: fffff5cc81266ac0 R14: 00000000000000db R15: 0000000000000000
> > > > > > [58691.161160] FS: 0000000000000000(0000) GS:ffff9c3bbbd40000(0000) knlGS:0000000000000000
> > > > > > [58691.164286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > > > [58691.166869] CR2: 00007f0fee9d3000 CR3: 000000005ee76001 CR4: 0000000000370ee0
> > > > > > [58691.169714] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > > > [58691.173102] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > > > [58691.176163] Call Trace:
> > > > > > [58691.177948] f2fs_cache_compressed_page+0x69/0x280 [f2fs]
> > > > > > [58691.180549] ? newidle_balance+0x253/0x3d0
> > > > > > [58691.183238] f2fs_end_read_compressed_page+0x5a/0x70 [f2fs]
> > > > > > [58691.188205] f2fs_post_read_work+0x11d/0x120 [f2fs]
> > > > > > [58691.192489] process_one_work+0x221/0x3a0
> > > > > > [58691.194482] worker_thread+0x4d/0x3f0
> > > > > > [58691.198867] kthread+0x114/0x150
> > > > > > [58691.202243] ? process_one_work+0x3a0/0x3a0
> > > > > > [58691.205367] ? kthread_park+0x90/0x90
> > > > > > [58691.208244] ret_from_fork+0x22/0x30
> > > > >
> > > > > Below patch fixes two issues, I expect this can fix above warning at least.
> > > >
> > > > [106115.591837] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#1] SMP PTI
> > > > [106115.595584] CPU: 3 PID: 10109 Comm: fsstress Tainted: G O 5.11.0-rc3-custom #1
> > > > [106115.601087] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> > > > [106115.601087] RIP: 0010:f2fs_read_multi_pages+0x415/0xa70 [f2fs]
> > >
> > > Jaegeuk,
> > >
> > > Could you please help to run:
> > >
> > > gdb f2fs.ko
> > > (gdb) l *(f2fs_read_multi_pages+0x415)
> > >
> > > to see where we hit the panic.
> >
> > It's fs/f2fs/data.c:2203
> >
> > 2199 goto out_put_dnode;
> > 2200 }
> > 2201
> > 2202 for (i = 0; i < dic->nr_cpages; i++) {
>
> I doubt when i == cc->nr_cpages, dic was released in below condition,
> then dic->nr_cpages can be any value (may be larger than cc->nr_cpages),
> then we may continue the loop, and will access invalid pointer dic.
>
> if (f2fs_load_compressed_page(sbi, page, blkaddr)) {
> if (atomic_dec_and_test(&dic->remaining_pages))
> f2fs_decompress_cluster(dic);
> continue;
> }
>
>
> I'd like to add a condition here (in between line 2202 and line 2203) to
> make sure whether this can happen, could you please help to verify this?
>
> f2fs_bug_on(sbi, i >= cc->nr_cpages);
No, it seems this is not the case.
>
> Thanks,
>
> > 2203 struct page *page = dic->cpages[i];
> > 2204 block_t blkaddr;
> > 2205 struct bio_post_read_ctx *ctx;
> > 2206
> > 2207 blkaddr = data_blkaddr(dn.inode, dn.node_page,
> > 2208 dn.ofs_in_node + i + 1);
> >
> >
> > >
> > > Thanks,
> > >
> > > > [106115.601087] Code: ff ff ff 45 31 ff f7 d0 25 00 00 08 00 89 45 80 48 8b 45 a0 48 83 c0 6c 48 89 85 78 ff ff ff 48 8b 7d a0 49 63 c7 48 8b 57 30 <48> 8b 1c c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 c6 55 92 dc 48
> > > > [106115.601087] RSP: 0018:ffffc0a4822f7710 EFLAGS: 00010206
> > > > [106115.620978] RAX: 0000000000000001 RBX: ffffe801820034c0 RCX: 0000000000200000
> > > > [106115.620978] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc09487af RDI: ffff9bc1d87c4200
> > > > [106115.627351] RBP: ffffc0a4822f77c0 R08: 0000000000000000 R09: 0000000000000000
> > > > [106115.627351] R10: ffff9bc1d87c4200 R11: 0000000000000001 R12: 0000000000105343
> > > > [106115.627351] R13: ffff9bc2d2184000 R14: 0000000000000000 R15: 0000000000000001
> > > > [106115.635587] FS: 00007f188e909b80(0000) GS:ffff9bc2fbcc0000(0000) knlGS:0000000000000000
> > > > [106115.635587] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > [106115.635587] CR2: 000056446d88b358 CR3: 00000000534b4002 CR4: 0000000000370ee0
> > > > [106115.635587] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > [106115.635587] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > [106115.635587] Call Trace:
> > > > [106115.635587] f2fs_mpage_readpages+0x4e4/0xac0 [f2fs]
> > > > [106115.635587] f2fs_readahead+0x47/0x90 [f2fs]
> > > > [106115.635587] read_pages+0x8e/0x280
> > > > [106115.635587] page_cache_ra_unbounded+0x11f/0x1f0
> > > > [106115.665909] do_page_cache_ra+0x3d/0x40
> > > > [106115.670756] ondemand_readahead+0x2c1/0x2e0
> > > > [106115.671682] page_cache_sync_ra+0xd4/0xe0
> > > > [106115.675622] generic_file_buffered_read_get_pages+0x126/0x8d0
> > > > [106115.679158] generic_file_buffered_read+0x113/0x4a0
> > > > [106115.679158] ? __filemap_fdatawrite_range+0xd8/0x110
> > > > [106115.685672] ? __mark_inode_dirty+0x98/0x330
> > > > [106115.691168] ? f2fs_direct_IO+0x80/0x6f0 [f2fs]
> > > > [106115.691168] generic_file_read_iter+0xdf/0x140
> > > > [106115.691168] f2fs_file_read_iter+0x34/0xb0 [f2fs]
> > > > [106115.699450] aio_read+0xef/0x1b0
> > > > [106115.699450] ? do_user_addr_fault+0x1b8/0x450
> > > > [106115.699450] io_submit_one+0x217/0xbc0
> > > > [106115.699450] ? io_submit_one+0x217/0xbc0
> > > > [106115.699450] __x64_sys_io_submit+0x8d/0x180
> > > > [106115.699450] ? __x64_sys_io_submit+0x8d/0x180
> > > > [106115.712018] ? exit_to_user_mode_prepare+0x3d/0x1a0
> > > > [106115.717468] do_syscall_64+0x38/0x90
> > > > [106115.723157] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > > >
> > > > >
> > > > > - detect truncation during f2fs_cache_compressed_page()
> > > > > - don't set PageUptodate for temporary page in f2fs_load_compressed_page()
> > > > >
> > > > > From: Chao Yu <yuchao0@huawei.com>
> > > > >
> > > > > Signed-off-by: Chao Yu <yuchao0@huawei.com>
> > > > > ---
> > > > > fs/f2fs/compress.c | 20 +++++++++++++-------
> > > > > fs/f2fs/data.c | 3 +--
> > > > > fs/f2fs/f2fs.h | 6 +++---
> > > > > 3 files changed, 17 insertions(+), 12 deletions(-)
> > > > >
> > > > > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > > > > index 0fec71e40001..f364c10c506c 100644
> > > > > --- a/fs/f2fs/compress.c
> > > > > +++ b/fs/f2fs/compress.c
> > > > > @@ -1741,7 +1741,7 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > if (!test_opt(sbi, COMPRESS_CACHE))
> > > > > return;
> > > > >
> > > > > - if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE))
> > > > > + if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
> > > > > return;
> > > > >
> > > > > si_meminfo(&si);
> > > > > @@ -1774,21 +1774,25 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > return;
> > > > > }
> > > > >
> > > > > - memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
> > > > > - SetPageUptodate(cpage);
> > > > > -
> > > > > f2fs_set_page_private(cpage, ino);
> > > > >
> > > > > + if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
> > > > > + goto out;
> > > > > +
> > > > > + memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
> > > > > + SetPageUptodate(cpage);
> > > > > +out:
> > > > > f2fs_put_page(cpage, 1);
> > > > > }
> > > > >
> > > > > -void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > +bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > block_t blkaddr)
> > > > > {
> > > > > struct page *cpage;
> > > > > + bool hitted = false;
> > > > >
> > > > > if (!test_opt(sbi, COMPRESS_CACHE))
> > > > > - return;
> > > > > + return false;
> > > > >
> > > > > cpage = f2fs_pagecache_get_page(COMPRESS_MAPPING(sbi),
> > > > > blkaddr, FGP_LOCK | FGP_NOWAIT, GFP_NOFS);
> > > > > @@ -1797,10 +1801,12 @@ void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > atomic_inc(&sbi->compress_page_hit);
> > > > > memcpy(page_address(page),
> > > > > page_address(cpage), PAGE_SIZE);
> > > > > - SetPageUptodate(page);
> > > > > + hitted = true;
> > > > > }
> > > > > f2fs_put_page(cpage, 1);
> > > > > }
> > > > > +
> > > > > + return hitted;
> > > > > }
> > > > >
> > > > > void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino)
> > > > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > > > > index b3973494b102..3705c272b76a 100644
> > > > > --- a/fs/f2fs/data.c
> > > > > +++ b/fs/f2fs/data.c
> > > > > @@ -2211,8 +2211,7 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
> > > > > blkaddr = data_blkaddr(dn.inode, dn.node_page,
> > > > > dn.ofs_in_node + i + 1);
> > > > >
> > > > > - f2fs_load_compressed_page(sbi, page, blkaddr);
> > > > > - if (PageUptodate(page)) {
> > > > > + if (f2fs_load_compressed_page(sbi, page, blkaddr)) {
> > > > > if (atomic_dec_and_test(&dic->remaining_pages))
> > > > > f2fs_decompress_cluster(dic);
> > > > > continue;
> > > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > > > index 9f79a6825f06..b807970d67b1 100644
> > > > > --- a/fs/f2fs/f2fs.h
> > > > > +++ b/fs/f2fs/f2fs.h
> > > > > @@ -3951,7 +3951,7 @@ 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);
> > > > > -void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > +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);
> > > > > #else
> > > > > @@ -3990,8 +3990,8 @@ 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_load_compressed_page(struct f2fs_sb_info *sbi,
> > > > > - struct page *page, 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,
> > > > > nid_t ino) { }
> > > > > #endif
> > > > > --
> > > > > 2.29.2
> > > > .
> > > >
> >
> >
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> >
WARNING: multiple messages have this Message-ID (diff)
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <chao@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks
Date: Tue, 19 Jan 2021 13:42:39 -0800 [thread overview]
Message-ID: <YAdSTzYF8Hvxdcqy@google.com> (raw)
In-Reply-To: <20a1dbd3-808e-e62a-53f3-7f1e2a316b3c@kernel.org>
On 01/16, Chao Yu wrote:
> On 2021/1/15 22:59, Jaegeuk Kim wrote:
> > On 01/15, Chao Yu wrote:
> > > On 2021/1/14 12:06, Jaegeuk Kim wrote:
> > > > On 01/14, Chao Yu wrote:
> > > > > On 2021/1/13 23:41, Jaegeuk Kim wrote:
> > > > > > [58690.961685] F2FS-fs (vdb) : inject page get in f2fs_pagecache_get_page of f2fs_quota_write+0x150/0x1f0 [f2fs]
> > > > > > [58691.071481] F2FS-fs (vdb): Inconsistent error blkaddr:31058, sit bitmap:0
> > > > > > [58691.077338] ------------[ cut here ]------------
> > > > > > [58691.081461] WARNING: CPU: 5 PID: 8308 at fs/f2fs/checkpoint.c:151 f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> > > > > > [58691.086734] Modules linked in: f2fs(O) quota_v2 quota_tree dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua ppdev intel_rapl_msr intel_rapl_common sb_edac kvm_intel kvm irqbypass joydev parport_pc parport input_leds serio_raw mac_hid qemu_fw_cfg sch_fq_codel ip_tables x_tables autofs4 btrfs blake2b_generic raid10 raid456 async_raid6_recov async_memcpy asy
> > > > > > [58691.120632] CPU: 5 PID: 8308 Comm: kworker/u17:5 Tainted: G D O 5.11.0-rc3-custom #1
> > > > > > [58691.125438] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> > > > > > [58691.129625] Workqueue: f2fs_post_read_wq f2fs_post_read_work [f2fs]
> > > > > > [58691.133142] RIP: 0010:f2fs_is_valid_blkaddr+0x1e9/0x280 [f2fs]
> > > > > > [58691.136221] Code: 3c 07 b8 01 00 00 00 d3 e0 21 f8 75 57 83 fa 07 75 52 89 f2 31 c9 48 c7 c6 20 6a a7 c0 48 89 df e8 bc d6 03 00 f0 80 4b 48 04 <0f> 0b 31 c0 e9 5e fe ff ff 48 8b 57 10 8b 42 30 d3 e0 03 42 48 39
> > > > > > [58691.143142] RSP: 0018:ffffb429047afd40 EFLAGS: 00010206
> > > > > > [58691.145639] RAX: 0000000000000000 RBX: ffff9c3b84041000 RCX: 0000000000000000
> > > > > > [58691.148899] RDX: 0000000000000000 RSI: ffff9c3bbbd58940 RDI: ffff9c3bbbd58940
> > > > > > [58691.152130] RBP: ffffb429047afd48 R08: ffff9c3bbbd58940 R09: ffffb429047afaa8
> > > > > > [58691.155266] R10: 00000000001ba090 R11: 0000000000000003 R12: 0000000000007952
> > > > > > [58691.158304] R13: fffff5cc81266ac0 R14: 00000000000000db R15: 0000000000000000
> > > > > > [58691.161160] FS: 0000000000000000(0000) GS:ffff9c3bbbd40000(0000) knlGS:0000000000000000
> > > > > > [58691.164286] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > > > [58691.166869] CR2: 00007f0fee9d3000 CR3: 000000005ee76001 CR4: 0000000000370ee0
> > > > > > [58691.169714] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > > > [58691.173102] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > > > [58691.176163] Call Trace:
> > > > > > [58691.177948] f2fs_cache_compressed_page+0x69/0x280 [f2fs]
> > > > > > [58691.180549] ? newidle_balance+0x253/0x3d0
> > > > > > [58691.183238] f2fs_end_read_compressed_page+0x5a/0x70 [f2fs]
> > > > > > [58691.188205] f2fs_post_read_work+0x11d/0x120 [f2fs]
> > > > > > [58691.192489] process_one_work+0x221/0x3a0
> > > > > > [58691.194482] worker_thread+0x4d/0x3f0
> > > > > > [58691.198867] kthread+0x114/0x150
> > > > > > [58691.202243] ? process_one_work+0x3a0/0x3a0
> > > > > > [58691.205367] ? kthread_park+0x90/0x90
> > > > > > [58691.208244] ret_from_fork+0x22/0x30
> > > > >
> > > > > Below patch fixes two issues, I expect this can fix above warning at least.
> > > >
> > > > [106115.591837] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b73: 0000 [#1] SMP PTI
> > > > [106115.595584] CPU: 3 PID: 10109 Comm: fsstress Tainted: G O 5.11.0-rc3-custom #1
> > > > [106115.601087] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1 04/01/2014
> > > > [106115.601087] RIP: 0010:f2fs_read_multi_pages+0x415/0xa70 [f2fs]
> > >
> > > Jaegeuk,
> > >
> > > Could you please help to run:
> > >
> > > gdb f2fs.ko
> > > (gdb) l *(f2fs_read_multi_pages+0x415)
> > >
> > > to see where we hit the panic.
> >
> > It's fs/f2fs/data.c:2203
> >
> > 2199 goto out_put_dnode;
> > 2200 }
> > 2201
> > 2202 for (i = 0; i < dic->nr_cpages; i++) {
>
> I doubt when i == cc->nr_cpages, dic was released in below condition,
> then dic->nr_cpages can be any value (may be larger than cc->nr_cpages),
> then we may continue the loop, and will access invalid pointer dic.
>
> if (f2fs_load_compressed_page(sbi, page, blkaddr)) {
> if (atomic_dec_and_test(&dic->remaining_pages))
> f2fs_decompress_cluster(dic);
> continue;
> }
>
>
> I'd like to add a condition here (in between line 2202 and line 2203) to
> make sure whether this can happen, could you please help to verify this?
>
> f2fs_bug_on(sbi, i >= cc->nr_cpages);
No, it seems this is not the case.
>
> Thanks,
>
> > 2203 struct page *page = dic->cpages[i];
> > 2204 block_t blkaddr;
> > 2205 struct bio_post_read_ctx *ctx;
> > 2206
> > 2207 blkaddr = data_blkaddr(dn.inode, dn.node_page,
> > 2208 dn.ofs_in_node + i + 1);
> >
> >
> > >
> > > Thanks,
> > >
> > > > [106115.601087] Code: ff ff ff 45 31 ff f7 d0 25 00 00 08 00 89 45 80 48 8b 45 a0 48 83 c0 6c 48 89 85 78 ff ff ff 48 8b 7d a0 49 63 c7 48 8b 57 30 <48> 8b 1c c2 8b 45 c4 8d 50 01 48 8b 45 b8 48 2b 05 c6 55 92 dc 48
> > > > [106115.601087] RSP: 0018:ffffc0a4822f7710 EFLAGS: 00010206
> > > > [106115.620978] RAX: 0000000000000001 RBX: ffffe801820034c0 RCX: 0000000000200000
> > > > [106115.620978] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc09487af RDI: ffff9bc1d87c4200
> > > > [106115.627351] RBP: ffffc0a4822f77c0 R08: 0000000000000000 R09: 0000000000000000
> > > > [106115.627351] R10: ffff9bc1d87c4200 R11: 0000000000000001 R12: 0000000000105343
> > > > [106115.627351] R13: ffff9bc2d2184000 R14: 0000000000000000 R15: 0000000000000001
> > > > [106115.635587] FS: 00007f188e909b80(0000) GS:ffff9bc2fbcc0000(0000) knlGS:0000000000000000
> > > > [106115.635587] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > [106115.635587] CR2: 000056446d88b358 CR3: 00000000534b4002 CR4: 0000000000370ee0
> > > > [106115.635587] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > [106115.635587] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > [106115.635587] Call Trace:
> > > > [106115.635587] f2fs_mpage_readpages+0x4e4/0xac0 [f2fs]
> > > > [106115.635587] f2fs_readahead+0x47/0x90 [f2fs]
> > > > [106115.635587] read_pages+0x8e/0x280
> > > > [106115.635587] page_cache_ra_unbounded+0x11f/0x1f0
> > > > [106115.665909] do_page_cache_ra+0x3d/0x40
> > > > [106115.670756] ondemand_readahead+0x2c1/0x2e0
> > > > [106115.671682] page_cache_sync_ra+0xd4/0xe0
> > > > [106115.675622] generic_file_buffered_read_get_pages+0x126/0x8d0
> > > > [106115.679158] generic_file_buffered_read+0x113/0x4a0
> > > > [106115.679158] ? __filemap_fdatawrite_range+0xd8/0x110
> > > > [106115.685672] ? __mark_inode_dirty+0x98/0x330
> > > > [106115.691168] ? f2fs_direct_IO+0x80/0x6f0 [f2fs]
> > > > [106115.691168] generic_file_read_iter+0xdf/0x140
> > > > [106115.691168] f2fs_file_read_iter+0x34/0xb0 [f2fs]
> > > > [106115.699450] aio_read+0xef/0x1b0
> > > > [106115.699450] ? do_user_addr_fault+0x1b8/0x450
> > > > [106115.699450] io_submit_one+0x217/0xbc0
> > > > [106115.699450] ? io_submit_one+0x217/0xbc0
> > > > [106115.699450] __x64_sys_io_submit+0x8d/0x180
> > > > [106115.699450] ? __x64_sys_io_submit+0x8d/0x180
> > > > [106115.712018] ? exit_to_user_mode_prepare+0x3d/0x1a0
> > > > [106115.717468] do_syscall_64+0x38/0x90
> > > > [106115.723157] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > > >
> > > > >
> > > > > - detect truncation during f2fs_cache_compressed_page()
> > > > > - don't set PageUptodate for temporary page in f2fs_load_compressed_page()
> > > > >
> > > > > From: Chao Yu <yuchao0@huawei.com>
> > > > >
> > > > > Signed-off-by: Chao Yu <yuchao0@huawei.com>
> > > > > ---
> > > > > fs/f2fs/compress.c | 20 +++++++++++++-------
> > > > > fs/f2fs/data.c | 3 +--
> > > > > fs/f2fs/f2fs.h | 6 +++---
> > > > > 3 files changed, 17 insertions(+), 12 deletions(-)
> > > > >
> > > > > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > > > > index 0fec71e40001..f364c10c506c 100644
> > > > > --- a/fs/f2fs/compress.c
> > > > > +++ b/fs/f2fs/compress.c
> > > > > @@ -1741,7 +1741,7 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > if (!test_opt(sbi, COMPRESS_CACHE))
> > > > > return;
> > > > >
> > > > > - if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE))
> > > > > + if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
> > > > > return;
> > > > >
> > > > > si_meminfo(&si);
> > > > > @@ -1774,21 +1774,25 @@ void f2fs_cache_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > return;
> > > > > }
> > > > >
> > > > > - memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
> > > > > - SetPageUptodate(cpage);
> > > > > -
> > > > > f2fs_set_page_private(cpage, ino);
> > > > >
> > > > > + if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE_READ))
> > > > > + goto out;
> > > > > +
> > > > > + memcpy(page_address(cpage), page_address(page), PAGE_SIZE);
> > > > > + SetPageUptodate(cpage);
> > > > > +out:
> > > > > f2fs_put_page(cpage, 1);
> > > > > }
> > > > >
> > > > > -void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > +bool f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > block_t blkaddr)
> > > > > {
> > > > > struct page *cpage;
> > > > > + bool hitted = false;
> > > > >
> > > > > if (!test_opt(sbi, COMPRESS_CACHE))
> > > > > - return;
> > > > > + return false;
> > > > >
> > > > > cpage = f2fs_pagecache_get_page(COMPRESS_MAPPING(sbi),
> > > > > blkaddr, FGP_LOCK | FGP_NOWAIT, GFP_NOFS);
> > > > > @@ -1797,10 +1801,12 @@ void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > atomic_inc(&sbi->compress_page_hit);
> > > > > memcpy(page_address(page),
> > > > > page_address(cpage), PAGE_SIZE);
> > > > > - SetPageUptodate(page);
> > > > > + hitted = true;
> > > > > }
> > > > > f2fs_put_page(cpage, 1);
> > > > > }
> > > > > +
> > > > > + return hitted;
> > > > > }
> > > > >
> > > > > void f2fs_invalidate_compress_pages(struct f2fs_sb_info *sbi, nid_t ino)
> > > > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > > > > index b3973494b102..3705c272b76a 100644
> > > > > --- a/fs/f2fs/data.c
> > > > > +++ b/fs/f2fs/data.c
> > > > > @@ -2211,8 +2211,7 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
> > > > > blkaddr = data_blkaddr(dn.inode, dn.node_page,
> > > > > dn.ofs_in_node + i + 1);
> > > > >
> > > > > - f2fs_load_compressed_page(sbi, page, blkaddr);
> > > > > - if (PageUptodate(page)) {
> > > > > + if (f2fs_load_compressed_page(sbi, page, blkaddr)) {
> > > > > if (atomic_dec_and_test(&dic->remaining_pages))
> > > > > f2fs_decompress_cluster(dic);
> > > > > continue;
> > > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > > > > index 9f79a6825f06..b807970d67b1 100644
> > > > > --- a/fs/f2fs/f2fs.h
> > > > > +++ b/fs/f2fs/f2fs.h
> > > > > @@ -3951,7 +3951,7 @@ 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);
> > > > > -void f2fs_load_compressed_page(struct f2fs_sb_info *sbi, struct page *page,
> > > > > +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);
> > > > > #else
> > > > > @@ -3990,8 +3990,8 @@ 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_load_compressed_page(struct f2fs_sb_info *sbi,
> > > > > - struct page *page, 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,
> > > > > nid_t ino) { }
> > > > > #endif
> > > > > --
> > > > > 2.29.2
> > > > .
> > > >
> >
> >
> > _______________________________________________
> > 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
next prev parent reply other threads:[~2021-01-19 21:47 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-07 9:31 [PATCH v3 1/5] f2fs: compress: add compress_inode to cache compressed blocks Chao Yu
2021-01-07 9:31 ` [f2fs-dev] " Chao Yu
2021-01-11 9:48 ` Jaegeuk Kim
2021-01-11 9:48 ` [f2fs-dev] " Jaegeuk Kim
2021-01-11 10:31 ` Chao Yu
2021-01-11 10:31 ` [f2fs-dev] " Chao Yu
2021-01-11 11:45 ` Chao Yu
2021-01-11 11:45 ` Chao Yu
2021-01-12 1:33 ` Chao Yu
2021-01-12 1:33 ` Chao Yu
2021-01-12 2:04 ` Jaegeuk Kim
2021-01-12 2:04 ` Jaegeuk Kim
2021-01-12 9:02 ` Chao Yu
2021-01-12 9:02 ` Chao Yu
2021-01-12 22:36 ` Jaegeuk Kim
2021-01-12 22:36 ` Jaegeuk Kim
2021-01-13 1:09 ` Chao Yu
2021-01-13 1:09 ` Chao Yu
2021-01-13 15:41 ` Jaegeuk Kim
2021-01-13 15:41 ` Jaegeuk Kim
2021-01-14 2:45 ` Chao Yu
2021-01-14 2:45 ` Chao Yu
2021-01-14 4:06 ` Jaegeuk Kim
2021-01-14 4:06 ` Jaegeuk Kim
2021-01-15 9:32 ` Chao Yu
2021-01-15 9:32 ` Chao Yu
2021-01-15 14:59 ` Jaegeuk Kim
2021-01-15 14:59 ` Jaegeuk Kim
2021-01-16 12:07 ` Chao Yu
2021-01-16 12:07 ` Chao Yu
2021-01-19 21:42 ` Jaegeuk Kim [this message]
2021-01-19 21:42 ` Jaegeuk Kim
2021-01-22 2:17 ` Chao Yu
2021-01-22 2:17 ` Chao Yu
2021-01-28 2:50 ` Chao Yu
2021-01-28 2:50 ` Chao Yu
2021-01-28 16:12 ` Jaegeuk Kim
2021-01-28 16:12 ` Jaegeuk Kim
2021-01-29 1:13 ` Chao Yu
2021-01-29 1:13 ` Chao Yu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YAdSTzYF8Hvxdcqy@google.com \
--to=jaegeuk@kernel.org \
--cc=chao@kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=yuchao0@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.