Reducing the verbosity of inserting a USB storage device

Reducing the verbosity of inserting a USB storage device

[Matthew Wilcox]

The current kernel is very verbose when you insert a new USB storage
device:

usb 1-2: new high speed USB device using ehci_hcd and address 9
usb 1-2: configuration #1 chosen from 1 choice
scsi4 : SCSI emulation for USB Mass Storage devices
usb-storage: device found at 9
usb-storage: waiting for device to settle before scanning
usb-storage: device scan complete
scsi 4:0:0:0: Direct-Access     GENERIC  USB DISK DEVICE  0100 PQ: 0 ANSI: 0 CCS
sd 4:0:0:0: Attached scsi generic sg2 type 0
sd 4:0:0:0: [sdc] 15728128 512-byte hardware sectors: (8.05 GB/7.49 GiB)
sd 4:0:0:0: [sdc] Write Protect is off
sd 4:0:0:0: [sdc] Mode Sense: 00 12 00 00
sd 4:0:0:0: [sdc] Assuming drive cache: write through
sd 4:0:0:0: [sdc] Assuming drive cache: write through
 sdc:
sd 4:0:0:0: [sdc] Attached SCSI removable disk

I think we can print a bit less information, and format it more densely.
With the following three patches, my kernel prints:

usb 1-4: new high speed USB device using ehci_hcd and address 5
usb 1-4: configuration #1 chosen from 1 choice
scsi 6:0:0:0: Direct-Access              Flash Disk       5.00 PQ: 0 ANSI: 2
sd 6:0:0:0: Attached scsi generic sg1 type 0
sd 6:0:0:0: [sdb] Assuming drive cache: write through
sd 6:0:0:0: [sdb] 2009088 512-byte logical blocks: (1.02 GB/981 MiB)
sd 6:0:0:0: [sdb] Removable disk, Write Protect is off
 sdb: unknown partition table

which doesn't take up nearly as much of my syslog ;-)

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] Allow SCSI hosts to be less verbose

[Matthew Wilcox]

From: Matthew Wilcox <willy-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Introduce a 'quiet' flag in the scsi_host_template which drivers can
set to prevent (in the first instance) printing their name when they
register with the SCSI core.  Set the quiet flag in the USB Storage
driver.

Signed-off-by: Matthew Wilcox <willy-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/scsi/hosts.c           |    3 ++-
 drivers/usb/storage/scsiglue.c |    3 +++
 include/scsi/scsi_host.h       |    3 +++
 3 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 5fd2da4..7741f1d 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -192,7 +192,8 @@ int scsi_add_host(struct Scsi_Host *shost, struct device *dev)
 	struct scsi_host_template *sht = shost->hostt;
 	int error = -EINVAL;
 
