> > > > >> >> +uint debug; > > > > >> >> +module_param(debug, uint, 0); > > > > > >>> +MODULE_PARM_DESC(debug, "Default debug msglevel"); > > > > >> > > > > >> >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]. > > > > > > > > > Can you give us an example where dynamic debug and tracing > > > > > infrastructures > > > > > are not enough? > > > > > > > > > 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. > > > > > > > > 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 > > > > > 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.
