All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] f2fs: avoid false positive of free secs check
@ 2017-11-27  6:54 ` Yunlong Song
  0 siblings, 0 replies; 9+ messages in thread
From: Yunlong Song @ 2017-11-27  6:54 UTC (permalink / raw)
  To: jaegeuk, chao, yuchao0, yunlong.song, yunlong.song
  Cc: miaoxie, bintian.wang, linux-fsdevel, linux-f2fs-devel, linux-kernel

Sometimes f2fs_gc is called with no target victim (e.g. xfstest
generic/027, ndirty_node:545 ndiry_dent:1 ndirty_imeta:513 rsvd_segs:21
free_segs:27, has_not_enough_free_secs will return true). This patch
first merges pages and then converts into sections.

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
---
 fs/f2fs/f2fs.h    |  9 ---------
 fs/f2fs/segment.c | 12 +++++++-----
 fs/f2fs/segment.h | 13 +++++++++----
 3 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index ca6b0c9..e89cff7 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1675,15 +1675,6 @@ static inline int get_dirty_pages(struct inode *inode)
 	return atomic_read(&F2FS_I(inode)->dirty_pages);
 }
 
-static inline int get_blocktype_secs(struct f2fs_sb_info *sbi, int block_type)
-{
-	unsigned int pages_per_sec = sbi->segs_per_sec * sbi->blocks_per_seg;
-	unsigned int segs = (get_pages(sbi, block_type) + pages_per_sec - 1) >>
-						sbi->log_blocks_per_seg;
-
-	return segs / sbi->segs_per_sec;
-}
-
 static inline block_t valid_user_blocks(struct f2fs_sb_info *sbi)
 {
 	return sbi->total_valid_block_count;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index c117e09..603f805 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -171,17 +171,19 @@ static unsigned long __find_rev_next_zero_bit(const unsigned long *addr,
 
 bool need_SSR(struct f2fs_sb_info *sbi)
 {
-	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
-	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
-	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
+	s64 node_pages = get_pages(sbi, F2FS_DIRTY_NODES);
+	s64 dent_pages = get_pages(sbi, F2FS_DIRTY_DENTS);
+	s64 imeta_pages = get_pages(sbi, F2FS_DIRTY_IMETA);
 
 	if (test_opt(sbi, LFS))
 		return false;
 	if (sbi->gc_thread && sbi->gc_thread->gc_urgent)
 		return true;
 
-	return free_sections(sbi) <= (node_secs + 2 * dent_secs + imeta_secs +
-			SM_I(sbi)->min_ssr_sections + reserved_sections(sbi));
+	return free_sections(sbi) <=
+		(PAGE2SEC(sbi, node_pages + imeta_pages) +
+		PAGE2SEC(sbi, 2 * dent_pages) +
+		SM_I(sbi)->min_ssr_sections + reserved_sections(sbi));
 }
 
 void register_inmem_page(struct inode *inode, struct page *page)
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index d1d394c..723d79e 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -115,6 +115,10 @@
 #define SECTOR_TO_BLOCK(sectors)					\
 	((sectors) >> F2FS_LOG_SECTORS_PER_BLOCK)
 
+#define PAGE2SEC(sbi, pages)				\
+	((((pages) + BLKS_PER_SEC(sbi) - 1)		\
+	>> sbi->log_blocks_per_seg) / sbi->segs_per_sec)
+
 /*
  * indicate a block allocation direction: RIGHT and LEFT.
  * RIGHT means allocating new sections towards the end of volume.
@@ -527,9 +531,9 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
 static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
 					int freed, int needed)
 {
-	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
-	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
-	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
+	s64 node_pages = get_pages(sbi, F2FS_DIRTY_NODES);
+	s64 dent_pages = get_pages(sbi, F2FS_DIRTY_DENTS);
+	s64 imeta_pages = get_pages(sbi, F2FS_DIRTY_IMETA);
 
 	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
 		return false;
@@ -538,7 +542,8 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
 			has_curseg_enough_space(sbi))
 		return false;
 	return (free_sections(sbi) + freed) <=
-		(node_secs + 2 * dent_secs + imeta_secs +
+		(PAGE2SEC(sbi, node_pages + imeta_pages) +
+		PAGE2SEC(sbi, 2 * dent_pages) +
 		reserved_sections(sbi) + needed);
 }
 
-- 
1.8.5.2

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

* [PATCH] f2fs: avoid false positive of free secs check
@ 2017-11-27  6:54 ` Yunlong Song
  0 siblings, 0 replies; 9+ messages in thread
From: Yunlong Song @ 2017-11-27  6:54 UTC (permalink / raw)
  To: jaegeuk, chao, yuchao0, yunlong.song, yunlong.song
  Cc: miaoxie, bintian.wang, linux-fsdevel, linux-f2fs-devel, linux-kernel

