| Message ID | 20220111010302.8864-4-richard.weiyang@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] mm/memcg: use NUMA_NO_NODE to indicate allocation from unspecified node | expand |
On Tue 11-01-22 01:03:02, Wei Yang wrote: > mem_cgroup_threshold_ary->current_threshold points to the last entry > who's threshold is less or equal to usage. > > Instead of iterating entries to get the correct index, we can leverage > primary->current_threshold to get it. If the threshold added is less or > equal to usage, current_threshold should increase by one. Otherwise, it > doesn't change. Why do we want/need this change? > Signed-off-by: Wei Yang <richard.weiyang@gmail.com> > --- > mm/memcontrol.c | 31 +++++++++++++++++-------------- > 1 file changed, 17 insertions(+), 14 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index a504616f904a..ce7060907df2 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -4161,7 +4161,7 @@ static int __mem_cgroup_usage_register_event(struct mem_cgroup *memcg, > struct mem_cgroup_threshold_ary *new; > unsigned long threshold; > unsigned long usage; > - int i, size, ret; > + int size, ret; > > ret = page_counter_memparse(args, "-1", &threshold); > if (ret) > @@ -4193,9 +4193,13 @@ static int __mem_cgroup_usage_register_event(struct mem_cgroup *memcg, > new->size = size; > > /* Copy thresholds (if any) to new array */ > - if (thresholds->primary) > + if (thresholds->primary) { > memcpy(new->entries, thresholds->primary->entries, > flex_array_size(new, entries, size - 1)); > + new->current_threshold = thresholds->primary->current_threshold; > + } else { > + new->current_threshold = -1; > + } > > /* Add new threshold */ > new->entries[size - 1].eventfd = eventfd; > @@ -4205,18 +4209,17 @@ static int __mem_cgroup_usage_register_event(struct mem_cgroup *memcg, > sort(new->entries, size, sizeof(*new->entries), > compare_thresholds, NULL); > > - /* Find current threshold */ > - new->current_threshold = -1; > - for (i = 0; i < size; i++) { > - if (new->entries[i].threshold <= usage) { > - /* > - * new->current_threshold will not be used until > - * rcu_assign_pointer(), so it's safe to increment > - * it here. > - */ > - ++new->current_threshold; > - } else > - break; > + /* > + * If the threshold added here is less or equal to usage, this means > + * current_threshold need to increase by one. > + */ > + if (threshold <= usage) { > + /* > + * new->current_threshold will not be used until > + * rcu_assign_pointer(), so it's safe to increment > + * it here. > + */ > + new->current_threshold++; > } > > /* Free old spare buffer and save old primary buffer as spare */ > -- > 2.33.1
On Tue, Jan 11, 2022 at 01:03:02AM +0000, Wei Yang wrote: > mem_cgroup_threshold_ary->current_threshold points to the last entry > who's threshold is less or equal to usage. > > Instead of iterating entries to get the correct index, we can leverage > primary->current_threshold to get it. If the threshold added is less or > equal to usage, current_threshold should increase by one. Otherwise, it > doesn't change. How big is usually an array of thresholds? If it's not huge, likely any savings won't be really noticeable (it's not a hot path and there is an rc_synchronize() below). So I agree with Michal that a better justification is really needed. Thanks!
On Tue, Jan 11, 2022 at 10:23:41AM -0800, Roman Gushchin wrote: >On Tue, Jan 11, 2022 at 01:03:02AM +0000, Wei Yang wrote: >> mem_cgroup_threshold_ary->current_threshold points to the last entry >> who's threshold is less or equal to usage. >> >> Instead of iterating entries to get the correct index, we can leverage >> primary->current_threshold to get it. If the threshold added is less or >> equal to usage, current_threshold should increase by one. Otherwise, it >> doesn't change. > >How big is usually an array of thresholds? If it's not huge, likely >any savings won't be really noticeable (it's not a hot path and there >is an rc_synchronize() below). Usually the size is small I think. > >So I agree with Michal that a better justification is really needed. Yep. > >Thanks!
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index a504616f904a..ce7060907df2 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -4161,7 +4161,7 @@ static int __mem_cgroup_usage_register_event(struct mem_cgroup *memcg, struct mem_cgroup_threshold_ary *new; unsigned long threshold; unsigned long usage; - int i, size, ret; + int size, ret; ret = page_counter_memparse(args, "-1", &threshold); if (ret) @@ -4193,9 +4193,13 @@ static int __mem_cgroup_usage_register_event(struct mem_cgroup *memcg, new->size = size; /* Copy thresholds (if any) to new array */ - if (thresholds->primary) + if (thresholds->primary) { memcpy(new->entries, thresholds->primary->entries, flex_array_size(new, entries, size - 1)); + new->current_threshold = thresholds->primary->current_threshold; + } else { + new->current_threshold = -1; + } /* Add new threshold */ new->entries[size - 1].eventfd = eventfd; @@ -4205,18 +4209,17 @@ static int __mem_cgroup_usage_register_event(struct mem_cgroup *memcg, sort(new->entries, size, sizeof(*new->entries), compare_thresholds, NULL); - /* Find current threshold */ - new->current_threshold = -1; - for (i = 0; i < size; i++) { - if (new->entries[i].threshold <= usage) { - /* - * new->current_threshold will not be used until - * rcu_assign_pointer(), so it's safe to increment - * it here. - */ - ++new->current_threshold; - } else - break; + /* + * If the threshold added here is less or equal to usage, this means + * current_threshold need to increase by one. + */ + if (threshold <= usage) { + /* + * new->current_threshold will not be used until + * rcu_assign_pointer(), so it's safe to increment + * it here. + */ + new->current_threshold++; } /* Free old spare buffer and save old primary buffer as spare */
mem_cgroup_threshold_ary->current_threshold points to the last entry who's threshold is less or equal to usage. Instead of iterating entries to get the correct index, we can leverage primary->current_threshold to get it. If the threshold added is less or equal to usage, current_threshold should increase by one. Otherwise, it doesn't change. Signed-off-by: Wei Yang <richard.weiyang@gmail.com> --- mm/memcontrol.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-)