[PATCH iwl-next] iavf: convert crit_section to DECLARE_BITMAP

[PATCH iwl-next] iavf: convert crit_section to DECLARE_BITMAP

[Aleksandr Loktionov]

struct iavf_adapter::crit_section is a bit-lock container indexed
by values from enum iavf_critical_section_t (__IAVF_IN_REMOVE_TASK).
It is manipulated exclusively through the kernel atomic bitops API:
set_bit(), clear_bit(), test_bit(), test_and_set_bit().

It is declared as a bare 'unsigned long' and every call site passes
'&adapter->crit_section'. That is functionally correct -- the bit
index is a compile-time enum value well below BITS_PER_LONG so
BIT_WORD(nr) is always 0 and only the singleton word is ever
touched -- but it relies on layout coincidence rather than on the
documented contract of the bitops API, which is defined in terms of
'unsigned long *' arrays.

Static analyzers that model the same contract flag every such call
site with ARRAY_VS_SINGLETON because the address of a scalar is
passed where an array pointer is expected.

Convert the field to a proper bitmap using DECLARE_BITMAP() sized by
a new sentinel __IAVF_CRIT_SECTION_NBITS at the end of the enum. The
underlying storage is unchanged (a single unsigned long word on every
architecture Linux supports), so there is no functional or ABI change;
the type simply becomes 'unsigned long[1]' which matches what the
bitops API expects and silences the analyzer warnings permanently.

Drop the leading '&' at every call site, since arrays decay to
pointers.

No functional change intended.

Suggested-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
---
 drivers/net/ethernet/intel/iavf/iavf.h      |  3 ++-
 drivers/net/ethernet/intel/iavf/iavf_main.c | 16 ++++++++--------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
