linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] mm/list_lru: optimize condition of exiting the loop
@ 2020-10-28 14:16 Hui Su
  2020-11-02 13:34 ` Vlastimil Babka
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Hui Su @ 2020-10-28 14:16 UTC (permalink / raw)
  To: akpm, gustavo, songmuchun, vbabka, sh_def, linux-kernel, linux-mm

In list_lru_walk(), nr_to_walk type is 'unsigned long',
so nr_to_walk won't be '< 0'.

In list_lru_walk_node(), nr_to_walk type is 'unsigned long',
so *nr_to_walk won't be '< 0' too.

We can use '!nr_to_walk' instead of 'nr_to_walk <= 0', which
is more precise.

Signed-off-by: Hui Su <sh_def@163.com>
---
 include/linux/list_lru.h | 2 +-
 mm/list_lru.c            | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/list_lru.h b/include/linux/list_lru.h
index 9dcaa3e582c9..b7bc4a2636b9 100644
--- a/include/linux/list_lru.h
+++ b/include/linux/list_lru.h
@@ -214,7 +214,7 @@ list_lru_walk(struct list_lru *lru, list_lru_walk_cb isolate,
 	for_each_node_state(nid, N_NORMAL_MEMORY) {
 		isolated += list_lru_walk_node(lru, nid, isolate,
 					       cb_arg, &nr_to_walk);
-		if (nr_to_walk <= 0)
+		if (!nr_to_walk)
 			break;
 	}
 	return isolated;
diff --git a/mm/list_lru.c b/mm/list_lru.c
index 5aa6e44bc2ae..35be4de9fd77 100644
--- a/mm/list_lru.c
+++ b/mm/list_lru.c
@@ -294,7 +294,7 @@ unsigned long list_lru_walk_node(struct list_lru *lru, int nid,
 
 	isolated += list_lru_walk_one(lru, nid, NULL, isolate, cb_arg,
 				      nr_to_walk);
-	if (*nr_to_walk > 0 && list_lru_memcg_aware(lru)) {
+	if (*nr_to_walk && list_lru_memcg_aware(lru)) {
 		for_each_memcg_cache_index(memcg_idx) {
 			struct list_lru_node *nlru = &lru->node[nid];
 
@@ -304,7 +304,7 @@ unsigned long list_lru_walk_node(struct list_lru *lru, int nid,
 							nr_to_walk);
 			spin_unlock(&nlru->lock);
 
-			if (*nr_to_walk <= 0)
+			if (!*nr_to_walk)
 				break;
 		}
 	}
-- 
2.29.0




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

* Re: [PATCH v2] mm/list_lru: optimize condition of exiting the loop
  2020-10-28 14:16 [PATCH v2] mm/list_lru: optimize condition of exiting the loop Hui Su
@ 2020-11-02 13:34 ` Vlastimil Babka
  2020-11-02 13:44 ` Pankaj Gupta
  2020-11-02 13:48 ` [External] " Muchun Song
  2 siblings, 0 replies; 4+ messages in thread
From: Vlastimil Babka @ 2020-11-02 13:34 UTC (permalink / raw)
  To: Hui Su, akpm, gustavo, songmuchun, linux-kernel, linux-mm

On 10/28/20 3:16 PM, Hui Su wrote:
> In list_lru_walk(), nr_to_walk type is 'unsigned long',
> so nr_to_walk won't be '< 0'.
> 
> In list_lru_walk_node(), nr_to_walk type is 'unsigned long',
> so *nr_to_walk won't be '< 0' too.
> 
> We can use '!nr_to_walk' instead of 'nr_to_walk <= 0', which
> is more precise.

Yes, imho comparisons that make no sense are only misleading for the readers. 
Compilers can probably find out easier, so maybe there's no code generation 
change, but for making it less misleading:

