All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] f2fs: cover update_free_nid_bitmap with free_list_lock
@ 2017-03-13 12:10 ` Chao Yu
  0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2017-03-13 12:10 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

free_nid_bitmap and free_nid_count in update_free_nid_bitmap should be
updated atomically, use free_list_lock cover them to avoid race in
concurrent scenario.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/f2fs.h |  1 -
 fs/f2fs/node.c | 27 +++++++++++----------------
 2 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 843ce39502bc..d905e53ba524 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -561,7 +561,6 @@ struct f2fs_nm_info {
 	unsigned char (*free_nid_bitmap)[NAT_ENTRY_BITMAP_SIZE];
 	unsigned char *nat_block_bitmap;
 	unsigned short *free_nid_count;	/* free nid count of NAT block */
-	spinlock_t free_nid_lock;	/* protect updating of nid count */
 
 	/* for checkpoint */
 	char *nat_bitmap;		/* NAT bitmap pointer */
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 11df8ab32478..3bfffd744f87 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1815,7 +1815,7 @@ static void remove_free_nid(struct f2fs_sb_info *sbi, nid_t nid)
 }
 
 static void update_free_nid_bitmap(struct f2fs_sb_info *sbi, nid_t nid,
-			bool set, bool build, bool locked)
+							bool set, bool build)
 {
 	struct f2fs_nm_info *nm_i = NM_I(sbi);
 	unsigned int nat_ofs = NAT_BLOCK_OFFSET(nid);
@@ -1829,14 +1829,10 @@ static void update_free_nid_bitmap(struct f2fs_sb_info *sbi, nid_t nid,
 	else
 		__clear_bit_le(nid_ofs, nm_i->free_nid_bitmap[nat_ofs]);
 
-	if (!locked)
-		spin_lock(&nm_i->free_nid_lock);
 	if (set)
 		nm_i->free_nid_count[nat_ofs]++;
 	else if (!build)
 		nm_i->free_nid_count[nat_ofs]--;
-	if (!locked)
-		spin_unlock(&nm_i->free_nid_lock);
 }
 
 static void scan_nat_page(struct f2fs_sb_info *sbi,
@@ -1865,7 +1861,9 @@ static void scan_nat_page(struct f2fs_sb_info *sbi,
 		f2fs_bug_on(sbi, blk_addr == NEW_ADDR);
 		if (blk_addr == NULL_ADDR)
 			freed = add_free_nid(sbi, start_nid, true);
-		update_free_nid_bitmap(sbi, start_nid, freed, true, false);
+		spin_lock(&NM_I(sbi)->nid_list_lock);
+		update_free_nid_bitmap(sbi, start_nid, freed, true);
+		spin_unlock(&NM_I(sbi)->nid_list_lock);
 	}
 }
 
@@ -2020,7 +2018,7 @@ bool alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid)
 		__insert_nid_to_list(sbi, i, ALLOC_NID_LIST, false);
 		nm_i->available_nids--;
 
-		update_free_nid_bitmap(sbi, *nid, false, false, false);
+		update_free_nid_bitmap(sbi, *nid, false, false);
 
 		spin_unlock(&nm_i->nid_list_lock);
 		return true;
@@ -2076,7 +2074,7 @@ void alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid)
 
 	nm_i->available_nids++;
 
-	update_free_nid_bitmap(sbi, nid, true, false, false);
+	update_free_nid_bitmap(sbi, nid, true, false);
 
 	spin_unlock(&nm_i->nid_list_lock);
 
@@ -2406,11 +2404,11 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
 			add_free_nid(sbi, nid, false);
 			spin_lock(&NM_I(sbi)->nid_list_lock);
 			NM_I(sbi)->available_nids++;
-			update_free_nid_bitmap(sbi, nid, true, false, false);
+			update_free_nid_bitmap(sbi, nid, true, false);
 			spin_unlock(&NM_I(sbi)->nid_list_lock);
 		} else {
 			spin_lock(&NM_I(sbi)->nid_list_lock);
-			update_free_nid_bitmap(sbi, nid, false, false, false);
+			update_free_nid_bitmap(sbi, nid, false, false);
 			spin_unlock(&NM_I(sbi)->nid_list_lock);
 		}
 	}
@@ -2535,10 +2533,10 @@ inline void load_free_nid_bitmap(struct f2fs_sb_info *sbi)
 		nid = i * NAT_ENTRY_PER_BLOCK;
 		last_nid = (i + 1) * NAT_ENTRY_PER_BLOCK;
 
-		spin_lock(&nm_i->free_nid_lock);
+		spin_lock(&NM_I(sbi)->nid_list_lock);
 		for (; nid < last_nid; nid++)
-			update_free_nid_bitmap(sbi, nid, true, true, true);
-		spin_unlock(&nm_i->free_nid_lock);
+			update_free_nid_bitmap(sbi, nid, true, true);
+		spin_unlock(&NM_I(sbi)->nid_list_lock);
 	}
 
 	for (i = 0; i < nm_i->nat_blocks; i++) {
@@ -2629,9 +2627,6 @@ static int init_free_nid_cache(struct f2fs_sb_info *sbi)
 					sizeof(unsigned short), GFP_KERNEL);
 	if (!nm_i->free_nid_count)
 		return -ENOMEM;
-
-	spin_lock_init(&nm_i->free_nid_lock);
-
 	return 0;
 }
 
-- 
2.8.2.295.g3f1c1d0

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH] f2fs: cover update_free_nid_bitmap with free_list_lock
@ 2017-03-13 12:10 ` Chao Yu
  0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2017-03-13 12:10 UTC (permalink / raw)
  To: jaegeuk; +Cc: chao, linux-kernel, linux-f2fs-devel

