Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Message-ID: <708eda13-6615-4efe-87e1-f3610e90e116@redhat.com>
Date:   Tue, 28 Nov 2023 11:46:18 -0500
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v4 2/3] cgroup/rstat: Optimize cgroup_rstat_updated_list()
Content-Language: en-US
To:     Tejun Heo <tj@kernel.org>
Cc:     Zefan Li <lizefan.x@bytedance.com>,
        Johannes Weiner <hannes@cmpxchg.org>, cgroups@vger.kernel.org,
        linux-kernel@vger.kernel.org, Joe Mario <jmario@redhat.com>,
        Sebastian Jug <sejug@redhat.com>,
        Yosry Ahmed <yosryahmed@google.com>
References: <20231106210543.717486-1-longman@redhat.com>
 <20231106210543.717486-3-longman@redhat.com>
 <a9aa2809-95f5-4f60-b936-0d857c971fea@redhat.com>
 <ZWYYrJVMUOrl9r2g@slm.duckdns.org>
From:   Waiman Long <longman@redhat.com>
In-Reply-To: <ZWYYrJVMUOrl9r2g@slm.duckdns.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 11/28/23 11:43, Tejun Heo wrote:
> On Mon, Nov 27, 2023 at 11:01:22PM -0500, Waiman Long wrote:
> ...
>>> + * Recursively traverse down the cgroup_rstat_cpu updated tree and push
>>> + * parent first before its children into a singly linked list built from
>>> + * the tail backward like "pushing" cgroups into a stack. The parent is
>>> + * pushed by the caller. The recursion depth is the depth of the current
>>> + * updated subtree.
>>> + */
>>> +static struct cgroup *cgroup_rstat_push_children(struct cgroup *head,
>>> +				struct cgroup_rstat_cpu *prstatc, int cpu)
>>> +{
>>> +	struct cgroup *child, *parent;
>>> +	struct cgroup_rstat_cpu *crstatc;
>>> +
>>> +	parent = head;
>>> +	child = prstatc->updated_children;
>>> +	prstatc->updated_children = parent;
>>> +
>>> +	/* updated_next is parent cgroup terminated */
>>> +	while (child != parent) {
>>> +		child->rstat_flush_next = head;
>>> +		head = child;
>>> +		crstatc = cgroup_rstat_cpu(child, cpu);
>>> +		if (crstatc->updated_children != child)
>>> +			head = cgroup_rstat_push_children(head, crstatc, cpu);
>>> +		child = crstatc->updated_next;
>>> +		crstatc->updated_next = NULL;
>>> +	}
>>> +	return head;
> The recursion bothers me. We don't really have a hard limit on nesting
> depth. We might need to add another pointer field but can make this
> iterative, right?

I see. Yes, I think it is possible to make it iterative. Using recursion 
is just an easier way to do it. Will look into that.

Thanks,
Longman

>
> Thanks.
>