Skip to content

Commit d0023f8

Browse files
davem330davem330
committed
Merge branch 'gfar-ethtool-atomic' of git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux
Paul Gortmaker says: ==================== Eric noticed that the handling of local u64 ethtool counters for this driver commonly found on Freescale ppc-32 boards was racy. However, before converting them over to atomic64_t, I noticed that an internal struct was being used to determine the offsets for exporting this data into the ethtool buffer, and in doing so, it assumed that the counters would always be u64. Rather than keep this implicit assumption, a simple code cleanup gets rid of the struct completely, and leaves less conversion sites. The alternative solution would have been to take advantage of the fact that the counters are all relating to error conditions, and hence make them internally u32. In doing so, we'd be assuming that U32_MAX of any particular error condition is highly unlikely. This might have made sense if any increments were in a hot path. Tested with "ethtool -S eth0" on sbc8548 board. ==================== Signed-off-by: David S. Miller <davem@davemloft.net>
2 parents 959d5fd + 212079d commit d0023f8

File tree

3 files changed

+37
-45
lines changed

3 files changed

+37
-45
lines changed

‎drivers/net/ethernet/freescale/gianfar.c

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2648,7 +2648,7 @@ static inline void count_errors(unsigned short status, struct net_device *dev)
26482648
if (status & RXBD_TRUNCATED) {
26492649
stats->rx_length_errors++;
26502650

2651-
estats->rx_trunc++;
2651+
atomic64_inc(&estats->rx_trunc);
26522652

26532653
return;
26542654
}
@@ -2657,20 +2657,20 @@ static inline void count_errors(unsigned short status, struct net_device *dev)
26572657
stats->rx_length_errors++;
26582658

26592659
if (status & RXBD_LARGE)
2660-
estats->rx_large++;
2660+
atomic64_inc(&estats->rx_large);
26612661
else
2662-
estats->rx_short++;
2662+
atomic64_inc(&estats->rx_short);
26632663
}
26642664
if (status & RXBD_NONOCTET) {
26652665
stats->rx_frame_errors++;
2666-
estats->rx_nonoctet++;
2666+
atomic64_inc(&estats->rx_nonoctet);
26672667
}
26682668
if (status & RXBD_CRCERR) {
2669-
estats->rx_crcerr++;
2669+
atomic64_inc(&estats->rx_crcerr);
26702670
stats->rx_crc_errors++;
26712671
}
26722672
if (status & RXBD_OVERRUN) {
2673-
estats->rx_overrun++;
2673+
atomic64_inc(&estats->rx_overrun);
26742674
stats->rx_crc_errors++;
26752675
}
26762676
}
@@ -2744,7 +2744,7 @@ static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
27442744
ret = napi_gro_receive(napi, skb);
27452745

27462746
if (GRO_DROP == ret)
2747-
priv->extra_stats.kernel_dropped++;
2747+
atomic64_inc(&priv->extra_stats.kernel_dropped);
27482748

27492749
return 0;
27502750
}
@@ -2812,7 +2812,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
28122812
} else {
28132813
netif_warn(priv, rx_err, dev, "Missing skb!\n");
28142814
rx_queue->stats.rx_dropped++;
2815-
priv->extra_stats.rx_skbmissing++;
2815+
atomic64_inc(&priv->extra_stats.rx_skbmissing);
28162816
}
28172817

28182818
}
@@ -3245,7 +3245,7 @@ static irqreturn_t gfar_error(int irq, void *grp_id)
32453245
netif_dbg(priv, tx_err, dev,
32463246
"TX FIFO underrun, packet dropped\n");
32473247
dev->stats.tx_dropped++;
3248-
priv->extra_stats.tx_underrun++;
3248+
atomic64_inc(&priv->extra_stats.tx_underrun);
32493249

32503250
local_irq_save(flags);
32513251
lock_tx_qs(priv);
@@ -3260,7 +3260,7 @@ static irqreturn_t gfar_error(int irq, void *grp_id)
32603260
}
32613261
if (events & IEVENT_BSY) {
32623262
dev->stats.rx_errors++;
3263-
priv->extra_stats.rx_bsy++;
3263+
atomic64_inc(&priv->extra_stats.rx_bsy);
32643264

32653265
gfar_receive(irq, grp_id);
32663266

@@ -3269,19 +3269,19 @@ static irqreturn_t gfar_error(int irq, void *grp_id)
32693269
}
32703270
if (events & IEVENT_BABR) {
32713271
dev->stats.rx_errors++;
3272-
priv->extra_stats.rx_babr++;
3272+
atomic64_inc(&priv->extra_stats.rx_babr);
32733273

32743274
netif_dbg(priv, rx_err, dev, "babbling RX error\n");
32753275
}
32763276
if (events & IEVENT_EBERR) {
3277-
priv->extra_stats.eberr++;
3277+
atomic64_inc(&priv->extra_stats.eberr);
32783278
netif_dbg(priv, rx_err, dev, "bus error\n");
32793279
}
32803280
if (events & IEVENT_RXC)
32813281
netif_dbg(priv, rx_status, dev, "control frame\n");
32823282

