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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id A571DC25B74 for ; Mon, 27 May 2024 17:26:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E84D66B0089; Mon, 27 May 2024 13:26:57 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E33DB6B008C; Mon, 27 May 2024 13:26:57 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CFB096B0092; Mon, 27 May 2024 13:26:57 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id B24376B0089 for ; Mon, 27 May 2024 13:26:57 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 0BD53C11E1 for ; Mon, 27 May 2024 17:26:57 +0000 (UTC) X-FDA: 82164856074.21.1058DC3 Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) by imf04.hostedemail.com (Postfix) with ESMTP id 0980C4000C for ; Mon, 27 May 2024 17:26:54 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=layalina-io.20230601.gappssmtp.com header.s=20230601 header.b=m7MAnRwO; spf=pass (imf04.hostedemail.com: domain of qyousef@layalina.io designates 209.85.128.43 as permitted sender) smtp.mailfrom=qyousef@layalina.io; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1716830815; h=from:from:sender: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:dkim-signature; bh=5xaz9ANbbqqtDnoE1v5r0IIUcdg877x8+F/QngDnD6I=; b=3kIj2zuhn77Vqyfb3Rapfu4gqP9K2BNgXjYPVWnauN5QT0aKEViiuLyUKmAALY0oFI/7B7 00+v3iA1pydyMr3KwozeVyHV2Ge5I+uNVrFU/pgeYCSVWw3urVnKHYVr7MuwOGJQGPS3i6 eGzRVeDDAsDyrAi1Le1a9H/rzGiXfo4= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1716830815; a=rsa-sha256; cv=none; b=EvEWan6ODAfprIu8weaKa5BDxaHoGgByRJs4ylfKWh6YToZoJ618ms2W7luJSR7F7LvMIb G1RdAaAjYGv/rs8HwDJ9ZCXR7xC4JOryz68dsz/EZJT9Js2oAIsR9X7XjFcMhlVsOf6msx tKhc9M2eURBF1JSGtomsb9FapMTTh/8= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=layalina-io.20230601.gappssmtp.com header.s=20230601 header.b=m7MAnRwO; spf=pass (imf04.hostedemail.com: domain of qyousef@layalina.io designates 209.85.128.43 as permitted sender) smtp.mailfrom=qyousef@layalina.io; dmarc=none Received: by mail-wm1-f43.google.com with SMTP id 5b1f17b1804b1-42108822e3cso18169215e9.0 for ; Mon, 27 May 2024 10:26:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=layalina-io.20230601.gappssmtp.com; s=20230601; t=1716830813; x=1717435613; darn=kvack.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=5xaz9ANbbqqtDnoE1v5r0IIUcdg877x8+F/QngDnD6I=; b=m7MAnRwOabDiwBMiWKBz2FF2aMulyZ4dEclKgExI5xzZSMa78V0tfR9fg3RslQZPga cXommNFFxH67DeS/4NotfWlrlCz3h2tsPN+7+wTp5Yfl+t0DHiwoLAsyn7C6/ulCoYmy 0JugIv9OiZuZ04TSSi4HkxExF/XMpFnGMvoYUN8bvyrh3GSPSwsSs5ICHgrykd7jmIIO Fhx/BdiXU4v4YJ/RTjaOYo1tMrFKho13i6xsGwLr+R61PR1/imrKQQZ3SI7kWudRaecS FjX5BItQvt1ITGusgmlbJ5mAGHPlsk24sAv9XdLmKCyeruymb0+5nuPDQ/is/T4M2mOI mkBg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716830813; x=1717435613; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=5xaz9ANbbqqtDnoE1v5r0IIUcdg877x8+F/QngDnD6I=; b=WZPU5YbVovj9GRE+uzbIr6pkPMYIhUZQt1zXKCH1GFRHeBPQKnd1y6oHLojzgAtYyJ zk17LejjoTIH/QJEj3xFEcz7veYttYGdl7jOIrRXaQ5DFVqQFMIK8L4UBYGYRKcXz5eW 63TcNl7kw4g2XwdTjBI3ZQ8OgZeBsl1m9my4nawQE+286g8tlqJYpWtgRJ7bX84YALbq nBPkVxoTC9rBME3LiMpTj6JPr+y0DRLIuBvdMY1XzMDqjNDOgFx87g/olgMA1FDgaynM Mb+DRtX96l8cGmoVfesawjnVzzdp1nbGAdI0i+Fjdklh9ilFr4m2qpeFZN2CG2VF6LOH NuwA== X-Forwarded-Encrypted: i=1; AJvYcCU9+3IzYChhXEjWuf/E71yp9VtW/XpxLci5k1tkNcW0g3ZLJSKE5nOqdpK0GkzVafUcx8bEWD+tghiaCoX1qRBD8kw= X-Gm-Message-State: AOJu0Ywc78wiFb7LOcpMl1u0NIqV0HQlb+AWaMARUwkpe9F2YqCp6+ee gWZfNDvz5dp1gkWx74T6ukV5gRD2T9j07sQ69VG6HNImsmXJC3G5sSR6fJFgvzM= X-Google-Smtp-Source: AGHT+IFArA1tSNa8aXEYto+F+xmnxEWR8BmJTuNot07bbSgyyxQsRiM4eehBTdcgh4HRplq3rNQiuw== X-Received: by 2002:a05:600c:54c5:b0:41f:dc27:a7c1 with SMTP id 5b1f17b1804b1-421081ae3f7mr94812535e9.5.1716830813135; Mon, 27 May 2024 10:26:53 -0700 (PDT) Received: from airbuntu (host81-157-90-255.range81-157.btcentralplus.com. [81.157.90.255]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-421089708f8sm113693955e9.20.2024.05.27.10.26.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 May 2024 10:26:52 -0700 (PDT) Date: Mon, 27 May 2024 18:26:50 +0100 From: Qais Yousef To: Sebastian Andrzej Siewior Cc: Ingo Molnar , Peter Zijlstra , Juri Lelli , Steven Rostedt , Vincent Guittot , Daniel Bristot de Oliveira , Thomas Gleixner , Alexander Viro , Christian Brauner , Andrew Morton , Jens Axboe , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-mm@kvack.org, Phil Auld Subject: Re: [PATCH v2] sched/rt: Clean up usage of rt_task() Message-ID: <20240527172650.kieptfl3zhyljkzx@airbuntu> References: <20240515220536.823145-1-qyousef@layalina.io> <20240521110035.KRIwllGe@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20240521110035.KRIwllGe@linutronix.de> X-Rspam-User: X-Stat-Signature: dwf1zioijwpnu3j31j4wjcuaf6ho5ad5 X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 0980C4000C X-HE-Tag: 1716830814-538119 X-HE-Meta: U2FsdGVkX18i2QNgLeRvMJRX9gL0qeSJLAGTHgYr7jvBtdXGmx5CJD3hurgeEjt7JmYvOMddxjAgXYvPQ7kCujwXMRfeG6TJ8/eDWip1ciI1Wz9kG3BVx3Nm8jCQPyqAttyGvKlpegdEaX0biBm7jBF9es5/RJK7SECfWz2ZnOTjGgKxOCZCGNIWlofuyDdLl+TvfTP0D4v1Vv3MTEIHrzNMe13zK7S3rKzFapsiSNqvetwQujGTEZ4OWrejj33YBFmshWesKmuOwglPOydwuIqwQokAoTO+d/QoMOtR+inXTM1p82fRwE+ib61NwaGFZIHMDofCDBMVCX5BznNcFHWUu8tZ9fs4Jf4Ca9qwMqbOdIpRudSr3/0L8EL6d7ZyoY3Aae5OqufA9NL1wd5kfw6SCJkpzjJ5QCuYagLyXrmBCM16H4sqKN/C04h2b36bCailh5jV7SRps/VsVIRussCR58O2FevbRBTj81G1vt92oqx9Of7RA5q/uEOvxDwi2cjbCIsxxGWRCGMx6tAHBZrHYlwS4sOE/K5G2bNSMhNpbGeWbFC8Q6Uk27heL+C8GN90MQMw8f+e1aG6Pu70i2D0MDL1NYEr19R0LQPAKFGh8HUOn62UjPUZIgeKSLkFr/gTBgvftYD96yBI10/URhkqNm+gvc+GRcYaaAC39MdE47VGTc/FKWm1WYCiK29MxcQIEV89wdN2vcN6g2+TUI0EwCQF/5hPXXV2z6EVzD0ZyC8XtrQVWiEJBjXxSTPjcTjz6YXm8D531HFURHjJZQv7mthGVtbZh7X1Jc7XjINI6JWOzGw2cqGtXFi5DKt4nf5tQWV8bTXqZrIDw8D+uNOqC8Yhb1W7/f7LQ+/xNOuReZKU3fiuAf8bt42+PI456RPamUXqyGCn3ABjqohCP9gIxCeJaHm6tSJnbfx68byBH59ASp7qbAuPmYUE2Ko+fVhAz3Dl2hhUEMIqiO5 73D8oaxe 3UjEn+V8dh+CPRxRMExG9hpkK4QvMasQKZdHno0N8zOKKOedTvahLvqSQLBiCIf+ewWJUXTAyzgbxQ+kLJXLGx81dN+VkYRdsXhhBJpWONoN4jQUQLzu7Y/RUdZihOb2bZ7OvfQd/JuPhxR++SFLTWQSCFPkW52UkTba4s8nB066kaJGPP3zRZoVMqNs1n+0nI5R83TDCRIbUnKUavKBmtiSDXSim/9S5xA3Bkpn2xOUtxdDEivCg3OwdR3qhhn8OG4PMtKIaKciybpPnVbw3KBg6yLkJWaye+4S6CBblJsEj7D4+Ncm7rhq2LhLWOtQTeWRPDD6zSvsBw103VNCT1YlnITSxp5Va3gNakSwB42IB0Vryz5o3wsKfmuu4yfx6Hi9ZjeMi01JTcJbAVX0Q46iM67NYMMjI/6BG8kUDdj/M3tHTwLtXhAOIbES/TT4POSZs+PCEd07XzO7IZyj/ZEgkRn62Q4GmYaulP2sJ6C9pQVP4zU6++qTrpmolgp/Vf5uFgut3DYfKi8rPO5iYCnuzw/PPshMQDg3pTUGg1v+WkuIr/vIWdEB94W6fg2Wt3FddoOmfI5Wm/gD88CG/z+qlLMzuQaZ+uT8p X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 05/21/24 13:00, Sebastian Andrzej Siewior wrote: > On 2024-05-15 23:05:36 [+0100], Qais Yousef wrote: > > rt_task() checks if a task has RT priority. But depends on your > > dictionary, this could mean it belongs to RT class, or is a 'realtime' > > task, which includes RT and DL classes. > > > > Since this has caused some confusion already on discussion [1], it > > seemed a clean up is due. > > > > I define the usage of rt_task() to be tasks that belong to RT class. > > Make sure that it returns true only for RT class and audit the users and > > replace the ones required the old behavior with the new realtime_task() > > which returns true for RT and DL classes. Introduce similar > > realtime_prio() to create similar distinction to rt_prio() and update > > the users that required the old behavior to use the new function. > > > > Move MAX_DL_PRIO to prio.h so it can be used in the new definitions. > > > > Document the functions to make it more obvious what is the difference > > between them. PI-boosted tasks is a factor that must be taken into > > account when choosing which function to use. > > > > Rename task_is_realtime() to realtime_task_policy() as the old name is > > confusing against the new realtime_task(). > > I *think* everyone using rt_task() means to include DL tasks. And > everyone means !SCHED-people since they know when the difference matters. yes, this makes sense > > > No functional changes were intended. > > > > [1] https://lore.kernel.org/lkml/20240506100509.GL40213@noisy.programming.kicks-ass.net/ > > > > Reviewed-by: Phil Auld > > Signed-off-by: Qais Yousef > > --- > > > > Changes since v1: > > > > * Use realtime_task_policy() instead task_has_realtime_policy() (Peter) > > * Improve commit message readability about replace some rt_task() > > users. > > > > v1 discussion: https://lore.kernel.org/lkml/20240514234112.792989-1-qyousef@layalina.io/ > > > > fs/select.c | 2 +- > > fs/bcachefs/six.c > six_owner_running() has rt_task(). But imho should have realtime_task() > to consider DL. But I think it is way worse that it has its own locking > rather than using what everyone else but then again it wouldn't be the > new hot thing… I think I missed this one. Converted now. Thanks! > > > include/linux/ioprio.h | 2 +- > > include/linux/sched/deadline.h | 6 ++++-- > > include/linux/sched/prio.h | 1 + > > include/linux/sched/rt.h | 27 ++++++++++++++++++++++++++- > > kernel/locking/rtmutex.c | 4 ++-- > > kernel/locking/rwsem.c | 4 ++-- > > kernel/locking/ww_mutex.h | 2 +- > > kernel/sched/core.c | 6 +++--- > > kernel/time/hrtimer.c | 6 +++--- > > kernel/trace/trace_sched_wakeup.c | 2 +- > > mm/page-writeback.c | 4 ++-- > > mm/page_alloc.c | 2 +- > > 13 files changed, 48 insertions(+), 20 deletions(-) > … > > diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c > > index 70625dff62ce..08b95e0a41ab 100644 > > --- a/kernel/time/hrtimer.c > > +++ b/kernel/time/hrtimer.c > > @@ -1996,7 +1996,7 @@ static void __hrtimer_init_sleeper(struct hrtimer_sleeper *sl, > > * expiry. > > */ > > if (IS_ENABLED(CONFIG_PREEMPT_RT)) { > > - if (task_is_realtime(current) && !(mode & HRTIMER_MODE_SOFT)) > > + if (realtime_task_policy(current) && !(mode & HRTIMER_MODE_SOFT)) > > mode |= HRTIMER_MODE_HARD; > > } > > > > @@ -2096,7 +2096,7 @@ long hrtimer_nanosleep(ktime_t rqtp, const enum hrtimer_mode mode, > > u64 slack; > > > > slack = current->timer_slack_ns; > > - if (rt_task(current)) > > + if (realtime_task(current)) > > slack = 0; > > > > hrtimer_init_sleeper_on_stack(&t, clockid, mode); > > @@ -2301,7 +2301,7 @@ schedule_hrtimeout_range_clock(ktime_t *expires, u64 delta, > > * Override any slack passed by the user if under > > * rt contraints. > > */ > > - if (rt_task(current)) > > + if (realtime_task(current)) > > delta = 0; > > I know this is just converting what is already here but… > __hrtimer_init_sleeper() looks at the policy to figure out if the task > is realtime do decide if should expire in HARD-IRQ context. This is > correct, a boosted task should not sleep. > > hrtimer_nanosleep() + schedule_hrtimeout_range_clock() is looking at > priority to decide if slack should be removed. This should also look at > policy since a boosted task shouldn't sleep. I have to admit I never dug deep enough into this code. Happy to convert these users. I'll add that as a separate patch as this is somewhat changing behavior which this patch intends to do a clean up only. > > In order to be PI-boosted you need to acquire a lock and the only lock > you can sleep while acquired without generating a warning is a mutex_t > (or equivalent sleeping lock) on PREEMPT_RT. Note we care about the behavior for !PREEMPT_RT. PI issues are important there too. I assume the fact the PREEMPT_RT changes the locks behavior is what you're referring to here and not applicable to normal case. Thanks! -- Qais Yousef > > > hrtimer_init_sleeper_on_stack(&t, clock_id, mode); > > diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c > > index 0469a04a355f..19d737742e29 100644 > > --- a/kernel/trace/trace_sched_wakeup.c > > +++ b/kernel/trace/trace_sched_wakeup.c > > @@ -545,7 +545,7 @@ probe_wakeup(void *ignore, struct task_struct *p) > > * - wakeup_dl handles tasks belonging to sched_dl class only. > > */ > > if (tracing_dl || (wakeup_dl && !dl_task(p)) || > > - (wakeup_rt && !dl_task(p) && !rt_task(p)) || > > + (wakeup_rt && !realtime_task(p)) || > > (!dl_task(p) && (p->prio >= wakeup_prio || p->prio >= current->prio))) > > return; > > > > Sebastian