* Reducing the verbosity of inserting a USB storage device
@ 2009-09-20 22:53 Matthew Wilcox
2009-09-20 22:57 ` [PATCH 2/3] USB Storage: Make driver less chatty when it finds a new device Matthew Wilcox
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Matthew Wilcox @ 2009-09-20 22:53 UTC (permalink / raw)
To: linux-scsi-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
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
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 2/3] USB Storage: Make driver less chatty when it finds a new device 2009-09-20 22:53 Reducing the verbosity of inserting a USB storage device Matthew Wilcox @ 2009-09-20 22:57 ` Matthew Wilcox 2009-09-21 2:09 ` Alan Stern 2009-09-20 22:58 ` [PATCH 3/3] sd.c: Make initialisation less verbose Matthew Wilcox [not found] ` <20090920225348.GB3690-6jwH94ZQLHl74goWV3ctuw@public.gmane.org> 2 siblings, 1 reply; 15+ messages in thread From: Matthew Wilcox @ 2009-09-20 22:57 UTC (permalink / raw) To: linux-scsi, linux-usb 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." ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] USB Storage: Make driver less chatty when it finds a new device 2009-09-20 22:57 ` [PATCH 2/3] USB Storage: Make driver less chatty when it finds a new device Matthew Wilcox @ 2009-09-21 2:09 ` Alan Stern 0 siblings, 0 replies; 15+ messages in thread From: Alan Stern @ 2009-09-21 2:09 UTC (permalink / raw) To: Matthew Wilcox; +Cc: linux-scsi, linux-usb 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/3] sd.c: Make initialisation less verbose 2009-09-20 22:53 Reducing the verbosity of inserting a USB storage device Matthew Wilcox 2009-09-20 22:57 ` [PATCH 2/3] USB Storage: Make driver less chatty when it finds a new device Matthew Wilcox @ 2009-09-20 22:58 ` Matthew Wilcox [not found] ` <20090920225348.GB3690-6jwH94ZQLHl74goWV3ctuw@public.gmane.org> 2 siblings, 0 replies; 15+ messages in thread From: Matthew Wilcox @ 2009-09-20 22:58 UTC (permalink / raw) To: linux-scsi, linux-usb 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." ^ permalink raw reply related [flat|nested] 15+ messages in thread
[parent not found: <20090920225348.GB3690-6jwH94ZQLHl74goWV3ctuw@public.gmane.org>]
* [PATCH 1/3] Allow SCSI hosts to be less verbose [not found] ` <20090920225348.GB3690-6jwH94ZQLHl74goWV3ctuw@public.gmane.org> @ 2009-09-20 22:56 ` Matthew Wilcox 2009-09-22 13:09 ` Christoph Hellwig 2009-09-21 1:09 ` Reducing the verbosity of inserting a USB storage device Alan Stern 2009-09-21 2:07 ` Reducing the verbosity of inserting a USB storage device Matthew Dharm 2 siblings, 1 reply; 15+ messages in thread From: Matthew Wilcox @ 2009-09-20 22:56 UTC (permalink / raw) To: linux-scsi-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] Allow SCSI hosts to be less verbose 2009-09-20 22:56 ` [PATCH 1/3] Allow SCSI hosts to be " Matthew Wilcox @ 2009-09-22 13:09 ` Christoph Hellwig 0 siblings, 0 replies; 15+ messages in thread From: Christoph Hellwig @ 2009-09-22 13:09 UTC (permalink / raw) To: Matthew Wilcox; +Cc: linux-scsi, linux-usb 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Reducing the verbosity of inserting a USB storage device [not found] ` <20090920225348.GB3690-6jwH94ZQLHl74goWV3ctuw@public.gmane.org> 2009-09-20 22:56 ` [PATCH 1/3] Allow SCSI hosts to be " Matthew Wilcox @ 2009-09-21 1:09 ` Alan Stern [not found] ` <Pine.LNX.4.44L0.0909202104300.14465-100000-pYrvlCTfrz9XsRXLowluHWD2FQJk+8+b@public.gmane.org> 2009-09-21 2:07 ` Reducing the verbosity of inserting a USB storage device Matthew Dharm 2 siblings, 1 reply; 15+ messages in thread From: Alan Stern @ 2009-09-21 1:09 UTC (permalink / raw) To: Matthew Wilcox Cc: linux-scsi-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <Pine.LNX.4.44L0.0909202104300.14465-100000-pYrvlCTfrz9XsRXLowluHWD2FQJk+8+b@public.gmane.org>]
* Re: Reducing the verbosity of inserting a USB storage device [not found] ` <Pine.LNX.4.44L0.0909202104300.14465-100000-pYrvlCTfrz9XsRXLowluHWD2FQJk+8+b@public.gmane.org> @ 2009-09-24 22:17 ` Matthew Wilcox 2009-09-24 22:18 ` [PATCH 1/4] Convert a dev_info to a dev_dbg Matthew Wilcox ` (3 more replies) 0 siblings, 4 replies; 15+ messages in thread From: Matthew Wilcox @ 2009-09-24 22:17 UTC (permalink / raw) To: Alan Stern Cc: linux-scsi-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/4] Convert a dev_info to a dev_dbg 2009-09-24 22:17 ` Matthew Wilcox @ 2009-09-24 22:18 ` Matthew Wilcox [not found] ` <20090924221702.GA7943-6jwH94ZQLHl74goWV3ctuw@public.gmane.org> ` (2 subsequent siblings) 3 siblings, 0 replies; 15+ messages in thread From: Matthew Wilcox @ 2009-09-24 22:18 UTC (permalink / raw) To: Alan Stern; +Cc: linux-scsi, linux-usb 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
[parent not found: <20090924221702.GA7943-6jwH94ZQLHl74goWV3ctuw@public.gmane.org>]
* [PATCH 2/4] usb-storage: Associate the name of the interface with the scsi host [not found] ` <20090924221702.GA7943-6jwH94ZQLHl74goWV3ctuw@public.gmane.org> @ 2009-09-24 22:19 ` Matthew Wilcox 2009-09-25 14:29 ` Alan Stern 0 siblings, 1 reply; 15+ messages in thread From: Matthew Wilcox @ 2009-09-24 22:19 UTC (permalink / raw) To: Alan Stern Cc: linux-scsi-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] usb-storage: Associate the name of the interface with the scsi host 2009-09-24 22:19 ` [PATCH 2/4] usb-storage: Associate the name of the interface with the scsi host Matthew Wilcox @ 2009-09-25 14:29 ` Alan Stern 0 siblings, 0 replies; 15+ messages in thread From: Alan Stern @ 2009-09-25 14:29 UTC (permalink / raw) To: Matthew Wilcox; +Cc: linux-scsi, linux-usb 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 */ ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/4] USB Storage: Make driver less chatty when it finds a new device 2009-09-24 22:17 ` Matthew Wilcox 2009-09-24 22:18 ` [PATCH 1/4] Convert a dev_info to a dev_dbg Matthew Wilcox [not found] ` <20090924221702.GA7943-6jwH94ZQLHl74goWV3ctuw@public.gmane.org> @ 2009-09-24 22:19 ` Matthew Wilcox 2009-09-24 22:20 ` [PATCH 4/4] sd.c: Make initialisation less verbose Matthew Wilcox 3 siblings, 0 replies; 15+ messages in thread From: Matthew Wilcox @ 2009-09-24 22:19 UTC (permalink / raw) To: Alan Stern; +Cc: linux-scsi, linux-usb 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/4] sd.c: Make initialisation less verbose 2009-09-24 22:17 ` Matthew Wilcox ` (2 preceding siblings ...) 2009-09-24 22:19 ` [PATCH 3/4] USB Storage: Make driver less chatty when it finds a new device Matthew Wilcox @ 2009-09-24 22:20 ` Matthew Wilcox [not found] ` <20090924222056.GE7943-6jwH94ZQLHl74goWV3ctuw@public.gmane.org> 3 siblings, 1 reply; 15+ messages in thread From: Matthew Wilcox @ 2009-09-24 22:20 UTC (permalink / raw) To: Alan Stern; +Cc: linux-scsi, linux-usb 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
[parent not found: <20090924222056.GE7943-6jwH94ZQLHl74goWV3ctuw@public.gmane.org>]
* Re: [PATCH 4/4] sd.c: Make initialisation less verbose [not found] ` <20090924222056.GE7943-6jwH94ZQLHl74goWV3ctuw@public.gmane.org> @ 2009-10-09 17:54 ` Greg KH 0 siblings, 0 replies; 15+ messages in thread From: Greg KH @ 2009-10-09 17:54 UTC (permalink / raw) To: Matthew Wilcox Cc: Alan Stern, linux-scsi-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Reducing the verbosity of inserting a USB storage device [not found] ` <20090920225348.GB3690-6jwH94ZQLHl74goWV3ctuw@public.gmane.org> 2009-09-20 22:56 ` [PATCH 1/3] Allow SCSI hosts to be " Matthew Wilcox 2009-09-21 1:09 ` Reducing the verbosity of inserting a USB storage device Alan Stern @ 2009-09-21 2:07 ` Matthew Dharm 2 siblings, 0 replies; 15+ messages in thread From: Matthew Dharm @ 2009-09-21 2:07 UTC (permalink / raw) To: Matthew Wilcox Cc: linux-scsi-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA [-- 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 --] ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2009-10-09 17:54 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-09-20 22:53 Reducing the verbosity of inserting a USB storage device Matthew Wilcox
2009-09-20 22:57 ` [PATCH 2/3] USB Storage: Make driver less chatty when it finds a new device Matthew Wilcox
2009-09-21 2:09 ` Alan Stern
2009-09-20 22:58 ` [PATCH 3/3] sd.c: Make initialisation less verbose Matthew Wilcox
[not found] ` <20090920225348.GB3690-6jwH94ZQLHl74goWV3ctuw@public.gmane.org>
2009-09-20 22:56 ` [PATCH 1/3] Allow SCSI hosts to be " Matthew Wilcox
2009-09-22 13:09 ` Christoph Hellwig
2009-09-21 1:09 ` Reducing the verbosity of inserting a USB storage device Alan Stern
[not found] ` <Pine.LNX.4.44L0.0909202104300.14465-100000-pYrvlCTfrz9XsRXLowluHWD2FQJk+8+b@public.gmane.org>
2009-09-24 22:17 ` Matthew Wilcox
2009-09-24 22:18 ` [PATCH 1/4] Convert a dev_info to a dev_dbg Matthew Wilcox
[not found] ` <20090924221702.GA7943-6jwH94ZQLHl74goWV3ctuw@public.gmane.org>
2009-09-24 22:19 ` [PATCH 2/4] usb-storage: Associate the name of the interface with the scsi host Matthew Wilcox
2009-09-25 14:29 ` Alan Stern
2009-09-24 22:19 ` [PATCH 3/4] USB Storage: Make driver less chatty when it finds a new device Matthew Wilcox
2009-09-24 22:20 ` [PATCH 4/4] sd.c: Make initialisation less verbose Matthew Wilcox
[not found] ` <20090924222056.GE7943-6jwH94ZQLHl74goWV3ctuw@public.gmane.org>
2009-10-09 17:54 ` Greg KH
2009-09-21 2:07 ` Reducing the verbosity of inserting a USB storage device Matthew Dharm
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).