Re: [RFC 02/11] Add RoCE driver framework

quoted

quoted

quoted

quoted

quoted

quoted

quoted

+uint debug;
+module_param(debug, uint, 0);
+MODULE_PARM_DESC(debug, "Default debug msglevel");

quoted

Why are you adding this as a module parameter?

 I believe this is mostly to follow same line as qede which also defines
'debug' module parameter for allowing easy user control of debug
prints [& specifically for probe prints, which can't be controlled
otherwise].

quoted

Can you give us an example where dynamic debug and tracing infrastructures
are not enough?

quoted

AFAIK, most of these debug module parameters are legacy copy/paste
code which is useless in real life scenarios.

Define 'enough'; Using dynamic debug you can provide all the necessary
information and at an even better granularity that's achieved by suggested

 > > infrastructure,  but is harder for an end-user to use. Same goes for tracing.

quoted

quoted

The 'debug' option provides an easy grouping for prints related to a specific
area in the driver.

It is hard to agree with you that user which knows how-to load modules
with parameters won't success to enable debug prints.

I think you're giving too much credit to the end-user. :-D

quoted

In addition, global increase in debug level for whole driver will create
printk storm in dmesg and give nothing to debuggability.

So basically, what you're claiming is that ethtool 'msglvl' setting for devices
is completely obselete. While this *might* be true, we use it extensively
in our qede and qed drivers; The debug module parameter merely provides
a manner of setting the debug value prior to initial probe for all interfaces.
qedr follows the same practice.

Thanks for this excellent example. Ethtool 'msglvl' adds this
dynamically, while your DEBUG argument works for loading module
only.

If you want dynamic prints, you have two options:
1. Add support of ethtool to whole RDMA stack.
2. Use dynamic tracing infrastructure.

Which option do you prefer?

Option 3 - continuing this discussion. :-)

Perhaps I misread your intentions - I thought that by dynamic debug
you meant that all debug in RDMA should be pr_debug() based, and
therefore my objection regarding the ease with which users can
configure it. 
If all you meant was 'dynamically set' as opposed to 'statically set'
then I agree that having that sort of configurability is preferable
[Even though end-user would still probably prefer a module
parameter for reproductions; As the name implies, 'debug' isn't
meant to be used in other situations].

The other thing to consider are the probe-time prints.
Problem is, you wouldn't have a control node between probe 
and until after the probing would be over, so it would be a bit
hard to configure that.
You can always think of some generic method of fixing that as well
[sysfs node for the entire system for probe-time prints, perhaps?]

Do notice you would be harming user-experience of reproductions
though - as it would have to follow different mechanisms to open
debug prints of various qed* components.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html