Re: [PATCH] SMSC LAN9500 USB2.0 10/100 ethernet adapter driver

[David Brownell <david-b@pacbell.net>]

On Monday 08 September 2008, Steve Glendinning wrote:
> +
> +#define SMSC_WARNING(msg, args...) \
> +       printk(KERN_WARNING "%s: WARNING: " msg "\n", __func__, ## args)
> +
> +#ifdef USE_DEBUG
> +#define SMSC_TRACE(debug_bit, msg, args...) \
> +       if (debug_mode & debug_bit) { \
> +               printk(KERN_WARNING "%s: " msg "\n", __func__, ## args); \
> +       }
> +#else
> +#define SMSC_TRACE(dbgBit, msg, args...)   ({ do {} while (0); 0; })
> +#endif

Standard feedback for such stuff:

 - avoid printk() for diagnostics, use dev_*() driver model calls
 - ... or in this case, pr_warning() if you really must
 - provide better messages and avoid passing __func__ 
 - use inline functions instead of CPP macros
 - make dead code elimination remove debug code, not #ifdefs

And specific to network devices:

 - use the ethtool message level flags not private debug_mode


> +int turbo_mode = true;
> +module_param(turbo_mode, bool, 0);
> +MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");

I'm curious ... does this actually improve things on Linux?

My understanding is that on MS-Windows the costs of sending
a transaction through the USB stack are so high that their
APIs encourage such batching, and this has in some cases
extended down to wire protocols and (as seems true here)
even to hardware.

The approach Linux prefers is just to ensure that a queue
of transactions works efficiently ... so that for example
each Ethernet frame maps directly to one URB, and there's
no time wasted batching/unbatching.  (Neither in Linux,
nor in the peripheral.)

Last time I had any measurements on such issues, the
Linux stack outperformed MS-Windows in terms of per-packet
latency and also throughput, indicating that batching was
not necessary for performance on most network loads.  But
maybe that's not the case with your particular hardware,
or in certain significant/common use cases?

- Dave