linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] f2fs fg_gc: node segment is prior to data segment selected victim
@ 2017-02-22 10:28 Hou Pengyang
  2017-02-22 10:29 ` [PATCH 2/2] f2fs:use fggc_set_cold_data instead of set_cold_data in fggc Hou Pengyang
  2017-02-24  2:03 ` [PATCH 1/2] f2fs fg_gc: node segment is prior to data segment selected victim Chao Yu
  0 siblings, 2 replies; 8+ messages in thread
From: Hou Pengyang @ 2017-02-22 10:28 UTC (permalink / raw)
  To: jaegeuk, yuchao0; +Cc: linux-f2fs-devel

As data segment gc may lead dnode dirty, so the greedy cost for data segment should be
valid blocks * 2, that is data segment is prior to node segment.

Team:OTHERS
Feature or Bugfix:Bugfix
Binary Source:No

Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
---
 fs/f2fs/gc.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index e93aecb..4ee749f 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -242,6 +242,13 @@ static unsigned int get_cb_cost(struct f2fs_sb_info *sbi, unsigned int segno)
 	return UINT_MAX - ((100 * (100 - u) * age) / (100 + u));
 }
 
+static unsigned int get_greedy_cost(struct f2fs_sb_info *sbi, unsigned int segno)
+{
+	unsigned int valid_blocks = get_valid_blocks(sbi, segno, sbi->segs_per_sec);
+	return IS_DATASEG(get_seg_entry(sbi, segno)->type) ?
+			valid_blocks * 2 : valid_blocks;
+}
+
 static inline unsigned int get_gc_cost(struct f2fs_sb_info *sbi,
 			unsigned int segno, struct victim_sel_policy *p)
 {
@@ -250,7 +257,7 @@ static inline unsigned int get_gc_cost(struct f2fs_sb_info *sbi,
 
 	/* alloc_mode == LFS */
 	if (p->gc_mode == GC_GREEDY)
-		return get_valid_blocks(sbi, segno, sbi->segs_per_sec);
+		return get_greedy_cost(sbi, segno);
 	else
 		return get_cb_cost(sbi, segno);
 }
-- 
2.10.1


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] f2fs:use fggc_set_cold_data instead of set_cold_data in fggc
  2017-02-22 10:28 [PATCH 1/2] f2fs fg_gc: node segment is prior to data segment selected victim Hou Pengyang
@ 2017-02-22 10:29 ` Hou Pengyang
  2017-02-23  0:39   ` Jaegeuk Kim
  2017-02-24  2:03 ` [PATCH 1/2] f2fs fg_gc: node segment is prior to data segment selected victim Chao Yu
  1 sibling, 1 reply; 8+ messages in thread
From: Hou Pengyang @ 2017-02-22 10:29 UTC (permalink / raw)
  To: jaegeuk, yuchao0; +Cc: linux-f2fs-devel

When no dirty colde segment for fggc ssr, we do NOT set_cold_data.

Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
---
 fs/f2fs/gc.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 4ee749f..a0eed23 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -666,6 +666,25 @@ static void move_encrypted_block(struct inode *inode, block_t bidx,
 	f2fs_put_page(page, 1);
 }
 
+/*
+ * If NOT enough dirty cold data segments for fggc, fggc would consume
+ * a valuable free segment, which would slow down the fggc procedure,
+ * If there are NO COLD dirty segment, we do not set_cold_page.
+ */
+static void fggc_set_cold_data(struct page *page)
+{
+	struct f2fs_sb_info *sbi = F2FS_P_SB(page);
+	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
+	int nr_cold_dirties;
+
+	mutex_lock(&dirty_i->seglist_lock);
+	nr_cold_dirties = dirty_i->nr_dirty[DIRTY_COLD_DATA];
+	mutex_unlock(&dirty_i->seglist_lock);
+
+	if (nr_cold_dirties)
+		set_cold_data(page);
+
+}
 static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
 							unsigned int segno, int off)
 {
@@ -706,7 +725,7 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
 			remove_dirty_inode(inode);
 		}
 
-		set_cold_data(page);
+		fggc_set_cold_data(page);
 
 		err = do_write_data_page(&fio);
 		if (err == -ENOMEM && is_dirty) {
-- 
2.10.1


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] f2fs:use fggc_set_cold_data instead of set_cold_data in fggc
  2017-02-22 10:29 ` [PATCH 2/2] f2fs:use fggc_set_cold_data instead of set_cold_data in fggc Hou Pengyang
