From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E889FC54EAA for ; Fri, 27 Jan 2023 14:55:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234056AbjA0OzR (ORCPT ); Fri, 27 Jan 2023 09:55:17 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60614 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233140AbjA0OzP (ORCPT ); Fri, 27 Jan 2023 09:55:15 -0500 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AABC974C11 for ; Fri, 27 Jan 2023 06:54:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674831269; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ysIL8RukAYmBUB71mQtyT0OJ0/jFEvPNazkDdH1z3cQ=; b=f9rFpNdWGx7MJmxf22FRMKFjAvwxzkiQe0VkiaIb6Ar90xv5f+PLeyqiuY7SsHG3N+3FvK o24AvL4rDBKjPWF0WEIGACedeeDlwGmXbMKP3W4w0rR4pypCaKRkuCLwiNKQSrSmnTU7+z EwPDrdcffg65YF4t5JzzGT8jwZ9s0bA= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-399-s6hjROmeP6OX23gbZ5ufjA-1; Fri, 27 Jan 2023 09:54:28 -0500 X-MC-Unique: s6hjROmeP6OX23gbZ5ufjA-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 859DE29DD993; Fri, 27 Jan 2023 14:54:27 +0000 (UTC) Received: from [10.22.10.246] (unknown [10.22.10.246]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8F3272166B26; Fri, 27 Jan 2023 14:54:26 +0000 (UTC) Message-ID: <43da8f9d-f0fd-d67b-7384-fc03ad159f29@redhat.com> Date: Fri, 27 Jan 2023 09:54:26 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.0 Subject: Re: [PATCH v3] sched: Store restrict_cpus_allowed_ptr() call state To: Will Deacon Cc: Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Daniel Bristot de Oliveira , Valentin Schneider , Phil Auld , Linus Torvalds , linux-kernel@vger.kernel.org References: <20230127015527.466367-1-longman@redhat.com> <20230127125946.GA30605@willie-the-truck> Content-Language: en-US From: Waiman Long In-Reply-To: <20230127125946.GA30605@willie-the-truck> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 3.1 on 10.11.54.6 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 1/27/23 07:59, Will Deacon wrote: > Hi Waiman, > > On Thu, Jan 26, 2023 at 08:55:27PM -0500, Waiman Long wrote: >> The user_cpus_ptr field was originally added by commit b90ca8badbd1 >> ("sched: Introduce task_struct::user_cpus_ptr to track requested >> affinity"). It was used only by arm64 arch due to possible asymmetric >> CPU setup. >> >> Since commit 8f9ea86fdf99 ("sched: Always preserve the user requested >> cpumask"), task_struct::user_cpus_ptr is repurposed to store user >> requested cpu affinity specified in the sched_setaffinity(). >> >> This results in a slight performance regression on an arm64 >> system when booted with "allow_mismatched_32bit_el0" >> on the command-line. The arch code will (amongst >> other things) calls force_compatible_cpus_allowed_ptr() and >> relax_compatible_cpus_allowed_ptr() when exec()'ing a 32-bit or a 64-bit >> task respectively. Now a call to relax_compatible_cpus_allowed_ptr() >> will always result in a __sched_setaffinity() call whether there is a >> previous force_compatible_cpus_allowed_ptr() call or not. >> >> In order to fix this regression, a new scheduler flag >> task_struct::cpus_allowed_restricted is now added to track if >> force_compatible_cpus_allowed_ptr() has been called before or not. This >> patch also updates the comments in force_compatible_cpus_allowed_ptr() >> and relax_compatible_cpus_allowed_ptr() and handles their interaction >> with sched_setaffinity(). >> >> This patch also removes the task_user_cpus() helper. In the case of >> relax_compatible_cpus_allowed_ptr(), cpu_possible_mask as user_cpu_ptr >> masking will be performed within __sched_setaffinity() anyway. >> >> Fixes: 8f9ea86fdf99 ("sched: Always preserve the user requested cpumask") >> Reported-by: Will Deacon >> Signed-off-by: Waiman Long >> --- >> include/linux/sched.h | 3 +++ >> kernel/sched/core.c | 25 +++++++++++++++++-------- >> kernel/sched/sched.h | 8 +------- >> 3 files changed, 21 insertions(+), 15 deletions(-) > So this doesn't even build... > >> diff --git a/kernel/sched/core.c b/kernel/sched/core.c >> index bb1ee6d7bdde..d7bc809c109e 100644 >> --- a/kernel/sched/core.c >> +++ b/kernel/sched/core.c >> @@ -2999,6 +2999,10 @@ static int __set_cpus_allowed_ptr(struct task_struct *p, >> struct rq *rq; >> >> rq = task_rq_lock(p, &rf); >> + >> + if (ctx->flags & SCA_CLR_RESTRICT) >> + p->cpus_allowed_restricted = 0; >> + >> /* >> * Masking should be skipped if SCA_USER or any of the SCA_MIGRATE_* >> * flags are set. >> @@ -3025,8 +3029,8 @@ EXPORT_SYMBOL_GPL(set_cpus_allowed_ptr); >> /* >> * Change a given task's CPU affinity to the intersection of its current >> * affinity mask and @subset_mask, writing the resulting mask to @new_mask. >> - * If user_cpus_ptr is defined, use it as the basis for restricting CPU >> - * affinity or use cpu_online_mask instead. >> + * The cpus_allowed_restricted bit is set to indicate to a later >> + * relax_compatible_cpus_allowed_ptr() call to relax the cpumask. >> * >> * If the resulting mask is empty, leave the affinity unchanged and return >> * -EINVAL. >> @@ -3044,6 +3048,7 @@ static int restrict_cpus_allowed_ptr(struct task_struct *p, >> int err; >> >> rq = task_rq_lock(p, &rf); >> + p->cpus_allowed_restricted = 1; >> >> /* >> * Forcefully restricting the affinity of a deadline task is >> @@ -3055,7 +3060,8 @@ static int restrict_cpus_allowed_ptr(struct task_struct *p, >> goto err_unlock; >> } >> >> - if (!cpumask_and(new_mask, task_user_cpus(p), subset_mask)) { >> + if (p->user_cpu_ptr && >> + !cpumask_and(new_mask, p->user_cpu_ptr, subset_mask)) { > s/user_cpu_ptr/user_cpus_ptr/ > >> err = -EINVAL; >> goto err_unlock; >> } >> @@ -3069,9 +3075,8 @@ static int restrict_cpus_allowed_ptr(struct task_struct *p, >> >> /* >> * Restrict the CPU affinity of task @p so that it is a subset of >> - * task_cpu_possible_mask() and point @p->user_cpus_ptr to a copy of the >> - * old affinity mask. If the resulting mask is empty, we warn and walk >> - * up the cpuset hierarchy until we find a suitable mask. >> + * task_cpu_possible_mask(). If the resulting mask is empty, we warn >> + * and walk up the cpuset hierarchy until we find a suitable mask. >> */ >> void force_compatible_cpus_allowed_ptr(struct task_struct *p) >> { >> @@ -3125,11 +3130,15 @@ __sched_setaffinity(struct task_struct *p, struct affinity_context *ctx); >> void relax_compatible_cpus_allowed_ptr(struct task_struct *p) >> { >> struct affinity_context ac = { >> - .new_mask = task_user_cpus(p), >> - .flags = 0, >> + .new_mask = cpu_possible_mask; > s/;/,/ > > But even with those two things fixed, I'm seeing new failures in my > testing which I think are because restrict_cpus_allowed_ptr() is failing > unexpectedly when called by force_compatible_cpus_allowed_ptr(). > > For example, just running a 32-bit task on an asymmetric system results > in: > > $ ./hello32 > [ 1690.855341] Overriding affinity for process 580 (hello32) to CPUs 2-3 > > That then has knock-on effects such as losing track of the initial affinity > mask and not being able to restore it if the forcefully-affined 32-bit task > exec()s a 64-bit program. I thought I have fixed the build failure. Apparently it is still there. I will fix it. BTW, which arm64 cpus support "allow_mismatched_32bit_el0"? I am trying to see if I can reproduce the issue, but I am not sure if I have any access to the cpus that have this capability. Cheers, Longman