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 X-Spam-Level: X-Spam-Status: No, score=-10.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A6CD4C433ED for ; Fri, 30 Apr 2021 13:00:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7632B61477 for ; Fri, 30 Apr 2021 13:00:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232335AbhD3NAy (ORCPT ); Fri, 30 Apr 2021 09:00:54 -0400 Received: from foss.arm.com ([217.140.110.172]:47844 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231696AbhD3NAw (ORCPT ); Fri, 30 Apr 2021 09:00:52 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 89488ED1; Fri, 30 Apr 2021 06:00:04 -0700 (PDT) Received: from [192.168.178.6] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9DA9C3F73B; Fri, 30 Apr 2021 06:00:01 -0700 (PDT) Subject: Re: [PATCH v2] sched: Fix out-of-bound access in uclamp To: Vincent Guittot , Quentin Perret Cc: Ingo Molnar , Peter Zijlstra , Juri Lelli , Steven Rostedt , Ben Segall , Mel Gorman , Daniel Bristot de Oliveira , Qais Yousef , Android Kernel Team , linux-kernel , Patrick Bellasi References: <20210429152656.4118460-1-qperret@google.com> From: Dietmar Eggemann Message-ID: Date: Fri, 30 Apr 2021 15:00:00 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 30/04/2021 14:03, Vincent Guittot wrote: > On Fri, 30 Apr 2021 at 11:40, Quentin Perret wrote: >> >> On Friday 30 Apr 2021 at 10:49:50 (+0200), Vincent Guittot wrote: >>> 20 buckets is probably not the best example because of the rounding of >>> the division. With 16 buckets, each bucket should be exactly 64 steps >>> large except the last one which will have 65 steps because of the >>> value 1024. With your change, buckets will be 65 large and the last >>> one will be only 49 large >> >> OK, so what do you think of this? > > Looks good to me +1 >> diff --git a/kernel/sched/core.c b/kernel/sched/core.c >> index c5fb230dc604..dceeb5821797 100644 >> --- a/kernel/sched/core.c >> +++ b/kernel/sched/core.c >> @@ -920,14 +920,14 @@ static struct uclamp_se uclamp_default[UCLAMP_CNT]; >> */ >> DEFINE_STATIC_KEY_FALSE(sched_uclamp_used); >> >> -#define UCLAMP_BUCKET_DELTA (SCHED_CAPACITY_SCALE / UCLAMP_BUCKETS + 1) >> +#define UCLAMP_BUCKET_DELTA DIV_ROUND_CLOSEST(SCHED_CAPACITY_SCALE, UCLAMP_BUCKETS) >> >> #define for_each_clamp_id(clamp_id) \ >> for ((clamp_id) = 0; (clamp_id) < UCLAMP_CNT; (clamp_id)++) >> >> static inline unsigned int uclamp_bucket_id(unsigned int clamp_value) >> { >> - return clamp_value / UCLAMP_BUCKET_DELTA; >> + return min(clamp_value / UCLAMP_BUCKET_DELTA, UCLAMP_BUCKETS - 1); IMHO, this asks for min_t(unsigned int, clamp_value/UCLAMP_BUCKET_DELTA, UCLAMP_BUCKETS-1); >> } >> >> static inline unsigned int uclamp_none(enum uclamp_id clamp_id) Looks like this will fix a lot of possible configs: nbr buckets 1-4, 7-8, 10-12, 14-17, *20*, 26, 29-32 ... We would still introduce larger last buckets, right? Examples: nbr_buckets delta last bucket size 20 51 +5 = 56 26 39 +10 = 49 29 35 +9 = 44 ...