@ 2017-02-23  0:39   ` Jaegeuk Kim
  2017-02-23  2:04     ` Hou Pengyang
  0 siblings, 1 reply; 8+ messages in thread
From: Jaegeuk Kim @ 2017-02-23  0:39 UTC (permalink / raw)
  To: Hou Pengyang; +Cc: linux-f2fs-devel

Hi Pengyang,

On 02/22, Hou Pengyang wrote:
> When no dirty colde segment for fggc ssr, we do NOT set_cold_data.

The get_ssr_segment() finds a victim segment across all the hot/warm/cold data
logs. When does this happen?

Thanks,

> 
> Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
> ---
>  fs/f2fs/gc.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 4ee749f..a0eed23 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -666,6 +666,25 @@ static void move_encrypted_block(struct inode *inode, block_t bidx,
>  	f2fs_put_page(page, 1);
>  }
>  
> +/*
> + * If NOT enough dirty cold data segments for fggc, fggc would consume
> + * a valuable free segment, which would slow down the fggc procedure,
> + * If there are NO COLD dirty segment, we do not set_cold_page.
> + */
> +static void fggc_set_cold_data(struct page *page)
> +{
> +	struct f2fs_sb_info *sbi = F2FS_P_SB(page);
> +	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
> +	int nr_cold_dirties;
> +
> +	mutex_lock(&dirty_i->seglist_lock);
> +	nr_cold_dirties = dirty_i->nr_dirty[DIRTY_COLD_DATA];
> +	mutex_unlock(&dirty_i->seglist_lock);
> +
> +	if (nr_cold_dirties)
> +		set_cold_data(page);
> +
> +}
>  static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
>  							unsigned int segno, int off)
>  {
> @@ -706,7 +725,7 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
>  			remove_dirty_inode(inode);
>  		}
>  
> -		set_cold_data(page);
> +		fggc_set_cold_data(page);
>  
>  		err = do_write_data_page(&fio);
>  		if (err == -ENOMEM && is_dirty) {
> -- 
> 2.10.1
> 
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] f2fs:use fggc_set_cold_data instead of set_cold_data in fggc
  2017-02-23  0:39   ` Jaegeuk Kim
@ 2017-02-23  2:04     ` Hou Pengyang
  2017-02-23  2:14       ` Jaegeuk Kim
  2017-02-24  2:07       ` Chao Yu
  0 siblings, 2 replies; 8+ messages in thread
From: Hou Pengyang @ 2017-02-23  2:04 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel

On 2017/2/23 8:39, Jaegeuk Kim wrote:
> Hi Pengyang,
>
> On 02/22, Hou Pengyang wrote:
>> When no dirty colde segment for fggc ssr, we do NOT set_cold_data.
>
> The get_ssr_segment() finds a victim segment across all the hot/warm/cold data
> logs. When does this happen?
>
Oh, I miss the travel across hot/warm/cold, sorry.
how about the following filter in get_victim_segment, which can avoid
unnecessary afterwards bitmap scanning.