free_nid_bitmap and free_nid_count in update_free_nid_bitmap should be
updated atomically, use free_list_lock cover them to avoid race in
concurrent scenario.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/f2fs.h |  1 -
 fs/f2fs/node.c | 27 +++++++++++----------------
 2 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 843ce39502bc..d905e53ba524 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -561,7 +561,6 @@ struct f2fs_nm_info {
 	unsigned char (*free_nid_bitmap)[NAT_ENTRY_BITMAP_SIZE];
 	unsigned char *nat_block_bitmap;
 	unsigned short *free_nid_count;	/* free nid count of NAT block */
-	spinlock_t free_nid_lock;	/* protect updating of nid count */
 
 	/* for checkpoint */
 	char *nat_bitmap;		/* NAT bitmap pointer */
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 11df8ab32478..3bfffd744f87 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1815,7 +1815,7 @@ static void remove_free_nid(struct f2fs_sb_info *sbi, nid_t nid)
 }
 
 static void update_free_nid_bitmap(struct f2fs_sb_info *sbi, nid_t nid,
-			bool set, bool build, bool locked)
+							bool set, bool build)
 {
 	struct f2fs_nm_info *nm_i = NM_I(sbi);
 	unsigned int nat_ofs = NAT_BLOCK_OFFSET(nid);
@@ -1829,14 +1829,10 @@ static void update_free_nid_bitmap(struct f2fs_sb_info *sbi, nid_t nid,
 	else
 		__clear_bit_le(nid_ofs, nm_i->free_nid_bitmap[nat_ofs]);
 
-	if (!locked)
-		spin_lock(&nm_i->free_nid_lock);
 	if (set)
 		nm_i->free_nid_count[nat_ofs]++;
 	else if (!build)
 		nm_i->free_nid_count[nat_ofs]--;
-	if (!locked)
-		spin_unlock(&nm_i->free_nid_lock);
 }
 
 static void scan_nat_page(struct f2fs_sb_info *sbi,
@@ -1865,7 +1861,9 @@ static void scan_nat_page(struct f2fs_sb_info *sbi,
 		f2fs_bug_on(sbi, blk_addr == NEW_ADDR);
 		if (blk_addr == NULL_ADDR)
 			freed = add_free_nid(sbi, start_nid, true);
-		update_free_nid_bitmap(sbi, start_nid, freed, true, false);
+		spin_lock(&NM_I(sbi)->nid_list_lock);
+		update_free_nid_bitmap(sbi, start_nid, freed, true);
+		spin_unlock(&NM_I(sbi)->nid_list_lock);
 	}
 }
 
@@ -2020,7 +2018,7 @@ bool alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid)
 		__insert_nid_to_list(sbi, i, ALLOC_NID_LIST, false);
 		nm_i->available_nids--;
 
-		update_free_nid_bitmap(sbi, *nid, false, false, false);
+		update_free_nid_bitmap(sbi, *nid, false, false);
 
 		spin_unlock(&nm_i->nid_list_lock);
 		return true;
@@ -2076,7 +2074,7 @@ void alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid)
 
 	nm_i->available_nids++;
 
-	update_free_nid_bitmap(sbi, nid, true, false, false);
+	update_free_nid_bitmap(sbi, nid, true, false);
 
 	spin_unlock(&nm_i->nid_list_lock);
 
@@ -2406,11 +2404,11 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
 			add_free_nid(sbi, nid, false);
 			spin_lock(&NM_I(sbi)->nid_list_lock);
 			NM_I(sbi)->available_nids++;
-			update_free_nid_bitmap(sbi, nid, true, false, false);
+			update_free_nid_bitmap(sbi, nid, true, false);
 			spin_unlock(&NM_I(sbi)->nid_list_lock);
 		} else {
 			spin_lock(&NM_I(sbi)->nid_list_lock);
-			update_free_nid_bitmap(sbi, nid, false, false, false);
+			update_free_nid_bitmap(sbi, nid, false, false);
 			spin_unlock(&NM_I(sbi)->nid_list_lock);
 		}
 	}
@@ -2535,10 +2533,10 @@ inline void load_free_nid_bitmap(struct f2fs_sb_info *sbi)
 		nid = i * NAT_ENTRY_PER_BLOCK;
 		last_nid = (i + 1) * NAT_ENTRY_PER_BLOCK;
 
-		spin_lock(&nm_i->free_nid_lock);
+		spin_lock(&NM_I(sbi)->nid_list_lock);
 		for (; nid < last_nid; nid++)
-			update_free_nid_bitmap(sbi, nid, true, true, true);
-		spin_unlock(&nm_i->free_nid_lock);
+			update_free_nid_bitmap(sbi, nid, true, true);
+		spin_unlock(&NM_I(sbi)->nid_list_lock);
 	}
 
 	for (i = 0; i < nm_i->nat_blocks; i++) {
@@ -2629,9 +2627,6 @@ static int init_free_nid_cache(struct f2fs_sb_info *sbi)
 					sizeof(unsigned short), GFP_KERNEL);
 	if (!nm_i->free_nid_count)
 		return -ENOMEM;
-
-	spin_lock_init(&nm_i->free_nid_lock);
-
 	return 0;
 }
 
-- 
2.8.2.295.g3f1c1d0


------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: cover update_free_nid_bitmap with free_list_lock
  2017-03-13 12:10 ` Chao Yu
@ 2017-03-13 12:22   ` Kinglong Mee
  -1 siblings, 0 replies; 8+ messages in thread
From: Kinglong Mee @ 2017-03-13 12:22 UTC (permalink / raw)
  To: Chao Yu, jaegeuk; +Cc: chao, linux-kernel, linux-f2fs-devel, Kinglong Mee

On 3/13/2017 20:10, Chao Yu wrote:
> free_nid_bitmap and free_nid_count in update_free_nid_bitmap should be
> updated atomically, use free_list_lock cover them to avoid race in

nid_list_lock?

Reviewed-by: Kinglong Mee <kinglongmee@gmail.com>

