All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: <dev@dpdk.org>, Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>,
	Thomas Monjalon <thomas@monjalon.net>,
	Ravi Kerur <rkerur@brocade.com>
Subject: Re: [PATCH v2] eal: fail initialization if TSC frequency is zero
Date: Tue, 29 Sep 2026 09:39:36 +0100	[thread overview]
Message-ID: <art5SCrwPVJ8JkKQ@bricha3-mobl1.ger.corp.intel.com> (raw)
In-Reply-To: <20260928231637.733450-1-stephen@networkplumber.org>

On Mon, Sep 28, 2026 at 04:15:58PM -0700, Stephen Hemminger wrote:
> Many parts of DPDK will fail with divide by zero and
> other errors if the initialization logic ever TSC hz was ever
> determined to be zero. This might happen on a broken get_tsc_freq_arch()
> or bad emulation in QEMU.
> 
> If TSC hz is zero, log the error and propagate back to
> fail rte_eal_init().
> 
> This fix doesn't need to go to stable since it is a purely
> theoretical problem; we aren't getting divide by zero reports
> from users.
> 

How was this discovered? Is there a coverity issue id, or was it just AI
discovered?

> Fixes: 040cf8a41187 ("eal: deduplicate timer functions")
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> v2 - cleanups: mostly squash useless comments
> 

Code looks generally ok to me.

>  lib/eal/common/eal_common_timer.c    | 19 ++++++++++++++++---
>  lib/eal/common/eal_private.h         |  2 +-
>  lib/eal/freebsd/eal_timer.c          |  3 +--
>  lib/eal/include/generic/rte_cycles.h |  2 +-
>  lib/eal/linux/eal_timer.c            |  3 +--
>  lib/eal/windows/eal_timer.c          |  3 +--
>  6 files changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/eal/common/eal_common_timer.c b/lib/eal/common/eal_common_timer.c
> index bbf8b8b11b..e9f8b56b59 100644
> --- a/lib/eal/common/eal_common_timer.c
> +++ b/lib/eal/common/eal_common_timer.c
> @@ -52,7 +52,7 @@ estimate_tsc_freq(void)
>  	return RTE_ALIGN_MUL_NEAR(rte_rdtsc() - start, CYC_PER_10MHZ);
>  }
>  
> -void
> +int
>  set_tsc_freq(void)
>  {
>  	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
> @@ -65,18 +65,31 @@ set_tsc_freq(void)
>  		 * systems where arch-specific frequency detection is not
>  		 * available.
>  		 */
> +		if (mcfg->tsc_hz == 0) {
> +			EAL_LOG(ERR, "Primary process TSC frequency is zero");
> +			return -1;
> +		}
> +
>  		eal_tsc_resolution_hz = mcfg->tsc_hz;
> -		return;
> +		return 0;
>  	}
>  
>  	freq = get_tsc_freq_arch();
>  	freq = get_tsc_freq(freq);
> -	if (!freq)
> +	if (freq == 0) {
>  		freq = estimate_tsc_freq();
>  
> +		/* Check if TSC is not moving */
> +		if (freq == 0) {
> +			EAL_LOG(ERR, "TSC frequency is not changing");
> +			return -1;
> +		}
> +	}
> +
>  	EAL_LOG(DEBUG, "TSC frequency is ~%" PRIu64 " KHz", freq / 1000);
>  	eal_tsc_resolution_hz = freq;
>  	mcfg->tsc_hz = freq;
> +	return 0;
>  }
>  
>  RTE_EXPORT_SYMBOL(rte_delay_us_callback_register)
> diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h
> index 6340bab8be..952cb5a03e 100644
> --- a/lib/eal/common/eal_private.h
> +++ b/lib/eal/common/eal_private.h
> @@ -409,7 +409,7 @@ int eal_cpu_detected(unsigned lcore_id);
>   *
>   * This function is private to the EAL.
>   */
> -void set_tsc_freq(void);
> +int set_tsc_freq(void);
>  

One minor suggestion: set_tsc_freq name implies that the user passes in a
value to be set. I wonder if "init_tsc_freq" might be a better name here,
since you are updating all calls anyway to handle an error return.

>  /**
>   * Get precise TSC frequency from system
> diff --git a/lib/eal/freebsd/eal_timer.c b/lib/eal/freebsd/eal_timer.c
> index d21ffa2694..84127d876b 100644
> --- a/lib/eal/freebsd/eal_timer.c
> +++ b/lib/eal/freebsd/eal_timer.c
> @@ -65,6 +65,5 @@ get_tsc_freq(uint64_t arch_hz)
>  int
>  rte_eal_timer_init(void)
>  {
> -	set_tsc_freq();
> -	return 0;
> +	return set_tsc_freq();
>  }
> diff --git a/lib/eal/include/generic/rte_cycles.h b/lib/eal/include/generic/rte_cycles.h
> index 7cfd51f0eb..f8e1cde332 100644
> --- a/lib/eal/include/generic/rte_cycles.h
> +++ b/lib/eal/include/generic/rte_cycles.h
> @@ -34,7 +34,7 @@ extern enum timer_source eal_timer_source;
>   * Get the measured frequency of the RDTSC counter
>   *
>   * @return
> - *   The TSC frequency for this lcore
> + *   The TSC frequency for all lcores, always non-zero
>   */
>  uint64_t
>  rte_get_tsc_hz(void);
> diff --git a/lib/eal/linux/eal_timer.c b/lib/eal/linux/eal_timer.c
> index 39f975b6b9..bccff60ff8 100644
> --- a/lib/eal/linux/eal_timer.c
> +++ b/lib/eal/linux/eal_timer.c
> @@ -99,6 +99,5 @@ rte_eal_timer_init(void)
>  
>  	eal_timer_source = EAL_TIMER_TSC;
>  
> -	set_tsc_freq();
> -	return 0;
> +	return set_tsc_freq();
>  }
> diff --git a/lib/eal/windows/eal_timer.c b/lib/eal/windows/eal_timer.c
> index 33cbac6a03..aec8ea854d 100644
> --- a/lib/eal/windows/eal_timer.c
> +++ b/lib/eal/windows/eal_timer.c
> @@ -94,6 +94,5 @@ get_tsc_freq(uint64_t arch_hz)
>  int
>  rte_eal_timer_init(void)
>  {
> -	set_tsc_freq();
> -	return 0;
> +	return set_tsc_freq();
>  }
> -- 
> 2.53.0
> 

  reply	other threads:[~2026-09-29  8:39 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-27 19:54 [PATCH] eal: fail initialization if TSC frequency is zero Stephen Hemminger
2026-09-27 20:09 ` Stephen Hemminger
2026-09-28 23:15 ` [PATCH v2] " Stephen Hemminger
2026-09-29  8:39   ` Bruce Richardson [this message]
2026-09-29 13:42     ` Stephen Hemminger
2026-09-29 13:57       ` Bruce Richardson
2026-09-29 14:10         ` Stephen Hemminger
2026-09-29 14:13   ` [PATCH v3] " Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=art5SCrwPVJ8JkKQ@bricha3-mobl1.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=dmitry.kozliuk@gmail.com \
    --cc=rkerur@brocade.com \
    --cc=stephen@networkplumber.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.