From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5F28545A81; Wed, 2 Oct 2024 21:15:13 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 01992402A7; Wed, 2 Oct 2024 21:15:13 +0200 (CEST) Received: from mail-pj1-f52.google.com (mail-pj1-f52.google.com [209.85.216.52]) by mails.dpdk.org (Postfix) with ESMTP id EA71640299 for ; Wed, 2 Oct 2024 21:15:10 +0200 (CEST) Received: by mail-pj1-f52.google.com with SMTP id 98e67ed59e1d1-2e078d28fe9so147149a91.2 for ; Wed, 02 Oct 2024 12:15:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1727896510; x=1728501310; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=ZLSDvgl/L3wqhuJrOy+8Biwky+JQPh+71hLHFo64QxQ=; b=HIbXUzpLGsSQV96ptjwklFB5Wq5A3JfXwDY1G3CHfx+fqoknf40CLOkc4t1KN8xcQW 3VlNvNhWQOnKiPyClU02ykw2apPGUhXXbmXHK2Sx1c6aOoACfat2R39PBiTO0orVLKks kWV7lG5itdknwoYdVr2UoFbqg0Z7SMvGCRBFbeVfW7/C6a2vDkdP78fwNwj328bvTqHb DhMdLY0XeQNy/mlAswgj2B8FSsYrJnpp8XprbjeVNZZS66Z77iwQzncDFwlfgq4VfY5E 1YbDm3a/++wgrtyQvUE12jz5EfajndJn6W/xi71qiu9x7tnvnOvwYudEj7+e1z+k0xnd tZkg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727896510; x=1728501310; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ZLSDvgl/L3wqhuJrOy+8Biwky+JQPh+71hLHFo64QxQ=; b=k3OgOGpqEZlcsXZQq78lD6DNcixrLAmYQy7CHOF1QmWcvP7/hopd1zCTSO6GpksLPM rNZFkwP/QmpA6r8DFKFZg6jB1qjT7Qt20upvo68P9/Ed4TICfQWuyH4ogYMvw7brkw+V TgJWjNB3Dr/IZJcK7xovwnURUfTq7XoBLpIw6T8tykhJWZDn9Te4DPACOzj4bH95q9+Z VuIms9mHrXgxn1J4aTTFYUnkDQkUEBMOWidFtKz6HwFf5rwGyZgP6ojUI52ov4AFJDIA MP8TVZDrmzq2zmC1CIt8WFC4eoAaCSUz8sy9e/FFJ4YDUTHZJSL9VkkZNo3Zikv2KLZz fE7A== X-Gm-Message-State: AOJu0Yyw9j75FuaEwOZcs8VHiU60vAeX+VCxoVvp3ec5VMQPRZ1xQ8DZ UNJ6C6J2rDH2CoqE5BgyqKNM38dX1kEcUuQZmO5vvRTuk8c3yFRe6ZoI2eKHCZLSZXVCXLNwQTE fhH910Z6DRp3yh0EpNJEPeAXvDHE= X-Google-Smtp-Source: AGHT+IE6BczBqOcZEo9dGAj0guny8xzWnc0GK2tQyNDCxwzGOJ1s/gcD14uI2REXoyGKlyg0hPs1Wv9aAIBjjkSmb9U= X-Received: by 2002:a17:90a:d084:b0:2d1:c9c8:301c with SMTP id 98e67ed59e1d1-2e1848e2bf0mr4975552a91.29.1727896509843; Wed, 02 Oct 2024 12:15:09 -0700 (PDT) MIME-Version: 1.0 References: <20240921140022.107239-1-iboukris@gmail.com> <20241002165840.341116-1-iboukris@gmail.com> <20241002165840.341116-3-iboukris@gmail.com> In-Reply-To: From: Isaac Boukris Date: Wed, 2 Oct 2024 22:14:57 +0300 Message-ID: Subject: Re: [PATCH v4 2/2] timer: allow platform to override cpu TSC frequency To: Bruce Richardson Cc: dev@dpdk.org, stephen@networkplumber.org, roretzla@linux.microsoft.com, dmitry.kozliuk@gmail.com, david.marchand@redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Wed, Oct 2, 2024 at 8:11=E2=80=AFPM Bruce Richardson wrote: > > On Wed, Oct 02, 2024 at 07:56:36PM +0300, Isaac Boukris wrote: > > The CPU value is often not accurate, allow overriding it based > > on info from the host OS. > > > > On Linux X86, if the tsc_known_freq cpu flag is missing, it means > > the kernel doesn't trust it and calculates its own. We should do > > the same to avoid drift. > > > > On freebsd we have access to the kernel tsc_hz value, just use it. > > > > Signed-off-by: Isaac Boukris > > --- > > Looks good to me. Hope to test it out soon on my systems to see how it > goes. > > One comment inline below. > > /Bruce > > > lib/eal/common/eal_common_timer.c | 3 +- > > lib/eal/common/eal_private.h | 2 +- > > lib/eal/freebsd/eal_timer.c | 8 +++-- > > lib/eal/linux/eal_timer.c | 53 +++++++++++++++++++++++++++++-- > > lib/eal/windows/eal_timer.c | 5 ++- > > 5 files changed, 62 insertions(+), 9 deletions(-) > > > > diff --git a/lib/eal/common/eal_common_timer.c b/lib/eal/common/eal_com= mon_timer.c > > index c5c4703f15..e00be0a5c8 100644 > > --- a/lib/eal/common/eal_common_timer.c > > +++ b/lib/eal/common/eal_common_timer.c > > @@ -66,8 +66,7 @@ set_tsc_freq(void) > > } > > > > freq =3D get_tsc_freq_arch(); > > - if (!freq) > > - freq =3D get_tsc_freq(); > > + freq =3D get_tsc_freq(freq); > > if (!freq) > > freq =3D estimate_tsc_freq(); > > > > diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.= h > > index af09620426..bb315dab04 100644 > > --- a/lib/eal/common/eal_private.h > > +++ b/lib/eal/common/eal_private.h > > @@ -374,7 +374,7 @@ void set_tsc_freq(void); > > * > > * This function is private to the EAL. > > */ > > -uint64_t get_tsc_freq(void); > > +uint64_t get_tsc_freq(uint64_t arch_hz); > > > > /** > > * Get TSC frequency if the architecture supports. > > diff --git a/lib/eal/freebsd/eal_timer.c b/lib/eal/freebsd/eal_timer.c > > index 3dd70e24ba..19fc9a7228 100644 > > --- a/lib/eal/freebsd/eal_timer.c > > +++ b/lib/eal/freebsd/eal_timer.c > > @@ -26,7 +26,7 @@ > > enum timer_source eal_timer_source =3D EAL_TIMER_TSC; > > > > uint64_t > > -get_tsc_freq(void) > > +get_tsc_freq(uint64_t arch_hz) > > { > > size_t sz; > > int tmp; > > @@ -50,9 +50,13 @@ get_tsc_freq(void) > > sz =3D sizeof(tsc_hz); > > if (sysctlbyname("machdep.tsc_freq", &tsc_hz, &sz, NULL, 0)) { > > EAL_LOG(WARNING, "%s", strerror(errno)); > > - return 0; > > + return arch_hz; > > } > > > > + if (arch_hz && arch_hz - tsc_hz > arch_hz / 100) > > Do we not need an "abs()" call on the "arch_hz - tsc_hz" in case tsc_hz i= s > the bigger value? As they are unsigned it will overflow and result the absolute value, I actually did use abs() at first, and the compiler yelled at me: warning: taking the absolute value of unsigned type =E2=80=98uint64_t=E2=80= =99 {aka =E2=80=98long unsigned int=E2=80=99} has no effect [-Wabsolute-value]