Re: [f2fs-dev] [PATCH v3] f2fs: fix race of pending_pages in decompression

2020-12-08 Thread Eric Biggers
On Wed, Dec 09, 2020 at 09:34:06AM +0800, Chao Yu wrote: > On 2020/12/9 7:55, Jaegeuk Kim wrote: > > On 12/07, Eric Biggers wrote: > > > On Tue, Dec 08, 2020 at 08:51:45AM +0900, Daeho Jeong wrote: > > > > > I am trying to review this but it is very hard, as the f2fs > > > > > compression code is

Re: [f2fs-dev] [PATCH v3] f2fs: fix race of pending_pages in decompression

2020-12-08 Thread Chao Yu
On 2020/12/9 7:55, Jaegeuk Kim wrote: On 12/07, Eric Biggers wrote: On Tue, Dec 08, 2020 at 08:51:45AM +0900, Daeho Jeong wrote: I am trying to review this but it is very hard, as the f2fs compression code is very hard to understand. It looks like a 'struct decompress_io_ctx' represents the

Re: [f2fs-dev] [PATCH v3] f2fs: fix race of pending_pages in decompression

2020-12-08 Thread Jaegeuk Kim
On 12/07, Eric Biggers wrote: > On Tue, Dec 08, 2020 at 08:51:45AM +0900, Daeho Jeong wrote: > > > I am trying to review this but it is very hard, as the f2fs compression > > > code is > > > very hard to understand. > > > > > > It looks like a 'struct decompress_io_ctx' represents the work to >

Re: [f2fs-dev] [PATCH v3] f2fs: fix race of pending_pages in decompression

2020-12-07 Thread Eric Biggers
On Tue, Dec 08, 2020 at 08:51:45AM +0900, Daeho Jeong wrote: > > I am trying to review this but it is very hard, as the f2fs compression > > code is > > very hard to understand. > > > > It looks like a 'struct decompress_io_ctx' represents the work to > > decompress a > > particular cluster.

Re: [f2fs-dev] [PATCH v3] f2fs: fix race of pending_pages in decompression

2020-12-07 Thread Daeho Jeong
Chao, Jaegeuk, Thanks. I'll update it as your comments. :) Eric, Decompression and verity can be executed in different thread contexts in different timing, so we need separate counts for each. We already use STEP_VERITY for non-compression case, so I think using this flag in here looks more

Re: [f2fs-dev] [PATCH v3] f2fs: fix race of pending_pages in decompression

2020-12-07 Thread Eric Biggers
On Sat, Dec 05, 2020 at 01:26:26PM +0900, Daeho Jeong wrote: > From: Daeho Jeong > > I found out f2fs_free_dic() is invoked in a wrong timing, but > f2fs_verify_bio() still needed the dic info and it triggered the > below kernel panic. It has been caused by the race condition of > pending_pages

Re: [f2fs-dev] [PATCH v3] f2fs: fix race of pending_pages in decompression

2020-12-07 Thread Jaegeuk Kim
On 12/07, Chao Yu wrote: > On 2020/12/7 15:28, Daeho Jeong wrote: > > > It looks like it will be better to move this into merge condition? > > > > > > if (bio && (!page_is_mergeable(sbi, bio, > > > *last_block_in_bio, blkaddr) || > > >

Re: [f2fs-dev] [PATCH v3] f2fs: fix race of pending_pages in decompression

2020-12-06 Thread Chao Yu
On 2020/12/7 15:28, Daeho Jeong wrote: It looks like it will be better to move this into merge condition? if (bio && (!page_is_mergeable(sbi, bio, *last_block_in_bio, blkaddr) || !f2fs_crypt_mergeable_bio(bio, inode,

Re: [f2fs-dev] [PATCH v3] f2fs: fix race of pending_pages in decompression

2020-12-06 Thread Daeho Jeong
> It looks like it will be better to move this into merge condition? > > if (bio && (!page_is_mergeable(sbi, bio, > *last_block_in_bio, blkaddr) || > !f2fs_crypt_mergeable_bio(bio, inode, page->index, NULL) > || >

Re: [f2fs-dev] [PATCH v3] f2fs: fix race of pending_pages in decompression

2020-12-06 Thread Chao Yu
On 2020/12/5 12:26, Daeho Jeong wrote: From: Daeho Jeong I found out f2fs_free_dic() is invoked in a wrong timing, but f2fs_verify_bio() still needed the dic info and it triggered the below kernel panic. It has been caused by the race condition of pending_pages value between decompression and