Ed Lin | 1 Sep 2006 08:31

Re: [PATCH] block: add support for shared tag maps


======= On 2006-09-01 06:21:45, James Bottomley wrote:

>On Thu, 2006-08-31 at 16:55 +0800, Ed Lin wrote:
>> EXPORT_SYMBOL(blk_free_tags) ?
>> Also, add new function definitions in blkdev.h?
>
>Yes .. fixed that up when I tried to use it in a module.
>
>So, given these two plus another patch that should fix all of the sync
>cache issues, how about this as the final patch for stex (to replace the
>[PATCH 3/3] stex: use block layer tagging)?
>

Thanks. It's good. But here are a few issues.

1.Maybe we need to check the result of scsi_init_shared_tag_map.
It's unlikely to fail. In case it really fails, we can simply exit
since we need tagging. Just checking.

2.During shutdown/unload, we need to notify firmware to stop some
background activities, that we are going to shutdown. If we don't do this,
and the power is switched off when firmware is not ready, there may be
error. It's possible the firmware is doing something that need to be
stopped before shutdown. So it's different from a simple cache issue.

So it's better to keep the stex_hba_stop function and the .shutdown
entry. In normal cases, there is no outside command at the stage when
shutdown functions are called. So we can safely assume the tag is 0,
and issue the notifying command, and then exit. It's only one and it
(Continue reading)

Jeff Garzik | 1 Sep 2006 09:08
Favicon

Re: [PATCH 1/3] stex: adjust command timeout in slave_config routine

I put patches 1-2 into misc-2.6.git#stex...

	Jeff

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jeff Garzik | 1 Sep 2006 09:12
Favicon

[PATCH] Add Promise SuperTrak driver


Add Promise SuperTrak 'stex' driver, supporting SuperTrak
EX8350/8300/16350/16300 controllers.  The controller's firmware accepts
SCSI commands, handing them to the underlying RAID or JBOD disks.

The driver consisted of the following cleanups and fixes, beyond its
initial submission:

Ed Lin:
      stex: cleanup and minor fixes
      stex: add new device ids
      stex: update internal copy code path
      stex: add hard reset function
      stex: adjust command timeout in slave_config routine
      stex: use more efficient method for unload/shutdown flush

Jeff Garzik:
      [SCSI] Add Promise SuperTrak 'shasta' driver.
      Rename drivers/scsi/shasta.c to stex.c ("SuperTrak EX").
      [SCSI] stex: update with community comments from 'Promise SuperTrak' thread
      [SCSI] stex: Fix warning, trim trailing whitespace.
      [SCSI] stex: remove last remnants of "shasta" project code name
      [SCSI] stex: removed 6-byte command emulation
      [SCSI] stex: minor cleanups
      [SCSI] stex: minor fixes: irq flag, error return value
      [SCSI] stex: use dma_alloc_coherent()

Signed-off-by: Jeff Garzik <jeff <at> garzik.org>
---

(Continue reading)

Stefan Richter | 1 Sep 2006 11:02
Picon

Re: [PATCH] sd: fix cache flushing on module removal (and individual device removal)