Thanks,

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index a0eed23..00791d2 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -290,9 +290,16 @@ static int get_victim_by_default(struct 
f2fs_sb_info *sbi,
         unsigned int secno, last_victim;
         unsigned int last_segment = MAIN_SEGS(sbi);
         unsigned int nsearched = 0;
+       int nr_dirty;

         mutex_lock(&dirty_i->seglist_lock);

+       if (alloc_mode == SSR) {
+               nr_dirty = dirty_i->nr_dirty[type];
+               if (!nr_dirty)
+                       goto out;
+       }
+
         p.alloc_mode = alloc_mode;
         select_policy(sbi, gc_type, type, &p);


> Thanks,
>
>>
>> Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
>> ---
>>   fs/f2fs/gc.c | 21 ++++++++++++++++++++-
>>   1 file changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>> index 4ee749f..a0eed23 100644
>> --- a/fs/f2fs/gc.c
>> +++ b/fs/f2fs/gc.c
>> @@ -666,6 +666,25 @@ static void move_encrypted_block(struct inode *inode, block_t bidx,
>>   	f2fs_put_page(page, 1);
>>   }
>>
>> +/*
>> + * If NOT enough dirty cold data segments for fggc, fggc would consume
>> + * a valuable free segment, which would slow down the fggc procedure,
>> + * If there are NO COLD dirty segment, we do not set_cold_page.
>> + */
>> +static void fggc_set_cold_data(struct page *page)
>> +{
>> +	struct f2fs_sb_info *sbi = F2FS_P_SB(page);
>> +	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
>> +	int nr_cold_dirties;
>> +
>> +	mutex_lock(&dirty_i->seglist_lock);
>> +	nr_cold_dirties = dirty_i->nr_dirty[DIRTY_COLD_DATA];
>> +	mutex_unlock(&dirty_i->seglist_lock);
>> +
>> +	if (nr_cold_dirties)
>> +		set_cold_data(page);
>> +
>> +}
>>   static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
>>   							unsigned int segno, int off)
>>   {
>> @@ -706,7 +725,7 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
>>   			remove_dirty_inode(inode);
>>   		}
>>
>> -		set_cold_data(page);
>> +		fggc_set_cold_data(page);
>>
>>   		err = do_write_data_page(&fio);
>>   		if (err == -ENOMEM && is_dirty) {
>> --
>> 2.10.1
>>
>>
>> ------------------------------------------------------------------------------
>> Check out the vibrant tech community on one of the world's most
>> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>
> .
>



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] f2fs:use fggc_set_cold_data instead of set_cold_data in fggc
  2017-02-23  2:04     ` Hou Pengyang
@ 2017-02-23  2:14       ` Jaegeuk Kim
  2017-02-24  2:07       ` Chao Yu
  1 sibling, 0 replies; 8+ messages in thread
From: Jaegeuk Kim @ 2017-02-23  2:14 UTC (permalink / raw)
  To: Hou Pengyang; +Cc: linux-f2fs-devel

On 02/23, Hou Pengyang wrote:
> On 2017/2/23 8:39, Jaegeuk Kim wrote:
> > Hi Pengyang,
> > 
> > On 02/22, Hou Pengyang wrote:
> > > When no dirty colde segment for fggc ssr, we do NOT set_cold_data.
> > 
> > The get_ssr_segment() finds a victim segment across all the hot/warm/cold data
> > logs. When does this happen?
> > 
> Oh, I miss the travel across hot/warm/cold, sorry.
> how about the following filter in get_victim_segment, which can avoid
> unnecessary afterwards bitmap scanning.

Yup, it makes sense.

Thanks,

> 
> Thanks,
> 
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index a0eed23..00791d2 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -290,9 +290,16 @@ static int get_victim_by_default(struct f2fs_sb_info
> *sbi,
>         unsigned int secno, last_victim;
>         unsigned int last_segment = MAIN_SEGS(sbi);
>         unsigned int nsearched = 0;
> +       int nr_dirty;
> 
>         mutex_lock(&dirty_i->seglist_lock);
> 
> +       if (alloc_mode == SSR) {
> +               nr_dirty = dirty_i->nr_dirty[type];
> +               if (!nr_dirty)
> +                       goto out;
> +       }
> +
>         p.alloc_mode = alloc_mode;
>         select_policy(sbi, gc_type, type, &p);
> 
> 
> > Thanks,
> > 
> > > 
> > > Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
> > > ---
> > >   fs/f2fs/gc.c | 21 ++++++++++++++++++++-
> > >   1 file changed, 20 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> > > index 4ee749f..a0eed23 100644
> > > --- a/fs/f2fs/gc.c
> > > +++ b/fs/f2fs/gc.c
> > > @@ -666,6 +666,25 @@ static void move_encrypted_block(struct inode *inode, block_t bidx,
> > >   	f2fs_put_page(page, 1);
> > >   }
> > > 
> > > +/*
> > > + * If NOT enough dirty cold data segments for fggc, fggc would consume
> > > + * a valuable free segment, which would slow down the fggc procedure,
> > > + * If there are NO COLD dirty segment, we do not set_cold_page.
> > > + */
> > > +static void fggc_set_cold_data(struct page *page)
> > > +{
> > > +	struct f2fs_sb_info *sbi = F2FS_P_SB(page);
> > > +	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
> > > +	int nr_cold_dirties;
> > > +
> > > +	mutex_lock(&dirty_i->seglist_lock);
> > > +	nr_cold_dirties = dirty_i->nr_dirty[DIRTY_COLD_DATA];
> > > +	mutex_unlock(&dirty_i->seglist_lock);
> > > +
> > > +	if (nr_cold_dirties)
> > > +		set_cold_data(page);
> > > +
> > > +}
> > >   static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
> > >   							unsigned int segno, int off)
> > >   {
> > > @@ -706,7 +725,7 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
> > >   			remove_dirty_inode(inode);
> > >   		}
> > > 
> > > -		set_cold_data(page);
> > > +		fggc_set_cold_data(page);
> > > 
> > >   		err = do_write_data_page(&fio);
> > >   		if (err == -ENOMEM && is_dirty) {
> > > --
> > > 2.10.1
> > > 
> > > 
> > > ------------------------------------------------------------------------------
> > > Check out the vibrant tech community on one of the world's most
> > > engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> > > _______________________________________________
> > > Linux-f2fs-devel mailing list
> > > Linux-f2fs-devel@lists.sourceforge.net
> > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > 
> > .
> > 
> 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] f2fs fg_gc: node segment is prior to data segment selected victim
  2017-02-22 10:28 [PATCH 1/2] f2fs fg_gc: node segment is prior to data segment selected victim Hou Pengyang
  2017-02-22 10:29 ` [PATCH 2/2] f2fs:use fggc_set_cold_data instead of set_cold_data in fggc Hou Pengyang
