* [PATCH 1/2] f2fs: fix to return correct trimmed block number in FITRIM interface @ 2016-06-30 8:42 Chao Yu 2016-06-30 8:42 ` [PATCH 2/2] f2fs: fix to avoid data update racing between GC and DIO Chao Yu 2016-07-07 4:29 ` [PATCH 1/2] f2fs: fix to return correct trimmed block number in FITRIM interface Chao Yu 0 siblings, 2 replies; 9+ messages in thread From: Chao Yu @ 2016-06-30 8:42 UTC (permalink / raw) To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu During tiggering fstrim, in case of issuing discard for prefree segments, we miss acclumulating trimmed block number which will be return to user. Fix it. Signed-off-by: Chao Yu <yuchao0@huawei.com> --- fs/f2fs/segment.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 6d16ecf..5dc14d6 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -732,15 +732,20 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc) if (!test_opt(sbi, LFS) || sbi->segs_per_sec == 1) { f2fs_issue_discard(sbi, START_BLOCK(sbi, start), (end - start) << sbi->log_blocks_per_seg); + cpc->trimmed += + (end - start) << sbi->log_blocks_per_seg; continue; } next: secno = GET_SECNO(sbi, start); start_segno = secno * sbi->segs_per_sec; if (!IS_CURSEC(sbi, secno) && - !get_valid_blocks(sbi, start, sbi->segs_per_sec)) + !get_valid_blocks(sbi, start, sbi->segs_per_sec)) { f2fs_issue_discard(sbi, START_BLOCK(sbi, start_segno), sbi->segs_per_sec << sbi->log_blocks_per_seg); + cpc->trimmed += + sbi->segs_per_sec << sbi->log_blocks_per_seg; + } start = start_segno + sbi->segs_per_sec; if (start < end) -- 2.8.2.311.gee88674 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] f2fs: fix to avoid data update racing between GC and DIO 2016-06-30 8:42 [PATCH 1/2] f2fs: fix to return correct trimmed block number in FITRIM interface Chao Yu @ 2016-06-30 8:42 ` Chao Yu 2016-07-01 0:03 ` Jaegeuk Kim 2016-07-07 4:29 ` [PATCH 1/2] f2fs: fix to return correct trimmed block number in FITRIM interface Chao Yu 1 sibling, 1 reply; 9+ messages in thread From: Chao Yu @ 2016-06-30 8:42 UTC (permalink / raw) To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu Datas in file can be operated by GC and DIO simultaneously, so we will face race case as below: For write case: Thread A Thread B - generic_file_direct_write - invalidate_inode_pages2_range - f2fs_direct_IO - do_blockdev_direct_IO - do_direct_IO - get_more_blocks - f2fs_gc - do_garbage_collect - gc_data_segment - move_data_page - do_write_data_page migrate data block to new block address - dio_bio_submit update user data to old block address For read case: Thread A Thread B - generic_file_direct_write - invalidate_inode_pages2_range - f2fs_direct_IO - do_blockdev_direct_IO - do_direct_IO - get_more_blocks - f2fs_balance_fs - f2fs_gc - do_garbage_collect - gc_data_segment - move_data_page - do_write_data_page migrate data block to new block address - write_checkpoint - do_checkpoint - clear_prefree_segments - f2fs_issue_discard discard old block adress - dio_bio_submit update user buffer from obsolete block address In order to fix this, for one file, we should let DIO and GC getting exclusion against with each other. Signed-off-by: Chao Yu <yuchao0@huawei.com> --- fs/f2fs/data.c | 2 ++ fs/f2fs/f2fs.h | 1 + fs/f2fs/gc.c | 14 +++++++++++++- fs/f2fs/super.c | 1 + 4 files changed, 17 insertions(+), 1 deletion(-) diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index ba4963f..08dc060 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -1716,7 +1716,9 @@ static ssize_t f2fs_direct_IO(struct kiocb *iocb, struct iov_iter *iter) trace_f2fs_direct_IO_enter(inode, offset, count, iov_iter_rw(iter)); + mutex_lock(&F2FS_I(inode)->dio_mutex); err = blockdev_direct_IO(iocb, inode, iter, get_data_block_dio); + mutex_unlock(&F2FS_I(inode)->dio_mutex); if (iov_iter_rw(iter) == WRITE) { if (err > 0) set_inode_flag(inode, FI_UPDATE_WRITE); diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index bd82b6d..a241576 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -474,6 +474,7 @@ struct f2fs_inode_info { struct list_head inmem_pages; /* inmemory pages managed by f2fs */ struct mutex inmem_lock; /* lock for inmemory pages */ struct extent_tree *extent_tree; /* cached extent_tree entry */ + struct mutex dio_mutex; /* avoid racing between dio and gc */ }; static inline void get_extent_info(struct extent_info *ext, diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index c2c4ac3..98e3763 100644 --- a/fs/f2fs/gc.c +++ b/fs/f2fs/gc.c @@ -744,12 +744,24 @@ next_step: /* phase 3 */ inode = find_gc_inode(gc_list, dni.ino); if (inode) { + bool locked = false; + + if (S_ISREG(inode->i_mode)) { + if (!mutex_trylock(&F2FS_I(inode)->dio_mutex)) + continue; + locked = true; + } + start_bidx = start_bidx_of_node(nofs, inode) + ofs_in_node; - if (f2fs_encrypted_inode(inode) && S_ISREG(inode->i_mode)) + if (f2fs_encrypted_inode(inode) && + S_ISREG(inode->i_mode)) move_encrypted_block(inode, start_bidx); else move_data_page(inode, start_bidx, gc_type); + if (locked) + mutex_unlock(&F2FS_I(inode)->dio_mutex); + stat_inc_data_blk_count(sbi, 1, gc_type); } } diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 8c698e1..24aab3f 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -575,6 +575,7 @@ static struct inode *f2fs_alloc_inode(struct super_block *sb) INIT_LIST_HEAD(&fi->gdirty_list); INIT_LIST_HEAD(&fi->inmem_pages); mutex_init(&fi->inmem_lock); + mutex_init(&fi->dio_mutex); /* Will be used by directory only */ fi->i_dir_level = F2FS_SB(sb)->dir_level; -- 2.8.2.311.gee88674 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] f2fs: fix to avoid data update racing between GC and DIO 2016-06-30 8:42 ` [PATCH 2/2] f2fs: fix to avoid data update racing between GC and DIO Chao Yu @ 2016-07-01 0:03 ` Jaegeuk Kim 2016-07-01 6:03 ` Chao Yu 0 siblings, 1 reply; 9+ messages in thread From: Jaegeuk Kim @ 2016-07-01 0:03 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel Hi Chao, On Thu, Jun 30, 2016 at 04:42:48PM +0800, Chao Yu wrote: > Datas in file can be operated by GC and DIO simultaneously, so we will > face race case as below: > > For write case: > Thread A Thread B > - generic_file_direct_write > - invalidate_inode_pages2_range > - f2fs_direct_IO > - do_blockdev_direct_IO > - do_direct_IO > - get_more_blocks > - f2fs_gc > - do_garbage_collect > - gc_data_segment > - move_data_page > - do_write_data_page > migrate data block to new block address > - dio_bio_submit > update user data to old block address > > For read case: > Thread A Thread B > - generic_file_direct_write > - invalidate_inode_pages2_range > - f2fs_direct_IO > - do_blockdev_direct_IO > - do_direct_IO > - get_more_blocks > - f2fs_balance_fs > - f2fs_gc > - do_garbage_collect > - gc_data_segment > - move_data_page > - do_write_data_page > migrate data block to new block address > - write_checkpoint > - do_checkpoint > - clear_prefree_segments > - f2fs_issue_discard > discard old block adress > - dio_bio_submit > update user buffer from obsolete block address > > In order to fix this, for one file, we should let DIO and GC getting exclusion > against with each other. > > Signed-off-by: Chao Yu <yuchao0@huawei.com> > --- > fs/f2fs/data.c | 2 ++ > fs/f2fs/f2fs.h | 1 + > fs/f2fs/gc.c | 14 +++++++++++++- > fs/f2fs/super.c | 1 + > 4 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > index ba4963f..08dc060 100644 > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -1716,7 +1716,9 @@ static ssize_t f2fs_direct_IO(struct kiocb *iocb, struct iov_iter *iter) > > trace_f2fs_direct_IO_enter(inode, offset, count, iov_iter_rw(iter)); > > + mutex_lock(&F2FS_I(inode)->dio_mutex); > err = blockdev_direct_IO(iocb, inode, iter, get_data_block_dio); > + mutex_unlock(&F2FS_I(inode)->dio_mutex); This means we need to sacrifice entire parallism even in the normal cases? Can we find another way? Thanks, > if (iov_iter_rw(iter) == WRITE) { > if (err > 0) > set_inode_flag(inode, FI_UPDATE_WRITE); > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index bd82b6d..a241576 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -474,6 +474,7 @@ struct f2fs_inode_info { > struct list_head inmem_pages; /* inmemory pages managed by f2fs */ > struct mutex inmem_lock; /* lock for inmemory pages */ > struct extent_tree *extent_tree; /* cached extent_tree entry */ > + struct mutex dio_mutex; /* avoid racing between dio and gc */ > }; > > static inline void get_extent_info(struct extent_info *ext, > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > index c2c4ac3..98e3763 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -744,12 +744,24 @@ next_step: > /* phase 3 */ > inode = find_gc_inode(gc_list, dni.ino); > if (inode) { > + bool locked = false; > + > + if (S_ISREG(inode->i_mode)) { > + if (!mutex_trylock(&F2FS_I(inode)->dio_mutex)) > + continue; > + locked = true; > + } > + > start_bidx = start_bidx_of_node(nofs, inode) > + ofs_in_node; > - if (f2fs_encrypted_inode(inode) && S_ISREG(inode->i_mode)) > + if (f2fs_encrypted_inode(inode) && > + S_ISREG(inode->i_mode)) > move_encrypted_block(inode, start_bidx); > else > move_data_page(inode, start_bidx, gc_type); > + if (locked) > + mutex_unlock(&F2FS_I(inode)->dio_mutex); > + > stat_inc_data_blk_count(sbi, 1, gc_type); > } > } > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index 8c698e1..24aab3f 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -575,6 +575,7 @@ static struct inode *f2fs_alloc_inode(struct super_block *sb) > INIT_LIST_HEAD(&fi->gdirty_list); > INIT_LIST_HEAD(&fi->inmem_pages); > mutex_init(&fi->inmem_lock); > + mutex_init(&fi->dio_mutex); > > /* Will be used by directory only */ > fi->i_dir_level = F2FS_SB(sb)->dir_level; > -- > 2.8.2.311.gee88674 ------------------------------------------------------------------------------ Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San Francisco, CA to explore cutting-edge tech and listen to tech luminaries present their vision of the future. This family event has something for everyone, including kids. Get more information and register today. http://sdm.link/attshape ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] f2fs: fix to avoid data update racing between GC and DIO 2016-07-01 0:03 ` Jaegeuk Kim @ 2016-07-01 6:03 ` Chao Yu 2016-07-06 0:24 ` Jaegeuk Kim 0 siblings, 1 reply; 9+ messages in thread From: Chao Yu @ 2016-07-01 6:03 UTC (permalink / raw) To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel Hi Jaegeuk, On 2016/7/1 8:03, Jaegeuk Kim wrote: > Hi Chao, > > On Thu, Jun 30, 2016 at 04:42:48PM +0800, Chao Yu wrote: >> Datas in file can be operated by GC and DIO simultaneously, so we will >> face race case as below: >> >> For write case: >> Thread A Thread B >> - generic_file_direct_write >> - invalidate_inode_pages2_range >> - f2fs_direct_IO >> - do_blockdev_direct_IO >> - do_direct_IO >> - get_more_blocks >> - f2fs_gc >> - do_garbage_collect >> - gc_data_segment >> - move_data_page >> - do_write_data_page >> migrate data block to new block address >> - dio_bio_submit >> update user data to old block address >> >> For read case: >> Thread A Thread B >> - generic_file_direct_write >> - invalidate_inode_pages2_range >> - f2fs_direct_IO >> - do_blockdev_direct_IO >> - do_direct_IO >> - get_more_blocks >> - f2fs_balance_fs >> - f2fs_gc >> - do_garbage_collect >> - gc_data_segment >> - move_data_page >> - do_write_data_page >> migrate data block to new block address >> - write_checkpoint >> - do_checkpoint >> - clear_prefree_segments >> - f2fs_issue_discard >> discard old block adress >> - dio_bio_submit >> update user buffer from obsolete block address >> >> In order to fix this, for one file, we should let DIO and GC getting exclusion >> against with each other. >> >> Signed-off-by: Chao Yu <yuchao0@huawei.com> >> --- >> fs/f2fs/data.c | 2 ++ >> fs/f2fs/f2fs.h | 1 + >> fs/f2fs/gc.c | 14 +++++++++++++- >> fs/f2fs/super.c | 1 + >> 4 files changed, 17 insertions(+), 1 deletion(-) >> >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >> index ba4963f..08dc060 100644 >> --- a/fs/f2fs/data.c >> +++ b/fs/f2fs/data.c >> @@ -1716,7 +1716,9 @@ static ssize_t f2fs_direct_IO(struct kiocb *iocb, struct iov_iter *iter) >> >> trace_f2fs_direct_IO_enter(inode, offset, count, iov_iter_rw(iter)); >> >> + mutex_lock(&F2FS_I(inode)->dio_mutex); >> err = blockdev_direct_IO(iocb, inode, iter, get_data_block_dio); >> + mutex_unlock(&F2FS_I(inode)->dio_mutex); > > This means we need to sacrifice entire parallism even in the normal cases? > Can we find another way? 1. For dio write vs dio write, writer will grab i_mutex before dio_mutex, so anyway, concurrent dio writes will be exclusive. 2. For dio write vs gc, keep using dio_mutex for making them exclusive. 3. For dio read vs dio read, and dio read vs gc, what about adding dio_rwsem to control the parallelism? 4. For dio write vs dio read, we grab different lock (write grabs dio_mutex, read grabs dio_rwsem), so there is no race condition. Thanks, > > Thanks, > >> if (iov_iter_rw(iter) == WRITE) { >> if (err > 0) >> set_inode_flag(inode, FI_UPDATE_WRITE); >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >> index bd82b6d..a241576 100644 >> --- a/fs/f2fs/f2fs.h >> +++ b/fs/f2fs/f2fs.h >> @@ -474,6 +474,7 @@ struct f2fs_inode_info { >> struct list_head inmem_pages; /* inmemory pages managed by f2fs */ >> struct mutex inmem_lock; /* lock for inmemory pages */ >> struct extent_tree *extent_tree; /* cached extent_tree entry */ >> + struct mutex dio_mutex; /* avoid racing between dio and gc */ >> }; >> >> static inline void get_extent_info(struct extent_info *ext, >> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c >> index c2c4ac3..98e3763 100644 >> --- a/fs/f2fs/gc.c >> +++ b/fs/f2fs/gc.c >> @@ -744,12 +744,24 @@ next_step: >> /* phase 3 */ >> inode = find_gc_inode(gc_list, dni.ino); >> if (inode) { >> + bool locked = false; >> + >> + if (S_ISREG(inode->i_mode)) { >> + if (!mutex_trylock(&F2FS_I(inode)->dio_mutex)) >> + continue; >> + locked = true; >> + } >> + >> start_bidx = start_bidx_of_node(nofs, inode) >> + ofs_in_node; >> - if (f2fs_encrypted_inode(inode) && S_ISREG(inode->i_mode)) >> + if (f2fs_encrypted_inode(inode) && >> + S_ISREG(inode->i_mode)) >> move_encrypted_block(inode, start_bidx); >> else >> move_data_page(inode, start_bidx, gc_type); >> + if (locked) >> + mutex_unlock(&F2FS_I(inode)->dio_mutex); >> + >> stat_inc_data_blk_count(sbi, 1, gc_type); >> } >> } >> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >> index 8c698e1..24aab3f 100644 >> --- a/fs/f2fs/super.c >> +++ b/fs/f2fs/super.c >> @@ -575,6 +575,7 @@ static struct inode *f2fs_alloc_inode(struct super_block *sb) >> INIT_LIST_HEAD(&fi->gdirty_list); >> INIT_LIST_HEAD(&fi->inmem_pages); >> mutex_init(&fi->inmem_lock); >> + mutex_init(&fi->dio_mutex); >> >> /* Will be used by directory only */ >> fi->i_dir_level = F2FS_SB(sb)->dir_level; >> -- >> 2.8.2.311.gee88674 > > . > ------------------------------------------------------------------------------ Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San Francisco, CA to explore cutting-edge tech and listen to tech luminaries present their vision of the future. This family event has something for everyone, including kids. Get more information and register today. http://sdm.link/attshape ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] f2fs: fix to avoid data update racing between GC and DIO 2016-07-01 6:03 ` Chao Yu @ 2016-07-06 0:24 ` Jaegeuk Kim 2016-07-06 2:10 ` Chao Yu 0 siblings, 1 reply; 9+ messages in thread From: Jaegeuk Kim @ 2016-07-06 0:24 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel On Fri, Jul 01, 2016 at 02:03:17PM +0800, Chao Yu wrote: > Hi Jaegeuk, > > On 2016/7/1 8:03, Jaegeuk Kim wrote: > > Hi Chao, > > > > On Thu, Jun 30, 2016 at 04:42:48PM +0800, Chao Yu wrote: > >> Datas in file can be operated by GC and DIO simultaneously, so we will > >> face race case as below: > >> > >> For write case: > >> Thread A Thread B > >> - generic_file_direct_write > >> - invalidate_inode_pages2_range > >> - f2fs_direct_IO > >> - do_blockdev_direct_IO > >> - do_direct_IO > >> - get_more_blocks > >> - f2fs_gc > >> - do_garbage_collect > >> - gc_data_segment > >> - move_data_page > >> - do_write_data_page > >> migrate data block to new block address > >> - dio_bio_submit > >> update user data to old block address > >> > >> For read case: > >> Thread A Thread B > >> - generic_file_direct_write > >> - invalidate_inode_pages2_range > >> - f2fs_direct_IO > >> - do_blockdev_direct_IO > >> - do_direct_IO > >> - get_more_blocks > >> - f2fs_balance_fs > >> - f2fs_gc > >> - do_garbage_collect > >> - gc_data_segment > >> - move_data_page > >> - do_write_data_page > >> migrate data block to new block address > >> - write_checkpoint > >> - do_checkpoint > >> - clear_prefree_segments > >> - f2fs_issue_discard > >> discard old block adress > >> - dio_bio_submit > >> update user buffer from obsolete block address > >> > >> In order to fix this, for one file, we should let DIO and GC getting exclusion > >> against with each other. > >> > >> Signed-off-by: Chao Yu <yuchao0@huawei.com> > >> --- > >> fs/f2fs/data.c | 2 ++ > >> fs/f2fs/f2fs.h | 1 + > >> fs/f2fs/gc.c | 14 +++++++++++++- > >> fs/f2fs/super.c | 1 + > >> 4 files changed, 17 insertions(+), 1 deletion(-) > >> > >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > >> index ba4963f..08dc060 100644 > >> --- a/fs/f2fs/data.c > >> +++ b/fs/f2fs/data.c > >> @@ -1716,7 +1716,9 @@ static ssize_t f2fs_direct_IO(struct kiocb *iocb, struct iov_iter *iter) > >> > >> trace_f2fs_direct_IO_enter(inode, offset, count, iov_iter_rw(iter)); > >> > >> + mutex_lock(&F2FS_I(inode)->dio_mutex); > >> err = blockdev_direct_IO(iocb, inode, iter, get_data_block_dio); > >> + mutex_unlock(&F2FS_I(inode)->dio_mutex); > > > > This means we need to sacrifice entire parallism even in the normal cases? > > Can we find another way? > > 1. For dio write vs dio write, writer will grab i_mutex before dio_mutex, so > anyway, concurrent dio writes will be exclusive. > > 2. For dio write vs gc, keep using dio_mutex for making them exclusive. > > 3. For dio read vs dio read, and dio read vs gc, what about adding dio_rwsem to > control the parallelism? > > 4. For dio write vs dio read, we grab different lock (write grabs dio_mutex, > read grabs dio_rwsem), so there is no race condition. How about adding a flag in a dio inode and avoiding GCs for there-in blocks? Thanks, > > Thanks, > > > > > Thanks, > > > >> if (iov_iter_rw(iter) == WRITE) { > >> if (err > 0) > >> set_inode_flag(inode, FI_UPDATE_WRITE); > >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > >> index bd82b6d..a241576 100644 > >> --- a/fs/f2fs/f2fs.h > >> +++ b/fs/f2fs/f2fs.h > >> @@ -474,6 +474,7 @@ struct f2fs_inode_info { > >> struct list_head inmem_pages; /* inmemory pages managed by f2fs */ > >> struct mutex inmem_lock; /* lock for inmemory pages */ > >> struct extent_tree *extent_tree; /* cached extent_tree entry */ > >> + struct mutex dio_mutex; /* avoid racing between dio and gc */ > >> }; > >> > >> static inline void get_extent_info(struct extent_info *ext, > >> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > >> index c2c4ac3..98e3763 100644 > >> --- a/fs/f2fs/gc.c > >> +++ b/fs/f2fs/gc.c > >> @@ -744,12 +744,24 @@ next_step: > >> /* phase 3 */ > >> inode = find_gc_inode(gc_list, dni.ino); > >> if (inode) { > >> + bool locked = false; > >> + > >> + if (S_ISREG(inode->i_mode)) { > >> + if (!mutex_trylock(&F2FS_I(inode)->dio_mutex)) > >> + continue; > >> + locked = true; > >> + } > >> + > >> start_bidx = start_bidx_of_node(nofs, inode) > >> + ofs_in_node; > >> - if (f2fs_encrypted_inode(inode) && S_ISREG(inode->i_mode)) > >> + if (f2fs_encrypted_inode(inode) && > >> + S_ISREG(inode->i_mode)) > >> move_encrypted_block(inode, start_bidx); > >> else > >> move_data_page(inode, start_bidx, gc_type); > >> + if (locked) > >> + mutex_unlock(&F2FS_I(inode)->dio_mutex); > >> + > >> stat_inc_data_blk_count(sbi, 1, gc_type); > >> } > >> } > >> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > >> index 8c698e1..24aab3f 100644 > >> --- a/fs/f2fs/super.c > >> +++ b/fs/f2fs/super.c > >> @@ -575,6 +575,7 @@ static struct inode *f2fs_alloc_inode(struct super_block *sb) > >> INIT_LIST_HEAD(&fi->gdirty_list); > >> INIT_LIST_HEAD(&fi->inmem_pages); > >> mutex_init(&fi->inmem_lock); > >> + mutex_init(&fi->dio_mutex); > >> > >> /* Will be used by directory only */ > >> fi->i_dir_level = F2FS_SB(sb)->dir_level; > >> -- > >> 2.8.2.311.gee88674 > > > > . > > ------------------------------------------------------------------------------ Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San Francisco, CA to explore cutting-edge tech and listen to tech luminaries present their vision of the future. This family event has something for everyone, including kids. Get more information and register today. http://sdm.link/attshape ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] f2fs: fix to avoid data update racing between GC and DIO 2016-07-06 0:24 ` Jaegeuk Kim @ 2016-07-06 2:10 ` Chao Yu 2016-07-06 22:37 ` Jaegeuk Kim 0 siblings, 1 reply; 9+ messages in thread From: Chao Yu @ 2016-07-06 2:10 UTC (permalink / raw) To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel On 2016/7/6 8:24, Jaegeuk Kim wrote: > On Fri, Jul 01, 2016 at 02:03:17PM +0800, Chao Yu wrote: >> Hi Jaegeuk, >> >> On 2016/7/1 8:03, Jaegeuk Kim wrote: >>> Hi Chao, >>> >>> On Thu, Jun 30, 2016 at 04:42:48PM +0800, Chao Yu wrote: >>>> Datas in file can be operated by GC and DIO simultaneously, so we will >>>> face race case as below: >>>> >>>> For write case: >>>> Thread A Thread B >>>> - generic_file_direct_write >>>> - invalidate_inode_pages2_range >>>> - f2fs_direct_IO >>>> - do_blockdev_direct_IO >>>> - do_direct_IO >>>> - get_more_blocks >>>> - f2fs_gc >>>> - do_garbage_collect >>>> - gc_data_segment >>>> - move_data_page >>>> - do_write_data_page >>>> migrate data block to new block address >>>> - dio_bio_submit >>>> update user data to old block address >>>> >>>> For read case: >>>> Thread A Thread B >>>> - generic_file_direct_write >>>> - invalidate_inode_pages2_range >>>> - f2fs_direct_IO >>>> - do_blockdev_direct_IO >>>> - do_direct_IO >>>> - get_more_blocks >>>> - f2fs_balance_fs >>>> - f2fs_gc >>>> - do_garbage_collect >>>> - gc_data_segment >>>> - move_data_page >>>> - do_write_data_page >>>> migrate data block to new block address >>>> - write_checkpoint >>>> - do_checkpoint >>>> - clear_prefree_segments >>>> - f2fs_issue_discard >>>> discard old block adress >>>> - dio_bio_submit >>>> update user buffer from obsolete block address >>>> >>>> In order to fix this, for one file, we should let DIO and GC getting exclusion >>>> against with each other. >>>> >>>> Signed-off-by: Chao Yu <yuchao0@huawei.com> >>>> --- >>>> fs/f2fs/data.c | 2 ++ >>>> fs/f2fs/f2fs.h | 1 + >>>> fs/f2fs/gc.c | 14 +++++++++++++- >>>> fs/f2fs/super.c | 1 + >>>> 4 files changed, 17 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >>>> index ba4963f..08dc060 100644 >>>> --- a/fs/f2fs/data.c >>>> +++ b/fs/f2fs/data.c >>>> @@ -1716,7 +1716,9 @@ static ssize_t f2fs_direct_IO(struct kiocb *iocb, struct iov_iter *iter) >>>> >>>> trace_f2fs_direct_IO_enter(inode, offset, count, iov_iter_rw(iter)); >>>> >>>> + mutex_lock(&F2FS_I(inode)->dio_mutex); >>>> err = blockdev_direct_IO(iocb, inode, iter, get_data_block_dio); >>>> + mutex_unlock(&F2FS_I(inode)->dio_mutex); >>> >>> This means we need to sacrifice entire parallism even in the normal cases? >>> Can we find another way? >> >> 1. For dio write vs dio write, writer will grab i_mutex before dio_mutex, so >> anyway, concurrent dio writes will be exclusive. >> >> 2. For dio write vs gc, keep using dio_mutex for making them exclusive. >> >> 3. For dio read vs dio read, and dio read vs gc, what about adding dio_rwsem to >> control the parallelism? >> >> 4. For dio write vs dio read, we grab different lock (write grabs dio_mutex, >> read grabs dio_rwsem), so there is no race condition. > > How about adding a flag in a dio inode and avoiding GCs for there-in blocks? Hmm.. IMO, without lock, it's hard to keep the sequence that let GC checking the flag after setting it, right? Thanks, > > Thanks, > >> >> Thanks, >> >>> >>> Thanks, >>> >>>> if (iov_iter_rw(iter) == WRITE) { >>>> if (err > 0) >>>> set_inode_flag(inode, FI_UPDATE_WRITE); >>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >>>> index bd82b6d..a241576 100644 >>>> --- a/fs/f2fs/f2fs.h >>>> +++ b/fs/f2fs/f2fs.h >>>> @@ -474,6 +474,7 @@ struct f2fs_inode_info { >>>> struct list_head inmem_pages; /* inmemory pages managed by f2fs */ >>>> struct mutex inmem_lock; /* lock for inmemory pages */ >>>> struct extent_tree *extent_tree; /* cached extent_tree entry */ >>>> + struct mutex dio_mutex; /* avoid racing between dio and gc */ >>>> }; >>>> >>>> static inline void get_extent_info(struct extent_info *ext, >>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c >>>> index c2c4ac3..98e3763 100644 >>>> --- a/fs/f2fs/gc.c >>>> +++ b/fs/f2fs/gc.c >>>> @@ -744,12 +744,24 @@ next_step: >>>> /* phase 3 */ >>>> inode = find_gc_inode(gc_list, dni.ino); >>>> if (inode) { >>>> + bool locked = false; >>>> + >>>> + if (S_ISREG(inode->i_mode)) { >>>> + if (!mutex_trylock(&F2FS_I(inode)->dio_mutex)) >>>> + continue; >>>> + locked = true; >>>> + } >>>> + >>>> start_bidx = start_bidx_of_node(nofs, inode) >>>> + ofs_in_node; >>>> - if (f2fs_encrypted_inode(inode) && S_ISREG(inode->i_mode)) >>>> + if (f2fs_encrypted_inode(inode) && >>>> + S_ISREG(inode->i_mode)) >>>> move_encrypted_block(inode, start_bidx); >>>> else >>>> move_data_page(inode, start_bidx, gc_type); >>>> + if (locked) >>>> + mutex_unlock(&F2FS_I(inode)->dio_mutex); >>>> + >>>> stat_inc_data_blk_count(sbi, 1, gc_type); >>>> } >>>> } >>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >>>> index 8c698e1..24aab3f 100644 >>>> --- a/fs/f2fs/super.c >>>> +++ b/fs/f2fs/super.c >>>> @@ -575,6 +575,7 @@ static struct inode *f2fs_alloc_inode(struct super_block *sb) >>>> INIT_LIST_HEAD(&fi->gdirty_list); >>>> INIT_LIST_HEAD(&fi->inmem_pages); >>>> mutex_init(&fi->inmem_lock); >>>> + mutex_init(&fi->dio_mutex); >>>> >>>> /* Will be used by directory only */ >>>> fi->i_dir_level = F2FS_SB(sb)->dir_level; >>>> -- >>>> 2.8.2.311.gee88674 >>> >>> . >>> > > . > ------------------------------------------------------------------------------ Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San Francisco, CA to explore cutting-edge tech and listen to tech luminaries present their vision of the future. This family event has something for everyone, including kids. Get more information and register today. http://sdm.link/attshape ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] f2fs: fix to avoid data update racing between GC and DIO 2016-07-06 2:10 ` Chao Yu @ 2016-07-06 22:37 ` Jaegeuk Kim 2016-07-07 4:25 ` Chao Yu 0 siblings, 1 reply; 9+ messages in thread From: Jaegeuk Kim @ 2016-07-06 22:37 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel On Wed, Jul 06, 2016 at 10:10:57AM +0800, Chao Yu wrote: > On 2016/7/6 8:24, Jaegeuk Kim wrote: > > On Fri, Jul 01, 2016 at 02:03:17PM +0800, Chao Yu wrote: > >> Hi Jaegeuk, > >> > >> On 2016/7/1 8:03, Jaegeuk Kim wrote: > >>> Hi Chao, > >>> > >>> On Thu, Jun 30, 2016 at 04:42:48PM +0800, Chao Yu wrote: > >>>> Datas in file can be operated by GC and DIO simultaneously, so we will > >>>> face race case as below: > >>>> > >>>> For write case: > >>>> Thread A Thread B > >>>> - generic_file_direct_write > >>>> - invalidate_inode_pages2_range > >>>> - f2fs_direct_IO > >>>> - do_blockdev_direct_IO > >>>> - do_direct_IO > >>>> - get_more_blocks > >>>> - f2fs_gc > >>>> - do_garbage_collect > >>>> - gc_data_segment > >>>> - move_data_page > >>>> - do_write_data_page > >>>> migrate data block to new block address > >>>> - dio_bio_submit > >>>> update user data to old block address > >>>> > >>>> For read case: > >>>> Thread A Thread B > >>>> - generic_file_direct_write > >>>> - invalidate_inode_pages2_range > >>>> - f2fs_direct_IO > >>>> - do_blockdev_direct_IO > >>>> - do_direct_IO > >>>> - get_more_blocks > >>>> - f2fs_balance_fs > >>>> - f2fs_gc > >>>> - do_garbage_collect > >>>> - gc_data_segment > >>>> - move_data_page > >>>> - do_write_data_page > >>>> migrate data block to new block address > >>>> - write_checkpoint > >>>> - do_checkpoint > >>>> - clear_prefree_segments > >>>> - f2fs_issue_discard > >>>> discard old block adress > >>>> - dio_bio_submit > >>>> update user buffer from obsolete block address > >>>> > >>>> In order to fix this, for one file, we should let DIO and GC getting exclusion > >>>> against with each other. > >>>> > >>>> Signed-off-by: Chao Yu <yuchao0@huawei.com> > >>>> --- > >>>> fs/f2fs/data.c | 2 ++ > >>>> fs/f2fs/f2fs.h | 1 + > >>>> fs/f2fs/gc.c | 14 +++++++++++++- > >>>> fs/f2fs/super.c | 1 + > >>>> 4 files changed, 17 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > >>>> index ba4963f..08dc060 100644 > >>>> --- a/fs/f2fs/data.c > >>>> +++ b/fs/f2fs/data.c > >>>> @@ -1716,7 +1716,9 @@ static ssize_t f2fs_direct_IO(struct kiocb *iocb, struct iov_iter *iter) > >>>> > >>>> trace_f2fs_direct_IO_enter(inode, offset, count, iov_iter_rw(iter)); > >>>> > >>>> + mutex_lock(&F2FS_I(inode)->dio_mutex); > >>>> err = blockdev_direct_IO(iocb, inode, iter, get_data_block_dio); > >>>> + mutex_unlock(&F2FS_I(inode)->dio_mutex); > >>> > >>> This means we need to sacrifice entire parallism even in the normal cases? > >>> Can we find another way? > >> > >> 1. For dio write vs dio write, writer will grab i_mutex before dio_mutex, so > >> anyway, concurrent dio writes will be exclusive. > >> > >> 2. For dio write vs gc, keep using dio_mutex for making them exclusive. > >> > >> 3. For dio read vs dio read, and dio read vs gc, what about adding dio_rwsem to > >> control the parallelism? > >> > >> 4. For dio write vs dio read, we grab different lock (write grabs dio_mutex, > >> read grabs dio_rwsem), so there is no race condition. > > > > How about adding a flag in a dio inode and avoiding GCs for there-in blocks? > > Hmm.. IMO, without lock, it's hard to keep the sequence that let GC checking the > flag after setting it, right? Okay, could you add dio_rwsem for now? Later, we may need to take a look at dio_overwrite case to mitigate inode_lock contention likewise xfs. :) Thanks, > >>> > >>>> if (iov_iter_rw(iter) == WRITE) { > >>>> if (err > 0) > >>>> set_inode_flag(inode, FI_UPDATE_WRITE); > >>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > >>>> index bd82b6d..a241576 100644 > >>>> --- a/fs/f2fs/f2fs.h > >>>> +++ b/fs/f2fs/f2fs.h > >>>> @@ -474,6 +474,7 @@ struct f2fs_inode_info { > >>>> struct list_head inmem_pages; /* inmemory pages managed by f2fs */ > >>>> struct mutex inmem_lock; /* lock for inmemory pages */ > >>>> struct extent_tree *extent_tree; /* cached extent_tree entry */ > >>>> + struct mutex dio_mutex; /* avoid racing between dio and gc */ > >>>> }; > >>>> > >>>> static inline void get_extent_info(struct extent_info *ext, > >>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > >>>> index c2c4ac3..98e3763 100644 > >>>> --- a/fs/f2fs/gc.c > >>>> +++ b/fs/f2fs/gc.c > >>>> @@ -744,12 +744,24 @@ next_step: > >>>> /* phase 3 */ > >>>> inode = find_gc_inode(gc_list, dni.ino); > >>>> if (inode) { > >>>> + bool locked = false; > >>>> + > >>>> + if (S_ISREG(inode->i_mode)) { > >>>> + if (!mutex_trylock(&F2FS_I(inode)->dio_mutex)) > >>>> + continue; > >>>> + locked = true; > >>>> + } > >>>> + > >>>> start_bidx = start_bidx_of_node(nofs, inode) > >>>> + ofs_in_node; > >>>> - if (f2fs_encrypted_inode(inode) && S_ISREG(inode->i_mode)) > >>>> + if (f2fs_encrypted_inode(inode) && > >>>> + S_ISREG(inode->i_mode)) > >>>> move_encrypted_block(inode, start_bidx); > >>>> else > >>>> move_data_page(inode, start_bidx, gc_type); > >>>> + if (locked) > >>>> + mutex_unlock(&F2FS_I(inode)->dio_mutex); > >>>> + > >>>> stat_inc_data_blk_count(sbi, 1, gc_type); > >>>> } > >>>> } > >>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > >>>> index 8c698e1..24aab3f 100644 > >>>> --- a/fs/f2fs/super.c > >>>> +++ b/fs/f2fs/super.c > >>>> @@ -575,6 +575,7 @@ static struct inode *f2fs_alloc_inode(struct super_block *sb) > >>>> INIT_LIST_HEAD(&fi->gdirty_list); > >>>> INIT_LIST_HEAD(&fi->inmem_pages); > >>>> mutex_init(&fi->inmem_lock); > >>>> + mutex_init(&fi->dio_mutex); > >>>> > >>>> /* Will be used by directory only */ > >>>> fi->i_dir_level = F2FS_SB(sb)->dir_level; > >>>> -- > >>>> 2.8.2.311.gee88674 > >>> > >>> . > >>> > > > > . > > ------------------------------------------------------------------------------ Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San Francisco, CA to explore cutting-edge tech and listen to tech luminaries present their vision of the future. This family event has something for everyone, including kids. Get more information and register today. http://sdm.link/attshape ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] f2fs: fix to avoid data update racing between GC and DIO 2016-07-06 22:37 ` Jaegeuk Kim @ 2016-07-07 4:25 ` Chao Yu 0 siblings, 0 replies; 9+ messages in thread From: Chao Yu @ 2016-07-07 4:25 UTC (permalink / raw) To: Jaegeuk Kim, Chao Yu; +Cc: linux-kernel, linux-f2fs-devel On 2016/7/7 6:37, Jaegeuk Kim wrote: > On Wed, Jul 06, 2016 at 10:10:57AM +0800, Chao Yu wrote: >> On 2016/7/6 8:24, Jaegeuk Kim wrote: >>> On Fri, Jul 01, 2016 at 02:03:17PM +0800, Chao Yu wrote: >>>> Hi Jaegeuk, >>>> >>>> On 2016/7/1 8:03, Jaegeuk Kim wrote: >>>>> Hi Chao, >>>>> >>>>> On Thu, Jun 30, 2016 at 04:42:48PM +0800, Chao Yu wrote: >>>>>> Datas in file can be operated by GC and DIO simultaneously, so we will >>>>>> face race case as below: >>>>>> >>>>>> For write case: >>>>>> Thread A Thread B >>>>>> - generic_file_direct_write >>>>>> - invalidate_inode_pages2_range >>>>>> - f2fs_direct_IO >>>>>> - do_blockdev_direct_IO >>>>>> - do_direct_IO >>>>>> - get_more_blocks >>>>>> - f2fs_gc >>>>>> - do_garbage_collect >>>>>> - gc_data_segment >>>>>> - move_data_page >>>>>> - do_write_data_page >>>>>> migrate data block to new block address >>>>>> - dio_bio_submit >>>>>> update user data to old block address >>>>>> >>>>>> For read case: >>>>>> Thread A Thread B >>>>>> - generic_file_direct_write >>>>>> - invalidate_inode_pages2_range >>>>>> - f2fs_direct_IO >>>>>> - do_blockdev_direct_IO >>>>>> - do_direct_IO >>>>>> - get_more_blocks >>>>>> - f2fs_balance_fs >>>>>> - f2fs_gc >>>>>> - do_garbage_collect >>>>>> - gc_data_segment >>>>>> - move_data_page >>>>>> - do_write_data_page >>>>>> migrate data block to new block address >>>>>> - write_checkpoint >>>>>> - do_checkpoint >>>>>> - clear_prefree_segments >>>>>> - f2fs_issue_discard >>>>>> discard old block adress >>>>>> - dio_bio_submit >>>>>> update user buffer from obsolete block address >>>>>> >>>>>> In order to fix this, for one file, we should let DIO and GC getting exclusion >>>>>> against with each other. >>>>>> >>>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com> >>>>>> --- >>>>>> fs/f2fs/data.c | 2 ++ >>>>>> fs/f2fs/f2fs.h | 1 + >>>>>> fs/f2fs/gc.c | 14 +++++++++++++- >>>>>> fs/f2fs/super.c | 1 + >>>>>> 4 files changed, 17 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >>>>>> index ba4963f..08dc060 100644 >>>>>> --- a/fs/f2fs/data.c >>>>>> +++ b/fs/f2fs/data.c >>>>>> @@ -1716,7 +1716,9 @@ static ssize_t f2fs_direct_IO(struct kiocb *iocb, struct iov_iter *iter) >>>>>> >>>>>> trace_f2fs_direct_IO_enter(inode, offset, count, iov_iter_rw(iter)); >>>>>> >>>>>> + mutex_lock(&F2FS_I(inode)->dio_mutex); >>>>>> err = blockdev_direct_IO(iocb, inode, iter, get_data_block_dio); >>>>>> + mutex_unlock(&F2FS_I(inode)->dio_mutex); >>>>> >>>>> This means we need to sacrifice entire parallism even in the normal cases? >>>>> Can we find another way? >>>> >>>> 1. For dio write vs dio write, writer will grab i_mutex before dio_mutex, so >>>> anyway, concurrent dio writes will be exclusive. >>>> >>>> 2. For dio write vs gc, keep using dio_mutex for making them exclusive. >>>> >>>> 3. For dio read vs dio read, and dio read vs gc, what about adding dio_rwsem to >>>> control the parallelism? >>>> >>>> 4. For dio write vs dio read, we grab different lock (write grabs dio_mutex, >>>> read grabs dio_rwsem), so there is no race condition. >>> >>> How about adding a flag in a dio inode and avoiding GCs for there-in blocks? >> >> Hmm.. IMO, without lock, it's hard to keep the sequence that let GC checking the >> flag after setting it, right? > > Okay, could you add dio_rwsem for now? > Later, we may need to take a look at dio_overwrite case to mitigate inode_lock > contention likewise xfs. :) Sounds good if we can support concurrent overwrite dio! :) Let me send v3. Thanks, > > Thanks, > >>>>> >>>>>> if (iov_iter_rw(iter) == WRITE) { >>>>>> if (err > 0) >>>>>> set_inode_flag(inode, FI_UPDATE_WRITE); >>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >>>>>> index bd82b6d..a241576 100644 >>>>>> --- a/fs/f2fs/f2fs.h >>>>>> +++ b/fs/f2fs/f2fs.h >>>>>> @@ -474,6 +474,7 @@ struct f2fs_inode_info { >>>>>> struct list_head inmem_pages; /* inmemory pages managed by f2fs */ >>>>>> struct mutex inmem_lock; /* lock for inmemory pages */ >>>>>> struct extent_tree *extent_tree; /* cached extent_tree entry */ >>>>>> + struct mutex dio_mutex; /* avoid racing between dio and gc */ >>>>>> }; >>>>>> >>>>>> static inline void get_extent_info(struct extent_info *ext, >>>>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c >>>>>> index c2c4ac3..98e3763 100644 >>>>>> --- a/fs/f2fs/gc.c >>>>>> +++ b/fs/f2fs/gc.c >>>>>> @@ -744,12 +744,24 @@ next_step: >>>>>> /* phase 3 */ >>>>>> inode = find_gc_inode(gc_list, dni.ino); >>>>>> if (inode) { >>>>>> + bool locked = false; >>>>>> + >>>>>> + if (S_ISREG(inode->i_mode)) { >>>>>> + if (!mutex_trylock(&F2FS_I(inode)->dio_mutex)) >>>>>> + continue; >>>>>> + locked = true; >>>>>> + } >>>>>> + >>>>>> start_bidx = start_bidx_of_node(nofs, inode) >>>>>> + ofs_in_node; >>>>>> - if (f2fs_encrypted_inode(inode) && S_ISREG(inode->i_mode)) >>>>>> + if (f2fs_encrypted_inode(inode) && >>>>>> + S_ISREG(inode->i_mode)) >>>>>> move_encrypted_block(inode, start_bidx); >>>>>> else >>>>>> move_data_page(inode, start_bidx, gc_type); >>>>>> + if (locked) >>>>>> + mutex_unlock(&F2FS_I(inode)->dio_mutex); >>>>>> + >>>>>> stat_inc_data_blk_count(sbi, 1, gc_type); >>>>>> } >>>>>> } >>>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >>>>>> index 8c698e1..24aab3f 100644 >>>>>> --- a/fs/f2fs/super.c >>>>>> +++ b/fs/f2fs/super.c >>>>>> @@ -575,6 +575,7 @@ static struct inode *f2fs_alloc_inode(struct super_block *sb) >>>>>> INIT_LIST_HEAD(&fi->gdirty_list); >>>>>> INIT_LIST_HEAD(&fi->inmem_pages); >>>>>> mutex_init(&fi->inmem_lock); >>>>>> + mutex_init(&fi->dio_mutex); >>>>>> >>>>>> /* Will be used by directory only */ >>>>>> fi->i_dir_level = F2FS_SB(sb)->dir_level; >>>>>> -- >>>>>> 2.8.2.311.gee88674 >>>>> >>>>> . >>>>> >>> >>> . >>> > > ------------------------------------------------------------------------------ > Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San > Francisco, CA to explore cutting-edge tech and listen to tech luminaries > present their vision of the future. This family event has something for > everyone, including kids. Get more information and register today. > http://sdm.link/attshape > _______________________________________________ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > ------------------------------------------------------------------------------ Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San Francisco, CA to explore cutting-edge tech and listen to tech luminaries present their vision of the future. This family event has something for everyone, including kids. Get more information and register today. http://sdm.link/attshape ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] f2fs: fix to return correct trimmed block number in FITRIM interface 2016-06-30 8:42 [PATCH 1/2] f2fs: fix to return correct trimmed block number in FITRIM interface Chao Yu 2016-06-30 8:42 ` [PATCH 2/2] f2fs: fix to avoid data update racing between GC and DIO Chao Yu @ 2016-07-07 4:29 ` Chao Yu 1 sibling, 0 replies; 9+ messages in thread From: Chao Yu @ 2016-07-07 4:29 UTC (permalink / raw) To: Chao Yu, jaegeuk; +Cc: linux-kernel, linux-f2fs-devel Hi all, I think this patch should be wrong, since during fstrim, we should not issue discard for prefree segment redundantly. So, Jaegeuk, could you please drop this patch in your branch? Sorry for the noise. Thanks, On 2016/6/30 16:42, Chao Yu wrote: > During tiggering fstrim, in case of issuing discard for prefree segments, > we miss acclumulating trimmed block number which will be return to user. > Fix it. > > Signed-off-by: Chao Yu <yuchao0@huawei.com> > --- > fs/f2fs/segment.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index 6d16ecf..5dc14d6 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -732,15 +732,20 @@ void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc) > if (!test_opt(sbi, LFS) || sbi->segs_per_sec == 1) { > f2fs_issue_discard(sbi, START_BLOCK(sbi, start), > (end - start) << sbi->log_blocks_per_seg); > + cpc->trimmed += > + (end - start) << sbi->log_blocks_per_seg; > continue; > } > next: > secno = GET_SECNO(sbi, start); > start_segno = secno * sbi->segs_per_sec; > if (!IS_CURSEC(sbi, secno) && > - !get_valid_blocks(sbi, start, sbi->segs_per_sec)) > + !get_valid_blocks(sbi, start, sbi->segs_per_sec)) { > f2fs_issue_discard(sbi, START_BLOCK(sbi, start_segno), > sbi->segs_per_sec << sbi->log_blocks_per_seg); > + cpc->trimmed += > + sbi->segs_per_sec << sbi->log_blocks_per_seg; > + } > > start = start_segno + sbi->segs_per_sec; > if (start < end) > ------------------------------------------------------------------------------ Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San Francisco, CA to explore cutting-edge tech and listen to tech luminaries present their vision of the future. This family event has something for everyone, including kids. Get more information and register today. http://sdm.link/attshape ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-07-07 4:29 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-06-30 8:42 [PATCH 1/2] f2fs: fix to return correct trimmed block number in FITRIM interface Chao Yu 2016-06-30 8:42 ` [PATCH 2/2] f2fs: fix to avoid data update racing between GC and DIO Chao Yu 2016-07-01 0:03 ` Jaegeuk Kim 2016-07-01 6:03 ` Chao Yu 2016-07-06 0:24 ` Jaegeuk Kim 2016-07-06 2:10 ` Chao Yu 2016-07-06 22:37 ` Jaegeuk Kim 2016-07-07 4:25 ` Chao Yu 2016-07-07 4:29 ` [PATCH 1/2] f2fs: fix to return correct trimmed block number in FITRIM interface Chao Yu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).