Re: [PATCH 01/20] dlb2: add skeleton for DLB 2.0 driver

[Greg KH]

On Mon, Jul 20, 2020 at 07:02:05PM +0000, Eads, Gage wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Sent: Saturday, July 18, 2020 1:47 AM
> > To: Eads, Gage <gage.eads@xxxxxxxxx>
> > Cc: linux-kernel@xxxxxxxxxxxxxxx; arnd@xxxxxxxx; Karlsson, Magnus
> > <magnus.karlsson@xxxxxxxxx>; Topel, Bjorn <bjorn.topel@xxxxxxxxx>
> > Subject: Re: [PATCH 01/20] dlb2: add skeleton for DLB 2.0 driver
> > 
> > On Fri, Jul 17, 2020 at 06:18:46PM +0000, Eads, Gage wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > > Sent: Sunday, July 12, 2020 10:58 AM
> > > > To: Eads, Gage <gage.eads@xxxxxxxxx>
> > > > Cc: linux-kernel@xxxxxxxxxxxxxxx; arnd@xxxxxxxx; Karlsson, Magnus
> > > > <magnus.karlsson@xxxxxxxxx>; Topel, Bjorn <bjorn.topel@xxxxxxxxx>
> > > > Subject: Re: [PATCH 01/20] dlb2: add skeleton for DLB 2.0 driver
> > > >
> > > > On Sun, Jul 12, 2020 at 08:43:12AM -0500, Gage Eads wrote:
> > > > > +static int dlb2_probe(struct pci_dev *pdev,
> > > > > +		      const struct pci_device_id *pdev_id) {
> > > > > +	struct dlb2_dev *dlb2_dev;
> > > > > +	int ret;
> > > > > +
> > > > > +	dev_dbg(&pdev->dev, "probe\n");
> > > >
> > > > ftrace is your friend.  Remove all of your debugging code now, you don't
> > need
> > > > it anymore, especially for stuff like this where you didn't even need it in
> > the
> > > > first place :(
> > >
> > > I'll remove this and other similar dev_dbg() calls. This was an oversight on
> > my part.
> > >
> > > I have other instances that a kprobe can't easily replace, such as printing
> > structure contents, that are useful for tracing the usage of the driver. It looks
> > like other misc drivers use dev_dbg() similarly -- do you consider this an
> > acceptable use of a debug print?
> > 
> > Why can't a kernel tracepoint print a structure?
> 
> I meant the command-line installed kprobes[1], but instrumenting the driver is
> certainly an option. We don't require the much lower overhead of a tracepoint,
> so I didn't choose it. This driver handles the (performance-insensitive)
> device configuration, while the fast-path operations take place in user-space.
> 
> Another reason is the "hardware access library" files use only non-GPL external
> symbols, and some tracepoint functions are exported GPL. Though it's probably
> feasible to lift that tracing code up into a (GPLv2-only) caller function.

Stop going through crazy gyrations for something that your own legal
team has told you not to do anymore in the first place.

No "hardware access library" files please, that's not how Linux drivers
are written.

you all know better...

> But if tracepoints are the preferred method and/or you think the driver would
> benefit, I'll make the change.

I don't think you need any of that stuff, now that the code works
properly, right?

greg k-h