netdev - Re: [PATCH 5/8] smc91x: add SMC91X_IO_SHIFT* macros and make SMC_IO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-id: <alpine.LFD.1.10.0806200754030.23059@xanadu.home>
Date:	Fri, 20 Jun 2008 08:02:18 -0400 (EDT)
From:	Nicolas Pitre <nico@....org>
To:	Eric Miao <eric.y.miao@...il.com>
Cc:	linux-netdev <netdev@...r.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@...ts.arm.linux.org.uk>,
	Magnus Damm <magnus.damm@...il.com>
Subject: Re: [PATCH 5/8] smc91x: add SMC91X_IO_SHIFT* macros and make
 SMC_IO_SHIFT a variable

On Fri, 20 Jun 2008, Eric Miao wrote:

> Nicolas Pitre wrote:
> > On Thu, 19 Jun 2008, Eric Miao wrote:
> > 
> >> SMC_IO_SHIFT is currently hardcoded, which makes some platforms (e.g.
> >> Lubbock) unable to use the newly introduced platform data. This patch
> >> introduces SMC91X_IO_SHIFT* macros and make SMC_IO_SHIFT a variable.
> >>
> >> Signed-off-by: Eric Miao <eric.miao@...vell.com>
> > 
> > NAK.
> > 
> > The very point of those macros is actually to optimize the IO accesses 
> > as much as possible at compile time.  By introducing a variable element 
> > in the definition of those macros (for when the driver is configured 
> > with constant params for those macros of course) you add a significant 
> > overhead to every access to the hardware, including when transferring 
> > data in and out of the chip.
> > 
> 
> Contrary to expected, the result shows a slight decrease on zylonite,
> PXA310@...MHz, result shown as below:
> 
> (by a simple measurement with "proc/uptime" and tftp)
> 
> with SMC_IO_SHIFT being a variable
> 
> trial 1: 2062776 bytes in (179.77 - 177.72 = 2.05) seconds = 1,006,232 Bps
> trial 2: 2062776 bytes in (183.00 - 180.95 = 2.05) seconds = 1,006,232 Bps
> trial 3: 2062776 bytes in (261.48 - 259.42 = 2.06) seconds = 1,001,347 Bps
> 
> with SMC_IO_SHIFT being a constant
> 
> trial 1: 2062776 bytes in (41.07 - 39.04 = 2.03) seconds = 1,016,145 Bps
> trial 2: 2062776 bytes in (97.19 - 95.16 = 2.03) seconds = 1,016,145 Bps
> trial 3: 2062776 bytes in (159.81 - 157.78 = 2.03) seconds = 1,016,145 Bps
> 
> The statistics were stable during the test, so I generally think it's
> typical.
> 
> On lubbock, PXA255@...MHz, however, the result shows a slight increase:
> 
> with SMC_IO_SHIFT being a variable
> 
> trial 1: 2062776 bytes in (49.42 - 42.20 = 7.22) seconds = 285,703 Bps
> trial 2: 2062776 bytes in (60.27 - 53.07 = 7.20) seconds = 286,497 Bps
> trial 3: 2062776 bytes in (141.04 - 133.84 = 7.20) seconds = 286,497 Bps
> 
> with SMC_IO_SHIFT being a constant
> 
> trial 1: 2062776 bytes in (58.93 - 51.62 = 7.31) seconds = 282,185 Bps
> trial 2: 2062776 bytes in (69.26 - 61.95 = 7.31) seconds = 282,185 Bps
> trial 3: 2062776 bytes in (151.58 - 144.27 = 7.31) seconds = 282,185 Bps
> 
> So I'm thinking that the overhead may not be so significant as expected,
> 1. control register accesses are rare compared to data register
> 2. data register access is usually fixed at one address and enclosed in
>    a loop, which the compiler may well optimize

You must also look at the CPU usage too.  A faster CPU may well mitigate 
the latency issue and make no significant throughput difference, but at 
a higher CPU cost.  That means fewer cycles for doing anything else, 
like drawing those pictures on the screen as they are received over the 
net for example.

> > And this is very important to have the lowest overhead possible with 
> > this chip that can do 100mbps on platforms with a CPU clock almost as 
> > slow.
> > 
> 
> Indeed, the overhead will be magnified on a system with slow CPU clock,
> maybe I should spend some time to have a test also. However, arguably,
> the smc91x chips are usually used as a debug ethernet on most (if not
> all) platforms, I don't think a serious design will deploy such a chip
> for performance critical application, though.

That's not acceptable as an argument to introduce what actually is a 
regression, especially when it should be possible to avoid it.  And the 
fact is that there are already designs out there using this chip in 
production, serious or not.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html