linux-kernel - Re: [PATCH] sched/deadline: Skip overflow check if 0 capacity

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <735344cd-1c32-451f-904e-235578e1a360@redhat.com>
Date: Fri, 8 Nov 2024 08:39:14 -0500
From: Waiman Long <llong@...hat.com>
To: Juri Lelli <juri.lelli@...hat.com>
Cc: Ingo Molnar <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>,
 Vincent Guittot <vincent.guittot@...aro.org>,
 Dietmar Eggemann <dietmar.eggemann@....com>,
 Steven Rostedt <rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>,
 Mel Gorman <mgorman@...e.de>, Valentin Schneider <vschneid@...hat.com>,
 linux-kernel@...r.kernel.org, Phil Auld <pauld@...hat.com>,
 Joel Fernandes <joel@...lfernandes.org>
Subject: Re: [PATCH] sched/deadline: Skip overflow check if 0 capacity

On 11/8/24 8:25 AM, Juri Lelli wrote:
> On 07/11/24 23:29, Waiman Long wrote:
>> By properly setting up a 1-cpu sched domain (partition) with no
>> task, it was found that offlining that particular CPU failed because
>> dl_bw_check_overflow() in cpuset_cpu_inactive() returned -EBUSY. This
>> is due to the fact that dl_bw_capacity() return 0 as the sched domain
>> has no active CPU causing a false positive in the overflow check.
>>
>> Fix this corner case by skipping the __dl_overflow() check in
>> dl_bw_manage() when the returned capacity is 0.
>>
>> Signed-off-by: Waiman Long <longman@...hat.com>
>> ---
>>   kernel/sched/deadline.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
>> index be1b917dc8ce..0195f350d6d3 100644
>> --- a/kernel/sched/deadline.c
>> +++ b/kernel/sched/deadline.c
>> @@ -3479,7 +3479,13 @@ static int dl_bw_manage(enum dl_bw_request req, int cpu, u64 dl_bw)
>>   	} else {
>>   		unsigned long cap = dl_bw_capacity(cpu);
>>   
>> -		overflow = __dl_overflow(dl_b, cap, 0, dl_bw);
>> +		/*
>> +		 * In the unlikely case of 0 capacity (e.g. a sched domain
>> +		 * with no active CPUs), skip the overflow check as it will
>> +		 * always return a false positive.
>> +		 */
>> +		if (likely(cap))
>> +			overflow = __dl_overflow(dl_b, cap, 0, dl_bw);
> The remaining total_bw that make this check fail should be the one
> relative to the dl_server on the cpu that is going offline. Wonder if we
> shouldn't rather clean that up (remove dl_server contribution) before we
> get to this point during an hotplug operation. Need to think about it a
> little more.
static inline bool
__dl_overflow(struct dl_bw *dl_b, unsigned long cap, u64 old_bw, u64 new_bw)
{
         return dl_b->bw != -1 &&
                cap_scale(dl_b->bw, cap) < dl_b->total_bw - old_bw + new_bw;
}

With a 0 cap, cap_scale(dl_b->bw, cap) will always be 0. As long as 
total_bw isn't 0 and bw isn't -1, the condition will be true.

Cheers,
Longman