Sometimes f2fs_gc is called with no target victim (e.g. xfstest
generic/027, ndirty_node:545 ndiry_dent:1 ndirty_imeta:513 rsvd_segs:21
free_segs:27, has_not_enough_free_secs will return true). This patch
first merges pages and then converts into sections.

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
---
 fs/f2fs/f2fs.h    |  9 ---------
 fs/f2fs/segment.c | 12 +++++++-----
 fs/f2fs/segment.h | 13 +++++++++----
 3 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index ca6b0c9..e89cff7 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1675,15 +1675,6 @@ static inline int get_dirty_pages(struct inode *inode)
 	return atomic_read(&F2FS_I(inode)->dirty_pages);
 }
 
-static inline int get_blocktype_secs(struct f2fs_sb_info *sbi, int block_type)
-{
-	unsigned int pages_per_sec = sbi->segs_per_sec * sbi->blocks_per_seg;
-	unsigned int segs = (get_pages(sbi, block_type) + pages_per_sec - 1) >>
-						sbi->log_blocks_per_seg;
-
-	return segs / sbi->segs_per_sec;
-}
-
 static inline block_t valid_user_blocks(struct f2fs_sb_info *sbi)
 {
 	return sbi->total_valid_block_count;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index c117e09..603f805 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -171,17 +171,19 @@ static unsigned long __find_rev_next_zero_bit(const unsigned long *addr,
 
 bool need_SSR(struct f2fs_sb_info *sbi)
 {
-	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
-	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
-	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
+	s64 node_pages = get_pages(sbi, F2FS_DIRTY_NODES);
+	s64 dent_pages = get_pages(sbi, F2FS_DIRTY_DENTS);
+	s64 imeta_pages = get_pages(sbi, F2FS_DIRTY_IMETA);
 
 	if (test_opt(sbi, LFS))
 		return false;
 	if (sbi->gc_thread && sbi->gc_thread->gc_urgent)
 		return true;
 
-	return free_sections(sbi) <= (node_secs + 2 * dent_secs + imeta_secs +
-			SM_I(sbi)->min_ssr_sections + reserved_sections(sbi));
+	return free_sections(sbi) <=
+		(PAGE2SEC(sbi, node_pages + imeta_pages) +
+		PAGE2SEC(sbi, 2 * dent_pages) +
+		SM_I(sbi)->min_ssr_sections + reserved_sections(sbi));
 }
 
 void register_inmem_page(struct inode *inode, struct page *page)
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index d1d394c..723d79e 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -115,6 +115,10 @@
 #define SECTOR_TO_BLOCK(sectors)					\
 	((sectors) >> F2FS_LOG_SECTORS_PER_BLOCK)
 
+#define PAGE2SEC(sbi, pages)				\
+	((((pages) + BLKS_PER_SEC(sbi) - 1)		\
+	>> sbi->log_blocks_per_seg) / sbi->segs_per_sec)
+
 /*
  * indicate a block allocation direction: RIGHT and LEFT.
  * RIGHT means allocating new sections towards the end of volume.
@@ -527,9 +531,9 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
 static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
 					int freed, int needed)
 {
-	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
-	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
-	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
+	s64 node_pages = get_pages(sbi, F2FS_DIRTY_NODES);
+	s64 dent_pages = get_pages(sbi, F2FS_DIRTY_DENTS);
+	s64 imeta_pages = get_pages(sbi, F2FS_DIRTY_IMETA);
 
 	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
 		return false;
@@ -538,7 +542,8 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
 			has_curseg_enough_space(sbi))
 		return false;
 	return (free_sections(sbi) + freed) <=
-		(node_secs + 2 * dent_secs + imeta_secs +
+		(PAGE2SEC(sbi, node_pages + imeta_pages) +
+		PAGE2SEC(sbi, 2 * dent_pages) +
 		reserved_sections(sbi) + needed);
 }
 
-- 
1.8.5.2

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

* Re: [PATCH] f2fs: avoid false positive of free secs check
  2017-11-27  6:54 ` Yunlong Song
@ 2017-11-28 13:03   ` kbuild test robot
  -1 siblings, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2017-11-28 13:03 UTC (permalink / raw)
  To: Yunlong Song
  Cc: kbuild-all, jaegeuk, chao, yuchao0, yunlong.song, yunlong.song,
	miaoxie, bintian.wang, linux-fsdevel, linux-f2fs-devel,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2818 bytes --]

Hi Yunlong,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on f2fs/dev-test]
[also build test ERROR on v4.15-rc1 next-20171128]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Yunlong-Song/f2fs-avoid-false-positive-of-free-secs-check/20171128-180549
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git dev-test
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   WARNING: modpost: missing MODULE_LICENSE() in drivers/auxdisplay/img-ascii-lcd.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/gpio/gpio-ath79.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/gpio/gpio-iop.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/iio/accel/kxsd9-i2c.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/iio/adc/qcom-vadc-common.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/media/platform/mtk-vcodec/mtk-vcodec-common.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/media/platform/soc_camera/soc_scale_crop.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/media/platform/tegra-cec/tegra_cec.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/mtd/nand/denali_pci.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/net/ethernet/cirrus/cs89x0.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/pinctrl/pxa/pinctrl-pxa2xx.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/power/reset/zx-reboot.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/staging/comedi/drivers/ni_atmio.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in sound/soc/codecs/snd-soc-pcm512x-spi.o
   see include/linux/module.h for more information
>> ERROR: "__divdi3" [fs/f2fs/f2fs.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 62438 bytes --]

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

* Re: [PATCH] f2fs: avoid false positive of free secs check
@ 2017-11-28 13:03   ` kbuild test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2017-11-28 13:03 UTC (permalink / raw)
  Cc: kbuild-all, jaegeuk, chao, yuchao0, yunlong.song, yunlong.song,
	miaoxie, bintian.wang, linux-fsdevel, linux-f2fs-devel,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2818 bytes --]

Hi Yunlong,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on f2fs/dev-test]
[also build test ERROR on v4.15-rc1 next-20171128]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Yunlong-Song/f2fs-avoid-false-positive-of-free-secs-check/20171128-180549
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git dev-test
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   WARNING: modpost: missing MODULE_LICENSE() in drivers/auxdisplay/img-ascii-lcd.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/gpio/gpio-ath79.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/gpio/gpio-iop.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/iio/accel/kxsd9-i2c.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/iio/adc/qcom-vadc-common.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/media/platform/mtk-vcodec/mtk-vcodec-common.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/media/platform/soc_camera/soc_scale_crop.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/media/platform/tegra-cec/tegra_cec.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/mtd/nand/denali_pci.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/net/ethernet/cirrus/cs89x0.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/pinctrl/pxa/pinctrl-pxa2xx.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/power/reset/zx-reboot.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in drivers/staging/comedi/drivers/ni_atmio.o
   see include/linux/module.h for more information
   WARNING: modpost: missing MODULE_LICENSE() in sound/soc/codecs/snd-soc-pcm512x-spi.o
   see include/linux/module.h for more information
>> ERROR: "__divdi3" [fs/f2fs/f2fs.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 62438 bytes --]

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

* Re: [PATCH] f2fs: avoid false positive of free secs check
  2017-11-27  6:54 ` Yunlong Song
  (?)
  (?)
@ 2017-11-29 11:56 ` Chao Yu
  2017-11-30  2:42     ` Yunlong Song
  -1 siblings, 1 reply; 9+ messages in thread
From: Chao Yu @ 2017-11-29 11:56 UTC (permalink / raw)
  To: Yunlong Song, jaegeuk, yuchao0, yunlong.song
  Cc: miaoxie, bintian.wang, linux-fsdevel, linux-f2fs-devel, linux-kernel

On 2017/11/27 14:54, Yunlong Song wrote:
> Sometimes f2fs_gc is called with no target victim (e.g. xfstest
> generic/027, ndirty_node:545 ndiry_dent:1 ndirty_imeta:513 rsvd_segs:21
> free_segs:27, has_not_enough_free_secs will return true). This patch
> first merges pages and then converts into sections.

I don't think this could be right, IMO, instead, it would be better to
account dirty hot/warm/cold nodes or imeta separately, as actually, they
will use different section, but currently, our calculation way is based
on that they could be written to same section.

Thanks,

> 
> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
> ---
>  fs/f2fs/f2fs.h    |  9 ---------
>  fs/f2fs/segment.c | 12 +++++++-----
>  fs/f2fs/segment.h | 13 +++++++++----
>  3 files changed, 16 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index ca6b0c9..e89cff7 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1675,15 +1675,6 @@ static inline int get_dirty_pages(struct inode *inode)
>  	return atomic_read(&F2FS_I(inode)->dirty_pages);
>  }
>  
> -static inline int get_blocktype_secs(struct f2fs_sb_info *sbi, int block_type)
> -{
> -	unsigned int pages_per_sec = sbi->segs_per_sec * sbi->blocks_per_seg;
> -	unsigned int segs = (get_pages(sbi, block_type) + pages_per_sec - 1) >>
> -						sbi->log_blocks_per_seg;
> -
> -	return segs / sbi->segs_per_sec;
> -}
> -
>  static inline block_t valid_user_blocks(struct f2fs_sb_info *sbi)
>  {
>  	return sbi->total_valid_block_count;
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index c117e09..603f805 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -171,17 +171,19 @@ static unsigned long __find_rev_next_zero_bit(const unsigned long *addr,
>  
>  bool need_SSR(struct f2fs_sb_info *sbi)
>  {
> -	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
> -	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
> -	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
> +	s64 node_pages = get_pages(sbi, F2FS_DIRTY_NODES);
> +	s64 dent_pages = get_pages(sbi, F2FS_DIRTY_DENTS);
> +	s64 imeta_pages = get_pages(sbi, F2FS_DIRTY_IMETA);
>  
>  	if (test_opt(sbi, LFS))
>  		return false;
>  	if (sbi->gc_thread && sbi->gc_thread->gc_urgent)
>  		return true;
>  
> -	return free_sections(sbi) <= (node_secs + 2 * dent_secs + imeta_secs +
> -			SM_I(sbi)->min_ssr_sections + reserved_sections(sbi));
> +	return free_sections(sbi) <=
> +		(PAGE2SEC(sbi, node_pages + imeta_pages) +
> +		PAGE2SEC(sbi, 2 * dent_pages) +
> +		SM_I(sbi)->min_ssr_sections + reserved_sections(sbi));
>  }
>  
>  void register_inmem_page(struct inode *inode, struct page *page)
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index d1d394c..723d79e 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -115,6 +115,10 @@
>  #define SECTOR_TO_BLOCK(sectors)					\
>  	((sectors) >> F2FS_LOG_SECTORS_PER_BLOCK)
>  
> +#define PAGE2SEC(sbi, pages)				\
> +	((((pages) + BLKS_PER_SEC(sbi) - 1)		\
> +	>> sbi->log_blocks_per_seg) / sbi->segs_per_sec)
> +
>  /*
>   * indicate a block allocation direction: RIGHT and LEFT.
>   * RIGHT means allocating new sections towards the end of volume.
> @@ -527,9 +531,9 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
>  static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>  					int freed, int needed)
>  {
> -	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
> -	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
> -	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
> +	s64 node_pages = get_pages(sbi, F2FS_DIRTY_NODES);
> +	s64 dent_pages = get_pages(sbi, F2FS_DIRTY_DENTS);
> +	s64 imeta_pages = get_pages(sbi, F2FS_DIRTY_IMETA);
>  
>  	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
>  		return false;
> @@ -538,7 +542,8 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>  			has_curseg_enough_space(sbi))
>  		return false;
>  	return (free_sections(sbi) + freed) <=
> -		(node_secs + 2 * dent_secs + imeta_secs +
> +		(PAGE2SEC(sbi, node_pages + imeta_pages) +
> +		PAGE2SEC(sbi, 2 * dent_pages) +
>  		reserved_sections(sbi) + needed);
>  }
>  
> 

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

* Re: [PATCH] f2fs: avoid false positive of free secs check
  2017-11-29 11:56 ` Chao Yu
@ 2017-11-30  2:42     ` Yunlong Song
  0 siblings, 0 replies; 9+ messages in thread
From: Yunlong Song @ 2017-11-30  2:42 UTC (permalink / raw)
  To: Chao Yu, jaegeuk, yuchao0, yunlong.song
  Cc: miaoxie, bintian.wang, linux-fsdevel, linux-f2fs-devel, linux-kernel

SSR can make hot/warm/cold nodes written together, so why should we account
them different?

On 2017/11/29 19:56, Chao Yu wrote:
> On 2017/11/27 14:54, Yunlong Song wrote:
>> Sometimes f2fs_gc is called with no target victim (e.g. xfstest
>> generic/027, ndirty_node:545 ndiry_dent:1 ndirty_imeta:513 rsvd_segs:21
>> free_segs:27, has_not_enough_free_secs will return true). This patch
>> first merges pages and then converts into sections.
> I don't think this could be right, IMO, instead, it would be better to
> account dirty hot/warm/cold nodes or imeta separately, as actually, they
> will use different section, but currently, our calculation way is based
> on that they could be written to same section.
>
> Thanks,
>
>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>> ---
>>   fs/f2fs/f2fs.h    |  9 ---------
>>   fs/f2fs/segment.c | 12 +++++++-----
>>   fs/f2fs/segment.h | 13 +++++++++----
>>   3 files changed, 16 insertions(+), 18 deletions(-)
>>
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index ca6b0c9..e89cff7 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -1675,15 +1675,6 @@ static inline int get_dirty_pages(struct inode *inode)
>>   	return atomic_read(&F2FS_I(inode)->dirty_pages);
>>   }
>>   
>> -static inline int get_blocktype_secs(struct f2fs_sb_info *sbi, int block_type)
>> -{
>> -	unsigned int pages_per_sec = sbi->segs_per_sec * sbi->blocks_per_seg;
>> -	unsigned int segs = (get_pages(sbi, block_type) + pages_per_sec - 1) >>
>> -						sbi->log_blocks_per_seg;
>> -
>> -	return segs / sbi->segs_per_sec;
>> -}
>> -
>>   static inline block_t valid_user_blocks(struct f2fs_sb_info *sbi)
>>   {
>>   	return sbi->total_valid_block_count;
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index c117e09..603f805 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -171,17 +171,19 @@ static unsigned long __find_rev_next_zero_bit(const unsigned long *addr,
>>   
>>   bool need_SSR(struct f2fs_sb_info *sbi)
>>   {
>> -	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
>> -	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
>> -	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
>> +	s64 node_pages = get_pages(sbi, F2FS_DIRTY_NODES);
>> +	s64 dent_pages = get_pages(sbi, F2FS_DIRTY_DENTS);
>> +	s64 imeta_pages = get_pages(sbi, F2FS_DIRTY_IMETA);
>>   
>>   	if (test_opt(sbi, LFS))
>>   		return false;
>>   	if (sbi->gc_thread && sbi->gc_thread->gc_urgent)
>>   		return true;
>>   
>> -	return free_sections(sbi) <= (node_secs + 2 * dent_secs + imeta_secs +
>> -			SM_I(sbi)->min_ssr_sections + reserved_sections(sbi));
>> +	return free_sections(sbi) <=
>> +		(PAGE2SEC(sbi, node_pages + imeta_pages) +
>> +		PAGE2SEC(sbi, 2 * dent_pages) +
>> +		SM_I(sbi)->min_ssr_sections + reserved_sections(sbi));
>>   }
>>   
>>   void register_inmem_page(struct inode *inode, struct page *page)
>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>> index d1d394c..723d79e 100644
>> --- a/fs/f2fs/segment.h
>> +++ b/fs/f2fs/segment.h
>> @@ -115,6 +115,10 @@
>>   #define SECTOR_TO_BLOCK(sectors)					\
>>   	((sectors) >> F2FS_LOG_SECTORS_PER_BLOCK)
>>   
>> +#define PAGE2SEC(sbi, pages)				\
>> +	((((pages) + BLKS_PER_SEC(sbi) - 1)		\
>> +	>> sbi->log_blocks_per_seg) / sbi->segs_per_sec)
>> +
>>   /*
>>    * indicate a block allocation direction: RIGHT and LEFT.
>>    * RIGHT means allocating new sections towards the end of volume.
>> @@ -527,9 +531,9 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
>>   static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>>   					int freed, int needed)
>>   {
>> -	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
>> -	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
>> -	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
>> +	s64 node_pages = get_pages(sbi, F2FS_DIRTY_NODES);
>> +	s64 dent_pages = get_pages(sbi, F2FS_DIRTY_DENTS);
>> +	s64 imeta_pages = get_pages(sbi, F2FS_DIRTY_IMETA);
>>   
>>   	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
>>   		return false;
>> @@ -538,7 +542,8 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>>   			has_curseg_enough_space(sbi))
>>   		return false;
>>   	return (free_sections(sbi) + freed) <=
>> -		(node_secs + 2 * dent_secs + imeta_secs +
>> +		(PAGE2SEC(sbi, node_pages + imeta_pages) +
>> +		PAGE2SEC(sbi, 2 * dent_pages) +
>>   		reserved_sections(sbi) + needed);
>>   }
>>   
>>
> .
>

-- 
Thanks,
Yunlong Song

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

* Re: [PATCH] f2fs: avoid false positive of free secs check
@ 2017-11-30  2:42     ` Yunlong Song
  0 siblings, 0 replies; 9+ messages in thread
From: Yunlong Song @ 2017-11-30  2:42 UTC (permalink / raw)
  To: Chao Yu, jaegeuk, yuchao0, yunlong.song
  Cc: miaoxie, bintian.wang, linux-fsdevel, linux-f2fs-devel, linux-kernel

SSR can make hot/warm/cold nodes written together, so why should we account
them different?

On 2017/11/29 19:56, Chao Yu wrote:
> On 2017/11/27 14:54, Yunlong Song wrote:
>> Sometimes f2fs_gc is called with no target victim (e.g. xfstest
>> generic/027, ndirty_node:545 ndiry_dent:1 ndirty_imeta:513 rsvd_segs:21
>> free_segs:27, has_not_enough_free_secs will return true). This patch
>> first merges pages and then converts into sections.
> I don't think this could be right, IMO, instead, it would be better to
> account dirty hot/warm/cold nodes or imeta separately, as actually, they
> will use different section, but currently, our calculation way is based
> on that they could be written to same section.
>
> Thanks,
>
>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>> ---
>>   fs/f2fs/f2fs.h    |  9 ---------
>>   fs/f2fs/segment.c | 12 +++++++-----
>>   fs/f2fs/segment.h | 13 +++++++++----
>>   3 files changed, 16 insertions(+), 18 deletions(-)
>>
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index ca6b0c9..e89cff7 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -1675,15 +1675,6 @@ static inline int get_dirty_pages(struct inode *inode)
>>   	return atomic_read(&F2FS_I(inode)->dirty_pages);
>>   }
>>   
>> -static inline int get_blocktype_secs(struct f2fs_sb_info *sbi, int block_type)
>> -{
>> -	unsigned int pages_per_sec = sbi->segs_per_sec * sbi->blocks_per_seg;
>> -	unsigned int segs = (get_pages(sbi, block_type) + pages_per_sec - 1) >>
>> -						sbi->log_blocks_per_seg;
>> -
>> -	return segs / sbi->segs_per_sec;
>> -}
>> -
>>   static inline block_t valid_user_blocks(struct f2fs_sb_info *sbi)
>>   {
>>   	return sbi->total_valid_block_count;
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index c117e09..603f805 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -171,17 +171,19 @@ static unsigned long __find_rev_next_zero_bit(const unsigned long *addr,
>>   
>>   bool need_SSR(struct f2fs_sb_info *sbi)
>>   {
>> -	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
>> -	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
>> -	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
>> +	s64 node_pages = get_pages(sbi, F2FS_DIRTY_NODES);
>> +	s64 dent_pages = get_pages(sbi, F2FS_DIRTY_DENTS);
>> +	s64 imeta_pages = get_pages(sbi, F2FS_DIRTY_IMETA);
>>   
>>   	if (test_opt(sbi, LFS))
>>   		return false;
>>   	if (sbi->gc_thread && sbi->gc_thread->gc_urgent)
>>   		return true;
>>   
>> -	return free_sections(sbi) <= (node_secs + 2 * dent_secs + imeta_secs +
>> -			SM_I(sbi)->min_ssr_sections + reserved_sections(sbi));
>> +	return free_sections(sbi) <=
>> +		(PAGE2SEC(sbi, node_pages + imeta_pages) +
>> +		PAGE2SEC(sbi, 2 * dent_pages) +
>> +		SM_I(sbi)->min_ssr_sections + reserved_sections(sbi));
>>   }
>>   
>>   void register_inmem_page(struct inode *inode, struct page *page)
>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>> index d1d394c..723d79e 100644
>> --- a/fs/f2fs/segment.h
>> +++ b/fs/f2fs/segment.h
>> @@ -115,6 +115,10 @@
>>   #define SECTOR_TO_BLOCK(sectors)					\
>>   	((sectors) >> F2FS_LOG_SECTORS_PER_BLOCK)
>>   
>> +#define PAGE2SEC(sbi, pages)				\
>> +	((((pages) + BLKS_PER_SEC(sbi) - 1)		\
>> +	>> sbi->log_blocks_per_seg) / sbi->segs_per_sec)
>> +
>>   /*
>>    * indicate a block allocation direction: RIGHT and LEFT.
>>    * RIGHT means allocating new sections towards the end of volume.
>> @@ -527,9 +531,9 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
>>   static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>>   					int freed, int needed)
>>   {
>> -	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
>> -	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
>> -	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
>> +	s64 node_pages = get_pages(sbi, F2FS_DIRTY_NODES);
>> +	s64 dent_pages = get_pages(sbi, F2FS_DIRTY_DENTS);
>> +	s64 imeta_pages = get_pages(sbi, F2FS_DIRTY_IMETA);
>>   
>>   	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
>>   		return false;
>> @@ -538,7 +542,8 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>>   			has_curseg_enough_space(sbi))
>>   		return false;
>>   	return (free_sections(sbi) + freed) <=
>> -		(node_secs + 2 * dent_secs + imeta_secs +
>> +		(PAGE2SEC(sbi, node_pages + imeta_pages) +
>> +		PAGE2SEC(sbi, 2 * dent_pages) +
>>   		reserved_sections(sbi) + needed);
>>   }
>>   
>>
> .
>

-- 
Thanks,
Yunlong Song

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

* Re: [PATCH] f2fs: avoid false positive of free secs check
  2017-11-30  2:42     ` Yunlong Song
@ 2017-11-30  7:15       ` Chao Yu
  -1 siblings, 0 replies; 9+ messages in thread
From: Chao Yu @ 2017-11-30  7:15 UTC (permalink / raw)
  To: Yunlong Song, Chao Yu, jaegeuk, yunlong.song
  Cc: miaoxie, bintian.wang, linux-fsdevel, linux-f2fs-devel, linux-kernel

On 2017/11/30 10:42, Yunlong Song wrote:
> SSR can make hot/warm/cold nodes written together, so why should we account
> them different?

Current segment which is using ssr allocation has only one valid type, so we
can not write data/node with different type into current segment which already
has fixed type, right?

Thanks,

> 
> On 2017/11/29 19:56, Chao Yu wrote:
>> On 2017/11/27 14:54, Yunlong Song wrote:
>>> Sometimes f2fs_gc is called with no target victim (e.g. xfstest
>>> generic/027, ndirty_node:545 ndiry_dent:1 ndirty_imeta:513 rsvd_segs:21
>>> free_segs:27, has_not_enough_free_secs will return true). This patch
>>> first merges pages and then converts into sections.
>> I don't think this could be right, IMO, instead, it would be better to
>> account dirty hot/warm/cold nodes or imeta separately, as actually, they
>> will use different section, but currently, our calculation way is based
>> on that they could be written to same section.
>>
>> Thanks,
>>
>>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>>> ---
>>>   fs/f2fs/f2fs.h    |  9 ---------
>>>   fs/f2fs/segment.c | 12 +++++++-----
>>>   fs/f2fs/segment.h | 13 +++++++++----
>>>   3 files changed, 16 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index ca6b0c9..e89cff7 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -1675,15 +1675,6 @@ static inline int get_dirty_pages(struct inode *inode)
>>>   	return atomic_read(&F2FS_I(inode)->dirty_pages);
>>>   }
>>>   
>>> -static inline int get_blocktype_secs(struct f2fs_sb_info *sbi, int block_type)
>>> -{
>>> -	unsigned int pages_per_sec = sbi->segs_per_sec * sbi->blocks_per_seg;
>>> -	unsigned int segs = (get_pages(sbi, block_type) + pages_per_sec - 1) >>
>>> -						sbi->log_blocks_per_seg;
>>> -
>>> -	return segs / sbi->segs_per_sec;
>>> -}
>>> -
>>>   static inline block_t valid_user_blocks(struct f2fs_sb_info *sbi)
>>>   {
>>>   	return sbi->total_valid_block_count;
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index c117e09..603f805 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -171,17 +171,19 @@ static unsigned long __find_rev_next_zero_bit(const unsigned long *addr,
>>>   
>>>   bool need_SSR(struct f2fs_sb_info *sbi)
>>>   {
>>> -	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
>>> -	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
>>> -	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
>>> +	s64 node_pages = get_pages(sbi, F2FS_DIRTY_NODES);
>>> +	s64 dent_pages = get_pages(sbi, F2FS_DIRTY_DENTS);
>>> +	s64 imeta_pages = get_pages(sbi, F2FS_DIRTY_IMETA);
>>>   
>>>   	if (test_opt(sbi, LFS))
>>>   		return false;
>>>   	if (sbi->gc_thread && sbi->gc_thread->gc_urgent)
>>>   		return true;
>>>   
>>> -	return free_sections(sbi) <= (node_secs + 2 * dent_secs + imeta_secs +
>>> -			SM_I(sbi)->min_ssr_sections + reserved_sections(sbi));
>>> +	return free_sections(sbi) <=
>>> +		(PAGE2SEC(sbi, node_pages + imeta_pages) +
>>> +		PAGE2SEC(sbi, 2 * dent_pages) +
>>> +		SM_I(sbi)->min_ssr_sections + reserved_sections(sbi));
>>>   }
>>>   
>>>   void register_inmem_page(struct inode *inode, struct page *page)
>>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>>> index d1d394c..723d79e 100644
>>> --- a/fs/f2fs/segment.h
>>> +++ b/fs/f2fs/segment.h
>>> @@ -115,6 +115,10 @@
>>>   #define SECTOR_TO_BLOCK(sectors)					\
>>>   	((sectors) >> F2FS_LOG_SECTORS_PER_BLOCK)
>>>   
>>> +#define PAGE2SEC(sbi, pages)				\
>>> +	((((pages) + BLKS_PER_SEC(sbi) - 1)		\
>>> +	>> sbi->log_blocks_per_seg) / sbi->segs_per_sec)
>>> +
>>>   /*
>>>    * indicate a block allocation direction: RIGHT and LEFT.
>>>    * RIGHT means allocating new sections towards the end of volume.
>>> @@ -527,9 +531,9 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
>>>   static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>>>   					int freed, int needed)
>>>   {
>>> -	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
>>> -	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
>>> -	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
>>> +	s64 node_pages = get_pages(sbi, F2FS_DIRTY_NODES);
>>> +	s64 dent_pages = get_pages(sbi, F2FS_DIRTY_DENTS);
>>> +	s64 imeta_pages = get_pages(sbi, F2FS_DIRTY_IMETA);
>>>   
>>>   	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
>>>   		return false;
>>> @@ -538,7 +542,8 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>>>   			has_curseg_enough_space(sbi))
>>>   		return false;
>>>   	return (free_sections(sbi) + freed) <=
>>> -		(node_secs + 2 * dent_secs + imeta_secs +
>>> +		(PAGE2SEC(sbi, node_pages + imeta_pages) +
>>> +		PAGE2SEC(sbi, 2 * dent_pages) +
>>>   		reserved_sections(sbi) + needed);
>>>   }
>>>   
>>>
>> .
>>
> 

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

* Re: [PATCH] f2fs: avoid false positive of free secs check
@ 2017-11-30  7:15       ` Chao Yu
  0 siblings, 0 replies; 9+ messages in thread
From: Chao Yu @ 2017-11-30  7:15 UTC (permalink / raw)
  To: Yunlong Song, Chao Yu, jaegeuk, yunlong.song
  Cc: miaoxie, bintian.wang, linux-fsdevel, linux-f2fs-devel, linux-kernel

On 2017/11/30 10:42, Yunlong Song wrote:
> SSR can make hot/warm/cold nodes written together, so why should we account
> them different?

Current segment which is using ssr allocation has only one valid type, so we
can not write data/node with different type into current segment which already
has fixed type, right?

Thanks,

> 
> On 2017/11/29 19:56, Chao Yu wrote:
>> On 2017/11/27 14:54, Yunlong Song wrote:
>>> Sometimes f2fs_gc is called with no target victim (e.g. xfstest
>>> generic/027, ndirty_node:545 ndiry_dent:1 ndirty_imeta:513 rsvd_segs:21
>>> free_segs:27, has_not_enough_free_secs will return true). This patch
>>> first merges pages and then converts into sections.
>> I don't think this could be right, IMO, instead, it would be better to
>> account dirty hot/warm/cold nodes or imeta separately, as actually, they
>> will use different section, but currently, our calculation way is based
>> on that they could be written to same section.
>>
>> Thanks,
>>
>>> Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
>>> ---
>>>   fs/f2fs/f2fs.h    |  9 ---------
>>>   fs/f2fs/segment.c | 12 +++++++-----
>>>   fs/f2fs/segment.h | 13 +++++++++----
>>>   3 files changed, 16 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index ca6b0c9..e89cff7 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -1675,15 +1675,6 @@ static inline int get_dirty_pages(struct inode *inode)
>>>   	return atomic_read(&F2FS_I(inode)->dirty_pages);
>>>   }
>>>   
>>> -static inline int get_blocktype_secs(struct f2fs_sb_info *sbi, int block_type)
>>> -{
>>> -	unsigned int pages_per_sec = sbi->segs_per_sec * sbi->blocks_per_seg;
>>> -	unsigned int segs = (get_pages(sbi, block_type) + pages_per_sec - 1) >>
>>> -						sbi->log_blocks_per_seg;
>>> -
>>> -	return segs / sbi->segs_per_sec;
>>> -}
>>> -
>>>   static inline block_t valid_user_blocks(struct f2fs_sb_info *sbi)
>>>   {
>>>   	return sbi->total_valid_block_count;
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index c117e09..603f805 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -171,17 +171,19 @@ static unsigned long __find_rev_next_zero_bit(const unsigned long *addr,
>>>   
>>>   bool need_SSR(struct f2fs_sb_info *sbi)
>>>   {
>>> -	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
>>> -	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
>>> -	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
>>> +	s64 node_pages = get_pages(sbi, F2FS_DIRTY_NODES);
>>> +	s64 dent_pages = get_pages(sbi, F2FS_DIRTY_DENTS);
>>> +	s64 imeta_pages = get_pages(sbi, F2FS_DIRTY_IMETA);
>>>   
>>>   	if (test_opt(sbi, LFS))
>>>   		return false;
>>>   	if (sbi->gc_thread && sbi->gc_thread->gc_urgent)
>>>   		return true;
>>>   
>>> -	return free_sections(sbi) <= (node_secs + 2 * dent_secs + imeta_secs +
>>> -			SM_I(sbi)->min_ssr_sections + reserved_sections(sbi));
>>> +	return free_sections(sbi) <=
>>> +		(PAGE2SEC(sbi, node_pages + imeta_pages) +
>>> +		PAGE2SEC(sbi, 2 * dent_pages) +
>>> +		SM_I(sbi)->min_ssr_sections + reserved_sections(sbi));
>>>   }
>>>   
>>>   void register_inmem_page(struct inode *inode, struct page *page)
>>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>>> index d1d394c..723d79e 100644
>>> --- a/fs/f2fs/segment.h
>>> +++ b/fs/f2fs/segment.h
>>> @@ -115,6 +115,10 @@
>>>   #define SECTOR_TO_BLOCK(sectors)					\
>>>   	((sectors) >> F2FS_LOG_SECTORS_PER_BLOCK)
>>>   
>>> +#define PAGE2SEC(sbi, pages)				\
>>> +	((((pages) + BLKS_PER_SEC(sbi) - 1)		\
>>> +	>> sbi->log_blocks_per_seg) / sbi->segs_per_sec)
>>> +
>>>   /*
>>>    * indicate a block allocation direction: RIGHT and LEFT.
>>>    * RIGHT means allocating new sections towards the end of volume.
>>> @@ -527,9 +531,9 @@ static inline bool has_curseg_enough_space(struct f2fs_sb_info *sbi)
>>>   static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>>>   					int freed, int needed)
>>>   {
>>> -	int node_secs = get_blocktype_secs(sbi, F2FS_DIRTY_NODES);
>>> -	int dent_secs = get_blocktype_secs(sbi, F2FS_DIRTY_DENTS);
>>> -	int imeta_secs = get_blocktype_secs(sbi, F2FS_DIRTY_IMETA);
>>> +	s64 node_pages = get_pages(sbi, F2FS_DIRTY_NODES);
>>> +	s64 dent_pages = get_pages(sbi, F2FS_DIRTY_DENTS);
>>> +	s64 imeta_pages = get_pages(sbi, F2FS_DIRTY_IMETA);
>>>   
>>>   	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
>>>   		return false;
>>> @@ -538,7 +542,8 @@ static inline bool has_not_enough_free_secs(struct f2fs_sb_info *sbi,
>>>   			has_curseg_enough_space(sbi))
>>>   		return false;
>>>   	return (free_sections(sbi) + freed) <=
>>> -		(node_secs + 2 * dent_secs + imeta_secs +
>>> +		(PAGE2SEC(sbi, node_pages + imeta_pages) +
>>> +		PAGE2SEC(sbi, 2 * dent_pages) +
>>>   		reserved_sections(sbi) + needed);
>>>   }
>>>   
>>>
>> .
>>
> 

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

end of thread, other threads:[~2017-11-30  7:15 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-27  6:54 [PATCH] f2fs: avoid false positive of free secs check Yunlong Song
2017-11-27  6:54 ` Yunlong Song
2017-11-28 13:03 ` kbuild test robot
2017-11-28 13:03   ` kbuild test robot
2017-11-29 11:56 ` Chao Yu
2017-11-30  2:42   ` Yunlong Song
2017-11-30  2:42     ` Yunlong Song
2017-11-30  7:15     ` Chao Yu
2017-11-30  7:15       ` Chao Yu

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.