[PATCH] igc: optimize igc_ptp_systim_to_hwtstamp()

Subsystems: intel ethernet drivers, networking drivers, the rest

STALE1810d

2 messages, 2 authors, 2021-08-27 · open the first message on its own page

[PATCH] igc: optimize igc_ptp_systim_to_hwtstamp()

From: trix@redhat.com
Date: 2021-08-27 17:15:37

From: Tom Rix <trix@redhat.com>

Static analysis reports this representative problem
igc_ptp.c:676:3: warning: The left operand of '+' is a garbage value
                ktime_add_ns(shhwtstamps.hwtstamp, adjust);
                ^            ~~~~~~~~~~~~~~~~~~~~

The issue is flagged because the setting of shhwtstamps is
in igc_ptp_systim_to_hwtstamp() it is set only in one path through
this switch.

	switch (adapter->hw.mac.type) {
	case igc_i225:
		memset(hwtstamps, 0, sizeof(*hwtstamps));
		/* Upper 32 bits contain s, lower 32 bits contain ns. */
		hwtstamps->hwtstamp = ktime_set(systim >> 32,
						systim & 0xFFFFFFFF);
		break;
	default:
		break;
	}

Changing the memset the a caller initialization is a small optimization
and will resolve uninitialized use issue.

A switch statement with one case is overkill, convert to an if statement.

This function is small and only called once, change to inline for an
expected small runtime and size improvement.

Signed-off-by: Tom Rix <trix@redhat.com>
---
 drivers/net/ethernet/intel/igc/igc_ptp.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
index 0f021909b430a0..1443a2da246e22 100644
--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
@@ -417,20 +417,14 @@ static int igc_ptp_verify_pin(struct ptp_clock_info *ptp, unsigned int pin,
  * We need to convert the system time value stored in the RX/TXSTMP registers
  * into a hwtstamp which can be used by the upper level timestamping functions.
  **/
-static void igc_ptp_systim_to_hwtstamp(struct igc_adapter *adapter,
-				       struct skb_shared_hwtstamps *hwtstamps,
-				       u64 systim)
+static inline void igc_ptp_systim_to_hwtstamp(struct igc_adapter *adapter,
+					      struct skb_shared_hwtstamps *hwtstamps,
+					      u64 systim)
 {
-	switch (adapter->hw.mac.type) {
-	case igc_i225:
-		memset(hwtstamps, 0, sizeof(*hwtstamps));
-		/* Upper 32 bits contain s, lower 32 bits contain ns. */
+	/* Upper 32 bits contain s, lower 32 bits contain ns. */
+	if (adapter->hw.mac.type == igc_i225)
 		hwtstamps->hwtstamp = ktime_set(systim >> 32,
 						systim & 0xFFFFFFFF);
-		break;
-	default:
-		break;
-	}
 }
 
 /**
@@ -645,7 +639,7 @@ void igc_ptp_tx_hang(struct igc_adapter *adapter)
 static void igc_ptp_tx_hwtstamp(struct igc_adapter *adapter)
 {
 	struct sk_buff *skb = adapter->ptp_tx_skb;
-	struct skb_shared_hwtstamps shhwtstamps;
+	struct skb_shared_hwtstamps shhwtstamps = { 0 };
 	struct igc_hw *hw = &adapter->hw;
 	int adjust = 0;
 	u64 regval;
-- 
2.26.3

Re: [PATCH] igc: optimize igc_ptp_systim_to_hwtstamp()

From: "Nguyen, Anthony L" <anthony.l.nguyen@intel.com>
Date: 2021-08-27 23:40:09

On Fri, 2021-08-27 at 10:15 -0700, trix@redhat.com wrote:
quoted hunk
From: Tom Rix <trix@redhat.com>

Static analysis reports this representative problem
igc_ptp.c:676:3: warning: The left operand of '+' is a garbage value
                ktime_add_ns(shhwtstamps.hwtstamp, adjust);
                ^            ~~~~~~~~~~~~~~~~~~~~

The issue is flagged because the setting of shhwtstamps is
in igc_ptp_systim_to_hwtstamp() it is set only in one path through
this switch.

	switch (adapter->hw.mac.type) {
	case igc_i225:
		memset(hwtstamps, 0, sizeof(*hwtstamps));
		/* Upper 32 bits contain s, lower 32 bits contain ns.
*/
		hwtstamps->hwtstamp = ktime_set(systim >> 32,
						systim & 0xFFFFFFFF);
		break;
	default:
		break;
	}

Changing the memset the a caller initialization is a small
optimization
and will resolve uninitialized use issue.

A switch statement with one case is overkill, convert to an if
statement.

This function is small and only called once, change to inline for an
expected small runtime and size improvement.

Signed-off-by: Tom Rix <trix@redhat.com>
---
 drivers/net/ethernet/intel/igc/igc_ptp.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c
b/drivers/net/ethernet/intel/igc/igc_ptp.c
index 0f021909b430a0..1443a2da246e22 100644
--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
@@ -417,20 +417,14 @@ static int igc_ptp_verify_pin(struct
ptp_clock_info *ptp, unsigned int pin,
  * We need to convert the system time value stored in the RX/TXSTMP
registers
  * into a hwtstamp which can be used by the upper level timestamping
functions.
  **/
-static void igc_ptp_systim_to_hwtstamp(struct igc_adapter *adapter,
-				       struct skb_shared_hwtstamps
*hwtstamps,
-				       u64 systim)
+static inline void igc_ptp_systim_to_hwtstamp(struct igc_adapter
*adapter,
Please don't use inline in C files, let the compiler decide.
quoted hunk
+					      struct
skb_shared_hwtstamps *hwtstamps,
+					      u64 systim)
 {
-	switch (adapter->hw.mac.type) {
-	case igc_i225:
-		memset(hwtstamps, 0, sizeof(*hwtstamps));
-		/* Upper 32 bits contain s, lower 32 bits contain ns.
*/
+	/* Upper 32 bits contain s, lower 32 bits contain ns. */
+	if (adapter->hw.mac.type == igc_i225)
 		hwtstamps->hwtstamp = ktime_set(systim >> 32,
 						systim & 0xFFFFFFFF);
-		break;
-	default:
-		break;
-	}
 }
 
 /**
@@ -645,7 +639,7 @@ void igc_ptp_tx_hang(struct igc_adapter *adapter)
 static void igc_ptp_tx_hwtstamp(struct igc_adapter *adapter)
 {
 	struct sk_buff *skb = adapter->ptp_tx_skb;
-	struct skb_shared_hwtstamps shhwtstamps;
+	struct skb_shared_hwtstamps shhwtstamps = { 0 };
Need to re-order for RCT.

Thanks,
Tony
 	struct igc_hw *hw = &adapter->hw;
 	int adjust = 0;
 	u64 regval;
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help