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  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]
Date:	Tue, 30 Jan 2007 22:52:09 -0600
From:	olof@...om.net (Olof Johansson)
To:	Francois Romieu <romieu@...zoreil.com>
Cc:	jgarzik@...ox.com, netdev@...r.kernel.org
Subject: Re: [PATCH] PA Semi PWRficient Ethernet driver

On Tue, Jan 30, 2007 at 10:45:18PM +0100, Francois Romieu wrote:
> Olof Johansson <olof@...om.net> :
> > On Mon, Jan 29, 2007 at 11:35:06PM +0100, Francois Romieu wrote:
> [...]
> > > - The driver does not contain a single SMP locking instruction but
> > >   http://www.pasemi.com claims the platform to be multicore.
> > >   Is the driver really designed to be lockless ?
> > 
> > Unless I misunderstood something, NAPI drivers that don't set NETIF_F_LLTX
> > will have all locking taken care of by higher layers, no?
> 
> It is not necessarily _that_ simple (it would be cool though :o) ).
> 
> For instance, what does prevent pasemi_mac_clean_tx() to be issued
> from IRQ context (pasemi_mac_tx_intr) and from the xmit handler
> (pasemi_mac_start_tx) at the same time ?

You're right. Bummer. I'll add locking on the rings.

> [...]
> > > unsigned int is supposed to save some cycles on ppc.
> > 
> > Who told you that? That's not true.
> 
> Jon D Mason <jonmason@...ibm.com> on 25/08/2004 about ppc64 (not ppc, sorry).

Interesting, I hadn't thought about that before.

There's nothing architectural in PPC that makes signed math slower than
unsigned, but in the case of modulo operations (which we do alot on the
rings), unsigned is per definition more complex to do the operations on.

It's pretty much within the noise on the current implementation, but
still an interesting tidbit. Thanks.

> [...]
> > > > +#define DESCR(ring, i) ((ring)->desc[i % ((ring)->count)])
> > > > +#define BUFF(ring, i) ((ring)->buffers[i % ((ring)->count)])
> > > > +#define INFO(ring, i) ((ring)->desc_info[i % ((ring)->count)])
> > > 
> > > A bit ugly/obfuscating/name clash prone imvho.
> > > 
> > > Use local variables ?
> > 
> > I'm open for suggestions here, not sure how local variables will help though?
> 
> 	struct pas_dma_xct_descr *desc = ring->desc[i % ring->count];

That makes sense. Done.


-Olof
-
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

Powered by blists - more mailing lists