From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f172.google.com (mail-pl1-f172.google.com [209.85.214.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0590C2309A4 for ; Tue, 14 Jan 2025 03:18:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736824721; cv=none; b=N5nwQjVWDie2brHPlADMN5M88/3r+jeDGN5r4PHSQeiawuB+8THa2bkxApFhEfGCEG3W2wYb90Y7I0z2k2WyAELzZpo2781f6iQm2Xp8UCbUUM2tRFVtJ/dI2pveYWAC1hbL3ewBCP4T0HbDlu5xmRCh+QZrYtmCnUAmlDjK8G0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736824721; c=relaxed/simple; bh=36LyTnvALeONirw57t4o1a7R1KhcrlFKs8AcXusfR04=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=VFFJQgFImPr5NoF9bVbNGxrdqTY6J/sfcFXwPQGcyHRK4AD7wAAu3ytAbcE/Z7VFhlMUwHBk3NyhdQB5SEa+iGr3dPISVdo8pIHKB/fy2eKGI5dQVoKH20oqhjU5MCd1aLvbLZDr2UNJ01zaLhmWo2ZmVY7uVSYQnlaYmO1JDZg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=f6PZpeWy; arc=none smtp.client-ip=209.85.214.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="f6PZpeWy" Received: by mail-pl1-f172.google.com with SMTP id d9443c01a7336-21649a7bcdcso84999515ad.1 for ; Mon, 13 Jan 2025 19:18:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1736824719; x=1737429519; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:references:cc:to:subject :user-agent:mime-version:date:message-id:from:to:cc:subject:date :message-id:reply-to; bh=UlORTER9Q2o4cE0X2zFPsEM9drfjOx6Mnz1wPmbp7x0=; b=f6PZpeWy4SnC47g1bIfKQ9A4i9Io9OjOdBHCzs5++i9czlCf/J0pAeXWVSdfT5SYMp Mi/B3P4z3oOWvVeYe630MsiFNEkFLoSV/0XbClRhTNF/9HyHEEfiLFvc6Ka01TF6Qa7J i1wZEsjLLWf6hKZvTUdfPowyUmzI/AWv1Xd0g20SpmWVoiA94jQCkIm9xyfTh21oJEzD wnrF7/piQDmOwby6tSCK8tm2M2u4U6wgB6GUvaQSnaBqc9mxeGAhwHIf1BlnTEnDkYmA RBkvGq7GbOiO5MbxaqPPvNKncFb+OOCNim98PwdjcKViAv+HFY6ldykD4IRN+9YdFpDT mB1g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736824719; x=1737429519; h=content-transfer-encoding:in-reply-to:from:references:cc:to:subject :user-agent:mime-version:date:message-id:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=UlORTER9Q2o4cE0X2zFPsEM9drfjOx6Mnz1wPmbp7x0=; b=rA3ehp1tlfVyZ+zow+vwM95w+gv5X9LBf25kGKB3NEpu9WYvjWucfaVpRwkpDFi4Ug 0uUDtpVpkoPSQK5pUDrb1t67JWR6s2SrFMibumcsmfATfcm2XNInHDWkAAPDKiOMUAMb 8ClR3PCEpfEyjxqqtcNMLfHmvZV0gVXlDYolqRoFmUm5a6oQNAiAjZ902gcL8gVDQkpR VEZS0YLkKUMkQXHG4Almu+kjDYOmV1E/teX7A27fDHWbIvJZlP2IYyTZnn9HPm0ArAN8 MUg6SyXThpdKf8XoxFCN9N6lWG6igy3qMii8psWm5yQ4tMPUjpM4V8keLAtKPtx6xAJC rhxA== X-Forwarded-Encrypted: i=1; AJvYcCXYcVwU5KPjqeQlr00qm4rke8hgQx8qEEber/r1l/6vftijC+1zOwb8cfH6HMmCZwI1ZN6Bg0MB0Nk9QUQ=@vger.kernel.org X-Gm-Message-State: AOJu0YzmSo5mQfGxfVOQddqumtnwfXc5e/A1KBrECYsHcZd+d4BB7O3D tHHYH3ACjPe43KDgD5mtRLmgUvnG60q7vukE9QytAjT4riS2+jxd X-Gm-Gg: ASbGncuEFwsCa0yNNJPubDBRQTpZmDcjYrrassjhPGm4l/v147G+mNxfSu1GN3nBTNb uBMMD2TWsPP++0CRhUVpeG0tR4GH9JPsKupV5JFoxIg8s5QVFNGAeq5bNDgvXQG6c8MzawBoY7V UfOIFlNByoACtUNI3Mso/g/4nh3c+JM3twkmpZ+1D+KIW5KnEW8nw9i7UK9IhURHzavwu7P1MGh B5eL1fPX8yga+QGIRztSLa9JwERRBgnAUobY3U6JuCAwerD956w4hB7ldM37YsI4Ovm4/Vr7sI3 X-Google-Smtp-Source: AGHT+IHMLDtrAWJKxv+Tu2y6ZE+pr7AsxIxPpZr+F1dO+yCm1MsJrX0DWpyadKgxtaGbsUVINikGGQ== X-Received: by 2002:a05:6a20:6a0c:b0:1e1:afa9:d38b with SMTP id adf61e73a8af0-1e88d0a1e37mr39594147637.8.1736824719110; Mon, 13 Jan 2025 19:18:39 -0800 (PST) Received: from [10.125.112.6] ([103.165.80.178]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72d4054942dsm6457516b3a.21.2025.01.13.19.18.35 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 13 Jan 2025 19:18:38 -0800 (PST) Message-ID: Date: Tue, 14 Jan 2025 11:18:32 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.15.0 Subject: Re: [PATCH v2] sched/core: Prioritize migrating eligible tasks in sched_balance_rq() To: Vincent Guittot Cc: mingo@redhat.com, peterz@infradead.org, mingo@kernel.org, juri.lelli@redhat.com, dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de, vschneid@redhat.com, linux-kernel@vger.kernel.org, Hao Jia References: <20241223091446.90208-1-jiahao.kernel@gmail.com> <597384e3-6519-b10e-081b-30c3f89b6e3f@gmail.com> From: Hao Jia In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 2025/1/14 00:40, Vincent Guittot wrote: > On Mon, 13 Jan 2025 at 10:21, Hao Jia wrote: >> >> Friendly ping... >> >> >> On 2024/12/23 17:14, Hao Jia wrote: >>> From: Hao Jia >>> >>> When the PLACE_LAG scheduling feature is enabled and >>> dst_cfs_rq->nr_queued is greater than 1, if a task is >>> ineligible (lag < 0) on the source cpu runqueue, it will >>> also be ineligible when it is migrated to the destination >>> cpu runqueue. Because we will keep the original equivalent >>> lag of the task in place_entity(). So if the task was >>> ineligible before, it will still be ineligible after >>> migration. >>> >>> So in sched_balance_rq(), we prioritize migrating eligible >>> tasks, and we soft-limit ineligible tasks, allowing them >>> to migrate only when nr_balance_failed is non-zero to >>> avoid load-balancing trying very hard to balance the load. > > Could you explain why you think it's better to balance eligible tasks > in priority and potentially skip a load balance ? In place_entity(), we maintain the task's original equivalent lag, even if we migrate the task to dst_rq, this does not change its eligibility attribute. When there are multiple tasks on src_rq, and the dst_cpu has some runnable tasks, migrating ineligible tasks to dst_rq will not allow them to run. Therefore, such task migration is inefficient. We should prioritize migrating tasks that can run on dst_rq. In other words, migrating ineligible tasks is merely moving them to another runqueue to wait until they become eligible. > > I can see an interest for idle and newly_idle load balance in order to > favor fairness as tasks will become eligible but I don't see why it > would be helpful if dst already has some runnable tasks. Furthermore, > when a cpu is idle or newly idle, we really want to migrate a task > even an non eligible one instead of possibly skipping this load > balance round. With your patch, we might end up not pulling any task, > increasing the nr_balance_failed and waiting next load balance > If I understand correctly, when the destination CPU is idle, my patch does not change the original behavior. it only prevents the migration of ineligible tasks when dst_cfs_rq->nr_queued is greater than 1. If I missed something, please correct me. Thanks, Hao >>> >>> Below are some benchmark test results. From my test results, >>> this patch shows a slight improvement on hackbench. >>> >>> Benchmark >>> ========= >>> >>> All of the benchmarks are done inside a normal cpu cgroup in a >>> clean environment with cpu turbo disabled, and test machine is: >>> >>> Single NUMA machine model is 13th Gen Intel(R) Core(TM) >>> i7-13700, 12 Core/24 HT. >>> >>> Based on master b86545e02e8c. >>> >>> Results >>> ======= >>> >>> hackbench-process-pipes >>> vanilla patched >>> Amean 1 0.5837 ( 0.00%) 0.5733 ( 1.77%) >>> Amean 4 1.4423 ( 0.00%) 1.4503 ( -0.55%) >>> Amean 7 2.5147 ( 0.00%) 2.4773 ( 1.48%) >>> Amean 12 3.9347 ( 0.00%) 3.8880 ( 1.19%) >>> Amean 21 5.3943 ( 0.00%) 5.3873 ( 0.13%) >>> Amean 30 6.7840 ( 0.00%) 6.6660 ( 1.74%) >>> Amean 48 9.8313 ( 0.00%) 9.6100 ( 2.25%) >>> Amean 79 15.4403 ( 0.00%) 14.9580 ( 3.12%) >>> Amean 96 18.4970 ( 0.00%) 17.9533 ( 2.94%) >>> >>> hackbench-process-sockets >>> vanilla patched >>> Amean 1 0.6297 ( 0.00%) 0.6223 ( 1.16%) >>> Amean 4 2.1517 ( 0.00%) 2.0887 ( 2.93%) >>> Amean 7 3.6377 ( 0.00%) 3.5670 ( 1.94%) >>> Amean 12 6.1277 ( 0.00%) 5.9290 ( 3.24%) >>> Amean 21 10.0380 ( 0.00%) 9.7623 ( 2.75%) >>> Amean 30 14.1517 ( 0.00%) 13.7513 ( 2.83%) >>> Amean 48 24.7253 ( 0.00%) 24.2287 ( 2.01%) >>> Amean 79 43.9523 ( 0.00%) 43.2330 ( 1.64%) >>> Amean 96 54.5310 ( 0.00%) 53.7650 ( 1.40%) >>> >>> tbench4 Throughput >>> vanilla patched >>> Hmean 1 255.97 ( 0.00%) 275.01 ( 7.44%) >>> Hmean 2 511.60 ( 0.00%) 544.27 ( 6.39%) >>> Hmean 4 996.70 ( 0.00%) 1006.57 ( 0.99%) >>> Hmean 8 1646.46 ( 0.00%) 1649.15 ( 0.16%) >>> Hmean 16 2259.42 ( 0.00%) 2274.35 ( 0.66%) >>> Hmean 32 4725.48 ( 0.00%) 4735.57 ( 0.21%) >>> Hmean 64 4411.47 ( 0.00%) 4400.05 ( -0.26%) >>> Hmean 96 4284.31 ( 0.00%) 4267.39 ( -0.39%) >>> >>> Signed-off-by: Hao Jia >>> Suggested-by: Peter Zijlstra (Intel) >>> --- >>> Previous discussion link: https://lore.kernel.org/all/20241128084858.25220-1-jiahao.kernel@gmail.com >>> Link to v1: https://lore.kernel.org/all/20241218080203.80556-1-jiahao.kernel@gmail.com >>> >>> v1 to v2: >>> - Modify dst_cfs_rq->nr_running to dst_cfs_rq->nr_queued to >>> resolve conflicts with commit 736c55a02c47 ("sched/fair: >>> Rename cfs_rq.nr_running into nr_queued"). >>> >>> kernel/sched/fair.c | 34 ++++++++++++++++++++++++++++++++++ >>> 1 file changed, 34 insertions(+) >>> >>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c >>> index 5599b0c1ba9b..c884bf631e66 100644 >>> --- a/kernel/sched/fair.c >>> +++ b/kernel/sched/fair.c >>> @@ -9396,6 +9396,30 @@ static inline int migrate_degrades_locality(struct task_struct *p, >>> } >>> #endif >>> >>> +/* >>> + * Check whether the task is ineligible on the destination cpu >>> + * >>> + * When the PLACE_LAG scheduling feature is enabled and >>> + * dst_cfs_rq->nr_queued is greater than 1, if the task >>> + * is ineligible, it will also be ineligible when >>> + * it is migrated to the destination cpu. >>> + */ >>> +static inline int task_is_ineligible_on_dst_cpu(struct task_struct *p, int dest_cpu) >>> +{ >>> + struct cfs_rq *dst_cfs_rq; >>> + >>> +#ifdef CONFIG_FAIR_GROUP_SCHED >>> + dst_cfs_rq = task_group(p)->cfs_rq[dest_cpu]; >>> +#else >>> + dst_cfs_rq = &cpu_rq(dest_cpu)->cfs; >>> +#endif >>> + if (sched_feat(PLACE_LAG) && dst_cfs_rq->nr_queued && >>> + !entity_eligible(task_cfs_rq(p), &p->se)) >>> + return 1; >>> + >>> + return 0; >>> +} >>> + >>> /* >>> * can_migrate_task - may task p from runqueue rq be migrated to this_cpu? >>> */ >>> @@ -9420,6 +9444,16 @@ int can_migrate_task(struct task_struct *p, struct lb_env *env) >>> if (throttled_lb_pair(task_group(p), env->src_cpu, env->dst_cpu)) >>> return 0; >>> >>> + /* >>> + * We want to prioritize the migration of eligible tasks. >>> + * For ineligible tasks we soft-limit them and only allow >>> + * them to migrate when nr_balance_failed is non-zero to >>> + * avoid load-balancing trying very hard to balance the load. >>> + */ >>> + if (!env->sd->nr_balance_failed && >>> + task_is_ineligible_on_dst_cpu(p, env->dst_cpu)) >>> + return 0; >>> + >>> /* Disregard percpu kthreads; they are where they need to be. */ >>> if (kthread_is_per_cpu(p)) >>> return 0;