-	printk(KERN_INFO "scsi%d : %s\n", shost->host_no,
+	if (!sht->quiet)
+		printk(KERN_INFO "scsi%d : %s\n", shost->host_no,
 			sht->info ? sht->info(shost) : sht->name);
 
 	if (!shost->can_queue) {
diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index cfa26d5..781fb6d 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -555,6 +555,9 @@ struct scsi_host_template usb_stor_host_template = {
 	/* we do our own delay after a device or bus reset */
 	.skip_settle_delay =		1,
 
+	/* We don't need the scsi midlayer to print our name */
+	.quiet =			1,
+
 	/* sysfs device attributes */
 	.sdev_attrs =			sysfs_device_attr_list,
 
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index b62a097..baa527c 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -446,6 +446,9 @@ struct scsi_host_template {
 	 */
 	unsigned ordered_tag:1;
 
+	/* Be less verbose */
+	unsigned quiet:1;
+
 	/*
 	 * Countdown for host blocking with no commands outstanding.
 	 */
-- 
1.6.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] Allow SCSI hosts to be less verbose

[Christoph Hellwig]

On Sun, Sep 20, 2009 at 04:56:39PM -0600, Matthew Wilcox wrote:
> From: Matthew Wilcox <willy@linux.intel.com>
> 
> Introduce a 'quiet' flag in the scsi_host_template which drivers can
> set to prevent (in the first instance) printing their name when they
> register with the SCSI core.  Set the quiet flag in the USB Storage
> driver.

Seems weird to make this conditional.  I'd vote for removing it
completely.

Re: Reducing the verbosity of inserting a USB storage device

[Alan Stern]

On Sun, 20 Sep 2009, Matthew Wilcox wrote:

> The current kernel is very verbose when you insert a new USB storage
> device:
> 
> usb 1-2: new high speed USB device using ehci_hcd and address 9
> usb 1-2: configuration #1 chosen from 1 choice
> scsi4 : SCSI emulation for USB Mass Storage devices
> usb-storage: device found at 9
> usb-storage: waiting for device to settle before scanning
> usb-storage: device scan complete
> scsi 4:0:0:0: Direct-Access     GENERIC  USB DISK DEVICE  0100 PQ: 0 ANSI: 0 CCS
> sd 4:0:0:0: Attached scsi generic sg2 type 0
> sd 4:0:0:0: [sdc] 15728128 512-byte hardware sectors: (8.05 GB/7.49 GiB)
> sd 4:0:0:0: [sdc] Write Protect is off
> sd 4:0:0:0: [sdc] Mode Sense: 00 12 00 00
> sd 4:0:0:0: [sdc] Assuming drive cache: write through
> sd 4:0:0:0: [sdc] Assuming drive cache: write through
>  sdc:
> sd 4:0:0:0: [sdc] Attached SCSI removable disk
> 
> I think we can print a bit less information, and format it more densely.
> With the following three patches, my kernel prints:
> 
> usb 1-4: new high speed USB device using ehci_hcd and address 5
> usb 1-4: configuration #1 chosen from 1 choice
> scsi 6:0:0:0: Direct-Access              Flash Disk       5.00 PQ: 0 ANSI: 2
> sd 6:0:0:0: Attached scsi generic sg1 type 0
> sd 6:0:0:0: [sdb] Assuming drive cache: write through
> sd 6:0:0:0: [sdb] 2009088 512-byte logical blocks: (1.02 GB/981 MiB)
> sd 6:0:0:0: [sdb] Removable disk, Write Protect is off
>  sdb: unknown partition table
> 
> which doesn't take up nearly as much of my syslog ;-)

I approve of these changes in general, but there should be one more
line of information.  In the "after" example, there's nothing to relate
the USB device mentioned in the first two lines with the SCSI device
mentioned in the remaining six.  I'd like to see one line in between
from usb-storage, to indicate which USB interface is being registered
as which SCSI host.

We don't print anything quite like that right now, so this would have 
to be something new.  The closest we come is the "scsi4 : SCSI 
emulation for USB Mass Storage devices" line, and it doesn't mention 
the USB interface.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Reducing the verbosity of inserting a USB storage device

[Matthew Wilcox]

On Sun, Sep 20, 2009 at 09:09:45PM -0400, Alan Stern wrote:
> I approve of these changes in general, but there should be one more
> line of information.  In the "after" example, there's nothing to relate
> the USB device mentioned in the first two lines with the SCSI device
> mentioned in the remaining six.  I'd like to see one line in between
> from usb-storage, to indicate which USB interface is being registered
> as which SCSI host.
> 
> We don't print anything quite like that right now, so this would have 
> to be something new.  The closest we come is the "scsi4 : SCSI 
> emulation for USB Mass Storage devices" line, and it doesn't mention 
> the USB interface.

I think you're right.  Here's my current dmesg:

usb 1-4: new high speed USB device using ehci_hcd and address 3
scsi4 : usb-storage 1-4:1.0
scsi 4:0:0:0: Direct-Access              Flash Disk       5.00 PQ: 0 ANSI: 2
sd 4:0:0:0: Attached scsi generic sg1 type 0
sd 4:0:0:0: [sdb] Assuming drive cache: write through
sd 4:0:0:0: [sdb] 2009088 512-byte logical blocks: (1.02 GB/981 MiB)
sd 4:0:0:0: [sdb] Removable disk, Write Protect is off
 sdb: unknown partition table

That actually makes the patchset slightly simpler; before:

 drivers/scsi/hosts.c           |    3 +
 drivers/scsi/sd.c              |   78 +++++++++++++++++++++++------------------
 drivers/usb/storage/scsiglue.c |    3 +
 drivers/usb/storage/usb.c      |   13 ++++--
 include/scsi/scsi_host.h       |    3 +
 5 files changed, 60 insertions(+), 40 deletions(-)

after:

 drivers/scsi/sd.c              |   78 +++++++++++++++++++++++------------------
 drivers/usb/core/generic.c     |    2 -
 drivers/usb/storage/scsiglue.c |    3 +
 drivers/usb/storage/usb.c      |   15 +++++--
 drivers/usb/storage/usb.h      |    1 
 5 files changed, 58 insertions(+), 41 deletions(-)

Patches to follow.

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] Convert a dev_info to a dev_dbg

[Matthew Wilcox]

From: Matthew Wilcox <willy@linux.intel.com>

Knowing which configuration was chosen is a debugging aid more than it
is informational.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/usb/core/generic.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
index 30ecac3..60368ae 100644
--- a/drivers/usb/core/generic.c
+++ b/drivers/usb/core/generic.c
@@ -139,7 +139,7 @@ int usb_choose_configuration(struct usb_device *udev)
 
 	if (best) {
 		i = best->desc.bConfigurationValue;
-		dev_info(&udev->dev,
+		dev_dbg(&udev->dev,
 			"configuration #%d chosen from %d choice%s\n",
 			i, num_configs, plural(num_configs));
 	} else {
-- 
1.6.3.3

[PATCH 2/4] usb-storage: Associate the name of the interface with the scsi host

[Matthew Wilcox]

From: Matthew Wilcox <willy-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Instead of reporting "SCSI emulation for USB Mass Storage devices",
report "usb-storage 1-4:1.0".

Signed-off-by: Matthew Wilcox <willy-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/usb/storage/scsiglue.c |    3 ++-
 drivers/usb/storage/usb.c      |    2 ++
 drivers/usb/storage/usb.h      |    1 +
 3 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index cfa26d5..e5e6df3 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -73,7 +73,8 @@
 
 static const char* host_info(struct Scsi_Host *host)
 {
-	return "SCSI emulation for USB Mass Storage devices";
+	struct us_data *us = host_to_us(host);
+	return us->scsi_name;
 }
 
 static int slave_alloc (struct scsi_device *sdev)
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 8060b85..39d3e54 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -929,6 +929,8 @@ int usb_stor_probe2(struct us_data *us)
 	result = usb_stor_acquire_resources(us);
 	if (result)
 		goto BadDevice;
+	snprintf(us->scsi_name, sizeof(us->scsi_name), "usb-storage %s",
+					dev_name(&us->pusb_intf->dev));
 	result = scsi_add_host(us_to_host(us), &us->pusb_intf->dev);
 	if (result) {
 		printk(KERN_WARNING USB_STORAGE
diff --git a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h
index 2609efb..6971713 100644
--- a/drivers/usb/storage/usb.h
+++ b/drivers/usb/storage/usb.h
@@ -132,6 +132,7 @@ struct us_data {
 	/* SCSI interfaces */
 	struct scsi_cmnd	*srb;		 /* current srb		*/
 	unsigned int		tag;		 /* current dCBWTag	*/
+	char			scsi_name[32];	 /* scsi_host name	*/
 
 	/* control and bulk communications data */
 	struct urb		*current_urb;	 /* USB requests	 */
-- 
1.6.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] usb-storage: Associate the name of the interface with the scsi host

[Alan Stern]

On Thu, 24 Sep 2009, Matthew Wilcox wrote:

> From: Matthew Wilcox <willy@linux.intel.com>
> 
> Instead of reporting "SCSI emulation for USB Mass Storage devices",
> report "usb-storage 1-4:1.0".
> 
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>

Acked-by: Alan Stern <stern@rowland.harvard.edu>

> ---
>  drivers/usb/storage/scsiglue.c |    3 ++-
>  drivers/usb/storage/usb.c      |    2 ++
>  drivers/usb/storage/usb.h      |    1 +
>  3 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
> index cfa26d5..e5e6df3 100644
> --- a/drivers/usb/storage/scsiglue.c
> +++ b/drivers/usb/storage/scsiglue.c
> @@ -73,7 +73,8 @@
>  
>  static const char* host_info(struct Scsi_Host *host)
>  {
> -	return "SCSI emulation for USB Mass Storage devices";
> +	struct us_data *us = host_to_us(host);
> +	return us->scsi_name;
>  }
>  
>  static int slave_alloc (struct scsi_device *sdev)
> diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
> index 8060b85..39d3e54 100644
> --- a/drivers/usb/storage/usb.c
> +++ b/drivers/usb/storage/usb.c
> @@ -929,6 +929,8 @@ int usb_stor_probe2(struct us_data *us)
>  	result = usb_stor_acquire_resources(us);
>  	if (result)
>  		goto BadDevice;
> +	snprintf(us->scsi_name, sizeof(us->scsi_name), "usb-storage %s",
> +					dev_name(&us->pusb_intf->dev));
>  	result = scsi_add_host(us_to_host(us), &us->pusb_intf->dev);
>  	if (result) {
>  		printk(KERN_WARNING USB_STORAGE
> diff --git a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h
> index 2609efb..6971713 100644
> --- a/drivers/usb/storage/usb.h
> +++ b/drivers/usb/storage/usb.h
> @@ -132,6 +132,7 @@ struct us_data {
>  	/* SCSI interfaces */
>  	struct scsi_cmnd	*srb;		 /* current srb		*/
>  	unsigned int		tag;		 /* current dCBWTag	*/
> +	char			scsi_name[32];	 /* scsi_host name	*/
>  
>  	/* control and bulk communications data */
>  	struct urb		*current_urb;	 /* USB requests	 */

[PATCH 3/4] USB Storage: Make driver less chatty when it finds a new device

[Matthew Wilcox]

From: Matthew Wilcox <willy@linux.intel.com>

Use dev_dbg() instead of an unconditional printk(KERN_DEBUG).  This has
two benefits; one is that it identifies the USB device which the messages
related to, and the other is that the messages won't be produced unless
debug is turned on.

Enable the debug messages when CONFIG_USB_STORAGE_DEBUG is set.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
---
 drivers/usb/storage/usb.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 39d3e54..92ca3c5 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -45,6 +45,10 @@
  * 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
+#ifdef CONFIG_USB_STORAGE_DEBUG
+#define DEBUG
+#endif
+
 #include <linux/sched.h>
 #include <linux/errno.h>
 #include <linux/freezer.h>
@@ -808,14 +812,13 @@ static int usb_stor_scan_thread(void * __us)
 {
 	struct us_data *us = (struct us_data *)__us;
 
-	printk(KERN_DEBUG
-		"usb-storage: device found at %d\n", us->pusb_dev->devnum);
+	dev_dbg(&us->pusb_intf->dev, "device found\n");
 
 	set_freezable();
 	/* Wait for the timeout to expire or for a disconnect */
 	if (delay_use > 0) {
-		printk(KERN_DEBUG "usb-storage: waiting for device "
-				"to settle before scanning\n");
+		dev_dbg(&us->pusb_intf->dev, "waiting for device to settle "
+				"before scanning\n");
 		wait_event_freezable_timeout(us->delay_wait,
 				test_bit(US_FLIDX_DONT_SCAN, &us->dflags),
 				delay_use * HZ);
@@ -832,7 +835,7 @@ static int usb_stor_scan_thread(void * __us)
 			mutex_unlock(&us->dev_mutex);
 		}
 		scsi_scan_host(us_to_host(us));
-		printk(KERN_DEBUG "usb-storage: device scan complete\n");
+		dev_dbg(&us->pusb_intf->dev, "scan complete\n");
 
 		/* Should we unbind if no devices were detected? */
 	}
-- 
1.6.3.3

[PATCH 4/4] sd.c: Make initialisation less verbose

[Matthew Wilcox]

From: Matthew Wilcox <willy@linux.intel.com>

Introduce a new function, sd_print_info() to print the information we
normally want to display to the user on revalidation.

Move the capacity display from sd_read_capacity() to sd_print_info().
Move the write protect display from sd_read_write_protect_flag() to
sd_print_info().
Delete the printk reporting Mode Sense as it is not generally useful.

Move the display of 'Removable' to the same line as reporting the Write
Protect status.

Only print the line about write cache once.
---
 drivers/scsi/sd.c |   78 ++++++++++++++++++++++++++++++-----------------------
 1 files changed, 44 insertions(+), 34 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index a89c421..beec7f3 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1441,7 +1441,6 @@ sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer)
 {
 	int sector_size;
 	struct scsi_device *sdp = sdkp->device;
-	sector_t old_capacity = sdkp->capacity;
 
 	if (sd_try_rc16_first(sdp)) {
 		sector_size = read_capacity_16(sdkp, sdp, buffer);
@@ -1524,28 +1523,6 @@ got_data:
 	}
 	blk_queue_logical_block_size(sdp->request_queue, sector_size);
 
-	{
-		char cap_str_2[10], cap_str_10[10];
-		u64 sz = (u64)sdkp->capacity << ilog2(sector_size);
-
-		string_get_size(sz, STRING_UNITS_2, cap_str_2,
-				sizeof(cap_str_2));
-		string_get_size(sz, STRING_UNITS_10, cap_str_10,
-				sizeof(cap_str_10));
-
-		if (sdkp->first_scan || old_capacity != sdkp->capacity) {
-			sd_printk(KERN_NOTICE, sdkp,
-				  "%llu %d-byte logical blocks: (%s/%s)\n",
-				  (unsigned long long)sdkp->capacity,
-				  sector_size, cap_str_10, cap_str_2);
-
-			if (sdkp->hw_sector_size != sector_size)
-				sd_printk(KERN_NOTICE, sdkp,
-					  "%u-byte physical blocks\n",
-					  sdkp->hw_sector_size);
-		}
-	}
-
 	/* Rescale capacity to 512-byte units */
 	if (sector_size == 4096)
 		sdkp->capacity <<= 3;
@@ -1581,7 +1558,6 @@ sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer)
 	int res;
 	struct scsi_device *sdp = sdkp->device;
 	struct scsi_mode_data data;
-	int old_wp = sdkp->write_prot;
 
 	set_disk_ro(sdkp->disk, 0);
 	if (sdp->skip_ms_page_3f) {
@@ -1622,13 +1598,6 @@ sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer)
 	} else {
 		sdkp->write_prot = ((data.device_specific & 0x80) != 0);
 		set_disk_ro(sdkp->disk, sdkp->write_prot);
-		if (sdkp->first_scan || old_wp != sdkp->write_prot) {
-			sd_printk(KERN_NOTICE, sdkp, "Write Protect is %s\n",
-				  sdkp->write_prot ? "on" : "off");
-			sd_printk(KERN_DEBUG, sdkp,
-				  "Mode Sense: %02x %02x %02x %02x\n",
-				  buffer[0], buffer[1], buffer[2], buffer[3]);
-		}
 	}
 }
 
@@ -1742,7 +1711,9 @@ bad_sense:
 		sd_printk(KERN_ERR, sdkp, "Asking for cache data failed\n");
 
 defaults:
-	sd_printk(KERN_ERR, sdkp, "Assuming drive cache: write through\n");
+	if (sdkp->first_scan)
+		sd_printk(KERN_ERR, sdkp,
+				"Assuming drive cache: write through\n");
 	sdkp->WCE = 0;
 	sdkp->RCD = 0;
 	sdkp->DPOFUA = 0;
@@ -1852,6 +1823,43 @@ static int sd_try_extended_inquiry(struct scsi_device *sdp)
 	return 0;
 }
 
+static
+void sd_print_info(struct scsi_disk *sdkp, sector_t old_capacity, int old_wp)
+{
+	unsigned sector_size = sdkp->device->sector_size;
+	char cap_str_2[10], cap_str_10[10];
+	u64 sz = (u64)sdkp->capacity * 512;
+
+	string_get_size(sz, STRING_UNITS_2, cap_str_2,
+			sizeof(cap_str_2));
+	string_get_size(sz, STRING_UNITS_10, cap_str_10,
+			sizeof(cap_str_10));
+
+	if (sdkp->first_scan || old_capacity != sdkp->capacity) {
+		u64 new_blocks = sdkp->capacity;
+		if (sector_size < 512)
+			new_blocks *= 512 / sector_size;
+		else if (sector_size > 512)
+			new_blocks = sector_div(sdkp->capacity,
+							512 / sector_size);
+
+		sd_printk(KERN_NOTICE, sdkp,
+			  "%llu %d-byte logical blocks: (%s/%s)\n",
+			  new_blocks, sector_size, cap_str_10, cap_str_2);
+
+		if (sdkp->hw_sector_size != sector_size)
+			sd_printk(KERN_NOTICE, sdkp,
+				  "%u-byte physical blocks\n",
+				  sdkp->hw_sector_size);
+	}
+
+	if (sdkp->first_scan || old_wp != sdkp->write_prot) {
+		sd_printk(KERN_NOTICE, sdkp, "%s disk, Write Protect is %s\n",
+			  sdkp->device->removable ? "Removable" : "Fixed",
+			  sdkp->write_prot ? "on" : "off");
+	}
+}
+
 /**
  *	sd_revalidate_disk - called the first time a new disk is seen,
  *	performs disk spin up, read_capacity, etc.
@@ -1861,6 +1869,8 @@ static int sd_revalidate_disk(struct gendisk *disk)
 {
 	struct scsi_disk *sdkp = scsi_disk(disk);
 	struct scsi_device *sdp = sdkp->device;
+	sector_t old_capacity = sdkp->capacity;
+	int old_wp = sdkp->write_prot;
 	unsigned char *buffer;
 	unsigned ordered;
 
@@ -1900,6 +1910,8 @@ static int sd_revalidate_disk(struct gendisk *disk)
 		sd_read_app_tag_own(sdkp, buffer);
 	}
 
+	sd_print_info(sdkp, old_capacity, old_wp);
+
 	sdkp->first_scan = 0;
 
 	/*
@@ -2019,8 +2031,6 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
 
 	sd_revalidate_disk(gd);
 
-	sd_printk(KERN_NOTICE, sdkp, "Attached SCSI %sdisk\n",
-		  sdp->removable ? "removable " : "");
 	put_device(&sdkp->dev);
 }
 
-- 
1.6.3.3

Re: [PATCH 4/4] sd.c: Make initialisation less verbose

[Greg KH]

On Thu, Sep 24, 2009 at 04:20:56PM -0600, Matthew Wilcox wrote:
> From: Matthew Wilcox <willy-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> 
> Introduce a new function, sd_print_info() to print the information we
> normally want to display to the user on revalidation.
> 
> Move the capacity display from sd_read_capacity() to sd_print_info().
> Move the write protect display from sd_read_write_protect_flag() to
> sd_print_info().
> Delete the printk reporting Mode Sense as it is not generally useful.
> 
> Move the display of 'Removable' to the same line as reporting the Write
> Protect status.
> 
> Only print the line about write cache once.

No signed off by line?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Reducing the verbosity of inserting a USB storage device

[Matthew Dharm]

[-- Attachment #1: Type: text/plain, Size: 668 bytes --]

On Sun, Sep 20, 2009 at 04:53:49PM -0600, Matthew Wilcox wrote:
> 
> The current kernel is very verbose when you insert a new USB storage
> device:
> 
> [...]
> 
> I think we can print a bit less information, and format it more densely.
> With the following three patches, my kernel prints:

Seems pretty reasonable, but perhaps we should put in a module parameter to
turn off the "quiet" flag?  The old data may be useful for debugging
purposes.

Matt

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

It was a new hope.
					-- Dust Puppy
User Friendly, 12/25/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

[PATCH 2/3] USB Storage: Make driver less chatty when it finds a new device

[Matthew Wilcox]

From: Matthew Wilcox <willy@linux.intel.com>

Use dev_dbg() instead of an unconditional printk(KERN_DEBUG).  This has
two benefits; one is that it identifies the USB device which the messages
related to, and the other is that the messages won't be produced unless
debug is turned on.

Enable the debug messages when CONFIG_USB_STORAGE_DEBUG is set.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/usb/storage/usb.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 8060b85..40c21a9 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -45,6 +45,10 @@
  * 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
+#ifdef CONFIG_USB_STORAGE_DEBUG
+#define DEBUG
+#endif
+
 #include <linux/sched.h>
 #include <linux/errno.h>
 #include <linux/freezer.h>
@@ -808,14 +812,13 @@ static int usb_stor_scan_thread(void * __us)
 {
 	struct us_data *us = (struct us_data *)__us;
 
-	printk(KERN_DEBUG
-		"usb-storage: device found at %d\n", us->pusb_dev->devnum);
+	dev_dbg(&us->pusb_dev->dev, "usb-storage device found\n");
 
 	set_freezable();
 	/* Wait for the timeout to expire or for a disconnect */
 	if (delay_use > 0) {
-		printk(KERN_DEBUG "usb-storage: waiting for device "
-				"to settle before scanning\n");
+		dev_dbg(&us->pusb_dev->dev, "usb-storage: waiting for device "
+			"to settle before scanning\n");
 		wait_event_freezable_timeout(us->delay_wait,
 				test_bit(US_FLIDX_DONT_SCAN, &us->dflags),
 				delay_use * HZ);
@@ -832,7 +835,7 @@ static int usb_stor_scan_thread(void * __us)
 			mutex_unlock(&us->dev_mutex);
 		}
 		scsi_scan_host(us_to_host(us));
-		printk(KERN_DEBUG "usb-storage: device scan complete\n");
+		dev_dbg(&us->pusb_dev->dev, "usb-storage scan complete\n");
 
 		/* Should we unbind if no devices were detected? */
 	}
-- 
1.6.3.3

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH 2/3] USB Storage: Make driver less chatty when it finds a new device

[Alan Stern]

On Sun, 20 Sep 2009, Matthew Wilcox wrote:

> From: Matthew Wilcox <willy@linux.intel.com>
> 
> Use dev_dbg() instead of an unconditional printk(KERN_DEBUG).  This has
> two benefits; one is that it identifies the USB device which the messages
> related to, and the other is that the messages won't be produced unless
> debug is turned on.
> 
> Enable the debug messages when CONFIG_USB_STORAGE_DEBUG is set.

> -	printk(KERN_DEBUG
> -		"usb-storage: device found at %d\n", us->pusb_dev->devnum);
> +	dev_dbg(&us->pusb_dev->dev, "usb-storage device found\n");
>  
>  	set_freezable();
>  	/* Wait for the timeout to expire or for a disconnect */
>  	if (delay_use > 0) {
> -		printk(KERN_DEBUG "usb-storage: waiting for device "
> -				"to settle before scanning\n");
> +		dev_dbg(&us->pusb_dev->dev, "usb-storage: waiting for device "
> +			"to settle before scanning\n");
>  		wait_event_freezable_timeout(us->delay_wait,
>  				test_bit(US_FLIDX_DONT_SCAN, &us->dflags),
>  				delay_use * HZ);
> @@ -832,7 +835,7 @@ static int usb_stor_scan_thread(void * __us)
>  			mutex_unlock(&us->dev_mutex);
>  		}
>  		scsi_scan_host(us_to_host(us));
> -		printk(KERN_DEBUG "usb-storage: device scan complete\n");
> +		dev_dbg(&us->pusb_dev->dev, "usb-storage scan complete\n");
>  
>  		/* Should we unbind if no devices were detected? */
>  	}

Each of these dev_dbg() lines should refer to pusb_intf instead of
pusb_dev.  See the existing code at the end of adjust_quirks().  Once
this change is made, the second dev_dbg() line won't need to specify
"usb-storage:" at the start of the message.

With those changes in place,

	Acked-by: Alan Stern <stern@rowland.harvard.edu>

Alan Stern

[PATCH 3/3] sd.c: Make initialisation less verbose

[Matthew Wilcox]

From: Matthew Wilcox <willy@linux.intel.com>

Introduce a new function, sd_print_info() to print the information we
normally want to display to the user on revalidation.

Move the capacity display from sd_read_capacity() to sd_print_info().
Move the write protect display from sd_read_write_protect_flag() to
sd_print_info().
Delete the printk reporting Mode Sense as it is not generally useful.

Move the display of 'Removable' to the same line as reporting the Write
Protect status.

Only print the line about write cache once.
---
 drivers/scsi/sd.c |   78 ++++++++++++++++++++++++++++++-----------------------
 1 files changed, 44 insertions(+), 34 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index a89c421..beec7f3 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1441,7 +1441,6 @@ sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer)
 {
 	int sector_size;
 	struct scsi_device *sdp = sdkp->device;
-	sector_t old_capacity = sdkp->capacity;
 
 	if (sd_try_rc16_first(sdp)) {
 		sector_size = read_capacity_16(sdkp, sdp, buffer);
@@ -1524,28 +1523,6 @@ got_data:
 	}
 	blk_queue_logical_block_size(sdp->request_queue, sector_size);
 
-	{
-		char cap_str_2[10], cap_str_10[10];
-		u64 sz = (u64)sdkp->capacity << ilog2(sector_size);
-
-		string_get_size(sz, STRING_UNITS_2, cap_str_2,
-				sizeof(cap_str_2));
-		string_get_size(sz, STRING_UNITS_10, cap_str_10,
-				sizeof(cap_str_10));
-
-		if (sdkp->first_scan || old_capacity != sdkp->capacity) {
-			sd_printk(KERN_NOTICE, sdkp,
-				  "%llu %d-byte logical blocks: (%s/%s)\n",
-				  (unsigned long long)sdkp->capacity,
-				  sector_size, cap_str_10, cap_str_2);
-
-			if (sdkp->hw_sector_size != sector_size)
-				sd_printk(KERN_NOTICE, sdkp,
-					  "%u-byte physical blocks\n",
-					  sdkp->hw_sector_size);
-		}
-	}
-
 	/* Rescale capacity to 512-byte units */
 	if (sector_size == 4096)
 		sdkp->capacity <<= 3;
@@ -1581,7 +1558,6 @@ sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer)
 	int res;
 	struct scsi_device *sdp = sdkp->device;
 	struct scsi_mode_data data;
-	int old_wp = sdkp->write_prot;
 
 	set_disk_ro(sdkp->disk, 0);
 	if (sdp->skip_ms_page_3f) {
@@ -1622,13 +1598,6 @@ sd_read_write_protect_flag(struct scsi_disk *sdkp, unsigned char *buffer)
 	} else {
 		sdkp->write_prot = ((data.device_specific & 0x80) != 0);
 		set_disk_ro(sdkp->disk, sdkp->write_prot);
-		if (sdkp->first_scan || old_wp != sdkp->write_prot) {
-			sd_printk(KERN_NOTICE, sdkp, "Write Protect is %s\n",
-				  sdkp->write_prot ? "on" : "off");
-			sd_printk(KERN_DEBUG, sdkp,
-				  "Mode Sense: %02x %02x %02x %02x\n",
-				  buffer[0], buffer[1], buffer[2], buffer[3]);
-		}
 	}
 }
 