32833283
if (events & IEVENT_BABT) {
3284-
priv->extra_stats.tx_babt++;
3284+
atomic64_inc(&priv->extra_stats.tx_babt);
32853285
netif_dbg(priv, tx_err, dev, "babbling TX error\n");
32863286
}
32873287
return IRQ_HANDLED;

‎drivers/net/ethernet/freescale/gianfar.h

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -627,34 +627,29 @@ struct rmon_mib
627627
};
628628

629629
struct gfar_extra_stats {
630-
u64 kernel_dropped;
631-
u64 rx_large;
632-
u64 rx_short;
633-
u64 rx_nonoctet;
634-
u64 rx_crcerr;
635-
u64 rx_overrun;
636-
u64 rx_bsy;
637-
u64 rx_babr;
638-
u64 rx_trunc;
639-
u64 eberr;
640-
u64 tx_babt;
641-
u64 tx_underrun;
642-
u64 rx_skbmissing;
643-
u64 tx_timeout;
630+
atomic64_t kernel_dropped;
631+
atomic64_t rx_large;
632+
atomic64_t rx_short;
633+
atomic64_t rx_nonoctet;
634+
atomic64_t rx_crcerr;
635+
atomic64_t rx_overrun;
636+
atomic64_t rx_bsy;
637+
atomic64_t rx_babr;
638+
atomic64_t rx_trunc;
639+
atomic64_t eberr;
640+
atomic64_t tx_babt;
641+
atomic64_t tx_underrun;
642+
atomic64_t rx_skbmissing;
643+
atomic64_t tx_timeout;
644644
};
645645

646646
#define GFAR_RMON_LEN ((sizeof(struct rmon_mib) - 16)/sizeof(u32))
647-
#define GFAR_EXTRA_STATS_LEN (sizeof(struct gfar_extra_stats)/sizeof(u64))
647+
#define GFAR_EXTRA_STATS_LEN \
648+
(sizeof(struct gfar_extra_stats)/sizeof(atomic64_t))
648649

649-
/* Number of stats in the stats structure (ignore car and cam regs)*/
650+
/* Number of stats exported via ethtool */
650651
#define GFAR_STATS_LEN (GFAR_RMON_LEN + GFAR_EXTRA_STATS_LEN)
651652

652-
struct gfar_stats {
653-
u64 extra[GFAR_EXTRA_STATS_LEN];
654-
u64 rmon[GFAR_RMON_LEN];
655-
};
656-
657-
658653
struct gfar {
659654
u32 tsec_id; /* 0x.000 - Controller ID register */
660655
u32 tsec_id2; /* 0x.004 - Controller ID2 register */

‎drivers/net/ethernet/freescale/gianfar_ethtool.c

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -149,20 +149,17 @@ static void gfar_fill_stats(struct net_device *dev, struct ethtool_stats *dummy,
149149
int i;
150150
struct gfar_private *priv = netdev_priv(dev);
151151
struct gfar __iomem *regs = priv->gfargrp[0].regs;
152-
u64 *extra = (u64 *) & priv->extra_stats;
152+
atomic64_t *extra = (atomic64_t *)&priv->extra_stats;
153+
154+
for (i = 0; i < GFAR_EXTRA_STATS_LEN; i++)
155+
buf[i] = atomic64_read(&extra[i]);
153156

154157
if (priv->device_flags & FSL_GIANFAR_DEV_HAS_RMON) {
155158
u32 __iomem *rmon = (u32 __iomem *) &regs->rmon;
156-
struct gfar_stats *stats = (struct gfar_stats *) buf;
157-
158-
for (i = 0; i < GFAR_RMON_LEN; i++)
159-
stats->rmon[i] = (u64) gfar_read(&rmon[i]);
160159

161-
for (i = 0; i < GFAR_EXTRA_STATS_LEN; i++)
162-
stats->extra[i] = extra[i];
163-
} else
164-
for (i = 0; i < GFAR_EXTRA_STATS_LEN; i++)
165-
buf[i] = extra[i];
160+
for (; i < GFAR_STATS_LEN; i++, rmon++)
161+
buf[i] = (u64) gfar_read(rmon);
162+
}
166163
}
167164

168165
static int gfar_sset_count(struct net_device *dev, int sset)

0 commit comments

Comments
 (0)