linux-kernel - Re: proper struct device selection for dev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 3 May 2012 13:21:42 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>
cc:	Greg KH <gregkh@...uxfoundation.org>, Kay Sievers <kay@...y.org>,
	<linux-kernel@...r.kernel.org>, <linux-usb@...r.kernel.org>,
	Henrik Rydberg <rydberg@...omail.se>
Subject: Re: proper struct device selection for dev_printk()

On Thu, 3 May 2012, Dmitry Torokhov wrote:

> On Thu, May 03, 2012 at 09:09:37AM -0700, Greg KH wrote:
> > Hi Kay,
> > 
> > I've been working on removing the old err() and dbg() functions in usb.h
> > that have been there since the 2.2 kernel and replace them with calls to
> > dev_err() and dev_dbg(), as that's what we want to have, especially with
> > your dev_printk() reworks.
> > 
> > In some recent changes in the input drivers, Dmitry noted that I was
> > picking the "wrong" struct device to pass to these functions.  I was
> > using the "farthest down the tree" struct device that I could get to, in
> > the USB input driver's case, the struct device for the input device, a
> > "class" device.
> > 
> > But that seems to produce an output that is less than helpful.  Dmitry
> > used this as an example output to show this for a serio device:
> > 	dev_warn(&input_dev->dev, "warning using input device\n");
> > 	dev_warn(&serio->dev, "warning using parent serio device\n");
> > 
> > Produces:
> > 	[    1.903608] input input6: warning using input device
> > 	[    1.903612] psmouse serio1: warning using parent serio device
> > 
> > Here it seems that the "one up from the lowest struct device" works
> > best.
> > 
> > So I tried this out with a usb to serial device, and got the following
> > results.  With the code:
> > 	dev_err(&port->dev, "dev_err port->dev output\n");
> > 	dev_err(&serial->dev->dev, "dev_err serial->dev->dev output\n");
> > 	dev_err(&serial->interface->dev, "dev_err serial->interface->dev output\n");
> > 	dev_err(port->port.tty->dev, "dev_err port->port.tty->dev output\n");
> > 
> > I get:
> > 	[   68.519639] pl2303 ttyUSB0: dev_err port->dev output
> > 	[   68.519645] usb 2-1.2: dev_err serial->dev->dev output
> > 	[   68.519649] pl2303 2-1.2:1.0: dev_err serial->interface->dev output
> > 	[   68.519653] tty ttyUSB0: dev_err port->port.tty->dev output
> > 
> > All of these "describe" the device being operated on in one fashion or
> > the other, as they are struct devices that are easily accessable from
> > the driver.
> > 
> > My question is, what is the "best" thing to be doing here?
> > 
> > I still think the "lowest" struct device would be best (in this case,
> > the last line above from the port->port.tty->dev pointer), but what do
> > you think is best for userspace to have here?
> 
> My $.02:
> 
> In general, drivers should emit messages for the devices they bind to.
> This way driver like psmouse (which is serio subsystem driver) will emit
> messages using serio port it is bound (or attempting to bind to):
> 
> 	[    1.903612] psmouse serio1: warning using parent serio device
> 
> and drivers like wacom which bind to a USB interface will emit messages
> using USB intraface, like:
> 
> 	[    1.234567] wacom 2-1.2:1.0: some error happened
> 
> The benefit of using the device we are binding to is that it allows us
> to crearly identify the driver that emits the error and we are using the
> same device throughout (leaf device might not have been created yet and
> we already need to emit an error or a warning).

That's just what I was going to say.

So for example, in the USB-serial example, the

	usb 2-1.2: dev_err serial->dev->dev output

line isn't specific enough because it doesn't say which interface it 
applies to.  The

	tty ttyUSB0: dev_err port->port.tty->dev output

line is appropriate for a message emanating from the tty core, but not 
from a usb-serial driver.

As for the other two:

	pl2303 ttyUSB0: dev_err port->dev output
	pl2303 2-1.2:1.0: dev_err serial->interface->dev output

either one would be okay.  You could choose between them depending on 
what part of the driver is involved.  A part that handles the tty 
functions would use the first and a part that handles the USB 
communication would use the second.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/