> concurrent scenario.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/f2fs.h |  1 -
>  fs/f2fs/node.c | 27 +++++++++++----------------
>  2 files changed, 11 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 843ce39502bc..d905e53ba524 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -561,7 +561,6 @@ struct f2fs_nm_info {
>  	unsigned char (*free_nid_bitmap)[NAT_ENTRY_BITMAP_SIZE];
>  	unsigned char *nat_block_bitmap;
>  	unsigned short *free_nid_count;	/* free nid count of NAT block */
> -	spinlock_t free_nid_lock;	/* protect updating of nid count */
>  
>  	/* for checkpoint */
>  	char *nat_bitmap;		/* NAT bitmap pointer */
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index 11df8ab32478..3bfffd744f87 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -1815,7 +1815,7 @@ static void remove_free_nid(struct f2fs_sb_info *sbi, nid_t nid)
>  }
>  
>  static void update_free_nid_bitmap(struct f2fs_sb_info *sbi, nid_t nid,
> -			bool set, bool build, bool locked)
> +							bool set, bool build)
>  {
>  	struct f2fs_nm_info *nm_i = NM_I(sbi);
>  	unsigned int nat_ofs = NAT_BLOCK_OFFSET(nid);
> @@ -1829,14 +1829,10 @@ static void update_free_nid_bitmap(struct f2fs_sb_info *sbi, nid_t nid,
>  	else
>  		__clear_bit_le(nid_ofs, nm_i->free_nid_bitmap[nat_ofs]);
>  
> -	if (!locked)
> -		spin_lock(&nm_i->free_nid_lock);
>  	if (set)
>  		nm_i->free_nid_count[nat_ofs]++;
>  	else if (!build)
>  		nm_i->free_nid_count[nat_ofs]--;
> -	if (!locked)
> -		spin_unlock(&nm_i->free_nid_lock);
>  }
>  
>  static void scan_nat_page(struct f2fs_sb_info *sbi,
> @@ -1865,7 +1861,9 @@ static void scan_nat_page(struct f2fs_sb_info *sbi,
>  		f2fs_bug_on(sbi, blk_addr == NEW_ADDR);
>  		if (blk_addr == NULL_ADDR)
>  			freed = add_free_nid(sbi, start_nid, true);
> -		update_free_nid_bitmap(sbi, start_nid, freed, true, false);
> +		spin_lock(&NM_I(sbi)->nid_list_lock);
> +		update_free_nid_bitmap(sbi, start_nid, freed, true);
> +		spin_unlock(&NM_I(sbi)->nid_list_lock);
>  	}
>  }
>  
> @@ -2020,7 +2018,7 @@ bool alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid)
>  		__insert_nid_to_list(sbi, i, ALLOC_NID_LIST, false);
>  		nm_i->available_nids--;
>  
> -		update_free_nid_bitmap(sbi, *nid, false, false, false);
> +		update_free_nid_bitmap(sbi, *nid, false, false);
>  
>  		spin_unlock(&nm_i->nid_list_lock);
>  		return true;
> @@ -2076,7 +2074,7 @@ void alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid)
>  
>  	nm_i->available_nids++;
>  
> -	update_free_nid_bitmap(sbi, nid, true, false, false);
> +	update_free_nid_bitmap(sbi, nid, true, false);
>  
>  	spin_unlock(&nm_i->nid_list_lock);
>  
> @@ -2406,11 +2404,11 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>  			add_free_nid(sbi, nid, false);
>  			spin_lock(&NM_I(sbi)->nid_list_lock);
>  			NM_I(sbi)->available_nids++;
> -			update_free_nid_bitmap(sbi, nid, true, false, false);
> +			update_free_nid_bitmap(sbi, nid, true, false);
>  			spin_unlock(&NM_I(sbi)->nid_list_lock);
>  		} else {
>  			spin_lock(&NM_I(sbi)->nid_list_lock);
> -			update_free_nid_bitmap(sbi, nid, false, false, false);
> +			update_free_nid_bitmap(sbi, nid, false, false);
>  			spin_unlock(&NM_I(sbi)->nid_list_lock);
>  		}
>  	}
> @@ -2535,10 +2533,10 @@ inline void load_free_nid_bitmap(struct f2fs_sb_info *sbi)
>  		nid = i * NAT_ENTRY_PER_BLOCK;
>  		last_nid = (i + 1) * NAT_ENTRY_PER_BLOCK;
>  
> -		spin_lock(&nm_i->free_nid_lock);
> +		spin_lock(&NM_I(sbi)->nid_list_lock);
>  		for (; nid < last_nid; nid++)
> -			update_free_nid_bitmap(sbi, nid, true, true, true);
> -		spin_unlock(&nm_i->free_nid_lock);
> +			update_free_nid_bitmap(sbi, nid, true, true);
> +		spin_unlock(&NM_I(sbi)->nid_list_lock);
>  	}
>  
>  	for (i = 0; i < nm_i->nat_blocks; i++) {
> @@ -2629,9 +2627,6 @@ static int init_free_nid_cache(struct f2fs_sb_info *sbi)
>  					sizeof(unsigned short), GFP_KERNEL);
>  	if (!nm_i->free_nid_count)
>  		return -ENOMEM;
> -
> -	spin_lock_init(&nm_i->free_nid_lock);
> -
>  	return 0;
>  }
>  
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] f2fs: cover update_free_nid_bitmap with free_list_lock
@ 2017-03-13 12:22   ` Kinglong Mee
  0 siblings, 0 replies; 8+ messages in thread
From: Kinglong Mee @ 2017-03-13 12:22 UTC (permalink / raw)
  To: Chao Yu, jaegeuk; +Cc: chao, linux-kernel, linux-f2fs-devel

On 3/13/2017 20:10, Chao Yu wrote:
> free_nid_bitmap and free_nid_count in update_free_nid_bitmap should be
> updated atomically, use free_list_lock cover them to avoid race in

nid_list_lock?

Reviewed-by: Kinglong Mee <kinglongmee@gmail.com>

> concurrent scenario.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/f2fs.h |  1 -
>  fs/f2fs/node.c | 27 +++++++++++----------------
>  2 files changed, 11 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 843ce39502bc..d905e53ba524 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -561,7 +561,6 @@ struct f2fs_nm_info {
>  	unsigned char (*free_nid_bitmap)[NAT_ENTRY_BITMAP_SIZE];
>  	unsigned char *nat_block_bitmap;
>  	unsigned short *free_nid_count;	/* free nid count of NAT block */
> -	spinlock_t free_nid_lock;	/* protect updating of nid count */
>  
>  	/* for checkpoint */
>  	char *nat_bitmap;		/* NAT bitmap pointer */
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index 11df8ab32478..3bfffd744f87 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -1815,7 +1815,7 @@ static void remove_free_nid(struct f2fs_sb_info *sbi, nid_t nid)
>  }
>  
>  static void update_free_nid_bitmap(struct f2fs_sb_info *sbi, nid_t nid,
> -			bool set, bool build, bool locked)
> +							bool set, bool build)
>  {
>  	struct f2fs_nm_info *nm_i = NM_I(sbi);
>  	unsigned int nat_ofs = NAT_BLOCK_OFFSET(nid);
> @@ -1829,14 +1829,10 @@ static void update_free_nid_bitmap(struct f2fs_sb_info *sbi, nid_t nid,
>  	else
>  		__clear_bit_le(nid_ofs, nm_i->free_nid_bitmap[nat_ofs]);
>  
> -	if (!locked)
> -		spin_lock(&nm_i->free_nid_lock);
>  	if (set)
>  		nm_i->free_nid_count[nat_ofs]++;
>  	else if (!build)
>  		nm_i->free_nid_count[nat_ofs]--;
> -	if (!locked)
> -		spin_unlock(&nm_i->free_nid_lock);
>  }
>  
>  static void scan_nat_page(struct f2fs_sb_info *sbi,
> @@ -1865,7 +1861,9 @@ static void scan_nat_page(struct f2fs_sb_info *sbi,
>  		f2fs_bug_on(sbi, blk_addr == NEW_ADDR);
>  		if (blk_addr == NULL_ADDR)
>  			freed = add_free_nid(sbi, start_nid, true);
> -		update_free_nid_bitmap(sbi, start_nid, freed, true, false);
> +		spin_lock(&NM_I(sbi)->nid_list_lock);
> +		update_free_nid_bitmap(sbi, start_nid, freed, true);
> +		spin_unlock(&NM_I(sbi)->nid_list_lock);
>  	}
>  }
>  
> @@ -2020,7 +2018,7 @@ bool alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid)
>  		__insert_nid_to_list(sbi, i, ALLOC_NID_LIST, false);
>  		nm_i->available_nids--;
>  
> -		update_free_nid_bitmap(sbi, *nid, false, false, false);
> +		update_free_nid_bitmap(sbi, *nid, false, false);
>  
>  		spin_unlock(&nm_i->nid_list_lock);
>  		return true;
> @@ -2076,7 +2074,7 @@ void alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid)
>  
>  	nm_i->available_nids++;
>  
> -	update_free_nid_bitmap(sbi, nid, true, false, false);
> +	update_free_nid_bitmap(sbi, nid, true, false);
>  
>  	spin_unlock(&nm_i->nid_list_lock);
>  
> @@ -2406,11 +2404,11 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>  			add_free_nid(sbi, nid, false);
>  			spin_lock(&NM_I(sbi)->nid_list_lock);
>  			NM_I(sbi)->available_nids++;
> -			update_free_nid_bitmap(sbi, nid, true, false, false);
> +			update_free_nid_bitmap(sbi, nid, true, false);
>  			spin_unlock(&NM_I(sbi)->nid_list_lock);
>  		} else {
>  			spin_lock(&NM_I(sbi)->nid_list_lock);
> -			update_free_nid_bitmap(sbi, nid, false, false, false);
> +			update_free_nid_bitmap(sbi, nid, false, false);
>  			spin_unlock(&NM_I(sbi)->nid_list_lock);
>  		}
>  	}
> @@ -2535,10 +2533,10 @@ inline void load_free_nid_bitmap(struct f2fs_sb_info *sbi)
>  		nid = i * NAT_ENTRY_PER_BLOCK;
>  		last_nid = (i + 1) * NAT_ENTRY_PER_BLOCK;
>  
> -		spin_lock(&nm_i->free_nid_lock);
> +		spin_lock(&NM_I(sbi)->nid_list_lock);
>  		for (; nid < last_nid; nid++)
> -			update_free_nid_bitmap(sbi, nid, true, true, true);
> -		spin_unlock(&nm_i->free_nid_lock);
> +			update_free_nid_bitmap(sbi, nid, true, true);
> +		spin_unlock(&NM_I(sbi)->nid_list_lock);
>  	}
>  
>  	for (i = 0; i < nm_i->nat_blocks; i++) {
> @@ -2629,9 +2627,6 @@ static int init_free_nid_cache(struct f2fs_sb_info *sbi)
>  					sizeof(unsigned short), GFP_KERNEL);
>  	if (!nm_i->free_nid_count)
>  		return -ENOMEM;
> -
> -	spin_lock_init(&nm_i->free_nid_lock);
> -
>  	return 0;
>  }
>  
> 

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: cover update_free_nid_bitmap with free_list_lock
  2017-03-13 12:22   ` Kinglong Mee