@ 2017-02-24  2:03 ` Chao Yu
  1 sibling, 0 replies; 8+ messages in thread
From: Chao Yu @ 2017-02-24  2:03 UTC (permalink / raw)
  To: Hou Pengyang, jaegeuk; +Cc: linux-f2fs-devel

On 2017/2/22 18:28, Hou Pengyang wrote:
> As data segment gc may lead dnode dirty, so the greedy cost for data segment should be
> valid blocks * 2, that is data segment is prior to node segment.
>
> Signed-off-by: Hou Pengyang <houpengyang@huawei.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>




------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] f2fs:use fggc_set_cold_data instead of set_cold_data in fggc
  2017-02-23  2:04     ` Hou Pengyang
  2017-02-23  2:14       ` Jaegeuk Kim
@ 2017-02-24  2:07       ` Chao Yu
  2017-02-24  3:04         ` Hou Pengyang
  1 sibling, 1 reply; 8+ messages in thread
From: Chao Yu @ 2017-02-24  2:07 UTC (permalink / raw)
  To: Hou Pengyang, Jaegeuk Kim; +Cc: linux-f2fs-devel

On 2017/2/23 10:04, Hou Pengyang wrote:
> On 2017/2/23 8:39, Jaegeuk Kim wrote:
>> Hi Pengyang,
>>
>> On 02/22, Hou Pengyang wrote:
>>> When no dirty colde segment for fggc ssr, we do NOT set_cold_data.
>>
>> The get_ssr_segment() finds a victim segment across all the hot/warm/cold data
>> logs. When does this happen?
>>
> Oh, I miss the travel across hot/warm/cold, sorry.
> how about the following filter in get_victim_segment, which can avoid
> unnecessary afterwards bitmap scanning.
> 
> Thanks,
> 
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index a0eed23..00791d2 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -290,9 +290,16 @@ static int get_victim_by_default(struct 
> f2fs_sb_info *sbi,
>          unsigned int secno, last_victim;
>          unsigned int last_segment = MAIN_SEGS(sbi);
>          unsigned int nsearched = 0;
> +       int nr_dirty;
> 
>          mutex_lock(&dirty_i->seglist_lock);
> 
> +       if (alloc_mode == SSR) {
> +               nr_dirty = dirty_i->nr_dirty[type];
> +               if (!nr_dirty)

Needs to clean up.

if (!dirty_i->nr_dirty[type])

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

> +                       goto out;
> +       }
> +
>          p.alloc_mode = alloc_mode;
>          select_policy(sbi, gc_type, type, &p);
> 
> 
>> Thanks,
>>
>>>
>>> Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
>>> ---
>>>   fs/f2fs/gc.c | 21 ++++++++++++++++++++-
>>>   1 file changed, 20 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>> index 4ee749f..a0eed23 100644
>>> --- a/fs/f2fs/gc.c
>>> +++ b/fs/f2fs/gc.c
>>> @@ -666,6 +666,25 @@ static void move_encrypted_block(struct inode *inode, block_t bidx,
>>>   	f2fs_put_page(page, 1);
>>>   }
>>>
>>> +/*
>>> + * If NOT enough dirty cold data segments for fggc, fggc would consume
>>> + * a valuable free segment, which would slow down the fggc procedure,
>>> + * If there are NO COLD dirty segment, we do not set_cold_page.
>>> + */
>>> +static void fggc_set_cold_data(struct page *page)
>>> +{
>>> +	struct f2fs_sb_info *sbi = F2FS_P_SB(page);
>>> +	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
>>> +	int nr_cold_dirties;
>>> +
>>> +	mutex_lock(&dirty_i->seglist_lock);
>>> +	nr_cold_dirties = dirty_i->nr_dirty[DIRTY_COLD_DATA];
>>> +	mutex_unlock(&dirty_i->seglist_lock);
>>> +
>>> +	if (nr_cold_dirties)
>>> +		set_cold_data(page);
>>> +
>>> +}
>>>   static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
>>>   							unsigned int segno, int off)
>>>   {
>>> @@ -706,7 +725,7 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
>>>   			remove_dirty_inode(inode);
>>>   		}
>>>
>>> -		set_cold_data(page);
>>> +		fggc_set_cold_data(page);
>>>
>>>   		err = do_write_data_page(&fio);
>>>   		if (err == -ENOMEM && is_dirty) {
>>> --
>>> 2.10.1
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> Check out the vibrant tech community on one of the world's most
>>> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
>>> _______________________________________________
>>> Linux-f2fs-devel mailing list
>>> Linux-f2fs-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>
>> .
>>
> 
> 
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] f2fs:use fggc_set_cold_data instead of set_cold_data in fggc
  2017-02-24  2:07       ` Chao Yu
