Linux-f2fs-devel Archive on lore.kernel.org
 help / color / Atom feed
* [f2fs-dev] [PATCH v2] f2fs: shrink spinlock coverage
@ 2020-05-08  9:50 Chao Yu
  0 siblings, 0 replies; 5+ messages in thread
From: Chao Yu @ 2020-05-08  9:50 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel

In f2fs_try_to_free_nids(), .nid_list_lock spinlock critical region will
increase as expected shrink number increase, to avoid spining other CPUs
for long time, we change to release nid caches with small batch each time
under .nid_list_lock coverage.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
v2:
- shrink free nid caches in batch under spinlock coverage.
 fs/f2fs/node.c | 25 +++++++++++++++----------
 fs/f2fs/node.h |  3 +++
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 4da0d8713df5..1db8cabf727e 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -2488,7 +2488,6 @@ void f2fs_alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid)
 int f2fs_try_to_free_nids(struct f2fs_sb_info *sbi, int nr_shrink)
 {
 	struct f2fs_nm_info *nm_i = NM_I(sbi);
-	struct free_nid *i, *next;
 	int nr = nr_shrink;
 
 	if (nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
@@ -2497,17 +2496,23 @@ int f2fs_try_to_free_nids(struct f2fs_sb_info *sbi, int nr_shrink)
 	if (!mutex_trylock(&nm_i->build_lock))
 		return 0;
 
-	spin_lock(&nm_i->nid_list_lock);
-	list_for_each_entry_safe(i, next, &nm_i->free_nid_list, list) {
-		if (nr_shrink <= 0 ||
-				nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
-			break;
+	while (nr_shrink && nm_i->nid_cnt[FREE_NID] > MAX_FREE_NIDS) {
+		struct free_nid *i, *next;
+		unsigned int batch = SHRINK_NID_BATCH_SIZE;
 
-		__remove_free_nid(sbi, i, FREE_NID);
-		kmem_cache_free(free_nid_slab, i);
-		nr_shrink--;
+		spin_lock(&nm_i->nid_list_lock);
+		list_for_each_entry_safe(i, next, &nm_i->free_nid_list, list) {
+			if (!nr_shrink || !batch ||
+				nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
+				break;
+			__remove_free_nid(sbi, i, FREE_NID);
+			kmem_cache_free(free_nid_slab, i);
+			nr_shrink--;
+			batch--;
+		}
+		spin_unlock(&nm_i->nid_list_lock);
 	}
-	spin_unlock(&nm_i->nid_list_lock);
+
 	mutex_unlock(&nm_i->build_lock);
 
 	return nr - nr_shrink;
diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h
index e05af5df5648..33d677f83569 100644
--- a/fs/f2fs/node.h
+++ b/fs/f2fs/node.h
@@ -15,6 +15,9 @@
 #define FREE_NID_PAGES	8
 #define MAX_FREE_NIDS	(NAT_ENTRY_PER_BLOCK * FREE_NID_PAGES)
 
+/* size of free nid batch when shrinking */
+#define SHRINK_NID_BATCH_SIZE	8
+
 #define DEF_RA_NID_PAGES	0	/* # of nid pages to be readaheaded */
 
 /* maximum readahead size for node during getting data blocks */
-- 
2.18.0.rc1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [f2fs-dev] [PATCH v2] f2fs: shrink spinlock coverage
  2020-05-07  3:03   ` Chao Yu
@ 2020-05-07 12:49     ` Jaegeuk Kim
  0 siblings, 0 replies; 5+ messages in thread
From: Jaegeuk Kim @ 2020-05-07 12:49 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel

On 05/07, Chao Yu wrote:
> On 2020/5/6 23:05, Jaegeuk Kim wrote:
> > On 05/06, Chao Yu wrote:
> >> In f2fs_try_to_free_nids(), .nid_list_lock spinlock critical region will
> >> increase as expected shrink number increase, to avoid spining other CPUs
> >> for long time, it's better to implement like extent cache and nats
> >> shrinker.
> >>
> >> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> >> ---
> >> v2:
> >> - fix unlock wrong spinlock.
> >>  fs/f2fs/node.c | 15 +++++++++++----
> >>  1 file changed, 11 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> >> index 4da0d8713df5..ad0b14f4dab8 100644
> >> --- a/fs/f2fs/node.c
> >> +++ b/fs/f2fs/node.c
> >> @@ -2488,7 +2488,6 @@ void f2fs_alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid)
> >>  int f2fs_try_to_free_nids(struct f2fs_sb_info *sbi, int nr_shrink)
> >>  {
> >>  	struct f2fs_nm_info *nm_i = NM_I(sbi);
> >> -	struct free_nid *i, *next;
> >>  	int nr = nr_shrink;
> >>  
> >>  	if (nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
> >> @@ -2498,14 +2497,22 @@ int f2fs_try_to_free_nids(struct f2fs_sb_info *sbi, int nr_shrink)
> >>  		return 0;
> >>  
> >>  	spin_lock(&nm_i->nid_list_lock);
> >> -	list_for_each_entry_safe(i, next, &nm_i->free_nid_list, list) {
> >> -		if (nr_shrink <= 0 ||
> >> -				nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
> >> +	while (nr_shrink) {
> >> +		struct free_nid *i;
> >> +
> >> +		if (nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
> >>  			break;
> >>  
> >> +		i = list_first_entry(&nm_i->free_nid_list,
> >> +					struct free_nid, list);
> >> +		list_del(&i->list);
> >> +		spin_unlock(&nm_i->nid_list_lock);
> >> +
> >>  		__remove_free_nid(sbi, i, FREE_NID);
> > 
> > __remove_free_nid() will do list_del again. btw, how about just splitting out
> 
> Oh, my bad.
> 
> How about moving __remove_free_nid into .nid_list_lock coverage?
> 
> > given nr_shrink into multiple trials?
> 
> Like this?

Yes.

> 
> 	while (shrink) {
> 		batch = DEFAULT_BATCH_NUMBER; // 16
> 		spinlock();
> 		list_for_each_entry_safe() {
> 			if (!shrink || !batch)
> 				break;
> 			remove_item_from_list;
> 			shrink--;
> 			batch--;
> 		}
> 		spin_unlock();
> 	}
> 
> Thanks,
> 
> > 
> >>  		kmem_cache_free(free_nid_slab, i);
> >>  		nr_shrink--;
> >> +
> >> +		spin_lock(&nm_i->nid_list_lock);
> >>  	}
> >>  	spin_unlock(&nm_i->nid_list_lock);
> >>  	mutex_unlock(&nm_i->build_lock);
> >> -- 
> >> 2.18.0.rc1
> > .
> > 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [f2fs-dev] [PATCH v2] f2fs: shrink spinlock coverage
  2020-05-06 15:05 ` Jaegeuk Kim
@ 2020-05-07  3:03   ` Chao Yu
  2020-05-07 12:49     ` Jaegeuk Kim
  0 siblings, 1 reply; 5+ messages in thread
From: Chao Yu @ 2020-05-07  3:03 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel

On 2020/5/6 23:05, Jaegeuk Kim wrote:
> On 05/06, Chao Yu wrote:
>> In f2fs_try_to_free_nids(), .nid_list_lock spinlock critical region will
>> increase as expected shrink number increase, to avoid spining other CPUs
>> for long time, it's better to implement like extent cache and nats
>> shrinker.
>>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>> v2:
>> - fix unlock wrong spinlock.
>>  fs/f2fs/node.c | 15 +++++++++++----
>>  1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>> index 4da0d8713df5..ad0b14f4dab8 100644
>> --- a/fs/f2fs/node.c
>> +++ b/fs/f2fs/node.c
>> @@ -2488,7 +2488,6 @@ void f2fs_alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid)
>>  int f2fs_try_to_free_nids(struct f2fs_sb_info *sbi, int nr_shrink)
>>  {
>>  	struct f2fs_nm_info *nm_i = NM_I(sbi);
>> -	struct free_nid *i, *next;
>>  	int nr = nr_shrink;
>>  
>>  	if (nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
>> @@ -2498,14 +2497,22 @@ int f2fs_try_to_free_nids(struct f2fs_sb_info *sbi, int nr_shrink)
>>  		return 0;
>>  
>>  	spin_lock(&nm_i->nid_list_lock);
>> -	list_for_each_entry_safe(i, next, &nm_i->free_nid_list, list) {
>> -		if (nr_shrink <= 0 ||
>> -				nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
>> +	while (nr_shrink) {
>> +		struct free_nid *i;
>> +
>> +		if (nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
>>  			break;
>>  
>> +		i = list_first_entry(&nm_i->free_nid_list,
>> +					struct free_nid, list);
>> +		list_del(&i->list);
>> +		spin_unlock(&nm_i->nid_list_lock);
>> +
>>  		__remove_free_nid(sbi, i, FREE_NID);
> 
> __remove_free_nid() will do list_del again. btw, how about just splitting out

Oh, my bad.

How about moving __remove_free_nid into .nid_list_lock coverage?

> given nr_shrink into multiple trials?

Like this?

	while (shrink) {
		batch = DEFAULT_BATCH_NUMBER; // 16
		spinlock();
		list_for_each_entry_safe() {
			if (!shrink || !batch)
				break;
			remove_item_from_list;
			shrink--;
			batch--;
		}
		spin_unlock();
	}

Thanks,

> 
>>  		kmem_cache_free(free_nid_slab, i);
>>  		nr_shrink--;
>> +
>> +		spin_lock(&nm_i->nid_list_lock);
>>  	}
>>  	spin_unlock(&nm_i->nid_list_lock);
>>  	mutex_unlock(&nm_i->build_lock);
>> -- 
>> 2.18.0.rc1
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [f2fs-dev] [PATCH v2] f2fs: shrink spinlock coverage
  2020-05-06 10:45 Chao Yu
@ 2020-05-06 15:05 ` Jaegeuk Kim
  2020-05-07  3:03   ` Chao Yu
  0 siblings, 1 reply; 5+ messages in thread
From: Jaegeuk Kim @ 2020-05-06 15:05 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel

On 05/06, Chao Yu wrote:
> In f2fs_try_to_free_nids(), .nid_list_lock spinlock critical region will
> increase as expected shrink number increase, to avoid spining other CPUs
> for long time, it's better to implement like extent cache and nats
> shrinker.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> v2:
> - fix unlock wrong spinlock.
>  fs/f2fs/node.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index 4da0d8713df5..ad0b14f4dab8 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -2488,7 +2488,6 @@ void f2fs_alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid)
>  int f2fs_try_to_free_nids(struct f2fs_sb_info *sbi, int nr_shrink)
>  {
>  	struct f2fs_nm_info *nm_i = NM_I(sbi);
> -	struct free_nid *i, *next;
>  	int nr = nr_shrink;
>  
>  	if (nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
> @@ -2498,14 +2497,22 @@ int f2fs_try_to_free_nids(struct f2fs_sb_info *sbi, int nr_shrink)
>  		return 0;
>  
>  	spin_lock(&nm_i->nid_list_lock);
> -	list_for_each_entry_safe(i, next, &nm_i->free_nid_list, list) {
> -		if (nr_shrink <= 0 ||
> -				nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
> +	while (nr_shrink) {
> +		struct free_nid *i;
> +
> +		if (nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
>  			break;
>  
> +		i = list_first_entry(&nm_i->free_nid_list,
> +					struct free_nid, list);
> +		list_del(&i->list);
> +		spin_unlock(&nm_i->nid_list_lock);
> +
>  		__remove_free_nid(sbi, i, FREE_NID);

__remove_free_nid() will do list_del again. btw, how about just splitting out
given nr_shrink into multiple trials?

>  		kmem_cache_free(free_nid_slab, i);
>  		nr_shrink--;
> +
> +		spin_lock(&nm_i->nid_list_lock);
>  	}
>  	spin_unlock(&nm_i->nid_list_lock);
>  	mutex_unlock(&nm_i->build_lock);
> -- 
> 2.18.0.rc1


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* [f2fs-dev] [PATCH v2] f2fs: shrink spinlock coverage
@ 2020-05-06 10:45 Chao Yu
  2020-05-06 15:05 ` Jaegeuk Kim
  0 siblings, 1 reply; 5+ messages in thread
From: Chao Yu @ 2020-05-06 10:45 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel

In f2fs_try_to_free_nids(), .nid_list_lock spinlock critical region will
increase as expected shrink number increase, to avoid spining other CPUs
for long time, it's better to implement like extent cache and nats
shrinker.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
v2:
- fix unlock wrong spinlock.
 fs/f2fs/node.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 4da0d8713df5..ad0b14f4dab8 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -2488,7 +2488,6 @@ void f2fs_alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid)
 int f2fs_try_to_free_nids(struct f2fs_sb_info *sbi, int nr_shrink)
 {
 	struct f2fs_nm_info *nm_i = NM_I(sbi);
-	struct free_nid *i, *next;
 	int nr = nr_shrink;
 
 	if (nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
@@ -2498,14 +2497,22 @@ int f2fs_try_to_free_nids(struct f2fs_sb_info *sbi, int nr_shrink)
 		return 0;
 
 	spin_lock(&nm_i->nid_list_lock);
-	list_for_each_entry_safe(i, next, &nm_i->free_nid_list, list) {
-		if (nr_shrink <= 0 ||
-				nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
+	while (nr_shrink) {
+		struct free_nid *i;
+
+		if (nm_i->nid_cnt[FREE_NID] <= MAX_FREE_NIDS)
 			break;
 
+		i = list_first_entry(&nm_i->free_nid_list,
+					struct free_nid, list);
+		list_del(&i->list);
+		spin_unlock(&nm_i->nid_list_lock);
+
 		__remove_free_nid(sbi, i, FREE_NID);
 		kmem_cache_free(free_nid_slab, i);
 		nr_shrink--;
+
+		spin_lock(&nm_i->nid_list_lock);
 	}
 	spin_unlock(&nm_i->nid_list_lock);
 	mutex_unlock(&nm_i->build_lock);
-- 
2.18.0.rc1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-08  9:50 [f2fs-dev] [PATCH v2] f2fs: shrink spinlock coverage Chao Yu
  -- strict thread matches above, loose matches on Subject: below --
2020-05-06 10:45 Chao Yu
2020-05-06 15:05 ` Jaegeuk Kim
2020-05-07  3:03   ` Chao Yu
2020-05-07 12:49     ` Jaegeuk Kim

Linux-f2fs-devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-f2fs-devel/0 linux-f2fs-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-f2fs-devel linux-f2fs-devel/ https://lore.kernel.org/linux-f2fs-devel \
		linux-f2fs-devel@lists.sourceforge.net
	public-inbox-index linux-f2fs-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/net.sourceforge.lists.linux-f2fs-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git