| Message ID | 20201027170420.GA61326@rlk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/list_lru: optimize condition of exiting the loop | expand |
On 10/27/20 6:04 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. > > Signed-off-by: Hui Su <sh_def@163.com> OK. Why not this too? --- 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]; > --- > include/linux/list_lru.h | 2 +- > mm/list_lru.c | 2 +- > 2 files changed, 2 insertions(+), 2 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..39b8d467159d 100644 > --- a/mm/list_lru.c > +++ b/mm/list_lru.c > @@ -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; > } > } >
On Tue, Oct 27, 2020 at 07:45:53PM +0100, Vlastimil Babka wrote: > On 10/27/20 6:04 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. > > > > Signed-off-by: Hui Su <sh_def@163.com> > > OK. Why not this too? > > --- 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]; > > Thanks for your fast reply. I did not notice that, and i would add this to my change. I will resend PATCH V2, and cc to you. Thanks.
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..39b8d467159d 100644 --- a/mm/list_lru.c +++ b/mm/list_lru.c @@ -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; } }
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 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)