> Signed-off-by: Hui Su <sh_def@163.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>   include/linux/list_lru.h | 2 +-
>   mm/list_lru.c            | 4 ++--
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/list_lru.h b/include/linux/list_lru.h
> index 9dcaa3e582c9..b7bc4a2636b9 100644
> --- a/include/linux/list_lru.h
> +++ b/include/linux/list_lru.h
> @@ -214,7 +214,7 @@ list_lru_walk(struct list_lru *lru, list_lru_walk_cb isolate,
>   	for_each_node_state(nid, N_NORMAL_MEMORY) {
>   		isolated += list_lru_walk_node(lru, nid, isolate,
>   					       cb_arg, &nr_to_walk);
> -		if (nr_to_walk <= 0)
> +		if (!nr_to_walk)
>   			break;
>   	}
>   	return isolated;
> diff --git a/mm/list_lru.c b/mm/list_lru.c
> index 5aa6e44bc2ae..35be4de9fd77 100644
> --- a/mm/list_lru.c
> +++ b/mm/list_lru.c
> @@ -294,7 +294,7 @@ unsigned long list_lru_walk_node(struct list_lru *lru, int nid,
>   
>   	isolated += list_lru_walk_one(lru, nid, NULL, isolate, cb_arg,
>   				      nr_to_walk);
> -	if (*nr_to_walk > 0 && list_lru_memcg_aware(lru)) {
> +	if (*nr_to_walk && list_lru_memcg_aware(lru)) {
>   		for_each_memcg_cache_index(memcg_idx) {
>   			struct list_lru_node *nlru = &lru->node[nid];
>   
> @@ -304,7 +304,7 @@ unsigned long list_lru_walk_node(struct list_lru *lru, int nid,
>   							nr_to_walk);
>   			spin_unlock(&nlru->lock);
>   
> -			if (*nr_to_walk <= 0)
> +			if (!*nr_to_walk)
>   				break;
>   		}
>   	}
> 



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

* Re: [PATCH v2] mm/list_lru: optimize condition of exiting the loop
  2020-10-28 14:16 [PATCH v2] mm/list_lru: optimize condition of exiting the loop Hui Su
  2020-11-02 13:34 ` Vlastimil Babka
@ 2020-11-02 13:44 ` Pankaj Gupta
  2020-11-02 13:48 ` [External] " Muchun Song
  2 siblings, 0 replies; 4+ messages in thread
From: Pankaj Gupta @ 2020-11-02 13:44 UTC (permalink / raw)
  To: Hui Su
  Cc: Andrew Morton, gustavo, Muchun Song, Vlastimil Babka, LKML, Linux MM

> In list_lru_walk(), nr_to_walk type is 'unsigned long',
> so nr_to_walk won't be '< 0'.
>
> In list_lru_walk_node(), nr_to_walk type is 'unsigned long',
> so *nr_to_walk won't be '< 0' too.
>
> We can use '!nr_to_walk' instead of 'nr_to_walk <= 0', which
> is more precise.
>
> Signed-off-by: Hui Su <sh_def@163.com>
> ---
>  include/linux/list_lru.h | 2 +-
>  mm/list_lru.c            | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/list_lru.h b/include/linux/list_lru.h
> index 9dcaa3e582c9..b7bc4a2636b9 100644
> --- a/include/linux/list_lru.h
> +++ b/include/linux/list_lru.h
> @@ -214,7 +214,7 @@ list_lru_walk(struct list_lru *lru, list_lru_walk_cb isolate,
>         for_each_node_state(nid, N_NORMAL_MEMORY) {
>                 isolated += list_lru_walk_node(lru, nid, isolate,
>                                                cb_arg, &nr_to_walk);
> -               if (nr_to_walk <= 0)
> +               if (!nr_to_walk)
>                         break;
>         }
>         return isolated;
> diff --git a/mm/list_lru.c b/mm/list_lru.c
> index 5aa6e44bc2ae..35be4de9fd77 100644
> --- a/mm/list_lru.c
> +++ b/mm/list_lru.c
> @@ -294,7 +294,7 @@ unsigned long list_lru_walk_node(struct list_lru *lru, int nid,
>
>         isolated += list_lru_walk_one(lru, nid, NULL, isolate, cb_arg,
>                                       nr_to_walk);
> -       if (*nr_to_walk > 0 && list_lru_memcg_aware(lru)) {
> +       if (*nr_to_walk && list_lru_memcg_aware(lru)) {
>                 for_each_memcg_cache_index(memcg_idx) {
>                         struct list_lru_node *nlru = &lru->node[nid];
>
> @@ -304,7 +304,7 @@ unsigned long list_lru_walk_node(struct list_lru *lru, int nid,
>                                                         nr_to_walk);
>                         spin_unlock(&nlru->lock);
>
> -                       if (*nr_to_walk <= 0)
> +                       if (!*nr_to_walk)
>                                 break;
>                 }
>         }

Acked-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com>


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

* Re: [External] [PATCH v2] mm/list_lru: optimize condition of exiting the loop
  2020-10-28 14:16 [PATCH v2] mm/list_lru: optimize condition of exiting the loop Hui Su
  2020-11-02 13:34 ` Vlastimil Babka
  2020-11-02 13:44 ` Pankaj Gupta
@ 2020-11-02 13:48 ` Muchun Song
  2 siblings, 0 replies; 4+ messages in thread
From: Muchun Song @ 2020-11-02 13:48 UTC (permalink / raw)
  To: Hui Su
  Cc: Andrew Morton, gustavo, Vlastimil Babka, LKML,
	Linux Memory Management List

On Wed, Oct 28, 2020 at 10:17 PM Hui Su <sh_def@163.com> wrote:
>
> In list_lru_walk(), nr_to_walk type is 'unsigned long',
> so nr_to_walk won't be '< 0'.
>
> In list_lru_walk_node(), nr_to_walk type is 'unsigned long',
> so *nr_to_walk won't be '< 0' too.
>
> We can use '!nr_to_walk' instead of 'nr_to_walk <= 0', which
> is more precise.
>
> Signed-off-by: Hui Su <sh_def@163.com>

Reviewed-by: Muchun Song <songmuchun@bytedance.com>

> ---
>  include/linux/list_lru.h | 2 +-
>  mm/list_lru.c            | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/list_lru.h b/include/linux/list_lru.h
> index 9dcaa3e582c9..b7bc4a2636b9 100644
> --- a/include/linux/list_lru.h
> +++ b/include/linux/list_lru.h
> @@ -214,7 +214,7 @@ list_lru_walk(struct list_lru *lru, list_lru_walk_cb isolate,
>         for_each_node_state(nid, N_NORMAL_MEMORY) {
>                 isolated += list_lru_walk_node(lru, nid, isolate,
>                                                cb_arg, &nr_to_walk);
> -               if (nr_to_walk <= 0)
> +               if (!nr_to_walk)
>                         break;
>         }
>         return isolated;
> diff --git a/mm/list_lru.c b/mm/list_lru.c
> index 5aa6e44bc2ae..35be4de9fd77 100644
> --- a/mm/list_lru.c
> +++ b/mm/list_lru.c
> @@ -294,7 +294,7 @@ unsigned long list_lru_walk_node(struct list_lru *lru, int nid,
>
>         isolated += list_lru_walk_one(lru, nid, NULL, isolate, cb_arg,
>                                       nr_to_walk);
> -       if (*nr_to_walk > 0 && list_lru_memcg_aware(lru)) {
> +       if (*nr_to_walk && list_lru_memcg_aware(lru)) {
>                 for_each_memcg_cache_index(memcg_idx) {
>                         struct list_lru_node *nlru = &lru->node[nid];
>
> @@ -304,7 +304,7 @@ unsigned long list_lru_walk_node(struct list_lru *lru, int nid,
>                                                         nr_to_walk);
>                         spin_unlock(&nlru->lock);
>
> -                       if (*nr_to_walk <= 0)
> +                       if (!*nr_to_walk)
>                                 break;
>                 }
>         }
> --
> 2.29.0
>
>


--
Yours,
Muchun


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

end of thread, other threads:[~2020-11-02 13:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-28 14:16 [PATCH v2] mm/list_lru: optimize condition of exiting the loop Hui Su
2020-11-02 13:34 ` Vlastimil Babka
2020-11-02 13:44 ` Pankaj Gupta
2020-11-02 13:48 ` [External] " Muchun Song

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox