| Submitter | Paul Gortmaker |
|---|---|
| Date | Feb. 13, 2013, 12:24 a.m. |
| Message ID | <1360715064-2689-1-git-send-email-paul.gortmaker@windriver.com> |
| Download | mbox |
| Permalink | /patch/220015/ |
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
Pull-request
git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux.git gfar-ethtool-atomicComments
From: Paul Gortmaker <paul.gortmaker@windriver.com> Date: Tue, 12 Feb 2013 19:24:22 -0500 > 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. ... > git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux.git gfar-ethtool-atomic Pulled, thanks Paul. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. Paul. -- The following changes since commit 0790bbb68f9d483348c1d65381f3dd92602bfd05: netpoll: cleanup sparse warnings (2013-02-11 19:19:58 -0500) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux.git gfar-ethtool-atomic for you to fetch changes up to 212079df6d77c0daada96b1d906f4b7749871411: gianfar: convert u64 status counters to atomic64_t (2013-02-12 19:08:27 -0500) ---------------------------------------------------------------- Paul Gortmaker (2): gianfar: remove largely unused gfar_stats struct gianfar: convert u64 status counters to atomic64_t drivers/net/ethernet/freescale/gianfar.c | 26 ++++++++-------- drivers/net/ethernet/freescale/gianfar.h | 39 +++++++++++------------- drivers/net/ethernet/freescale/gianfar_ethtool.c | 17 +++++------ 3 files changed, 37 insertions(+), 45 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html