Re: [PATCH net-next v2 3/6] eth: fbnic: cache hwmon sensor readings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Signed subtraction wrap-around in `time_is_after_eq_jiffies()` causes hwmon sensor readings to freeze for 24.8 days on 32-bit architectures.
--

--- Patch [3]: [PATCH net-next v2 3/6] eth: fbnic: cache hwmon sensor readings ---
commit 976f9fc351b80235f704002eae4e7012534c286d
Author: Zinc Lim <limzhineng2@gmail.com>

eth: fbnic: cache hwmon sensor readings

This commit introduces caching for hardware monitoring sensor readings in the
fbnic driver. It caches the temperature and voltage for the remainder of the
current jiffy to prevent flooding the firmware mailbox during rapid polling.

> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c b/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c
> index 38bb26cb8e6cb..f35cb00650937 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c
[ ... ]
> @@ -25,26 +26,32 @@ static umode_t fbnic_hwmon_is_visible(const void *drvdata,
>  
>  static int fbnic_hwmon_sensor_read(struct fbnic_dev *fbd, int id, long *val)
>  {
> +	struct fbnic_hwmon_cache *cache = &fbd->hwmon_cache;
>  	struct fbnic_fw_completion *fw_cmpl;
>  	int err = 0;
> -	s32 *sensor;
> -
> -	fw_cmpl = fbnic_fw_alloc_cmpl(FBNIC_TLV_MSG_ID_TSENE_READ_RESP);
> -	if (!fw_cmpl)
> -		return -ENOMEM;
> +	s32 *cached;
>  
>  	switch (id) {
>  	case FBNIC_SENSOR_TEMP:
> -		sensor = &fw_cmpl->u.tsene.millidegrees;
> +		cached = &cache->temp_mdeg;
>  		break;
>  	case FBNIC_SENSOR_VOLTAGE:
> -		sensor = &fw_cmpl->u.tsene.millivolts;
> +		cached = &cache->volt_mv;
>  		break;
>  	default:
> -		err = -EINVAL;
> -		goto exit_free;
> +		return -EINVAL;
> +	}
> +
> +	if (*cached != FBNIC_SENSOR_NO_DATA &&
> +	    time_is_after_eq_jiffies(cache->last_read)) {

[Severity: High]
On 32-bit architectures, if the sensor isn't read for over 24.8 days, will
time_is_after_eq_jiffies() evaluate the wrapped signed subtraction as a
false cache hit?

If this condition evaluates to true under those circumstances, the function
returns early without updating cache->last_read. Does this mean the sensor
readings will continue to falsely hit the cache, causing the temperature
and voltage values to remain frozen for the next 24.8 days?

> +		*val = *cached;
> +		return 0;
>  	}
>  
> +	fw_cmpl = fbnic_fw_alloc_cmpl(FBNIC_TLV_MSG_ID_TSENE_READ_RESP);
> +	if (!fw_cmpl)
> +		return -ENOMEM;
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824175059.2485048-1-limzhineng2@gmail.com?part=3