Received: by 2002:a05:7412:b995:b0:f9:9502:5bb8 with SMTP id it21csp6668727rdb; Tue, 2 Jan 2024 09:12:15 -0800 (PST) X-Google-Smtp-Source: AGHT+IFSCLxSkd0hErpr5E1kG89XdFECTObDobHqhNL+UO1fI1jpG1Qz9ovvj+52yTYXGUinanO4 X-Received: by 2002:a05:6e02:1a86:b0:360:e6b:bc4b with SMTP id k6-20020a056e021a8600b003600e6bbc4bmr18916405ilv.48.1704215535064; Tue, 02 Jan 2024 09:12:15 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1704215535; cv=none; d=google.com; s=arc-20160816; b=ohZL7rMGt5ofCwPyhdk7B9fHD7pkLRGXRFMw38sX4UATE4POmA9QrnYH3za53jmP5f McjgF2/JxOJ5Z5gQ+9yvle4P5f6U44orV42tRn5Dbfc9pEDXJrvuli5y17ejHF6aD3BN c2TPdx6QPMgxsXUsfU5SOrjzM/YP5jW6i9doZi03EieM1dWjJI6PzLSyCtYWZYSaDRhm vwTB/nn+NkvziwXwFP+Y9gPiXbXJOlKw7Fje6nhxkJRgwaAryadW/bIQ4HPB09M6+rUX 2fnlRhrFMtpFjgWUq2bmcaiQhuQXVH8Y8xIVk+kgLeXVGTQk1kx9Pmv7nfeEWwrKZ0pL mV4A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:from:user-agent:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:date:message-id; bh=0eGMr9CiLt8hFFUvLw+6UR0mNObMMbiQmEDqRuqu5sQ=; fh=6pmPfZp7P/Vfhw35xHBgzqY7wv05Azv1fj+RLFJ/g2s=; b=rAplo3ZUpPD9g+LpAvYgnLOU2xMSXZzLSOIFcyMak834K5zY1mJSOGitXZXDWV5RrZ xwmfecHtv+lzsfFvO0EIGsDK0ofqPJlA+FwIGJBfhBqF2/eYRlbu9lw5KdIjanJ95Nqz v51ZoUK9m28u8nGEZ0VUUtvXDbLKQ0ginqy79RsKBejZ3VSSYLaEAEkdG8PwRDYrmx2U BVbCKXKDK0tBPogZhNfe6jVcQ6jXOUoqCh/qbrSvCsj7uwv0HcZGfere7wptFQxXwAMm wLfZLwWd8QjLFSyvTFsfC9cS8AB8wunX8N1qOlbqJlAxsMm8OxMMKralmn2mJVJ2q0Pu zhZQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel+bounces-14643-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-14643-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Return-Path: Received: from sy.mirrors.kernel.org (sy.mirrors.kernel.org. [2604:1380:40f1:3f00::1]) by mx.google.com with ESMTPS id w23-20020a631617000000b005cd82e43aa3si18017710pgl.321.2024.01.02.09.12.14 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Jan 2024 09:12:15 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-14643-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel+bounces-14643-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-14643-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sy.mirrors.kernel.org (Postfix) with ESMTPS id 28485B22864 for ; Tue, 2 Jan 2024 17:10:33 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 08932154B1; Tue, 2 Jan 2024 17:10:01 +0000 (UTC) X-Original-To: linux-kernel@vger.kernel.org Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id CE7471549C for ; Tue, 2 Jan 2024 17:09:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 265C4C15; Tue, 2 Jan 2024 09:10:43 -0800 (PST) Received: from [10.34.100.129] (e126645.nice.arm.com [10.34.100.129]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1859F3F64C; Tue, 2 Jan 2024 09:09:55 -0800 (PST) Message-ID: <8e2263b7-e33b-43e1-b0df-92b560b8fa25@arm.com> Date: Tue, 2 Jan 2024 18:09:54 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Pierre Gondois Subject: Re: [PATCH v3 2/2] sched/topology: Sort asym_cap_list in descending order To: Qais Yousef , Ingo Molnar , Peter Zijlstra , Vincent Guittot , Dietmar Eggemann Cc: linux-kernel@vger.kernel.org References: <20231231175218.510721-1-qyousef@layalina.io> <20231231175218.510721-3-qyousef@layalina.io> Content-Language: en-US In-Reply-To: <20231231175218.510721-3-qyousef@layalina.io> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hello Qais, I just have some comments regarding the commit message/comments, otherwise the code (of the 2 patches) looks good to me, Regards, Pierre On 12/31/23 18:52, Qais Yousef wrote: > So that searches always start from biggest CPU which would help misfit > detection logic to be more efficient. > > I see the following when adding trace_printks() during add and del > operations > > init-1 [000] ..... 0.058128: asym_cpu_capacity_update_data: Added new capacity 250. Capacity list order: > init-1 [000] ..... 0.058132: asym_cpu_capacity_update_data: -- 250 > init-1 [000] ..... 0.058135: asym_cpu_capacity_update_data: Added new capacity 620. Capacity list order: > init-1 [000] ..... 0.058136: asym_cpu_capacity_update_data: -- 620 > init-1 [000] ..... 0.058137: asym_cpu_capacity_update_data: -- 250 > init-1 [000] ..... 0.058139: asym_cpu_capacity_update_data: Added new capacity 1024. Capacity list order: > init-1 [000] ..... 0.058140: asym_cpu_capacity_update_data: -- 1024 > init-1 [000] ..... 0.058141: asym_cpu_capacity_update_data: -- 620 > init-1 [000] ..... 0.058142: asym_cpu_capacity_update_data: -- 250 > init-1 [000] ..... 0.058143: asym_cpu_capacity_scan: Final capacity list order: > init-1 [000] ..... 0.058145: asym_cpu_capacity_scan: -- 1024 > init-1 [000] ..... 0.058145: asym_cpu_capacity_scan: -- 620 > init-1 [000] ..... 0.058146: asym_cpu_capacity_scan: -- 250 > <...>-244 [007] ..... 1.959174: asym_cpu_capacity_update_data: Added new capacity 160. Capacity list order: > <...>-244 [007] ..... 1.959175: asym_cpu_capacity_update_data: -- 1024 > <...>-244 [007] ..... 1.959176: asym_cpu_capacity_update_data: -- 620 > <...>-244 [007] ..... 1.959176: asym_cpu_capacity_update_data: -- 250 > <...>-244 [007] ..... 1.959176: asym_cpu_capacity_update_data: -- 160 > <...>-244 [007] ..... 1.959183: asym_cpu_capacity_update_data: Added new capacity 498. Capacity list order: > <...>-244 [007] ..... 1.959184: asym_cpu_capacity_update_data: -- 1024 > <...>-244 [007] ..... 1.959184: asym_cpu_capacity_update_data: -- 620 > <...>-244 [007] ..... 1.959185: asym_cpu_capacity_update_data: -- 498 > <...>-244 [007] ..... 1.959185: asym_cpu_capacity_update_data: -- 250 > <...>-244 [007] ..... 1.959186: asym_cpu_capacity_update_data: -- 160 > <...>-244 [007] ..... 1.959204: asym_cpu_capacity_scan: Deleted capacity 620 > <...>-244 [007] ..... 1.959208: asym_cpu_capacity_scan: Deleted capacity 250 > <...>-244 [007] ..... 1.959209: asym_cpu_capacity_scan: Final capacity list order: > <...>-244 [007] ..... 1.959209: asym_cpu_capacity_scan: -- 1024 > <...>-244 [007] ..... 1.959210: asym_cpu_capacity_scan: -- 498 > <...>-244 [007] ..... 1.959210: asym_cpu_capacity_scan: -- 160 > rcuop/7-66 [001] b.... 1.968114: free_asym_cap_entry: Freeing capacity 620 > rcuop/7-66 [001] b.... 1.968118: free_asym_cap_entry: Freeing capacity 250 IMO the trace is not necessary. > > Suggested-by: Pierre Gondois > Signed-off-by: Qais Yousef (Google) > --- > kernel/sched/topology.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c > index ba4a0b18ae25..1505677e4247 100644 > --- a/kernel/sched/topology.c > +++ b/kernel/sched/topology.c > @@ -1384,18 +1384,30 @@ static void free_asym_cap_entry(struct rcu_head *head) > static inline void asym_cpu_capacity_update_data(int cpu) > { > unsigned long capacity = arch_scale_cpu_capacity(cpu); > - struct asym_cap_data *entry = NULL; > + struct asym_cap_data *insert_entry = NULL; > + struct asym_cap_data *entry; > > + /* > + * Search if capacity already exits. If not, track which the entry > + * where we should insert to keep the list ordered descendingly. > + */ IMO just a small comment like the one suggested below should be enough, but this is just a suggestion. > list_for_each_entry(entry, &asym_cap_list, link) { > if (capacity == entry->capacity) > goto done; > + else if (!insert_entry && capacity > entry->capacity) > + insert_entry = list_prev_entry(entry, link); > } > > entry = kzalloc(sizeof(*entry) + cpumask_size(), GFP_KERNEL); > if (WARN_ONCE(!entry, "Failed to allocate memory for asymmetry data\n")) > return; > entry->capacity = capacity; > - list_add_rcu(&entry->link, &asym_cap_list); > + > + /* If NULL then the new capacity is the smallest, add last. */ (something like): /* asym_cap_list is sorted by descending order. */ > + if (!insert_entry) > + list_add_tail_rcu(&entry->link, &asym_cap_list); > + else > + list_add_rcu(&entry->link, &insert_entry->link); > done: > __cpumask_set_cpu(cpu, cpu_capacity_span(entry)); > }