@ 2017-03-13 19:57     ` Jaegeuk Kim
  -1 siblings, 0 replies; 8+ messages in thread
From: Jaegeuk Kim @ 2017-03-13 19:57 UTC (permalink / raw)
  To: Kinglong Mee; +Cc: Chao Yu, chao, linux-kernel, linux-f2fs-devel

On 03/13, Kinglong Mee wrote:
> On 3/13/2017 20:10, Chao Yu wrote:
> > free_nid_bitmap and free_nid_count in update_free_nid_bitmap should be
> > updated atomically, use free_list_lock cover them to avoid race in
> 
> nid_list_lock?

Think so.
I merged a modified one in dev-test and started to test it.

Thanks,

> 
> Reviewed-by: Kinglong Mee <kinglongmee@gmail.com>
> 
> > concurrent scenario.
> > 
> > Signed-off-by: Chao Yu <yuchao0@huawei.com>
> > ---
> >  fs/f2fs/f2fs.h |  1 -
> >  fs/f2fs/node.c | 27 +++++++++++----------------
> >  2 files changed, 11 insertions(+), 17 deletions(-)
> > 
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 843ce39502bc..d905e53ba524 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -561,7 +561,6 @@ struct f2fs_nm_info {
> >  	unsigned char (*free_nid_bitmap)[NAT_ENTRY_BITMAP_SIZE];
> >  	unsigned char *nat_block_bitmap;
> >  	unsigned short *free_nid_count;	/* free nid count of NAT block */
> > -	spinlock_t free_nid_lock;	/* protect updating of nid count */
> >  
> >  	/* for checkpoint */
> >  	char *nat_bitmap;		/* NAT bitmap pointer */
> > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> > index 11df8ab32478..3bfffd744f87 100644
> > --- a/fs/f2fs/node.c
> > +++ b/fs/f2fs/node.c
> > @@ -1815,7 +1815,7 @@ static void remove_free_nid(struct f2fs_sb_info *sbi, nid_t nid)
> >  }
> >  
> >  static void update_free_nid_bitmap(struct f2fs_sb_info *sbi, nid_t nid,
> > -			bool set, bool build, bool locked)
> > +							bool set, bool build)
> >  {
> >  	struct f2fs_nm_info *nm_i = NM_I(sbi);
> >  	unsigned int nat_ofs = NAT_BLOCK_OFFSET(nid);
> > @@ -1829,14 +1829,10 @@ static void update_free_nid_bitmap(struct f2fs_sb_info *sbi, nid_t nid,
> >  	else
> >  		__clear_bit_le(nid_ofs, nm_i->free_nid_bitmap[nat_ofs]);
> >  
> > -	if (!locked)
> > -		spin_lock(&nm_i->free_nid_lock);
> >  	if (set)
> >  		nm_i->free_nid_count[nat_ofs]++;
> >  	else if (!build)
> >  		nm_i->free_nid_count[nat_ofs]--;
> > -	if (!locked)
> > -		spin_unlock(&nm_i->free_nid_lock);
> >  }
> >  
> >  static void scan_nat_page(struct f2fs_sb_info *sbi,
> > @@ -1865,7 +1861,9 @@ static void scan_nat_page(struct f2fs_sb_info *sbi,
> >  		f2fs_bug_on(sbi, blk_addr == NEW_ADDR);
> >  		if (blk_addr == NULL_ADDR)
> >  			freed = add_free_nid(sbi, start_nid, true);
> > -		update_free_nid_bitmap(sbi, start_nid, freed, true, false);
> > +		spin_lock(&NM_I(sbi)->nid_list_lock);
> > +		update_free_nid_bitmap(sbi, start_nid, freed, true);
> > +		spin_unlock(&NM_I(sbi)->nid_list_lock);
> >  	}
> >  }
> >  
> > @@ -2020,7 +2018,7 @@ bool alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid)
> >  		__insert_nid_to_list(sbi, i, ALLOC_NID_LIST, false);
> >  		nm_i->available_nids--;
> >  
> > -		update_free_nid_bitmap(sbi, *nid, false, false, false);
> > +		update_free_nid_bitmap(sbi, *nid, false, false);
> >  
> >  		spin_unlock(&nm_i->nid_list_lock);
> >  		return true;
> > @@ -2076,7 +2074,7 @@ void alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid)
> >  
> >  	nm_i->available_nids++;
> >  
> > -	update_free_nid_bitmap(sbi, nid, true, false, false);
> > +	update_free_nid_bitmap(sbi, nid, true, false);
> >  
> >  	spin_unlock(&nm_i->nid_list_lock);
> >  
> > @@ -2406,11 +2404,11 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
> >  			add_free_nid(sbi, nid, false);
> >  			spin_lock(&NM_I(sbi)->nid_list_lock);
> >  			NM_I(sbi)->available_nids++;
> > -			update_free_nid_bitmap(sbi, nid, true, false, false);
> > +			update_free_nid_bitmap(sbi, nid, true, false);
> >  			spin_unlock(&NM_I(sbi)->nid_list_lock);
> >  		} else {
> >  			spin_lock(&NM_I(sbi)->nid_list_lock);
> > -			update_free_nid_bitmap(sbi, nid, false, false, false);
> > +			update_free_nid_bitmap(sbi, nid, false, false);
> >  			spin_unlock(&NM_I(sbi)->nid_list_lock);
> >  		}
> >  	}
> > @@ -2535,10 +2533,10 @@ inline void load_free_nid_bitmap(struct f2fs_sb_info *sbi)
> >  		nid = i * NAT_ENTRY_PER_BLOCK;
> >  		last_nid = (i + 1) * NAT_ENTRY_PER_BLOCK;
> >  
> > -		spin_lock(&nm_i->free_nid_lock);
> > +		spin_lock(&NM_I(sbi)->nid_list_lock);
> >  		for (; nid < last_nid; nid++)
> > -			update_free_nid_bitmap(sbi, nid, true, true, true);
> > -		spin_unlock(&nm_i->free_nid_lock);
> > +			update_free_nid_bitmap(sbi, nid, true, true);
> > +		spin_unlock(&NM_I(sbi)->nid_list_lock);
> >  	}
> >  
> >  	for (i = 0; i < nm_i->nat_blocks; i++) {
> > @@ -2629,9 +2627,6 @@ static int init_free_nid_cache(struct f2fs_sb_info *sbi)
> >  					sizeof(unsigned short), GFP_KERNEL);
> >  	if (!nm_i->free_nid_count)
> >  		return -ENOMEM;
> > -
> > -	spin_lock_init(&nm_i->free_nid_lock);
> > -
> >  	return 0;
> >  }
> >  
> > 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] f2fs: cover update_free_nid_bitmap with free_list_lock
@ 2017-03-13 19:57     ` Jaegeuk Kim
  0 siblings, 0 replies; 8+ messages in thread