@@ -1742,7 +1711,9 @@ bad_sense:
 		sd_printk(KERN_ERR, sdkp, "Asking for cache data failed\n");
 
 defaults:
-	sd_printk(KERN_ERR, sdkp, "Assuming drive cache: write through\n");
+	if (sdkp->first_scan)
+		sd_printk(KERN_ERR, sdkp,
+				"Assuming drive cache: write through\n");
 	sdkp->WCE = 0;
 	sdkp->RCD = 0;
 	sdkp->DPOFUA = 0;
@@ -1852,6 +1823,43 @@ static int sd_try_extended_inquiry(struct scsi_device *sdp)
 	return 0;
 }
 
+static
+void sd_print_info(struct scsi_disk *sdkp, sector_t old_capacity, int old_wp)
+{
+	unsigned sector_size = sdkp->device->sector_size;
+	char cap_str_2[10], cap_str_10[10];
+	u64 sz = (u64)sdkp->capacity * 512;
+
+	string_get_size(sz, STRING_UNITS_2, cap_str_2,
+			sizeof(cap_str_2));
+	string_get_size(sz, STRING_UNITS_10, cap_str_10,
+			sizeof(cap_str_10));
+
+	if (sdkp->first_scan || old_capacity != sdkp->capacity) {
+		u64 new_blocks = sdkp->capacity;
+		if (sector_size < 512)
+			new_blocks *= 512 / sector_size;
+		else if (sector_size > 512)
+			new_blocks = sector_div(sdkp->capacity,
+							512 / sector_size);
+
+		sd_printk(KERN_NOTICE, sdkp,
+			  "%llu %d-byte logical blocks: (%s/%s)\n",
+			  new_blocks, sector_size, cap_str_10, cap_str_2);
+
+		if (sdkp->hw_sector_size != sector_size)
+			sd_printk(KERN_NOTICE, sdkp,
+				  "%u-byte physical blocks\n",
+				  sdkp->hw_sector_size);
+	}
+
+	if (sdkp->first_scan || old_wp != sdkp->write_prot) {
+		sd_printk(KERN_NOTICE, sdkp, "%s disk, Write Protect is %s\n",
+			  sdkp->device->removable ? "Removable" : "Fixed",
+			  sdkp->write_prot ? "on" : "off");
+	}
+}
+
 /**
  *	sd_revalidate_disk - called the first time a new disk is seen,
  *	performs disk spin up, read_capacity, etc.
@@ -1861,6 +1869,8 @@ static int sd_revalidate_disk(struct gendisk *disk)
 {
 	struct scsi_disk *sdkp = scsi_disk(disk);
 	struct scsi_device *sdp = sdkp->device;
+	sector_t old_capacity = sdkp->capacity;
+	int old_wp = sdkp->write_prot;
 	unsigned char *buffer;
 	unsigned ordered;
 
@@ -1900,6 +1910,8 @@ static int sd_revalidate_disk(struct gendisk *disk)
 		sd_read_app_tag_own(sdkp, buffer);
 	}
 
+	sd_print_info(sdkp, old_capacity, old_wp);
+
 	sdkp->first_scan = 0;
 
 	/*
@@ -2019,8 +2031,6 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
 
 	sd_revalidate_disk(gd);
 
-	sd_printk(KERN_NOTICE, sdkp, "Attached SCSI %sdisk\n",
-		  sdp->removable ? "removable " : "");
 	put_device(&sdkp->dev);
 }
 
-- 
1.6.3.3

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."