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 E19CCC04FFE for ; Tue, 14 May 2024 23:59:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 367DC8D0054; Tue, 14 May 2024 19:59:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2F1608D004F; Tue, 14 May 2024 19:59:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 16B9D8D0054; Tue, 14 May 2024 19:59:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id E4C5A8D004F for ; Tue, 14 May 2024 19:59:17 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 9B3A4A204D for ; Tue, 14 May 2024 23:59:17 +0000 (UTC) X-FDA: 82118670354.03.D494FCD Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf27.hostedemail.com (Postfix) with ESMTP id BB54F4000F for ; Tue, 14 May 2024 23:59:15 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=IGGiAemD; spf=pass (imf27.hostedemail.com: domain of pauld@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=pauld@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1715731155; a=rsa-sha256; cv=none; b=WcuAGWVRwpN8UY/4D9qnSSrNi/QhA/iZx/HvLjTjSSO13WE30fdW7qB3cCIkyUgJy98oJa 3knUnLYZs+G8A/IGJzvyG9b0VsUgqo7T1qESzLrZNgfPjxftdZg4i3iYAkuwFhn0Flo7Bh U7RqoDOCEFEAlTJI/oXstMoUa9QweoI= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=IGGiAemD; spf=pass (imf27.hostedemail.com: domain of pauld@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=pauld@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1715731155; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=gsGXcq5SRCOdcCuxuUZi3nIuEQpczxVsVwRLQG1JRg8=; b=a6LwKv9EvGeKZfXdbdmVBvcF5cpriUD9Dtsb8MDNBUhI6JbQwmUA5+VKGBEHkkZubHYm4O 0LmiAHLK6c3TnpfZjTg7XqHhuV6E9Ouwn9BHprL8oeEhZxMeOE1mtKSiZOC3LDvypWJIsh Vw7WxX2gDU6FANEO2zYIBETsdqWtMjY= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1715731155; 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: in-reply-to:in-reply-to:references:references; bh=gsGXcq5SRCOdcCuxuUZi3nIuEQpczxVsVwRLQG1JRg8=; b=IGGiAemD0+xtw8zXCBvFKsDTOWSrfjBV2sBc3dVlIy30nD+pN7vW8HgDuAChFbyDCE8vJW hV5fWS/Eu5djf6i7QSXm7Z5MeqfHPGwsno6OEWSqVnGVFSZ3RHEnpIB6DliB9KHtxRVN71 33Zl8kwsZ1F1xIMcX+D5QOEbiGhHrKs= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-49-GmpqE6tBNu2ScdfjY0WdpQ-1; Tue, 14 May 2024 19:58:56 -0400 X-MC-Unique: GmpqE6tBNu2ScdfjY0WdpQ-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id A70C3380009F; Tue, 14 May 2024 23:58:55 +0000 (UTC) Received: from lorien.usersys.redhat.com (unknown [10.22.10.26]) by smtp.corp.redhat.com (Postfix) with ESMTPS id F1D7F400059; Tue, 14 May 2024 23:58:53 +0000 (UTC) Date: Tue, 14 May 2024 19:58:51 -0400 From: Phil Auld To: Qais Yousef Cc: Ingo Molnar , Peter Zijlstra , Juri Lelli , Steven Rostedt , Vincent Guittot , Daniel Bristot de Oliveira , Thomas Gleixner , Sebastian Andrzej Siewior , 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 Subject: Re: [PATCH] sched/rt: Clean up usage of rt_task() Message-ID: <20240514235851.GA6845@lorien.usersys.redhat.com> References: <20240514234112.792989-1-qyousef@layalina.io> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240514234112.792989-1-qyousef@layalina.io> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.9 X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: BB54F4000F X-Stat-Signature: znm7819hh3hgrue8xb7w8t4mcue376wf X-HE-Tag: 1715731155-273469 X-HE-Meta: U2FsdGVkX19y5t7INZQwC9aATIbta8Y7+5nUKjkeP1JPal7UNMkqfT/zdKWCf0rDImbikxRmtPWYAin13HWgrer3kmvWTrHkiind8+e+HCY4pCkPyKoyJitPnBlPItf67WLlA6zJCAjIhyy+U/ZmtV1rIIqhGaEGogmtm3bHtvJ89xHDasX5/Uu4Zb4pTMGnIzbnixKoMctHXMnXfaKJgl6b92mIzSgaZ4dBpd5UbKwcAjjrhCaQ+2gOTRbiQYd5biCzDlx5EEqKgmxeSUL+3KPzwWP2m+ikKL9WpXfASsoQ2xIADqowy8Enk6hWksdcYSS5vFfJcOSjKFmVk/Tuj0gsG8oQLWSzqL+9i2sYTFkVP4Q743DHOaum6BU+8hn9jUqZOLH2KquYJdQkuEWwDwaMjG9V0QKn6CjE63zg3+YeAMExjIU4liepdtnkpoXGUpYgKZuasw1FyZ4iaRQq2ZNzqUV7F911DJWZwOmjUEri0IwLNLh5u8VBe69rLVvInQfL72ZgARLlUGjWL3/LbT8MydVEP63vYKlos7t4wopn1JVYvX8eLEdsoSnV6yAzMoDuJy1yAv5kzh5XN2MrF1HHy8jhVwAnGGuYahXuvo6JQocK3oJnBQTHCnBzwfrKnorvjZpYHgkhg0m2/bC88IgAXMJbz6hBA4ICE51kCNurgZLQ9MKwxqeCmHILmATPwt7QancdsOUWN3HfjU5jBYmYd6FbuPaJl73LAT8kLdk5nni4PbX6ovBDhgJZpyRs1llZigJcNrVbLrBQ/SyOWVEyis7jj5qsBAOshHxnVGRO1jV241ccPoWSHKWG3PVCP8vlYJR+G7ma7gUHjDpMSQvQ95cZ7BEsBzdBNe8cd4vYrR9+QOAwogJogFIolDtax6kJ586RQfsV3wyOJ7nyonjfh51bORuO3Cb+ebq6MPgAFFkJPkSo5RZLvCyvMSixp932xoJxH2JW5LnRUJI ASE8vEsM m3xIOXUrIc+Yr1+ag8s8eWJnSzXNJ6hxh+3Qw65IsQzNJxgE7vPMEbOluwShLsIqeTxzmSZvsPnxpq1bVyWE6C7SF5TcBo6D5n3EdNksExH2VZzOou0awTieeaX74eTZSWH5FIkREPJxs5rVw7tLo2wMLozNmT5Xu0/QUSuaPMgdDNkJ38HPKYnn4wPS807Ir0B1y6SR+C3MMVezKqLGhNgzZjXMY7QWuOl1cNn619JW7yPmUK0zi9JZySdBO1tt1rvsG+sTcp5fH+q9kJ+UYp33ukXkcEBprT0721HkoerupM02HZarES7DNFt28VwH9oKKTj80waIryuEM= 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: Hi Qais, On Wed, May 15, 2024 at 12:41:12AM +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 them with the new realtime_task() which returns true for RT and > DL classes - the old behavior. Introduce similar realtime_prio() to > create similar distinction to rt_prio() and update the users. I think making the difference clear is good. However, I think rt_task() is a better name. We have dl_task() still. And rt tasks are things managed by rt.c, basically. Not realtime.c :) I know that doesn't work for deadline.c and dl_ but this change would be the reverse of that pattern. > > 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 task_has_realtime_policy() as the old name > is confusing against the new realtime_task(). Keeping it rt_task() above could mean this stays as it was but this change makes sense as you have written it too. Cheers, Phil > > No functional changes were intended. > > [1] https://lore.kernel.org/lkml/20240506100509.GL40213@noisy.programming.kicks-ass.net/ > > Signed-off-by: Qais Yousef > --- > fs/select.c | 2 +- > 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/fs/select.c b/fs/select.c > index 9515c3fa1a03..8d5c1419416c 100644 > --- a/fs/select.c > +++ b/fs/select.c > @@ -82,7 +82,7 @@ u64 select_estimate_accuracy(struct timespec64 *tv) > * Realtime tasks get a slack of 0 for obvious reasons. > */ > > - if (rt_task(current)) > + if (realtime_task(current)) > return 0; > > ktime_get_ts64(&now); > diff --git a/include/linux/ioprio.h b/include/linux/ioprio.h > index db1249cd9692..6c00342b6166 100644 > --- a/include/linux/ioprio.h > +++ b/include/linux/ioprio.h > @@ -40,7 +40,7 @@ static inline int task_nice_ioclass(struct task_struct *task) > { > if (task->policy == SCHED_IDLE) > return IOPRIO_CLASS_IDLE; > - else if (task_is_realtime(task)) > + else if (task_has_realtime_policy(task)) > return IOPRIO_CLASS_RT; > else > return IOPRIO_CLASS_BE; > diff --git a/include/linux/sched/deadline.h b/include/linux/sched/deadline.h > index df3aca89d4f5..5cb88b748ad6 100644 > --- a/include/linux/sched/deadline.h > +++ b/include/linux/sched/deadline.h > @@ -10,8 +10,6 @@ > > #include > > -#define MAX_DL_PRIO 0 > - > static inline int dl_prio(int prio) > { > if (unlikely(prio < MAX_DL_PRIO)) > @@ -19,6 +17,10 @@ static inline int dl_prio(int prio) > return 0; > } > > +/* > + * Returns true if a task has a priority that belongs to DL class. PI-boosted > + * tasks will return true. Use dl_policy() to ignore PI-boosted tasks. > + */ > static inline int dl_task(struct task_struct *p) > { > return dl_prio(p->prio); > diff --git a/include/linux/sched/prio.h b/include/linux/sched/prio.h > index ab83d85e1183..6ab43b4f72f9 100644 > --- a/include/linux/sched/prio.h > +++ b/include/linux/sched/prio.h > @@ -14,6 +14,7 @@ > */ > > #define MAX_RT_PRIO 100 > +#define MAX_DL_PRIO 0 > > #define MAX_PRIO (MAX_RT_PRIO + NICE_WIDTH) > #define DEFAULT_PRIO (MAX_RT_PRIO + NICE_WIDTH / 2) > diff --git a/include/linux/sched/rt.h b/include/linux/sched/rt.h > index b2b9e6eb9683..b31be3c50152 100644 > --- a/include/linux/sched/rt.h > +++ b/include/linux/sched/rt.h > @@ -7,18 +7,43 @@ > struct task_struct; > > static inline int rt_prio(int prio) > +{ > + if (unlikely(prio < MAX_RT_PRIO && prio >= MAX_DL_PRIO)) > + return 1; > + return 0; > +} > + > +static inline int realtime_prio(int prio) > { > if (unlikely(prio < MAX_RT_PRIO)) > return 1; > return 0; > } > > +/* > + * Returns true if a task has a priority that belongs to RT class. PI-boosted > + * tasks will return true. Use rt_policy() to ignore PI-boosted tasks. > + */ > static inline int rt_task(struct task_struct *p) > { > return rt_prio(p->prio); > } > > -static inline bool task_is_realtime(struct task_struct *tsk) > +/* > + * Returns true if a task has a priority that belongs to RT or DL classes. > + * PI-boosted tasks will return true. Use task_has_realtime_policy() to ignore > + * PI-boosted tasks. > + */ > +static inline int realtime_task(struct task_struct *p) > +{ > + return realtime_prio(p->prio); > +} > + > +/* > + * Returns true if a task has a policy that belongs to RT or DL classes. > + * PI-boosted tasks will return false. > + */ > +static inline bool task_has_realtime_policy(struct task_struct *tsk) > { > int policy = tsk->policy; > > diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c > index 88d08eeb8bc0..55c9dab37f33 100644 > --- a/kernel/locking/rtmutex.c > +++ b/kernel/locking/rtmutex.c > @@ -347,7 +347,7 @@ static __always_inline int __waiter_prio(struct task_struct *task) > { > int prio = task->prio; > > - if (!rt_prio(prio)) > + if (!realtime_prio(prio)) > return DEFAULT_PRIO; > > return prio; > @@ -435,7 +435,7 @@ static inline bool rt_mutex_steal(struct rt_mutex_waiter *waiter, > * Note that RT tasks are excluded from same priority (lateral) > * steals to prevent the introduction of an unbounded latency. > */ > - if (rt_prio(waiter->tree.prio) || dl_prio(waiter->tree.prio)) > + if (realtime_prio(waiter->tree.prio)) > return false; > > return rt_waiter_node_equal(&waiter->tree, &top_waiter->tree); > diff --git a/kernel/locking/rwsem.c b/kernel/locking/rwsem.c > index c6d17aee4209..ad8d4438bc91 100644 > --- a/kernel/locking/rwsem.c > +++ b/kernel/locking/rwsem.c > @@ -631,7 +631,7 @@ static inline bool rwsem_try_write_lock(struct rw_semaphore *sem, > * if it is an RT task or wait in the wait queue > * for too long. > */ > - if (has_handoff || (!rt_task(waiter->task) && > + if (has_handoff || (!realtime_task(waiter->task) && > !time_after(jiffies, waiter->timeout))) > return false; > > @@ -914,7 +914,7 @@ static bool rwsem_optimistic_spin(struct rw_semaphore *sem) > if (owner_state != OWNER_WRITER) { > if (need_resched()) > break; > - if (rt_task(current) && > + if (realtime_task(current) && > (prev_owner_state != OWNER_WRITER)) > break; > } > diff --git a/kernel/locking/ww_mutex.h b/kernel/locking/ww_mutex.h > index 3ad2cc4823e5..fa4b416a1f62 100644 > --- a/kernel/locking/ww_mutex.h > +++ b/kernel/locking/ww_mutex.h > @@ -237,7 +237,7 @@ __ww_ctx_less(struct ww_acquire_ctx *a, struct ww_acquire_ctx *b) > int a_prio = a->task->prio; > int b_prio = b->task->prio; > > - if (rt_prio(a_prio) || rt_prio(b_prio)) { > + if (realtime_prio(a_prio) || realtime_prio(b_prio)) { > > if (a_prio > b_prio) > return true; > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 1a914388144a..27f15de3d099 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -162,7 +162,7 @@ static inline int __task_prio(const struct task_struct *p) > if (p->sched_class == &stop_sched_class) /* trumps deadline */ > return -2; > > - if (rt_prio(p->prio)) /* includes deadline */ > + if (realtime_prio(p->prio)) /* includes deadline */ > return p->prio; /* [-1, 99] */ > > if (p->sched_class == &idle_sched_class) > @@ -2198,7 +2198,7 @@ static int effective_prio(struct task_struct *p) > * keep the priority unchanged. Otherwise, update priority > * to the normal priority: > */ > - if (!rt_prio(p->prio)) > + if (!realtime_prio(p->prio)) > return p->normal_prio; > return p->prio; > } > @@ -10282,7 +10282,7 @@ void normalize_rt_tasks(void) > schedstat_set(p->stats.sleep_start, 0); > schedstat_set(p->stats.block_start, 0); > > - if (!dl_task(p) && !rt_task(p)) { > + if (!realtime_task(p)) { > /* > * Renice negative nice level userspace > * tasks back to 0: > diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c > index 70625dff62ce..4150e98847fa 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 (task_has_realtime_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; > > 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; > > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > index 3e19b87049db..7372e40f225d 100644 > --- a/mm/page-writeback.c > +++ b/mm/page-writeback.c > @@ -418,7 +418,7 @@ static void domain_dirty_limits(struct dirty_throttle_control *dtc) > if (bg_thresh >= thresh) > bg_thresh = thresh / 2; > tsk = current; > - if (rt_task(tsk)) { > + if (realtime_task(tsk)) { > bg_thresh += bg_thresh / 4 + global_wb_domain.dirty_limit / 32; > thresh += thresh / 4 + global_wb_domain.dirty_limit / 32; > } > @@ -468,7 +468,7 @@ static unsigned long node_dirty_limit(struct pglist_data *pgdat) > else > dirty = vm_dirty_ratio * node_memory / 100; > > - if (rt_task(tsk)) > + if (realtime_task(tsk)) > dirty += dirty / 4; > > return dirty; > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 14d39f34d336..0af24a60ade0 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -3877,7 +3877,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask, unsigned int order) > */ > if (alloc_flags & ALLOC_MIN_RESERVE) > alloc_flags &= ~ALLOC_CPUSET; > - } else if (unlikely(rt_task(current)) && in_task()) > + } else if (unlikely(realtime_task(current)) && in_task()) > alloc_flags |= ALLOC_MIN_RESERVE; > > alloc_flags = gfp_to_alloc_flags_cma(gfp_mask, alloc_flags); > -- > 2.34.1 > > --