From: Jaegeuk Kim @ 2017-03-13 19:57 UTC (permalink / raw)
  To: Kinglong Mee; +Cc: linux-kernel, chao, linux-f2fs-devel

On 03/13, Kinglong Mee wrote:
> On 3/13/2017 20:10, Chao Yu wrote:
> > free_nid_bitmap and free_nid_count in update_free_nid_bitmap should be
> > updated atomically, use free_list_lock cover them to avoid race in
> 
> nid_list_lock?

Think so.
I merged a modified one in dev-test and started to test it.

Thanks,

> 
> Reviewed-by: Kinglong Mee <kinglongmee@gmail.com>
> 
> > concurrent scenario.
> > 
> > Signed-off-by: Chao Yu <yuchao0@huawei.com>
> > ---
> >  fs/f2fs/f2fs.h |  1 -
> >  fs/f2fs/node.c | 27 +++++++++++----------------
> >  2 files changed, 11 insertions(+), 17 deletions(-)
> > 
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 843ce39502bc..d905e53ba524 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -561,7 +561,6 @@ struct f2fs_nm_info {
> >  	unsigned char (*free_nid_bitmap)[NAT_ENTRY_BITMAP_SIZE];
> >  	unsigned char *nat_block_bitmap;
> >  	unsigned short *free_nid_count;	/* free nid count of NAT block */
> > -	spinlock_t free_nid_lock;	/* protect updating of nid count */
> >  
> >  	/* for checkpoint */
> >  	char *nat_bitmap;		/* NAT bitmap pointer */
> > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> > index 11df8ab32478..3bfffd744f87 100644
> > --- a/fs/f2fs/node.c
> > +++ b/fs/f2fs/node.c
> > @@ -1815,7 +1815,7 @@ static void remove_free_nid(struct f2fs_sb_info *sbi, nid_t nid)
> >  }
> >  
> >  static void update_free_nid_bitmap(struct f2fs_sb_info *sbi, nid_t nid,
> > -			bool set, bool build, bool locked)
> > +							bool set, bool build)
> >  {
> >  	struct f2fs_nm_info *nm_i = NM_I(sbi);
> >  	unsigned int nat_ofs = NAT_BLOCK_OFFSET(nid);
> > @@ -1829,14 +1829,10 @@ static void update_free_nid_bitmap(struct f2fs_sb_info *sbi, nid_t nid,
> >  	else
> >  		__clear_bit_le(nid_ofs, nm_i->free_nid_bitmap[nat_ofs]);
> >  
> > -	if (!locked)
> > -		spin_lock(&nm_i->free_nid_lock);
> >  	if (set)
> >  		nm_i->free_nid_count[nat_ofs]++;
> >  	else if (!build)
> >  		nm_i->free_nid_count[nat_ofs]--;
> > -	if (!locked)
> > -		spin_unlock(&nm_i->free_nid_lock);
> >  }
> >  
> >  static void scan_nat_page(struct f2fs_sb_info *sbi,
> > @@ -1865,7 +1861,9 @@ static void scan_nat_page(struct f2fs_sb_info *sbi,
> >  		f2fs_bug_on(sbi, blk_addr == NEW_ADDR);
> >  		if (blk_addr == NULL_ADDR)
> >  			freed = add_free_nid(sbi, start_nid, true);
> > -		update_free_nid_bitmap(sbi, start_nid, freed, true, false);
> > +		spin_lock(&NM_I(sbi)->nid_list_lock);
> > +		update_free_nid_bitmap(sbi, start_nid, freed, true);
> > +		spin_unlock(&NM_I(sbi)->nid_list_lock);
> >  	}
> >  }
> >  
> > @@ -2020,7 +2018,7 @@ bool alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid)
> >  		__insert_nid_to_list(sbi, i, ALLOC_NID_LIST, false);
> >  		nm_i->available_nids--;
> >  
> > -		update_free_nid_bitmap(sbi, *nid, false, false, false);
> > +		update_free_nid_bitmap(sbi, *nid, false, false);
> >  
> >  		spin_unlock(&nm_i->nid_list_lock);
> >  		return true;
> > @@ -2076,7 +2074,7 @@ void alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid)
> >  
> >  	nm_i->available_nids++;
> >  
> > -	update_free_nid_bitmap(sbi, nid, true, false, false);
> > +	update_free_nid_bitmap(sbi, nid, true, false);
> >  
> >  	spin_unlock(&nm_i->nid_list_lock);
> >  
> > @@ -2406,11 +2404,11 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
> >  			add_free_nid(sbi, nid, false);
> >  			spin_lock(&NM_I(sbi)->nid_list_lock);
> >  			NM_I(sbi)->available_nids++;
> > -			update_free_nid_bitmap(sbi, nid, true, false, false);
> > +			update_free_nid_bitmap(sbi, nid, true, false);
> >  			spin_unlock(&NM_I(sbi)->nid_list_lock);
> >  		} else {
> >  			spin_lock(&NM_I(sbi)->nid_list_lock);
> > -			update_free_nid_bitmap(sbi, nid, false, false, false);
> > +			update_free_nid_bitmap(sbi, nid, false, false);
> >  			spin_unlock(&NM_I(sbi)->nid_list_lock);
> >  		}
> >  	}
> > @@ -2535,10 +2533,10 @@ inline void load_free_nid_bitmap(struct f2fs_sb_info *sbi)
> >  		nid = i * NAT_ENTRY_PER_BLOCK;
> >  		last_nid = (i + 1) * NAT_ENTRY_PER_BLOCK;
> >  
> > -		spin_lock(&nm_i->free_nid_lock);
> > +		spin_lock(&NM_I(sbi)->nid_list_lock);
> >  		for (; nid < last_nid; nid++)
> > -			update_free_nid_bitmap(sbi, nid, true, true, true);
> > -		spin_unlock(&nm_i->free_nid_lock);
> > +			update_free_nid_bitmap(sbi, nid, true, true);
> > +		spin_unlock(&NM_I(sbi)->nid_list_lock);
> >  	}
> >  
> >  	for (i = 0; i < nm_i->nat_blocks; i++) {
> > @@ -2629,9 +2627,6 @@ static int init_free_nid_cache(struct f2fs_sb_info *sbi)
> >  					sizeof(unsigned short), GFP_KERNEL);
> >  	if (!nm_i->free_nid_count)
> >  		return -ENOMEM;
> > -
> > -	spin_lock_init(&nm_i->free_nid_lock);
> > -
> >  	return 0;
> >  }
> >  
> > 

