> > > Don't introduce a config option. If you do this by ifdef, use ifdef
> > > on the arches that you used as default. On those arches the memmove
> > > is definitely a win.
> > > You could even test it unconditionally on i386 or x86_64. A copy may
> > > be faster than keeping it in place, as you touch the data many
> > > times, but copy it only once.
> >
> > Do you thing that copy of whole frames (up to 1.5 kilobytes!) is
> > faster than unaligned accesses to header on x86/x86_64 (which is
> > definitly used by vast majority of this driver users)?
>
> You will in most cases touch more than just the header processing
> that frame. Eventually you will copy the frame anyway. It needs to
> be tested. The bottleneck is probably reading the buffer from a cold
> cache. The assumption cannot be made without testing, but is worth
> testing.
I can't do such a test myself - I'm doing my job remotely so I don't have
physical access to hardware, and people for whom I work are definitly not
interested in such things so they won't set up needed environment.
But, maybe it is not important?
Multiple-frames-per-urb happens only when network load is high (and who
uses USB network in high-load cases?), and even then - not too often.
So maybe performance impact is of minor interest?
Attached is a patch without Kconfig option.
Nikita
diff -ur net.orig/asix.c net/asix.c
--- net.orig/asix.c 2006-10-15 16:20:26.000000000 +0400
+++ net/asix.c 2006-11-24 11:45:00.133930152 +0300
@@ -690,6 +690,21 @@
return ret;
}
+
+/* AX88xxx hardware sometimes packs several frames into single urb,
+ * and second and subsequent frames are 2-byte-aligned.
+ *
+ * On RISC processors, frames must be aligned at (4n+2) adresses, to avoid
+ * unaligned accesses to IP header fields, so data move may be required.
+ *
+ * On PC processors, data move looks like unnecessary overhead.
+ */
+#if defined(CONFIG_X86_32) || defined(CONFIG_X86_64)
+#undef FIX_FRAME_ALIGNMENT
+#else
+#define FIX_FRAME_ALIGNMENT
+#endif
+
static int ax88772_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
{
u8 *head;
@@ -697,6 +712,7 @@
char *packet;
struct sk_buff *ax_skb;
u16 size;
+ int offset;
head = (u8 *) skb->data;
memcpy(&header, head, sizeof(header));
@@ -713,8 +729,20 @@
/* get the packet length */
size = (u16) (header & 0x0000ffff);
- if ((skb->len) - ((size + 1) & 0xfffe) == 0)
+ if ((skb->len) - ((size + 1) & 0xfffe) == 0) {
+#ifdef FIX_FRAME_ALIGNMENT
+ offset = ((unsigned long)packet + 2) & 3;
+#else
+ offset = 0;
+#endif
+ if (offset) {
+ skb->data -= offset;
+ skb->tail -= offset;
+ memmove(packet - offset, packet, size);
+ }
+
return 2;
+ }
if (size > ETH_FRAME_LEN) {
devdbg(dev,"invalid rx length %d", size);
return 0;
@@ -722,8 +750,17 @@
ax_skb = skb_clone(skb, GFP_ATOMIC);
if (ax_skb) {
ax_skb->len = size;
- ax_skb->data = packet;
- ax_skb->tail = packet + size;
+
+#ifdef FIX_FRAME_ALIGNMENT
+ offset = ((unsigned long)packet + 2) & 3;
+#else
+ offset = 0;
+#endif
+ ax_skb->data = packet - offset;
+ ax_skb->tail = packet - offset + size;
+ if (offset)
+ memmove(packet - offset, packet, size);
+
usbnet_skb_return(dev, ax_skb);
} else {
return 0;
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