index 050f824..ba9ad63 100644
--- a/drivers/net/ethernet/intel/iavf/iavf.h
+++ b/drivers/net/ethernet/intel/iavf/iavf.h
@@ -213,6 +213,7 @@ enum iavf_state_t {
 
 enum iavf_critical_section_t {
 	__IAVF_IN_REMOVE_TASK,	/* device being removed */
+	__IAVF_CRIT_SECTION_NBITS	/* must be last */
 };
 
 #define IAVF_CLOUD_FIELD_OMAC		0x01
@@ -389,7 +390,7 @@ struct iavf_adapter {
 
 	enum iavf_state_t state;
 	enum iavf_state_t last_state;
-	unsigned long crit_section;
+	DECLARE_BITMAP(crit_section, __IAVF_CRIT_SECTION_NBITS);
 
 	struct delayed_work watchdog_task;
 	bool link_up;
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index d2914c5..b3332ca 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -264,7 +264,7 @@ void iavf_free_virt_mem(struct iavf_hw *hw, struct iavf_virt_mem *mem)
  **/
 void iavf_schedule_reset(struct iavf_adapter *adapter, u64 flags)
 {
-	if (!test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section) &&
+	if (!test_bit(__IAVF_IN_REMOVE_TASK, adapter->crit_section) &&
 	    !(adapter->flags &
 	    (IAVF_FLAG_RESET_PENDING | IAVF_FLAG_RESET_NEEDED))) {
 		adapter->flags |= flags;
@@ -1355,7 +1355,7 @@ void iavf_down(struct iavf_adapter *adapter)
 	if (adapter->flags & IAVF_FLAG_PF_COMMS_FAILED)
 		return;
 
-	if (!test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section)) {
+	if (!test_bit(__IAVF_IN_REMOVE_TASK, adapter->crit_section)) {
 		/* cancel any current operation */
 		adapter->current_op = VIRTCHNL_OP_UNKNOWN;
 		/* Schedule operations to close down the HW. Don't wait
@@ -1960,7 +1960,7 @@ static void iavf_finish_config(struct work_struct *work)
 
 	if ((adapter->flags & IAVF_FLAG_SETUP_NETDEV_FEATURES) &&
 	    adapter->netdev->reg_state == NETREG_REGISTERED &&
-	    !test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section)) {
+	    !test_bit(__IAVF_IN_REMOVE_TASK, adapter->crit_section)) {
 		netdev_update_features(adapter->netdev);
 		adapter->flags &= ~IAVF_FLAG_SETUP_NETDEV_FEATURES;
 	}
@@ -2016,7 +2016,7 @@ static void iavf_finish_config(struct work_struct *work)
  **/
 void iavf_schedule_finish_config(struct iavf_adapter *adapter)
 {
-	if (!test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section))
+	if (!test_bit(__IAVF_IN_REMOVE_TASK, adapter->crit_section))
 		queue_work(adapter->wq, &adapter->finish_config);
 }
 
@@ -2870,7 +2870,7 @@ static int iavf_watchdog_step(struct iavf_adapter *adapter)
 		return 1;
 	case __IAVF_INIT_FAILED:
 		if (test_bit(__IAVF_IN_REMOVE_TASK,
-			     &adapter->crit_section)) {
+			     adapter->crit_section)) {
 			/* Do not update the state and do not reschedule
 			 * watchdog task, iavf_remove should handle this state
 			 * as it can loop forever
@@ -2889,7 +2889,7 @@ static int iavf_watchdog_step(struct iavf_adapter *adapter)
 		return 1000;
 	case __IAVF_COMM_FAILED:
 		if (test_bit(__IAVF_IN_REMOVE_TASK,
-			     &adapter->crit_section)) {
+			     adapter->crit_section)) {
 			/* Set state to __IAVF_INIT_FAILED and perform remove
 			 * steps. Remove IAVF_FLAG_PF_COMMS_FAILED so the task
 			 * doesn't bring the state back to __IAVF_COMM_FAILED.
@@ -3771,7 +3771,7 @@ static int __iavf_setup_tc(struct net_device *netdev, void *type_data)
 		}
 	}
 exit:
-	if (test_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section))
+	if (test_bit(__IAVF_IN_REMOVE_TASK, adapter->crit_section))
 		return 0;
 
 	netif_set_real_num_rx_queues(netdev, total_qps);
@@ -5513,7 +5513,7 @@ static void iavf_remove(struct pci_dev *pdev)
 	adapter = iavf_pdev_to_adapter(pdev);
 	hw = &adapter->hw;
 
-	if (test_and_set_bit(__IAVF_IN_REMOVE_TASK, &adapter->crit_section))
+	if (test_and_set_bit(__IAVF_IN_REMOVE_TASK, adapter->crit_section))
 		return;
 
 	/* Wait until port initialization is complete.
-- 
2.52.0

Re: [PATCH iwl-next] iavf: convert crit_section to DECLARE_BITMAP

[Simon Horman]

On Fri, May 15, 2026 at 08:38:36AM +0200, Aleksandr Loktionov wrote:
> struct iavf_adapter::crit_section is a bit-lock container indexed
> by values from enum iavf_critical_section_t (__IAVF_IN_REMOVE_TASK).
> It is manipulated exclusively through the kernel atomic bitops API:
> set_bit(), clear_bit(), test_bit(), test_and_set_bit().
> 
> It is declared as a bare 'unsigned long' and every call site passes
> '&adapter->crit_section'. That is functionally correct -- the bit
> index is a compile-time enum value well below BITS_PER_LONG so
> BIT_WORD(nr) is always 0 and only the singleton word is ever
> touched -- but it relies on layout coincidence rather than on the
> documented contract of the bitops API, which is defined in terms of
> 'unsigned long *' arrays.
> 
> Static analyzers that model the same contract flag every such call
> site with ARRAY_VS_SINGLETON because the address of a scalar is
> passed where an array pointer is expected.
> 
> Convert the field to a proper bitmap using DECLARE_BITMAP() sized by
> a new sentinel __IAVF_CRIT_SECTION_NBITS at the end of the enum. The
> underlying storage is unchanged (a single unsigned long word on every
> architecture Linux supports), so there is no functional or ABI change;
> the type simply becomes 'unsigned long[1]' which matches what the
> bitops API expects and silences the analyzer warnings permanently.
> 
> Drop the leading '&' at every call site, since arrays decay to
> pointers.
> 
> No functional change intended.
> 
> Suggested-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

Reviewed-by: Simon Horman <horms@kernel.org>