James Bottomley wrote:
[...]
> --- linux-2.6.orig/drivers/scsi/scsi.c
> +++ linux-2.6/drivers/scsi/scsi.c
>  <at>  <at>  -835,14 +835,14  <at>  <at>  EXPORT_SYMBOL(scsi_track_queue_full);
>   */
>  int scsi_device_get(struct scsi_device *sdev)
>  {
> -	if (sdev->sdev_state == SDEV_DEL || sdev->sdev_state == SDEV_CANCEL)
> +	if (sdev->sdev_state == SDEV_DEL)
>  		return -ENXIO;
>  	if (!get_device(&sdev->sdev_gendev))
>  		return -ENXIO;
> -	if (!try_module_get(sdev->host->hostt->module)) {
> -		put_device(&sdev->sdev_gendev);
> -		return -ENXIO;
> -	}
> +	/* We can fail this if we're doing SCSI operations
> +	 * from module exit (like cache flush) */
> +	try_module_get(sdev->host->hostt->module);
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL(scsi_device_get);
>  <at>  <at>  -857,7 +857,10  <at>  <at>  EXPORT_SYMBOL(scsi_device_get);
>   */
>  void scsi_device_put(struct scsi_device *sdev)
>  {
> -	module_put(sdev->host->hostt->module);
> +	/* The module refcount will be zero if scsi_device_get()
(Continue reading)

James Bottomley | 1 Sep 2006 15:28
Favicon

Re: [PATCH] block: add support for shared tag maps

On Fri, 2006-09-01 at 14:31 +0800, Ed Lin wrote:
> ======= On 2006-09-01 06:21:45, James Bottomley wrote:
> 
> >On Thu, 2006-08-31 at 16:55 +0800, Ed Lin wrote:
> >> EXPORT_SYMBOL(blk_free_tags) ?
> >> Also, add new function definitions in blkdev.h?
> >
> >Yes .. fixed that up when I tried to use it in a module.
> >
> >So, given these two plus another patch that should fix all of the sync
> >cache issues, how about this as the final patch for stex (to replace the
> >[PATCH 3/3] stex: use block layer tagging)?
> >
> 
> Thanks. It's good. But here are a few issues.
> 
> 1.Maybe we need to check the result of scsi_init_shared_tag_map.
> It's unlikely to fail. In case it really fails, we can simply exit
> since we need tagging. Just checking.

Yes, we should.  I generalised your error checking below.

> 2.During shutdown/unload, we need to notify firmware to stop some
> background activities, that we are going to shutdown. If we don't do this,
> and the power is switched off when firmware is not ready, there may be
> error. It's possible the firmware is doing something that need to be
> stopped before shutdown. So it's different from a simple cache issue.
> 
> So it's better to keep the stex_hba_stop function and the .shutdown
> entry. In normal cases, there is no outside command at the stage when
(Continue reading)

Hannes Reinecke | 1 Sep 2006 15:50
Picon

[PATCH] Wrong size information for devices with disabled read access

Hi James,

when accessing a device with disabled read access the capacity is set
somewhat arbitrary to 1GB. This makes it quite impossible for userspace
applications to detect this as an invalid capacity.
And it is inconsistent with other error conditions, where the capacity
is in fact set to 0.

Please apply.

Cheers,

Hannes
--

-- 
Dr. Hannes Reinecke			hare <at> suse.de
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de
Wrong size information for devices with disabled read access

When accessing a device with disabled read access the capacity is set
randomly to 1GB. This makes it impossible to userspace tools to detect
invalid device capacities.

Signed-off-by: Mike Anderson <andmike <at> us.ibm.com>
Acked-by: Chris Mason <mason <at> suse.com>
Signed-off-by: Hannes Reinecke <hare <at> suse.de>

(Continue reading)

James Bottomley | 1 Sep 2006 15:54
Favicon

Re: [PATCH] sd: fix cache flushing on module removal (and individual device removal)

On Fri, 2006-09-01 at 11:02 +0200, Stefan Richter wrote:
> Somehow the (void)try_module_get(...) looks dangerous to me. Is it
> really safe to always ignore failures to get the module? Why would we
> want to ignore failures? Couldn't there be border cases where a
> module_getter/_putter in a concurrent code path disturbs
> scsi_device_get/_put's underlying assumptions?

As long as we don't do spurious module_puts, yes.  However, there looks
to be another nasty module race that's orthogonal to this, in that the
final scsi_host_put() of a module doesn't necessarily wait for the host
actually to be released, so it's possible to free the host template when
the module exit finishes and still have a partially functioning host.

James

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Doug Maxey | 1 Sep 2006 21:04

Re: [PATCH] block: add support for shared tag maps


Ravi,

While working on a patch to add shared tags to qla4xxx was looking at
the shost->can_queue settings, I see the value is set pretty high:

qla4xxx_probe()
...
	host->can_queue = REQUEST_QUEUE_DEPTH + 128;

where REQUEST_QUEUE_DEPTH works out to be 1024.

My question:
what is the relationship between the can_queue and the
setting in
qla4xxx_slave_configure()
	if (sdev->tagged_supported)
		scsi_activate_tcq(sdev, 32);
	else
		scsi_deactivate_tcq(sdev, 32);

Does this imply that the firmware can ultimately track more requests
than we can possibly stuff in it?  Where do the other 1012 requests get
queued, in the block layer?

James' shared tag patch for stex has the following
+stex_slave_alloc(struct scsi_device *sdev)
+{
+	/* Cheat: usually extracted from Inquiry data */
+	sdev->tagged_supported = 1;
(Continue reading)

Mike Anderson | 1 Sep 2006 22:11
Picon
Favicon

Re: [PATCH] block: add support for shared tag maps

Doug Maxey <dwm <at> enoyolf.org> wrote:
> 
> Ravi,
> 
> While working on a patch to add shared tags to qla4xxx was looking at
> the shost->can_queue settings, I see the value is set pretty high:
> 
> qla4xxx_probe()
> ...
> 	host->can_queue = REQUEST_QUEUE_DEPTH + 128;
> 
> where REQUEST_QUEUE_DEPTH works out to be 1024.
> 
> My question:
> what is the relationship between the can_queue and the
> setting in
> qla4xxx_slave_configure()
> 	if (sdev->tagged_supported)
> 		scsi_activate_tcq(sdev, 32);
> 	else
> 		scsi_deactivate_tcq(sdev, 32);
> 
> Does this imply that the firmware can ultimately track more requests
> than we can possibly stuff in it?  Where do the other 1012 requests get
> queued, in the block layer?

Maybe I misreading your question. host->can_queue is the per host instance
(adapter) limit and scsi_activate_tcq will set the per dev (lun) limit.

host->can_queue being large is a good thing (if it is backed by real
(Continue reading)

Ravi Anand | 1 Sep 2006 22:21

Re: [PATCH] block: add support for shared tag maps

>On Fri, 01 Sep 2006, Doug Maxey wrote:
> Ravi,
> 
> While working on a patch to add shared tags to qla4xxx was looking at
> the shost->can_queue settings, I see the value is set pretty high:
> 
> qla4xxx_probe()
> ...
> 	host->can_queue = REQUEST_QUEUE_DEPTH + 128;
> 
> where REQUEST_QUEUE_DEPTH works out to be 1024.
> 
> My question:
> what is the relationship between the can_queue and the
> setting in
> qla4xxx_slave_configure()
> 	if (sdev->tagged_supported)
> 		scsi_activate_tcq(sdev, 32);
> 	else
> 		scsi_deactivate_tcq(sdev, 32);
> 
> Does this imply that the firmware can ultimately track more requests
> than we can possibly stuff in it?  Where do the other 1012 requests get
> queued, in the block layer?

My understanding is that the "can_queue" is on a per HBA basis.
In other words maximum no of cmd's a give host adapter can
queue at any given point of time. 

While the setting the slave_configure() is on a per lun basis.
(Continue reading)


Gmane