@ 2017-02-24  3:04         ` Hou Pengyang
  0 siblings, 0 replies; 8+ messages in thread
From: Hou Pengyang @ 2017-02-24  3:04 UTC (permalink / raw)
  To: Chao Yu; +Cc: Jaegeuk Kim, linux-f2fs-devel

On 2017/2/24 10:07, Chao Yu wrote:
> On 2017/2/23 10:04, Hou Pengyang wrote:
>> On 2017/2/23 8:39, Jaegeuk Kim wrote:
>>> Hi Pengyang,
Hi Chao,
>>>
>>> On 02/22, Hou Pengyang wrote:
>>>> When no dirty colde segment for fggc ssr, we do NOT set_cold_data.
>>>
>>> The get_ssr_segment() finds a victim segment across all the hot/warm/cold data
>>> logs. When does this happen?
>>>
>> Oh, I miss the travel across hot/warm/cold, sorry.
>> how about the following filter in get_victim_segment, which can avoid
>> unnecessary afterwards bitmap scanning.
>>
>> Thanks,
>>
>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>> index a0eed23..00791d2 100644
>> --- a/fs/f2fs/gc.c
>> +++ b/fs/f2fs/gc.c
>> @@ -290,9 +290,16 @@ static int get_victim_by_default(struct
>> f2fs_sb_info *sbi,
>>           unsigned int secno, last_victim;
>>           unsigned int last_segment = MAIN_SEGS(sbi);
>>           unsigned int nsearched = 0;
>> +       int nr_dirty;
>>
>>           mutex_lock(&dirty_i->seglist_lock);
>>
>> +       if (alloc_mode == SSR) {
>> +               nr_dirty = dirty_i->nr_dirty[type];
>> +               if (!nr_dirty)
>
> Needs to clean up.
>
> if (!dirty_i->nr_dirty[type])

As Jaegeuk explained, p.max_search has skipped no # of victims scenario

	if (p.max_search == 0)
		goto out;
Thanks,
>
> Reviewed-by: Chao Yu <yuchao0@huawei.com>
>
> Thanks,
>
>> +                       goto out;
>> +       }
>> +
>>           p.alloc_mode = alloc_mode;
>>           select_policy(sbi, gc_type, type, &p);
>>
>>
>>> Thanks,
>>>
>>>>
>>>> Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
>>>> ---
>>>>    fs/f2fs/gc.c | 21 ++++++++++++++++++++-
>>>>    1 file changed, 20 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>>> index 4ee749f..a0eed23 100644
>>>> --- a/fs/f2fs/gc.c
>>>> +++ b/fs/f2fs/gc.c
>>>> @@ -666,6 +666,25 @@ static void move_encrypted_block(struct inode *inode, block_t bidx,
>>>>    	f2fs_put_page(page, 1);
>>>>    }
>>>>
>>>> +/*
>>>> + * If NOT enough dirty cold data segments for fggc, fggc would consume
>>>> + * a valuable free segment, which would slow down the fggc procedure,
>>>> + * If there are NO COLD dirty segment, we do not set_cold_page.
>>>> + */
>>>> +static void fggc_set_cold_data(struct page *page)
>>>> +{
>>>> +	struct f2fs_sb_info *sbi = F2FS_P_SB(page);
>>>> +	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
>>>> +	int nr_cold_dirties;
>>>> +
>>>> +	mutex_lock(&dirty_i->seglist_lock);
>>>> +	nr_cold_dirties = dirty_i->nr_dirty[DIRTY_COLD_DATA];
>>>> +	mutex_unlock(&dirty_i->seglist_lock);
>>>> +
>>>> +	if (nr_cold_dirties)
>>>> +		set_cold_data(page);
>>>> +
>>>> +}
>>>>    static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
>>>>    							unsigned int segno, int off)
>>>>    {
>>>> @@ -706,7 +725,7 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
>>>>    			remove_dirty_inode(inode);
>>>>    		}
>>>>
>>>> -		set_cold_data(page);
>>>> +		fggc_set_cold_data(page);
>>>>
>>>>    		err = do_write_data_page(&fio);
>>>>    		if (err == -ENOMEM && is_dirty) {
>>>> --
>>>> 2.10.1
>>>>
>>>>
>>>> ------------------------------------------------------------------------------
>>>> Check out the vibrant tech community on one of the world's most
>>>> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
>>>> _______________________________________________
>>>> Linux-f2fs-devel mailing list
>>>> Linux-f2fs-devel@lists.sourceforge.net
>>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>>
>>> .
>>>
>>
>>
>>
>> .
>>
>
>
> .
>



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-02-24  3:05 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-22 10:28 [PATCH 1/2] f2fs fg_gc: node segment is prior to data segment selected victim Hou Pengyang
2017-02-22 10:29 ` [PATCH 2/2] f2fs:use fggc_set_cold_data instead of set_cold_data in fggc Hou Pengyang
2017-02-23  0:39   ` Jaegeuk Kim
2017-02-23  2:04     ` Hou Pengyang
2017-02-23  2:14       ` Jaegeuk Kim
2017-02-24  2:07       ` Chao Yu
2017-02-24  3:04         ` Hou Pengyang
2017-02-24  2:03 ` [PATCH 1/2] f2fs fg_gc: node segment is prior to data segment selected victim 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).