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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E2A1CC25B76 for ; Wed, 5 Jun 2024 10:15:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:References :In-Reply-To:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=2mXTfydX3yDR7Caw0VCiqaG/LHuXZj+TMxWDLSUlpJI=; b=h2fvm1X8xY3GVp w9gMTxK4L4hg4tNRkiO5UICiLiHf2bvGLo/1vQVcomF0mU47RJM8p6p7G4SztGIyE5Z1A/03VvCAG zg62q/O8xyuYB4hy9rybm1e/6AxB78IuTRAfLzyYOJxIpoEbAMfI++KRmR8k+6RGIRtiwgQ/UtaKR vGCNhBAoHJg+GIc/KVHqJKa9N1Bkz5qaPDAovAKAI8cgNXC9OBWOXlwuWftoenIjc0VllNzQllVwG T47Owy+cv59fRfEQ6Afz+qH4b9YEL9Qdyggd1u5Bq430UMRcVb1iQYJJYUm56k6UWsPWc2QNuY4a0 V1DnpF4BhFCxEq668mCw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sEnfg-00000005Xlq-0U2t; Wed, 05 Jun 2024 10:15:08 +0000 Received: from galois.linutronix.de ([193.142.43.55]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sEnfb-00000005Xkv-3qHU for linux-arm-kernel@lists.infradead.org; Wed, 05 Jun 2024 10:15:06 +0000 From: Anna-Maria Behnsen DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1717582500; 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=0AGW4NPcPgZ+kllaz1iPcGGi2RTc/mz6X+/BfY/cPz0=; b=pxVqLWi/Yb7a/LrLOJggqlpcYlf6UrMtTLNLpDVqZg0QlLi+CTc0XfTb+/poAi/GcXDmkX qvlJ3Yl5BQKDGI32Nd8o+YhOMGQ/XndHUKh+d2DYC5Pu2GwfbzVmYGTZ1WH3hyB7QB0iAY S8nsQLC/dLKtYjn8fM5RsbSdkNV+bi+j53TIy4pom4FcmR5s+KU8rTKPzZRMijtjWMDru1 NmrFibIHB/mHOx/FWvEn/5kt43i0ysP+D3XfrkHsOfVA1thz/y5Ev/ksq+FgO+SeFb3gXa cSSbbAxDmekJ2wvP/X8M96gzV828929XUtrHOrSKMh+1krLdObJGggLC+m5Vog== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1717582500; 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=0AGW4NPcPgZ+kllaz1iPcGGi2RTc/mz6X+/BfY/cPz0=; b=RWOe6OyLnPQkqmEDbUsgM6r2SGakTwisP2BQzYobq5K7UJMFKqbw24F9h3+s5zRwnAWpvp b/FKYIujy4Of3oAA== To: Phil Chang , phil.chang@mediatek.com Cc: frederic@kernel.org, tglx@linutronix.de, matthias.bgg@gmail.com, angelogioacchino.delregno@collabora.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, jy.ho@mediatek.com, alix.wu@mediatk.com Subject: Re: [PATCH] hrtimer: check hrtimer with a NULL function In-Reply-To: <20240605084149.6420-1-phil.chang@mediatek.com> References: <20240605084149.6420-1-phil.chang@mediatek.com> Date: Wed, 05 Jun 2024 12:15:00 +0200 Message-ID: <87plsvvgpn.fsf@somnus> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240605_031504_121831_126D5320 X-CRM114-Status: GOOD ( 17.95 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi, Phil Chang writes: > simillar with timers, check for timer->function == NULL. > If the pointer is NULL, discard the request silently. Can you please explain, why this change is required? The statement "similar to timers" is not a valid explaination as timer list timers and hrtimers are two different things. The function pointer for timer list timers is explicitly set to NULL in shutdown path to prevent unwanted rearming of the timer. For hrtimers there is no shutdown function implemented and function is never set to NULL by hrtimer code. > Signed-off-by: Phil Chang > --- > kernel/time/hrtimer.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c > index 492c14aac642..72d6e7bc9cd9 100644 > --- a/kernel/time/hrtimer.c > +++ b/kernel/time/hrtimer.c > @@ -1297,9 +1297,13 @@ void hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, > > base = lock_hrtimer_base(timer, &flags); > > + if (!timer->function) > + goto out; When this happens, user of hrtimers do not follow the semantics of hrtimers which means this is a bug. > + > if (__hrtimer_start_range_ns(timer, tim, delta_ns, mode, base)) > hrtimer_reprogram(timer, true); > > +out: > unlock_hrtimer_base(timer, &flags); > } > EXPORT_SYMBOL_GPL(hrtimer_start_range_ns); > @@ -1667,6 +1671,11 @@ static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base, > __remove_hrtimer(timer, base, HRTIMER_STATE_INACTIVE, 0); > fn = timer->function; > > + if (WARN_ON_ONCE(!fn)) { > + /* Should never happen. */ ...same as above... > + goto out; > + } > + > /* > * Clear the 'is relative' flag for the TIME_LOW_RES case. If the > * timer is restarted with a period then it becomes an absolute > @@ -1710,6 +1719,7 @@ static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base, > * hrtimer_active() cannot observe base->running.timer == NULL && > * timer->state == INACTIVE. > */ > +out: > raw_write_seqcount_barrier(&base->seq); > > WARN_ON_ONCE(base->running != timer); Thanks, Anna-Maria _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel