Received: by 2002:a05:6a10:206:0:0:0:0 with SMTP id 6csp4212295pxj; Tue, 25 May 2021 02:56:22 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyMDIQosT0ycjdX+HlpTv5p7WG2DYxburEtZlRTMWupLLtunpSUi7nWeTYwE2pXgq/v/44K X-Received: by 2002:a92:c149:: with SMTP id b9mr23272242ilh.195.1621936581830; Tue, 25 May 2021 02:56:21 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1621936581; cv=none; d=google.com; s=arc-20160816; b=k1jlEly8JK6U4IS254hpztyNAeL898dg3u6GrdNOL+VRNX/5TJFijhG9RL8IpnZC6M XXxrAFjszhXviNscNWrgrCavXHHc71XT6cTr1XZDwVHnraIrNXwDtv/9jXjYcZ4ByhIT pAoVMEzdXOKe6Ux2Ez7uRTxHfgcH1tsOM3WlGabO+iAbGJyuatP/gHtC7zWMq7q15euS 6eYuP83+mM3dOS/Q6xyj+fBqWV7QWzawmCNK9tsTclwBOmtOQL2GliIALHE0PyJeZeDA wg902+tlEisfuT6JYC6Sr8FMIKhfsY8bQ6Fh7OJSR0nNXS0B0ySk2g4bg/xqA6KmanVU KLOA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:message-id:date:references :in-reply-to:subject:cc:to:from; bh=fMwjCgzoKDXHVJLJCFwU6y1i+PliDLoi3TgEo5YPkW0=; b=XQAeRcmfBQgqLp61Rcl2KKPuIw+JtA5DDSf8SGJQvI2vw7jtM1y+fFm2OSpmtN1gQr RQzXby0sfzXUAHtOD/sPGyHrntvZaHYV+CmNpGyjl3yxBOCTCvKGBXFJUzhly9qF6Y3+ /0hgaU6AFd560SMN25ncVsjsbY2t81n/p8ccIyKOAqgBuMs+Cw/cikdoMLraZlRQ7QFK 51vxGcFRTNMBXuwvfEpMHxABhCr66hoysD8PVGVU4ZUVrrI0Uful8zzWK58V1z0GTZp0 FzhPaZYazt4XUH61oc3Iuftshz+asfR4V7qgdLN3zkwFqxHo5SCcT44tm0eEZzh+4Eyx wZHQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id b11si18796772jat.37.2021.05.25.02.56.08; Tue, 25 May 2021 02:56:21 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232724AbhEYJyy (ORCPT + 99 others); Tue, 25 May 2021 05:54:54 -0400 Received: from foss.arm.com ([217.140.110.172]:54042 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232723AbhEYJyo (ORCPT ); Tue, 25 May 2021 05:54:44 -0400 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 D22E8D6E; Tue, 25 May 2021 02:53:14 -0700 (PDT) Received: from e113632-lin (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7DFDC3F719; Tue, 25 May 2021 02:53:13 -0700 (PDT) From: Valentin Schneider To: Beata Michalska Cc: linux-kernel@vger.kernel.org, peterz@infradead.org, mingo@redhat.com, juri.lelli@redhat.com, vincent.guittot@linaro.org, dietmar.eggemann@arm.com, corbet@lwn.net, rdunlap@infradead.org, linux-doc@vger.kernel.org Subject: Re: [PATCH v5 2/3] sched/topology: Rework CPU capacity asymmetry detection In-Reply-To: <20210524225508.GA14880@e120325.cambridge.arm.com> References: <20210524101617.8965-1-beata.michalska@arm.com> <20210524101617.8965-3-beata.michalska@arm.com> <87fsyc6mfz.mognet@arm.com> <20210524225508.GA14880@e120325.cambridge.arm.com> Date: Tue, 25 May 2021 10:53:07 +0100 Message-ID: <87a6oj6sxo.mognet@arm.com> MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 24/05/21 23:55, Beata Michalska wrote: > On Mon, May 24, 2021 at 07:01:04PM +0100, Valentin Schneider wrote: >> On 24/05/21 11:16, Beata Michalska wrote: >> > This patch also removes the additional -dflags- parameter used when >> ^^^^^^^^^^^^^^^^^^^^^^^ >> s/^/Also remove/ > I would kind of ... disagree. > All the commit msg is constructed using passive structure, the suggestion > would actually break that. And it does 'sound' bit imperative but I guess > that is subjective. I'd rather stay with impersonal structure (which is > applied through out the whole patchset). It's mainly about the 'This patch' formulation, some take exception to that :-) >> >> > building sched domains as the asymmetry flags are now being set >> > directly in sd_init. >> > >> >> Few nits below, but beyond that: >> >> Tested-by: Valentin Schneider >> Reviewed-by: Valentin Schneider >> > Thanks a lot for the review and testing! > >> > +static inline int >> > +asym_cpu_capacity_classify(struct sched_domain *sd, >> > + const struct cpumask *cpu_map) >> > +{ >> > + int sd_asym_flags = SD_ASYM_CPUCAPACITY | SD_ASYM_CPUCAPACITY_FULL; >> > + struct asym_cap_data *entry; >> > + int asym_cap_count = 0; >> > + >> > + if (list_is_singular(&asym_cap_list)) >> > + goto leave; >> > + >> > + list_for_each_entry(entry, &asym_cap_list, link) { >> > + if (cpumask_intersects(sched_domain_span(sd), entry->cpu_mask)) { >> > + ++asym_cap_count; >> > + } else { >> > + /* >> > + * CPUs with given capacity might be offline >> > + * so make sure this is not the case >> > + */ >> > + if (cpumask_intersects(entry->cpu_mask, cpu_map)) { >> > + sd_asym_flags &= ~SD_ASYM_CPUCAPACITY_FULL; >> > + if (asym_cap_count > 1) >> > + break; >> > + } >> >> Readability nit: That could be made into an else if (). > It could but then this way the -comment- gets more exposed. > But that might be my personal perception so I can change that. As always those are quite subjective! Methink something like this would still draw attention to the offline case: /* * Count how many unique capacities this domain covers. If a * capacity isn't covered, we need to check if any CPU with * that capacity is actually online, otherwise it can be * ignored. */ if (cpumask_intersects(sched_domain_span(sd), entry->cpu_mask)) { ++asym_cap_count; } else if (cpumask_intersects(entry->cpu_mask, cpu_map)) { sd_asym_flags &= ~SD_ASYM_CPUCAPACITY_FULL; if (asym_cap_count > 1) break; }