------------------------------------------------------------------------------
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: [f2fs-dev] [PATCH] f2fs: cover update_free_nid_bitmap with free_list_lock
  2017-03-13 19:57     ` Jaegeuk Kim
@ 2017-03-14  1:34       ` Chao Yu
  -1 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2017-03-14  1:34 UTC (permalink / raw)
  To: Jaegeuk Kim, Kinglong Mee; +Cc: chao, linux-kernel, linux-f2fs-devel

On 2017/3/14 3:57, Jaegeuk Kim wrote:
> On 03/13, Kinglong Mee wrote:
>> On 3/13/2017 20:10, Chao Yu wrote:
>>> free_nid_bitmap and free_nid_count in update_free_nid_bitmap should be
>>> updated atomically, use free_list_lock cover them to avoid race in
>>
>> nid_list_lock?
> 
> Think so.
> I merged a modified one in dev-test and started to test it.

Thanks for fixing.

> 
> Thanks,
> 
>>
>> Reviewed-by: Kinglong Mee <kinglongmee@gmail.com>

Thanks for the review.

Thanks,

>>
>>> concurrent scenario.
>>>
>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>> ---
>>>  fs/f2fs/f2fs.h |  1 -
>>>  fs/f2fs/node.c | 27 +++++++++++----------------
>>>  2 files changed, 11 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 843ce39502bc..d905e53ba524 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -561,7 +561,6 @@ struct f2fs_nm_info {
>>>  	unsigned char (*free_nid_bitmap)[NAT_ENTRY_BITMAP_SIZE];
>>>  	unsigned char *nat_block_bitmap;
>>>  	unsigned short *free_nid_count;	/* free nid count of NAT block */
>>> -	spinlock_t free_nid_lock;	/* protect updating of nid count */
>>>  
>>>  	/* for checkpoint */
>>>  	char *nat_bitmap;		/* NAT bitmap pointer */
>>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>>> index 11df8ab32478..3bfffd744f87 100644
>>> --- a/fs/f2fs/node.c
>>> +++ b/fs/f2fs/node.c
>>> @@ -1815,7 +1815,7 @@ static void remove_free_nid(struct f2fs_sb_info *sbi, nid_t nid)
>>>  }
>>>  
>>>  static void update_free_nid_bitmap(struct f2fs_sb_info *sbi, nid_t nid,
>>> -			bool set, bool build, bool locked)
>>> +							bool set, bool build)
>>>  {
>>>  	struct f2fs_nm_info *nm_i = NM_I(sbi);
>>>  	unsigned int nat_ofs = NAT_BLOCK_OFFSET(nid);
>>> @@ -1829,14 +1829,10 @@ static void update_free_nid_bitmap(struct f2fs_sb_info *sbi, nid_t nid,
>>>  	else
>>>  		__clear_bit_le(nid_ofs, nm_i->free_nid_bitmap[nat_ofs]);
>>>  
>>> -	if (!locked)
>>> -		spin_lock(&nm_i->free_nid_lock);
>>>  	if (set)
>>>  		nm_i->free_nid_count[nat_ofs]++;
>>>  	else if (!build)
>>>  		nm_i->free_nid_count[nat_ofs]--;
>>> -	if (!locked)
>>> -		spin_unlock(&nm_i->free_nid_lock);
>>>  }
>>>  
>>>  static void scan_nat_page(struct f2fs_sb_info *sbi,
>>> @@ -1865,7 +1861,9 @@ static void scan_nat_page(struct f2fs_sb_info *sbi,
>>>  		f2fs_bug_on(sbi, blk_addr == NEW_ADDR);
>>>  		if (blk_addr == NULL_ADDR)
>>>  			freed = add_free_nid(sbi, start_nid, true);
>>> -		update_free_nid_bitmap(sbi, start_nid, freed, true, false);
>>> +		spin_lock(&NM_I(sbi)->nid_list_lock);
>>> +		update_free_nid_bitmap(sbi, start_nid, freed, true);
>>> +		spin_unlock(&NM_I(sbi)->nid_list_lock);
>>>  	}
>>>  }
>>>  
>>> @@ -2020,7 +2018,7 @@ bool alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid)
>>>  		__insert_nid_to_list(sbi, i, ALLOC_NID_LIST, false);
>>>  		nm_i->available_nids--;
>>>  
>>> -		update_free_nid_bitmap(sbi, *nid, false, false, false);
>>> +		update_free_nid_bitmap(sbi, *nid, false, false);
>>>  
>>>  		spin_unlock(&nm_i->nid_list_lock);
>>>  		return true;
>>> @@ -2076,7 +2074,7 @@ void alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid)
>>>  
>>>  	nm_i->available_nids++;
>>>  
>>> -	update_free_nid_bitmap(sbi, nid, true, false, false);
>>> +	update_free_nid_bitmap(sbi, nid, true, false);
>>>  
>>>  	spin_unlock(&nm_i->nid_list_lock);
>>>  
>>> @@ -2406,11 +2404,11 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>  			add_free_nid(sbi, nid, false);
>>>  			spin_lock(&NM_I(sbi)->nid_list_lock);
>>>  			NM_I(sbi)->available_nids++;
>>> -			update_free_nid_bitmap(sbi, nid, true, false, false);
>>> +			update_free_nid_bitmap(sbi, nid, true, false);
>>>  			spin_unlock(&NM_I(sbi)->nid_list_lock);
>>>  		} else {
>>>  			spin_lock(&NM_I(sbi)->nid_list_lock);
>>> -			update_free_nid_bitmap(sbi, nid, false, false, false);
>>> +			update_free_nid_bitmap(sbi, nid, false, false);
>>>  			spin_unlock(&NM_I(sbi)->nid_list_lock);
>>>  		}
>>>  	}
>>> @@ -2535,10 +2533,10 @@ inline void load_free_nid_bitmap(struct f2fs_sb_info *sbi)
>>>  		nid = i * NAT_ENTRY_PER_BLOCK;
>>>  		last_nid = (i + 1) * NAT_ENTRY_PER_BLOCK;
>>>  
>>> -		spin_lock(&nm_i->free_nid_lock);
>>> +		spin_lock(&NM_I(sbi)->nid_list_lock);
>>>  		for (; nid < last_nid; nid++)
>>> -			update_free_nid_bitmap(sbi, nid, true, true, true);
>>> -		spin_unlock(&nm_i->free_nid_lock);
>>> +			update_free_nid_bitmap(sbi, nid, true, true);
>>> +		spin_unlock(&NM_I(sbi)->nid_list_lock);
>>>  	}
>>>  
>>>  	for (i = 0; i < nm_i->nat_blocks; i++) {
>>> @@ -2629,9 +2627,6 @@ static int init_free_nid_cache(struct f2fs_sb_info *sbi)
>>>  					sizeof(unsigned short), GFP_KERNEL);
>>>  	if (!nm_i->free_nid_count)
>>>  		return -ENOMEM;
>>> -
>>> -	spin_lock_init(&nm_i->free_nid_lock);
>>> -
>>>  	return 0;
>>>  }
>>>  
>>>
> 
> .
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] f2fs: cover update_free_nid_bitmap with free_list_lock
@ 2017-03-14  1:34       ` Chao Yu
  0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2017-03-14  1:34 UTC (permalink / raw)
  To: Jaegeuk Kim, Kinglong Mee; +Cc: chao, linux-kernel, linux-f2fs-devel

On 2017/3/14 3:57, Jaegeuk Kim wrote:
> On 03/13, Kinglong Mee wrote:
>> On 3/13/2017 20:10, Chao Yu wrote:
>>> free_nid_bitmap and free_nid_count in update_free_nid_bitmap should be
>>> updated atomically, use free_list_lock cover them to avoid race in
>>
>> nid_list_lock?
> 
> Think so.
> I merged a modified one in dev-test and started to test it.

Thanks for fixing.

> 
> Thanks,
> 
>>
>> Reviewed-by: Kinglong Mee <kinglongmee@gmail.com>

Thanks for the review.

Thanks,

>>
>>> concurrent scenario.
>>>
>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>> ---
>>>  fs/f2fs/f2fs.h |  1 -
>>>  fs/f2fs/node.c | 27 +++++++++++----------------
>>>  2 files changed, 11 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 843ce39502bc..d905e53ba524 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -561,7 +561,6 @@ struct f2fs_nm_info {
>>>  	unsigned char (*free_nid_bitmap)[NAT_ENTRY_BITMAP_SIZE];
>>>  	unsigned char *nat_block_bitmap;
>>>  	unsigned short *free_nid_count;	/* free nid count of NAT block */
>>> -	spinlock_t free_nid_lock;	/* protect updating of nid count */
>>>  
>>>  	/* for checkpoint */
>>>  	char *nat_bitmap;		/* NAT bitmap pointer */
>>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>>> index 11df8ab32478..3bfffd744f87 100644
>>> --- a/fs/f2fs/node.c
>>> +++ b/fs/f2fs/node.c
>>> @@ -1815,7 +1815,7 @@ static void remove_free_nid(struct f2fs_sb_info *sbi, nid_t nid)
>>>  }
>>>  
>>>  static void update_free_nid_bitmap(struct f2fs_sb_info *sbi, nid_t nid,
>>> -			bool set, bool build, bool locked)
>>> +							bool set, bool build)
>>>  {
>>>  	struct f2fs_nm_info *nm_i = NM_I(sbi);
>>>  	unsigned int nat_ofs = NAT_BLOCK_OFFSET(nid);
>>> @@ -1829,14 +1829,10 @@ static void update_free_nid_bitmap(struct f2fs_sb_info *sbi, nid_t nid,
>>>  	else
>>>  		__clear_bit_le(nid_ofs, nm_i->free_nid_bitmap[nat_ofs]);
>>>  
>>> -	if (!locked)
>>> -		spin_lock(&nm_i->free_nid_lock);
>>>  	if (set)
>>>  		nm_i->free_nid_count[nat_ofs]++;
>>>  	else if (!build)
>>>  		nm_i->free_nid_count[nat_ofs]--;
>>> -	if (!locked)
>>> -		spin_unlock(&nm_i->free_nid_lock);
>>>  }
>>>  
>>>  static void scan_nat_page(struct f2fs_sb_info *sbi,
>>> @@ -1865,7 +1861,9 @@ static void scan_nat_page(struct f2fs_sb_info *sbi,
>>>  		f2fs_bug_on(sbi, blk_addr == NEW_ADDR);
>>>  		if (blk_addr == NULL_ADDR)
>>>  			freed = add_free_nid(sbi, start_nid, true);
>>> -		update_free_nid_bitmap(sbi, start_nid, freed, true, false);
>>> +		spin_lock(&NM_I(sbi)->nid_list_lock);
>>> +		update_free_nid_bitmap(sbi, start_nid, freed, true);
>>> +		spin_unlock(&NM_I(sbi)->nid_list_lock);
>>>  	}
>>>  }
>>>  
>>> @@ -2020,7 +2018,7 @@ bool alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid)
>>>  		__insert_nid_to_list(sbi, i, ALLOC_NID_LIST, false);
>>>  		nm_i->available_nids--;
>>>  
>>> -		update_free_nid_bitmap(sbi, *nid, false, false, false);
>>> +		update_free_nid_bitmap(sbi, *nid, false, false);
>>>  
>>>  		spin_unlock(&nm_i->nid_list_lock);
>>>  		return true;
>>> @@ -2076,7 +2074,7 @@ void alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid)
>>>  
>>>  	nm_i->available_nids++;
>>>  
>>> -	update_free_nid_bitmap(sbi, nid, true, false, false);
>>> +	update_free_nid_bitmap(sbi, nid, true, false);
>>>  
>>>  	spin_unlock(&nm_i->nid_list_lock);
>>>  
>>> @@ -2406,11 +2404,11 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
>>>  			add_free_nid(sbi, nid, false);
>>>  			spin_lock(&NM_I(sbi)->nid_list_lock);
>>>  			NM_I(sbi)->available_nids++;
>>> -			update_free_nid_bitmap(sbi, nid, true, false, false);
>>> +			update_free_nid_bitmap(sbi, nid, true, false);
>>>  			spin_unlock(&NM_I(sbi)->nid_list_lock);
>>>  		} else {
>>>  			spin_lock(&NM_I(sbi)->nid_list_lock);
>>> -			update_free_nid_bitmap(sbi, nid, false, false, false);
>>> +			update_free_nid_bitmap(sbi, nid, false, false);
>>>  			spin_unlock(&NM_I(sbi)->nid_list_lock);
>>>  		}
>>>  	}
>>> @@ -2535,10 +2533,10 @@ inline void load_free_nid_bitmap(struct f2fs_sb_info *sbi)
>>>  		nid = i * NAT_ENTRY_PER_BLOCK;
>>>  		last_nid = (i + 1) * NAT_ENTRY_PER_BLOCK;
>>>  
>>> -		spin_lock(&nm_i->free_nid_lock);
>>> +		spin_lock(&NM_I(sbi)->nid_list_lock);
>>>  		for (; nid < last_nid; nid++)
>>> -			update_free_nid_bitmap(sbi, nid, true, true, true);
>>> -		spin_unlock(&nm_i->free_nid_lock);
>>> +			update_free_nid_bitmap(sbi, nid, true, true);
>>> +		spin_unlock(&NM_I(sbi)->nid_list_lock);
>>>  	}
>>>  
>>>  	for (i = 0; i < nm_i->nat_blocks; i++) {
>>> @@ -2629,9 +2627,6 @@ static int init_free_nid_cache(struct f2fs_sb_info *sbi)
>>>  					sizeof(unsigned short), GFP_KERNEL);
>>>  	if (!nm_i->free_nid_count)
>>>  		return -ENOMEM;
>>> -
>>> -	spin_lock_init(&nm_i->free_nid_lock);
>>> -
>>>  	return 0;
>>>  }
>>>  
>>>
> 
> .
> 


------------------------------------------------------------------------------
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-03-14  1:34 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-13 12:10 [PATCH] f2fs: cover update_free_nid_bitmap with free_list_lock Chao Yu
2017-03-13 12:10 ` Chao Yu
2017-03-13 12:22 ` [f2fs-dev] " Kinglong Mee
2017-03-13 12:22   ` Kinglong Mee
2017-03-13 19:57   ` [f2fs-dev] " Jaegeuk Kim
2017-03-13 19:57     ` Jaegeuk Kim
2017-03-14  1:34     ` [f2fs-dev] " Chao Yu
2017-03-14  1:34       ` 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.