Re: [PATCH v4 net-next 6/9] net: enetc: add helpers to set/clear table bitmap

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The custom ID allocation helpers deceptively use atomic bit operations and needlessly reinvent the kernel's IDA framework.
--

commit f37b0be8d3c2fcb9018ffa7dc02a01123fbb4e22
Author: Wei Fang <wei.fang@nxp.com>

net: enetc: add helpers to set/clear table bitmap

This commit introduces ntmp_lookup_free_eid() and ntmp_clear_eid_bitmap()
to help allocate and manage entry IDs for NTMP index tables. It uses standard
bitmap operations to track which IDs are free and which are in use.

> diff --git a/drivers/net/ethernet/freescale/enetc/ntmp.c b/drivers/net/ethernet/freescale/enetc/ntmp.c
> index 601435966ed1a..9f38f885ebb52 100644
> --- a/drivers/net/ethernet/freescale/enetc/ntmp.c
> +++ b/drivers/net/ethernet/freescale/enetc/ntmp.c
> @@ -47,6 +47,30 @@
>  #define RSST_STSE_DATA_SIZE(n)		((n) * 8)
>  #define RSST_CFGE_DATA_SIZE(n)		(n)
>  
> +u32 ntmp_lookup_free_eid(unsigned long *bitmap, u32 size)
> +{
> +	u32 entry_id;
> +
> +	entry_id = find_first_zero_bit(bitmap, size);
> +	if (entry_id == size)
> +		return NTMP_NULL_ENTRY_ID;
> +
> +	/* Set the bit once we found it */
> +	set_bit(entry_id, bitmap);

[Severity: Low]
Does this mix of non-atomic find_first_zero_bit() followed by an atomic
set_bit() present a misleading concurrency guarantee in ntmp_lookup_free_eid()?

If this allocation happens under an external lock, would it be more efficient
to use the non-atomic __set_bit() here?

Since this implements dynamic integer ID allocation, would the standard kernel
IDA framework (ida_alloc() and ida_free()) be a better fit than managing
custom raw bitmaps?

> +
> +	return entry_id;
> +}
> +EXPORT_SYMBOL_GPL(ntmp_lookup_free_eid);
> +
> +void ntmp_clear_eid_bitmap(unsigned long *bitmap, u32 entry_id)
> +{
> +	if (entry_id == NTMP_NULL_ENTRY_ID)
> +		return;
> +
> +	clear_bit(entry_id, bitmap);

[Severity: Low]
If external locking is guaranteed for these ID management functions, should
ntmp_clear_eid_bitmap() use the non-atomic __clear_bit() instead of clear_bit()?

> +}
> +EXPORT_SYMBOL_GPL(ntmp_clear_eid_bitmap);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609032955.2066089-1-wei.fang@oss.nxp.com?part=6