Instead of do while unconditionally, let's put the loop variant in
while.
Signed-off-by: Wei Yang <[email protected]>
---
kernel/cgroup/rstat.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c
index 1486768f2318..a9d344e0521d 100644
--- a/kernel/cgroup/rstat.c
+++ b/kernel/cgroup/rstat.c
@@ -124,12 +124,10 @@ static struct cgroup *cgroup_rstat_cpu_pop_updated(struct cgroup *pos,
prstatc = cgroup_rstat_cpu(parent, cpu);
nextp = &prstatc->updated_children;
- while (true) {
+ while (*nextp != pos) {
struct cgroup_rstat_cpu *nrstatc;
nrstatc = cgroup_rstat_cpu(*nextp, cpu);
- if (*nextp == pos)
- break;
WARN_ON_ONCE(*nextp == parent);
nextp = &nrstatc->updated_next;
}
--
2.33.1
After commit dc26532aed0a ("cgroup: rstat: punt root-level optimization to
individual controllers"), each rstat on updated_children list has its
->updated_next not NULL.
This means we can remove the check on ->updated_next, if we make sure
the subtree from @root is on list, which could be done by checking
updated_next for root.
Signed-off-by: Wei Yang <[email protected]>
---
kernel/cgroup/rstat.c | 46 +++++++++++++++++++++----------------------
1 file changed, 22 insertions(+), 24 deletions(-)
diff --git a/kernel/cgroup/rstat.c b/kernel/cgroup/rstat.c
index a9d344e0521d..a00875375f7d 100644
--- a/kernel/cgroup/rstat.c
+++ b/kernel/cgroup/rstat.c
@@ -88,6 +88,7 @@ static struct cgroup *cgroup_rstat_cpu_pop_updated(struct cgroup *pos,
struct cgroup *root, int cpu)
{
struct cgroup_rstat_cpu *rstatc;
+ struct cgroup *parent;
if (pos == root)
return NULL;
@@ -96,9 +97,12 @@ static struct cgroup *cgroup_rstat_cpu_pop_updated(struct cgroup *pos,
* We're gonna walk down to the first leaf and visit/remove it. We
* can pick whatever unvisited node as the starting point.
*/
- if (!pos)
+ if (!pos) {
pos = root;
- else
+ // return NULL if this subtree is not on-list
+ if (!cgroup_rstat_cpu(pos, cpu)->updated_next)
+ return NULL;
+ } else
pos = cgroup_parent(pos);
/* walk down to the first leaf */
@@ -115,31 +119,25 @@ static struct cgroup *cgroup_rstat_cpu_pop_updated(struct cgroup *pos,
* However, due to the way we traverse, @pos will be the first
* child in most cases. The only exception is @root.
*/
- if (rstatc->updated_next) {
- struct cgroup *parent = cgroup_parent(pos);
-
- if (parent) {
- struct cgroup_rstat_cpu *prstatc;
- struct cgroup **nextp;
-
- prstatc = cgroup_rstat_cpu(parent, cpu);
- nextp = &prstatc->updated_children;
- while (*nextp != pos) {
- struct cgroup_rstat_cpu *nrstatc;
-
- nrstatc = cgroup_rstat_cpu(*nextp, cpu);
- WARN_ON_ONCE(*nextp == parent);
- nextp = &nrstatc->updated_next;
- }
- *nextp = rstatc->updated_next;
- }
+ parent = cgroup_parent(pos);
+ if (parent) {
+ struct cgroup_rstat_cpu *prstatc;
+ struct cgroup **nextp;
- rstatc->updated_next = NULL;
- return pos;
+ prstatc = cgroup_rstat_cpu(parent, cpu);
+ nextp = &prstatc->updated_children;
+ while (*nextp != pos) {
+ struct cgroup_rstat_cpu *nrstatc;
+
+ nrstatc = cgroup_rstat_cpu(*nextp, cpu);
+ WARN_ON_ONCE(*nextp == parent);
+ nextp = &nrstatc->updated_next;
+ }
+ *nextp = rstatc->updated_next;
}
- /* only happens for @root */
- return NULL;
+ rstatc->updated_next = NULL;
+ return pos;
}
/* see cgroup_rstat_flush() */
--
2.33.1
On Sat, Dec 25, 2021 at 12:09:32AM +0000, Wei Yang <[email protected]> wrote:
> This means we can remove the check on ->updated_next, if we make sure
> the subtree from @root is on list, which could be done by checking
> updated_next for root.
Nice refactoring.
> @@ -96,9 +97,12 @@ static struct cgroup *cgroup_rstat_cpu_pop_updated(struct cgroup *pos,
> * We're gonna walk down to the first leaf and visit/remove it. We
> * can pick whatever unvisited node as the starting point.
> */
> - if (!pos)
> + if (!pos) {
> pos = root;
> - else
> + // return NULL if this subtree is not on-list
> + if (!cgroup_rstat_cpu(pos, cpu)->updated_next)
> + return NULL;
> + } else
+ /* return NULL if this subtree is not on-list */
Just a coding style nitpick.
The patch is otherwise
Reviewed-by: Michal Koutn? <[email protected]>
On Sat, Dec 25, 2021 at 12:09:31AM +0000, Wei Yang <[email protected]> wrote:
> Instead of do while unconditionally, let's put the loop variant in
> while.
>
> Signed-off-by: Wei Yang <[email protected]>
> ---
> kernel/cgroup/rstat.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
Makes sense,
Reviewed-by: Michal Koutn? <[email protected]>
On Wed, Jan 05, 2022 at 08:35:04PM +0100, Michal Koutn? wrote:
>On Sat, Dec 25, 2021 at 12:09:32AM +0000, Wei Yang <[email protected]> wrote:
>> This means we can remove the check on ->updated_next, if we make sure
>> the subtree from @root is on list, which could be done by checking
>> updated_next for root.
>
>Nice refactoring.
>
>> @@ -96,9 +97,12 @@ static struct cgroup *cgroup_rstat_cpu_pop_updated(struct cgroup *pos,
>> * We're gonna walk down to the first leaf and visit/remove it. We
>> * can pick whatever unvisited node as the starting point.
>> */
>> - if (!pos)
>> + if (!pos) {
>> pos = root;
>> - else
>> + // return NULL if this subtree is not on-list
>> + if (!cgroup_rstat_cpu(pos, cpu)->updated_next)
>> + return NULL;
>> + } else
>+ /* return NULL if this subtree is not on-list */
>
>Just a coding style nitpick.
Thanks for comment. Would you like me to send a v2?
>
>The patch is otherwise
>Reviewed-by: Michal Koutn? <[email protected]>
--
Wei Yang
Help you, Help me
On Sat, Dec 25, 2021 at 12:09:31AM +0000, Wei Yang wrote:
> Instead of do while unconditionally, let's put the loop variant in
> while.
>
> Signed-off-by: Wei Yang <[email protected]>
Applied to cgorup/for-5.17.
Thanks.
--
tejun
On Sat, Dec 25, 2021 at 12:09:32AM +0000, Wei Yang wrote:
> After commit dc26532aed0a ("cgroup: rstat: punt root-level optimization to
> individual controllers"), each rstat on updated_children list has its
> ->updated_next not NULL.
>
> This means we can remove the check on ->updated_next, if we make sure
> the subtree from @root is on list, which could be done by checking
> updated_next for root.
>
> Signed-off-by: Wei Yang <[email protected]>
Applied to cgroup/for-5.17 w/ coding style fixes.
Thanks.
--
tejun