On 9/19/2016 2:15 AM, Leon Romanovsky wrote: > On Sun, Sep 18, 2016 at 04:35:37PM -0400, Doug Ledford wrote: > Doug, > The function pointers will cause us to create two similar functions > (debug/regular) for every important function. It will leave to code > duplication and/or introducing of new code to fill additional fields > in returns to debug functions, so they will be able to print it. That's situationally specific. The original patch you posted that I said no to is an example of where this wouldn't have been true. Basically it all boils down to whether or not your debug code can be a wrapper of your existing function or if it needs to be embedded in the middle of it. If the latter, then you are more likely to have code duplication as you point out. But, then refactoring code can help with that problem too. > I wanted to propose to use tracepoints to provide debug information in > data paths. The tracepoints infrastructure uses hooks in the code with > minimal impact on performance. Yes, but one of the things you said before was "zero impact on normal operations", but with tracepoints you just said "minimal impact". So it sounds like tracepoints don't meet your own initial requirements. I'm not against them, but utilizing the entry points allows for achievment of the "zero impact" requirement. And as Denny pointed out, some things can't be done with tracepoints. So, the final solution might be a mix of different things including different entry points, tracepoints, and some core infrastructure. > >> Then we can drop the CONFIG_*_DEBUG options out of all of the >> drivers. That would be my preference. I don't want any compile time >> options because you never will be running a debug kernel when you need >> it. Per device, run time selectable so that you get 0 overhead in the >> normal case is what I would like to see. > > Great, > We are aligned on this point, so do I. I see no reason in special compiled > kernel or global module parameter to print debug prints for whole driver. -- Doug Ledford GPG Key ID: 0E572FDD