[RFC Patch V1 20/30] mm, fcoe: Use cpu_to_mem()/numa_mem_id() to support memoryless node
When CONFIG_HAVE_MEMORYLESS_NODES is enabled, cpu_to_node()/numa_node_id()
may return a node without memory, and later cause system failure/panic
when calling kmalloc_node() and friends with returned node id.
So use cpu_to_mem()/numa_mem_id() instead to get the nearest node with
memory for the/current cpu.
If CONFIG_HAVE_MEMORYLESS_NODES is disabled, cpu_to_mem()/numa_mem_id()
is the same as cpu_to_node()/numa_node_id().
Signed-off-by: Jiang Liu
---
drivers/scsi/fcoe/fcoe.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 00ee0ed642aa..779a7af0e410 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1257,7 +1257,7 @@ static void fcoe_percpu_thread_create(unsigned int cpu)
p = &per_cpu(fcoe_percpu, cpu);
thread = kthread_create_on_node(fcoe_percpu_receive_thread,
- (void *)p, cpu_to_node(cpu),
+ (void *)p, cpu_to_mem(cpu),
"fcoethread/%d", cpu);
if (likely(!IS_ERR(thread))) {
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC Patch V1 18/30] mm, bnx2fc: Use cpu_to_mem()/numa_mem_id() to support memoryless node
When CONFIG_HAVE_MEMORYLESS_NODES is enabled, cpu_to_node()/numa_node_id()
may return a node without memory, and later cause system failure/panic
when calling kmalloc_node() and friends with returned node id.
So use cpu_to_mem()/numa_mem_id() instead to get the nearest node with
memory for the/current cpu.
If CONFIG_HAVE_MEMORYLESS_NODES is disabled, cpu_to_mem()/numa_mem_id()
is the same as cpu_to_node()/numa_node_id().
Signed-off-by: Jiang Liu
---
drivers/scsi/bnx2fc/bnx2fc_fcoe.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 785d0d71781e..144534a51cbb 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -2453,7 +2453,7 @@ static void bnx2fc_percpu_thread_create(unsigned int cpu)
p = &per_cpu(bnx2fc_percpu, cpu);
thread = kthread_create_on_node(bnx2fc_percpu_io_thread,
- (void *)p, cpu_to_node(cpu),
+ (void *)p, cpu_to_mem(cpu),
"bnx2fc_thread/%d", cpu);
/* bind thread to the cpu */
if (likely(!IS_ERR(thread))) {
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC Patch V1 19/30] mm, bnx2i: Use cpu_to_mem()/numa_mem_id() to support memoryless node
When CONFIG_HAVE_MEMORYLESS_NODES is enabled, cpu_to_node()/numa_node_id()
may return a node without memory, and later cause system failure/panic
when calling kmalloc_node() and friends with returned node id.
So use cpu_to_mem()/numa_mem_id() instead to get the nearest node with
memory for the/current cpu.
If CONFIG_HAVE_MEMORYLESS_NODES is disabled, cpu_to_mem()/numa_mem_id()
is the same as cpu_to_node()/numa_node_id().
Signed-off-by: Jiang Liu
---
drivers/scsi/bnx2i/bnx2i_init.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/bnx2i/bnx2i_init.c b/drivers/scsi/bnx2i/bnx2i_init.c
index 80c03b452d61..f67a5a63134e 100644
--- a/drivers/scsi/bnx2i/bnx2i_init.c
+++ b/drivers/scsi/bnx2i/bnx2i_init.c
@@ -423,7 +423,7 @@ static void bnx2i_percpu_thread_create(unsigned int cpu)
p = &per_cpu(bnx2i_percpu, cpu);
thread = kthread_create_on_node(bnx2i_percpu_io_thread, (void *)p,
- cpu_to_node(cpu),
+ cpu_to_mem(cpu),
"bnx2i_thread/%d", cpu);
/* bind thread to the cpu */
if (likely(!IS_ERR(thread))) {
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [PATCH 3/4] Introduce XEN scsiback module
On 06/28/2014 08:09 PM, Christoph Hellwig wrote: On Fri, Jun 27, 2014 at 04:34:35PM +0200, jgr...@suse.com wrote: From: Juergen Gross Introduces the XEN pvSCSI backend. With pvSCSI it is possible for a XEN domU to issue SCSI commands to a SCSI LUN assigned to that domU. The SCSI commands are passed to the pvSCSI backend in a driver domain (usually Dom0) which is owner of the physical device. This allows e.g. to use SCSI tape drives in a XEN domU. This should be written against the generic target core infrastructure in drivers/target, instead of introducing another simplistic pass through only SCSI target. . Just to be sure: you mean something like a combined version of tcm_vhost and virtio_scsi? Juergen -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] Introduce XEN scsiback module
On Fri, Jul 11, 2014 at 10:57:50AM +0200, Juergen Gross wrote: > >This should be written against the generic target core infrastructure > >in drivers/target, instead of introducing another simplistic pass > >through only SCSI target. > >. > > > > Just to be sure: you mean something like a combined version of > tcm_vhost and virtio_scsi? It would be roughly equivalent to tcm_vhost. virtio_scsi is the equivanet of scsifront in Xen. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: clean up command setup
On Sun, Jun 29, 2014 at 03:34:31PM +0200, Christoph Hellwig wrote: > There has been some confusion because scsi_setup_blk_pc_cmnd is used to set > up various special TYPE_FS commands. I've looked into the area and come > up with various cleanup, as well as few small bug fixes that were turned > up by this. > > The series is against the core-for-3.17 branch of my scsi-queue tree. I'd like to drop the RFC and officially propose this for inclusion. As noted elsewhere this also helps as a preparation for handling the WRITE SAME and discard requests properly for scsi-mq, so I'd love to get some reviews for it, and will report scsi-mq on top of it. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: eata - issue appeared in Linus git master in last 24-48 hours
On Mon, Jun 30, 2014 at 04:31:33AM +0930, Arthur Marsh wrote:
> Hi, I haven't had time to do a git bisect yet, but just saw this after
> rebuilding the kernel in the last day or so:
It seems like some of the routines called during the driver
initialization may sleep while the driver_lock is held and irqs are
disabled.
As eata2x_detect is only called during module load the lock seems
entirely pointless and should be removed, like in the patch below:
diff --git a/drivers/scsi/eata.c b/drivers/scsi/eata.c
index 03372cf..980898e 100644
--- a/drivers/scsi/eata.c
+++ b/drivers/scsi/eata.c
@@ -837,7 +837,6 @@ struct hostdata {
static struct Scsi_Host *sh[MAX_BOARDS];
static const char *driver_name = "EATA";
static char sha[MAX_BOARDS];
-static DEFINE_SPINLOCK(driver_lock);
/* Initialize num_boards so that ihdlr can work while detect is in progress */
static unsigned int num_boards = MAX_BOARDS;
@@ -1097,8 +1096,6 @@ static int port_detect(unsigned long port_base, unsigned
int j,
goto fail;
}
- spin_lock_irq(&driver_lock);
-
if (do_dma(port_base, 0, READ_CONFIG_PIO)) {
#if defined(DEBUG_DETECT)
printk("%s: detect, do_dma failed at 0x%03lx.\n", name,
@@ -1265,10 +1262,7 @@ static int port_detect(unsigned long port_base, unsigned
int j,
}
#endif
- spin_unlock_irq(&driver_lock);
sh[j] = shost = scsi_register(tpnt, sizeof(struct hostdata));
- spin_lock_irq(&driver_lock);
-
if (shost == NULL) {
printk("%s: unable to register host, detaching.\n", name);
goto freedma;
@@ -1345,8 +1339,6 @@ static int port_detect(unsigned long port_base, unsigned
int j,
else
sprintf(dma_name, "DMA %u", dma_channel);
- spin_unlock_irq(&driver_lock);
-
for (i = 0; i < shost->can_queue; i++)
ha->cp[i].cp_dma_addr = pci_map_single(ha->pdev,
&ha->cp[i],
@@ -1439,7 +1431,6 @@ static int port_detect(unsigned long port_base, unsigned
int j,
freeirq:
free_irq(irq, &sha[j]);
freelock:
- spin_unlock_irq(&driver_lock);
release_region(port_base, REGION_SIZE);
fail:
return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uas - kernel panic on drive connection
Hi, On 07/10/2014 01:31 AM, Jonathan wrote: > Hans, > > Thanks for getting back to me. It looks like the USB 3.0 controller is an > Etron. Etron, that is the first time I've heard of them. So that is the 5th manufacturer which is making xhci controllers now (the others are nec, intel, via and designware). I've managed to find a pci-e add-on card using the same Etron chipset as you have, and I've ordered one, so that I can try and reproduce this problem. It will be aprox. 3 weeks before I get it though. In the mean time, can you try the usb disk enclosure on a machine with a nec or intel xhci controller ? So that we can confirm that this is indeed a problem specific to the Etron controller? Regards, Hans > > Here is the output from lspci: > > 00:00.0 Host bridge: Intel Corporation 2nd Generation Core Processor Family > DRAM Controller (rev 09) > 00:02.0 VGA compatible controller: Intel Corporation 2nd Generation Core > Processor Family Integrated Graphics Controller (rev 09) > 00:16.0 Communication controller: Intel Corporation 6 Series/C200 Series > Chipset Family MEI Controller #1 (rev 04) > 00:1a.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset Family > USB Enhanced Host Controller #2 (rev 05) > 00:1b.0 Audio device: Intel Corporation 6 Series/C200 Series Chipset Family > High Definition Audio Controller (rev 05) > 00:1c.0 PCI bridge: Intel Corporation 6 Series/C200 Series Chipset Family PCI > Express Root Port 1 (rev b5) > 00:1c.2 PCI bridge: Intel Corporation 6 Series/C200 Series Chipset Family PCI > Express Root Port 3 (rev b5) > 00:1c.4 PCI bridge: Intel Corporation 82801 PCI Bridge (rev b5) > 00:1c.5 PCI bridge: Intel Corporation 6 Series/C200 Series Chipset Family PCI > Express Root Port 6 (rev b5) > 00:1d.0 USB controller: Intel Corporation 6 Series/C200 Series Chipset Family > USB Enhanced Host Controller #1 (rev 05) > 00:1f.0 ISA bridge: Intel Corporation Z68 Express Chipset Family LPC > Controller (rev 05) > 00:1f.2 SATA controller: Intel Corporation 6 Series/C200 Series Chipset > Family SATA AHCI Controller (rev 05) > 00:1f.3 SMBus: Intel Corporation 6 Series/C200 Series Chipset Family SMBus > Controller (rev 05) > 01:00.0 Ethernet controller: Intel Corporation 82574L Gigabit Network > Connection > 02:00.0 USB controller: Etron Technology, Inc. EJ168 USB 3.0 Host Controller > (rev 01) > 03:00.0 PCI bridge: ASMedia Technology Inc. ASM1083/1085 PCIe to PCI Bridge > (rev 01) > 04:00.0 FireWire (IEEE 1394): Texas Instruments TSB12LV26 IEEE-1394 > Controller (Link) > 05:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. > RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 06) > > Best, > > Jonathan > > On Wed, 09 Jul 2014 09:06:43 +0200 > Hans de Goede wrote: > >> >> This looks like something is going wrong in the XHCI code, likely something >> related to bulk-streams. >> >> I've a dock with an ASMedia ASM1053E chipset myself and that one works fine >> with both >> Nec and Intel XHCI controllers. What type of XHCI controller do you have ? >> >> Can you please do "lspci" on the machine in question and include the output >> in your >> next mail ? >> >> Regards, >> >> Hans -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] Drivers: scsi: storvsc: Implement an abort handler
On 07/11/2014 12:26 AM, KY Srinivasan wrote:
-Original Message-
From: Christoph Hellwig [mailto:h...@infradead.org]
Sent: Thursday, July 10, 2014 3:13 AM
To: KY Srinivasan
Cc: Christoph Hellwig; linux-ker...@vger.kernel.org;
de...@linuxdriverproject.org; oher...@suse.com;
jbottom...@parallels.com; jasow...@redhat.com; a...@canonical.com;
linux-scsi@vger.kernel.org
Subject: Re: [PATCH 6/8] Drivers: scsi: storvsc: Implement an abort handler
Note that you could increase the timeout and/or implement an
eh_timed_out handler that just returns BLK_EH_RESET_TIMER, but if the
completion takes too long the expectation is that a command will eventually
finish instead of beeing delayed by an unmound amount.
I like this idea; I will implement a eh_timed_out_handler.
Something like this should be sufficient:
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index e71a0d7..630ae81 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1468,6 +1468,12 @@ static int storvsc_get_chs(struct scsi_device
*sdev, stru
ct block_device * bdev,
return 0;
}
+static enum blk_eh_timer_return
+storvsc_timed_out_handler(struct scsi_cmnd *scmd)
+{
+ return BLK_EH_RESET_TIMER;
+}
+
static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd)
{
struct hv_host_device *host_dev =
shost_priv(scmnd->device->host);
@@ -1687,6 +1693,7 @@ static struct scsi_host_template scsi_driver = {
.name = "storvsc_host_t",
.bios_param = storvsc_get_chs,
.queuecommand = storvsc_queuecommand,
+ .eh_timed_out = storvsc_timed_out_handler,
.eh_host_reset_handler =storvsc_host_reset_handler,
.slave_alloc = storvsc_device_alloc,
.slave_destroy =storvsc_device_destroy,
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] Drivers: scsi: storvsc: Implement an abort handler
On Fri, Jul 11, 2014 at 11:52:55AM +0200, Hannes Reinecke wrote: > Something like this should be sufficient: Right. That plus a detailed comment explaining why it's there.. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] megaraid_sas: Fix LD/VF affiliation parsing
On Wed, Jul 09, 2014 at 03:17:56PM -0700, Adam Radford wrote:
> The following patch for megaraid_sas fixes the LD/VF affiliation policy
> parsing
> code to account for LD targetId's and Hidden LD's (not yet affiliated with any
> Virtual Functions). This also breaks megasas_get_ld_vf_affiliation() into 2
> separate functions: megasas_get_ld_vf_affiliation_111() and
> megasas_get_ld_Vf_affiliation_12() to reduce indentation levels.
>
> Signed-off-by: Adam Radford
> ---
> drivers/scsi/megaraid/megaraid_sas.h | 1 +
> drivers/scsi/megaraid/megaraid_sas_base.c | 318
> +++---
> 2 files changed, 208 insertions(+), 111 deletions(-)
>
> diff --git a/drivers/scsi/megaraid/megaraid_sas.h
> b/drivers/scsi/megaraid/megaraid_sas.h
> index 7d722fb..2e2fcb2 100644
> --- a/drivers/scsi/megaraid/megaraid_sas.h
> +++ b/drivers/scsi/megaraid/megaraid_sas.h
> @@ -1659,6 +1659,7 @@ struct MR_LD_VF_AFFILIATION {
> /* Plasma 1.11 FW backward compatibility structures */
> #define IOV_111_OFFSET 0x7CE
> #define MAX_VIRTUAL_FUNCTIONS 8
> +#define MR_LD_ACCESS_HIDDEN 15
>
> struct IOV_111 {
> u8 maxVFsSupported;
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
> b/drivers/scsi/megaraid/megaraid_sas_base.c
> index 112799b..b4c032c 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -1825,16 +1825,12 @@ void megasas_do_ocr(struct megasas_instance *instance)
> process_fw_state_change_wq(&instance->work_init);
> }
>
> -/* This function will get the current SR-IOV LD/VF affiliation */
> -static int megasas_get_ld_vf_affiliation(struct megasas_instance *instance,
> - int initial)
> +static int megasas_get_ld_vf_affiliation_111(struct megasas_instance
> *instance,
> + int initial)
> {
> struct megasas_cmd *cmd;
> struct megasas_dcmd_frame *dcmd;
> - struct MR_LD_VF_AFFILIATION *new_affiliation = NULL;
> struct MR_LD_VF_AFFILIATION_111 *new_affiliation_111 = NULL;
> - struct MR_LD_VF_MAP *newmap = NULL, *savedmap = NULL;
> - dma_addr_t new_affiliation_h;
> dma_addr_t new_affiliation_111_h;
> int ld, retval = 0;
> u8 thisVf;
> @@ -1842,15 +1838,15 @@ static int megasas_get_ld_vf_affiliation(struct
> megasas_instance *instance,
> cmd = megasas_get_cmd(instance);
>
> if (!cmd) {
> - printk(KERN_DEBUG "megasas: megasas_get_ld_vf_"
> -"affiliation: Failed to get cmd for scsi%d.\n",
> + printk(KERN_DEBUG "megasas: megasas_get_ld_vf_affiliation_111:"
You'd make your life easier if you just used __func__ to print the
function name.
I also think the initial variant should have their own set of functions,
but I'm not going to held this series over over these bits:
Reviewed-by: Christoph Hellwig
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] megaraid_sas: Add missing initial call to megasas_get_ld_vf_affiliation().
On Wed, Jul 09, 2014 at 03:17:57PM -0700, Adam Radford wrote: > The following patch for megaraid_sas adds a missing initial call to > megasas_get_ld_vf_affiliation() at the end of megasas_probe_one(). > > Signed-off-by: Adam Radford Looks good, Reviewed-by: Christoph Hellwig -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: arm: fas216: Use %llu to print 'u64' format
Hi Christoph, On Fri, Jul 11, 2014 at 3:23 AM, Christoph Hellwig wrote: > Thanks Fabio, > > this looks good to me. Are there any other warnings you got from that > autobuilder? Yes, the build results for various ARM defconfigs are available at: http://lists.linaro.org/pipermail/kernel-build-reports/2014-July/004338.html Regards, Fabio Estevam -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] scsi: move the nr_phys_segments assert into scsi_init_io
On 06/29/2014 03:34 PM, Christoph Hellwig wrote:
scsi_init_io should only be called for requests that transfer data,
so move the assert that a request has segments from the callers into
scsi_init_io.
Signed-off-by: Christoph Hellwig
---
drivers/scsi/scsi_lib.c | 16 +---
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 37038fb..b505b06 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -996,8 +996,11 @@ int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask)
{
struct scsi_device *sdev = cmd->device;
struct request *rq = cmd->request;
+ int error;
- int error = scsi_init_sgtable(rq, &cmd->sdb, gfp_mask);
+ BUG_ON(!rq->nr_phys_segments);
+
+ error = scsi_init_sgtable(rq, &cmd->sdb, gfp_mask);
if (error)
goto err_exit;
@@ -1088,11 +1091,7 @@ int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev,
struct request *req)
* submit a request without an attached bio.
*/
if (req->bio) {
- int ret;
-
- BUG_ON(!req->nr_phys_segments);
-
- ret = scsi_init_io(cmd, GFP_ATOMIC);
+ int ret = scsi_init_io(cmd, GFP_ATOMIC);
if (unlikely(ret))
return ret;
} else {
@@ -1131,11 +1130,6 @@ int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct
request *req)
return ret;
}
- /*
-* Filesystem requests must transfer data.
-*/
- BUG_ON(!req->nr_phys_segments);
-
memset(cmd->cmnd, 0, BLK_MAX_CDB);
return scsi_init_io(cmd, GFP_ATOMIC);
}
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/10] scsi: restructure command initialization for TYPE_FS requests
On 06/29/2014 03:34 PM, Christoph Hellwig wrote:
We should call the device handler prep_fn for all TYPE_FS requests,
not just simple read/write calls that are handled by the disk driver.
Restructure the common I/O code to call the prep_fn handler and zero
out the CDB, and just leave the call to scsi_init_io to the ULDs.
Signed-off-by: Christoph Hellwig
---
drivers/scsi/scsi_lib.c| 22 --
drivers/scsi/sd.c |2 +-
drivers/scsi/sr.c |3 +--
include/scsi/scsi_driver.h |1 -
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b505b06..bc84811 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1115,11 +1115,10 @@ int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev,
struct request *req)
EXPORT_SYMBOL(scsi_setup_blk_pc_cmnd);
/*
- * Setup a REQ_TYPE_FS command. These are simple read/write request
- * from filesystems that still need to be translated to SCSI CDBs from
- * the ULD.
+ * Setup a REQ_TYPE_FS command. These are simple request from filesystems
+ * that still need to be translated to SCSI CDBs from the ULD.
*/
-int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req)
+static int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req)
{
struct scsi_cmnd *cmd = req->special;
@@ -1131,9 +1130,8 @@ int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct
request *req)
}
memset(cmd->cmnd, 0, BLK_MAX_CDB);
- return scsi_init_io(cmd, GFP_ATOMIC);
+ return scsi_cmd_to_driver(cmd)->init_command(cmd);
}
-EXPORT_SYMBOL(scsi_setup_fs_cmnd);
static int
scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
@@ -1238,12 +1236,16 @@ static int scsi_prep_fn(struct request_queue *q, struct
request *req)
goto out;
}
- if (req->cmd_type == REQ_TYPE_FS)
- ret = scsi_cmd_to_driver(cmd)->init_command(cmd);
- else if (req->cmd_type == REQ_TYPE_BLOCK_PC)
+ switch (req->cmd_type) {
+ case REQ_TYPE_FS:
+ ret = scsi_setup_fs_cmnd(sdev, req);
+ break;
+ case REQ_TYPE_BLOCK_PC:
ret = scsi_setup_blk_pc_cmnd(sdev, req);
- else
+ break;
+ default:
ret = BLKPREP_KILL;
+ }
out:
return scsi_prep_return(q, req, ret);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 9c86e3d..001b3e8 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -894,7 +894,7 @@ static int sd_init_command(struct scsi_cmnd *SCpnt)
ret = scsi_setup_flush_cmnd(sdp, rq);
goto out;
}
- ret = scsi_setup_fs_cmnd(sdp, rq);
+ ret = scsi_init_io(SCpnt, GFP_ATOMIC);
if (ret != BLKPREP_OK)
goto out;
SCpnt = rq->special;
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index a7ea27c..9feeb37 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -385,10 +385,9 @@ static int sr_init_command(struct scsi_cmnd *SCpnt)
int block = 0, this_count, s_size;
struct scsi_cd *cd;
struct request *rq = SCpnt->request;
- struct scsi_device *sdp = SCpnt->device;
int ret;
- ret = scsi_setup_fs_cmnd(sdp, rq);
+ ret = scsi_init_io(SCpnt, GFP_ATOMIC);
if (ret != BLKPREP_OK)
goto out;
SCpnt = rq->special;
diff --git a/include/scsi/scsi_driver.h b/include/scsi/scsi_driver.h
index 36c4114..009d2ae 100644
--- a/include/scsi/scsi_driver.h
+++ b/include/scsi/scsi_driver.h
@@ -30,6 +30,5 @@ extern int scsi_register_interface(struct class_interface *);
class_interface_unregister(intf)
int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req);
-int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req);
#endif /* _SCSI_SCSI_DRIVER_H */
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/10] scsi: set sc_data_direction in common code
On 06/29/2014 03:34 PM, Christoph Hellwig wrote:
Signed-off-by: Christoph Hellwig
---
drivers/scsi/scsi_lib.c | 14 +++---
drivers/scsi/sd.c |2 --
drivers/scsi/sr.c |2 --
3 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index bc84811..ea23860 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1101,13 +1101,6 @@ int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev,
struct request *req)
}
cmd->cmd_len = req->cmd_len;
- if (!blk_rq_bytes(req))
- cmd->sc_data_direction = DMA_NONE;
- else if (rq_data_dir(req) == WRITE)
- cmd->sc_data_direction = DMA_TO_DEVICE;
- else
- cmd->sc_data_direction = DMA_FROM_DEVICE;
-
cmd->transfersize = blk_rq_bytes(req);
cmd->allowed = req->retries;
return BLKPREP_OK;
@@ -1236,6 +1229,13 @@ static int scsi_prep_fn(struct request_queue *q, struct
request *req)
goto out;
}
+ if (!blk_rq_bytes(req))
+ cmd->sc_data_direction = DMA_NONE;
+ else if (rq_data_dir(req) == WRITE)
+ cmd->sc_data_direction = DMA_TO_DEVICE;
+ else
+ cmd->sc_data_direction = DMA_FROM_DEVICE;
+
switch (req->cmd_type) {
case REQ_TYPE_FS:
ret = scsi_setup_fs_cmnd(sdev, req);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 001b3e8..d88bdc8 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -994,14 +994,12 @@ static int sd_init_command(struct scsi_cmnd *SCpnt)
goto out;
}
SCpnt->cmnd[0] = WRITE_6;
- SCpnt->sc_data_direction = DMA_TO_DEVICE;
if (blk_integrity_rq(rq))
sd_dif_prepare(rq, block, sdp->sector_size);
} else if (rq_data_dir(rq) == READ) {
SCpnt->cmnd[0] = READ_6;
- SCpnt->sc_data_direction = DMA_FROM_DEVICE;
} else {
scmd_printk(KERN_ERR, SCpnt, "Unknown command %llx\n", (unsigned
long long) rq->cmd_flags);
goto out;
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 9feeb37..cce4771 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -438,11 +438,9 @@ static int sr_init_command(struct scsi_cmnd *SCpnt)
if (!cd->device->writeable)
goto out;
SCpnt->cmnd[0] = WRITE_10;
- SCpnt->sc_data_direction = DMA_TO_DEVICE;
cd->cdi.media_written = 1;
} else if (rq_data_dir(rq) == READ) {
SCpnt->cmnd[0] = READ_10;
- SCpnt->sc_data_direction = DMA_FROM_DEVICE;
} else {
blk_dump_rq_flags(rq, "Unknown sr command");
goto out;
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/10] sd: don't use scsi_setup_blk_pc_cmnd for flush requests
On 06/29/2014 03:34 PM, Christoph Hellwig wrote:
Simplify handling of flush requests by setting up the command directly
instead of initializing request fields and then calling
scsi_setup_blk_pc_cmnd to propagate the information into the command.
Also rename scsi_setup_flush_cmnd to sd_setup_flush_cmnd for consistency.
Signed-off-by: Christoph Hellwig
---
drivers/scsi/sd.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index d88bdc8..620d32f 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -844,14 +844,20 @@ static int sd_setup_write_same_cmnd(struct scsi_device
*sdp, struct request *rq)
return ret;
}
-static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct request *rq)
+static int sd_setup_flush_cmnd(struct scsi_cmnd *cmd)
{
- rq->timeout *= SD_FLUSH_TIMEOUT_MULTIPLIER;
- rq->retries = SD_MAX_RETRIES;
- rq->cmd[0] = SYNCHRONIZE_CACHE;
- rq->cmd_len = 10;
+ struct request *rq = cmd->request;
+
+ /* flush requests don't perform I/O, zero the S/G table */
+ memset(&cmd->sdb, 0, sizeof(cmd->sdb));
- return scsi_setup_blk_pc_cmnd(sdp, rq);
+ cmd->cmnd[0] = SYNCHRONIZE_CACHE;
+ cmd->cmd_len = 10;
+ cmd->transfersize = 0;
+ cmd->allowed = SD_MAX_RETRIES;
+
+ rq->timeout *= SD_FLUSH_TIMEOUT_MULTIPLIER;
+ return BLKPREP_OK;
}
static void sd_uninit_command(struct scsi_cmnd *SCpnt)
@@ -891,7 +897,7 @@ static int sd_init_command(struct scsi_cmnd *SCpnt)
ret = sd_setup_write_same_cmnd(sdp, rq);
goto out;
} else if (rq->cmd_flags & REQ_FLUSH) {
- ret = scsi_setup_flush_cmnd(sdp, rq);
+ ret = sd_setup_flush_cmnd(SCpnt);
goto out;
}
ret = scsi_init_io(SCpnt, GFP_ATOMIC);
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/10] sd: don't use scsi_setup_blk_pc_cmnd for write same requests
On 06/29/2014 03:34 PM, Christoph Hellwig wrote:
Simplify handling of write same requests by setting up the command directly
instead of initializing request fields and then calling
scsi_setup_blk_pc_cmnd to propagate the information into the command.
Signed-off-by: Christoph Hellwig
---
drivers/scsi/sd.c | 44
1 file changed, 28 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 620d32f..25f25dd 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -799,14 +799,15 @@ out:
/**
* sd_setup_write_same_cmnd - write the same data to multiple blocks
- * @sdp: scsi device to operate one
- * @rq: Request to prepare
+ * @cmd: command to prepare
*
* Will issue either WRITE SAME(10) or WRITE SAME(16) depending on
* preference indicated by target device.
**/
-static int sd_setup_write_same_cmnd(struct scsi_device *sdp, struct request
*rq)
+static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
{
+ struct request *rq = cmd->request;
+ struct scsi_device *sdp = cmd->device;
struct scsi_disk *sdkp = scsi_disk(rq->rq_disk);
struct bio *bio = rq->bio;
sector_t sector = blk_rq_pos(rq);
@@ -822,25 +823,36 @@ static int sd_setup_write_same_cmnd(struct scsi_device
*sdp, struct request *rq)
sector >>= ilog2(sdp->sector_size) - 9;
nr_sectors >>= ilog2(sdp->sector_size) - 9;
- rq->__data_len = sdp->sector_size;
rq->timeout = SD_WRITE_SAME_TIMEOUT;
- memset(rq->cmd, 0, rq->cmd_len);
if (sdkp->ws16 || sector > 0x || nr_sectors > 0x) {
- rq->cmd_len = 16;
- rq->cmd[0] = WRITE_SAME_16;
- put_unaligned_be64(sector, &rq->cmd[2]);
- put_unaligned_be32(nr_sectors, &rq->cmd[10]);
+ cmd->cmd_len = 16;
+ cmd->cmnd[0] = WRITE_SAME_16;
+ put_unaligned_be64(sector, &cmd->cmnd[2]);
+ put_unaligned_be32(nr_sectors, &cmd->cmnd[10]);
} else {
- rq->cmd_len = 10;
- rq->cmd[0] = WRITE_SAME;
- put_unaligned_be32(sector, &rq->cmd[2]);
- put_unaligned_be16(nr_sectors, &rq->cmd[7]);
+ cmd->cmd_len = 10;
+ cmd->cmnd[0] = WRITE_SAME;
+ put_unaligned_be32(sector, &cmd->cmnd[2]);
+ put_unaligned_be16(nr_sectors, &cmd->cmnd[7]);
}
- ret = scsi_setup_blk_pc_cmnd(sdp, rq);
- rq->__data_len = nr_bytes;
+ cmd->transfersize = sdp->sector_size;
+ cmd->allowed = rq->retries;
+ /*
+* For WRITE_SAME the data transferred in the DATA IN buffer is
+* different from the amount of data actually written to the target.
+*
+* We set up __data_len to the amount of data transferred from the
+* DATA IN buffer so that blk_rq_map_sg set up the proper S/G list
+* to transfer a single sector of data first, but then reset it to
+* the amount of data to be written right after so that the I/O path
+* knows how much to actually write.
+*/
+ rq->__data_len = sdp->sector_size;
+ ret = scsi_init_io(cmd, GFP_ATOMIC);
+ rq->__data_len = nr_bytes;
return ret;
}
Hmm? __data_len is the amount of data written _on the target_.
Do we actually care about it?
And if so, why didn't it break with the original version?
In either case a short description in the patch would be nice.
@@ -894,7 +906,7 @@ static int sd_init_command(struct scsi_cmnd *SCpnt)
ret = sd_setup_discard_cmnd(sdp, rq);
goto out;
} else if (rq->cmd_flags & REQ_WRITE_SAME) {
- ret = sd_setup_write_same_cmnd(sdp, rq);
+ ret = sd_setup_write_same_cmnd(SCpnt);
goto out;
} else if (rq->cmd_flags & REQ_FLUSH) {
ret = sd_setup_flush_cmnd(SCpnt);
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/10] sd: don't use scsi_setup_blk_pc_cmnd for discard requests
On 06/29/2014 03:34 PM, Christoph Hellwig wrote:
Simplify handling of discard requests by setting up the command directly
instead of initializing request fields and then calling
scsi_setup_blk_pc_cmnd to propagate the information into the command.
Signed-off-by: Christoph Hellwig
---
drivers/scsi/sd.c | 43 ---
1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 25f25dd..9737e78 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -691,8 +691,10 @@ static void sd_config_discard(struct scsi_disk *sdkp,
unsigned int mode)
* Will issue either UNMAP or WRITE SAME(16) depending on preference
* indicated by target device.
**/
-static int sd_setup_discard_cmnd(struct scsi_device *sdp, struct request *rq)
+static int sd_setup_discard_cmnd(struct scsi_cmnd *cmd)
{
+ struct request *rq = cmd->request;
+ struct scsi_device *sdp = cmd->device;
struct scsi_disk *sdkp = scsi_disk(rq->rq_disk);
sector_t sector = blk_rq_pos(rq);
unsigned int nr_sectors = blk_rq_sectors(rq);
@@ -704,9 +706,6 @@ static int sd_setup_discard_cmnd(struct scsi_device *sdp,
struct request *rq)
sector >>= ilog2(sdp->sector_size) - 9;
nr_sectors >>= ilog2(sdp->sector_size) - 9;
- rq->timeout = SD_TIMEOUT;
-
- memset(rq->cmd, 0, rq->cmd_len);
page = alloc_page(GFP_ATOMIC | __GFP_ZERO);
if (!page)
@@ -716,9 +715,9 @@ static int sd_setup_discard_cmnd(struct scsi_device *sdp,
struct request *rq)
case SD_LBP_UNMAP:
buf = page_address(page);
- rq->cmd_len = 10;
- rq->cmd[0] = UNMAP;
- rq->cmd[8] = 24;
+ cmd->cmd_len = 10;
+ cmd->cmnd[0] = UNMAP;
+ cmd->cmnd[8] = 24;
put_unaligned_be16(6 + 16, &buf[0]);
put_unaligned_be16(16, &buf[2]);
@@ -729,23 +728,23 @@ static int sd_setup_discard_cmnd(struct scsi_device *sdp,
struct request *rq)
break;
case SD_LBP_WS16:
- rq->cmd_len = 16;
- rq->cmd[0] = WRITE_SAME_16;
- rq->cmd[1] = 0x8; /* UNMAP */
- put_unaligned_be64(sector, &rq->cmd[2]);
- put_unaligned_be32(nr_sectors, &rq->cmd[10]);
+ cmd->cmd_len = 16;
+ cmd->cmnd[0] = WRITE_SAME_16;
+ cmd->cmnd[1] = 0x8; /* UNMAP */
+ put_unaligned_be64(sector, &cmd->cmnd[2]);
+ put_unaligned_be32(nr_sectors, &cmd->cmnd[10]);
len = sdkp->device->sector_size;
break;
case SD_LBP_WS10:
case SD_LBP_ZERO:
- rq->cmd_len = 10;
- rq->cmd[0] = WRITE_SAME;
+ cmd->cmd_len = 10;
+ cmd->cmnd[0] = WRITE_SAME;
if (sdkp->provisioning_mode == SD_LBP_WS10)
- rq->cmd[1] = 0x8; /* UNMAP */
- put_unaligned_be32(sector, &rq->cmd[2]);
- put_unaligned_be16(nr_sectors, &rq->cmd[7]);
+ cmd->cmnd[1] = 0x8; /* UNMAP */
+ put_unaligned_be32(sector, &cmd->cmnd[2]);
+ put_unaligned_be16(nr_sectors, &cmd->cmnd[7]);
len = sdkp->device->sector_size;
break;
@@ -756,8 +755,14 @@ static int sd_setup_discard_cmnd(struct scsi_device *sdp,
struct request *rq)
}
rq->completion_data = page;
+ rq->timeout = SD_TIMEOUT;
+
blk_add_request_payload(rq, page, len);
- ret = scsi_setup_blk_pc_cmnd(sdp, rq);
+
+ cmd->transfersize = len;
+ cmd->allowed = rq->retries;
+
+ ret = scsi_init_io(cmd, GFP_ATOMIC);
rq->__data_len = nr_bytes;
out:
Ah. Here it's done properly.
@@ -903,7 +908,7 @@ static int sd_init_command(struct scsi_cmnd *SCpnt)
* block PC requests to make life easier.
*/
if (rq->cmd_flags & REQ_DISCARD) {
- ret = sd_setup_discard_cmnd(sdp, rq);
+ ret = sd_setup_discard_cmnd(SCpnt);
goto out;
} else if (rq->cmd_flags & REQ_WRITE_SAME) {
ret = sd_setup_write_same_cmnd(SCpnt);
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/10] sd: retry write same commands
On 06/29/2014 03:34 PM, Christoph Hellwig wrote: Currently cmd->allowed is initialized from rq->retries for write same commands, but retries is always 0 for non-BLOCK_PC requests. Set it to the standard number of retries instead. Signed-off-by: Christoph Hellwig --- drivers/scsi/sd.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 9737e78..777b141 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -843,7 +843,7 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd) } cmd->transfersize = sdp->sector_size; - cmd->allowed = rq->retries; + cmd->allowed = SD_MAX_RETRIES; /* * For WRITE_SAME the data transferred in the DATA IN buffer is Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] sd: retry discard commands
On 06/29/2014 03:34 PM, Christoph Hellwig wrote: Currently cmd->allowed is initialized from rq->retries for discard commands, but retries is always 0 for non-BLOCK_PC requests. Set it to the standard number of retries instead. Signed-off-by: Christoph Hellwig --- drivers/scsi/sd.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 777b141..5f85102 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -760,7 +760,7 @@ static int sd_setup_discard_cmnd(struct scsi_cmnd *cmd) blk_add_request_payload(rq, page, len); cmd->transfersize = len; - cmd->allowed = rq->retries; + cmd->allowed = SD_MAX_RETRIES; ret = scsi_init_io(cmd, GFP_ATOMIC); rq->__data_len = nr_bytes; Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/10] sd: split sd_init_command
On 06/29/2014 03:34 PM, Christoph Hellwig wrote:
Factor out a function to initialize regular read/write commands and leave
sd_init_command as a simple dispatcher to the different prepare routines.
Signed-off-by: Christoph Hellwig
---
drivers/scsi/sd.c | 58 ++---
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 5f85102..ba756b1 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -877,21 +877,7 @@ static int sd_setup_flush_cmnd(struct scsi_cmnd *cmd)
return BLKPREP_OK;
}
-static void sd_uninit_command(struct scsi_cmnd *SCpnt)
-{
- struct request *rq = SCpnt->request;
-
- if (rq->cmd_flags & REQ_DISCARD)
- __free_page(rq->completion_data);
-
- if (SCpnt->cmnd != rq->cmd) {
- mempool_free(SCpnt->cmnd, sd_cdb_pool);
- SCpnt->cmnd = NULL;
- SCpnt->cmd_len = 0;
- }
-}
-
-static int sd_init_command(struct scsi_cmnd *SCpnt)
+static int sd_setup_read_write_cmnd(struct scsi_cmnd *SCpnt)
{
struct request *rq = SCpnt->request;
struct scsi_device *sdp = SCpnt->device;
@@ -903,20 +889,6 @@ static int sd_init_command(struct scsi_cmnd *SCpnt)
int ret, host_dif;
unsigned char protect;
- /*
-* Discard request come in as REQ_TYPE_FS but we turn them into
-* block PC requests to make life easier.
-*/
- if (rq->cmd_flags & REQ_DISCARD) {
- ret = sd_setup_discard_cmnd(SCpnt);
- goto out;
- } else if (rq->cmd_flags & REQ_WRITE_SAME) {
- ret = sd_setup_write_same_cmnd(SCpnt);
- goto out;
- } else if (rq->cmd_flags & REQ_FLUSH) {
- ret = sd_setup_flush_cmnd(SCpnt);
- goto out;
- }
ret = scsi_init_io(SCpnt, GFP_ATOMIC);
if (ret != BLKPREP_OK)
goto out;
@@ -1148,6 +1120,34 @@ static int sd_init_command(struct scsi_cmnd *SCpnt)
return ret;
}
+static int sd_init_command(struct scsi_cmnd *cmd)
+{
+ struct request *rq = cmd->request;
+
+ if (rq->cmd_flags & REQ_DISCARD)
+ return sd_setup_discard_cmnd(cmd);
+ else if (rq->cmd_flags & REQ_WRITE_SAME)
+ return sd_setup_write_same_cmnd(cmd);
+ else if (rq->cmd_flags & REQ_FLUSH)
+ return sd_setup_flush_cmnd(cmd);
+ else
+ return sd_setup_read_write_cmnd(cmd);
+}
+
+static void sd_uninit_command(struct scsi_cmnd *SCpnt)
+{
+ struct request *rq = SCpnt->request;
+
+ if (rq->cmd_flags & REQ_DISCARD)
+ __free_page(rq->completion_data);
+
+ if (SCpnt->cmnd != rq->cmd) {
+ mempool_free(SCpnt->cmnd, sd_cdb_pool);
+ SCpnt->cmnd = NULL;
+ SCpnt->cmd_len = 0;
+ }
+}
+
/**
*sd_open - open a scsi disk device
*@inode: only i_rdev member may be used
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/10] scsi: mark scsi_setup_blk_pc_cmnd static
On 06/29/2014 03:34 PM, Christoph Hellwig wrote:
Signed-off-by: Christoph Hellwig
---
drivers/scsi/scsi_lib.c|3 +--
include/scsi/scsi_driver.h |2 --
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index ea23860..f0a3ef1 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1080,7 +1080,7 @@ static struct scsi_cmnd *scsi_get_cmd_from_req(struct
scsi_device *sdev,
return cmd;
}
-int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req)
+static int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request
*req)
{
struct scsi_cmnd *cmd = req->special;
@@ -1105,7 +1105,6 @@ int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev,
struct request *req)
cmd->allowed = req->retries;
return BLKPREP_OK;
}
-EXPORT_SYMBOL(scsi_setup_blk_pc_cmnd);
/*
* Setup a REQ_TYPE_FS command. These are simple request from filesystems
diff --git a/include/scsi/scsi_driver.h b/include/scsi/scsi_driver.h
index 009d2ae..c2b7598 100644
--- a/include/scsi/scsi_driver.h
+++ b/include/scsi/scsi_driver.h
@@ -29,6 +29,4 @@ extern int scsi_register_interface(struct class_interface *);
#define scsi_unregister_interface(intf) \
class_interface_unregister(intf)
-int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req);
-
#endif /* _SCSI_SCSI_DRIVER_H */
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] Drivers: scsi: storvsc: Filter WRITE_SAME_16
> "hch" == hch@infradead org writes: (Back from vacation: Bear with me while I'm catching up on two weeks of linux-scsi stuff...) hch> I think the problem is a differnet one. If we have the logical hch> provisioning EVPD it configures what method to use, but if we don't hch> have one we simply check for a max unmap blocks field, and if hch> that's not present use WRITE SAME. Yeah, that was done to accommodate the devices out there that predate the LBP VPD. There sadly are several still. And it's hard to motivate people to update their storage array firmware even when updates are readily available. hch> The patch checks the no_write_same flag before doing that, for hch> which we also have to do the write_same setup before the discard hch> setup in sd_revalidate_disk. The no_write_same was introduced to disable the REQ_WRITE_SAME use case where we have no INQUIRY/READ CAPACITY/VPD flags to key off of to determine whether it is safe to send the commands. no_write_same does not preclude the REQ_DISCARD variants of WRITE_SAME. Those are gated by the discard heuristics exclusively. So first of all I'd like to know whether it's a WRITE SAME(16) or a WRITE SAME(16) with the UNMAP bit set that's coming down. hch> Ky: does hyperv support UNMAP? If so any idea why it doesn't set hch> the maximum unmap block count field in the EVPD? We shouldn't be sending down WRITE SAME(16) with the UNMAP bit set unless the device sets LBPME=1 in the READ CAPACITY(16) response. So what does the storsvc report as its thin provisioning capabilities? -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: scsi-mq V2
> -Original Message-
> From: Christoph Hellwig [mailto:h...@infradead.org]
> Sent: Friday, 11 July, 2014 1:15 AM
> To: Elliott, Robert (Server Storage)
> Cc: Jeff Moyer; Christoph Hellwig; Jens Axboe; dgilb...@interlog.com; James
> Bottomley; Bart Van Assche; Benjamin LaHaise; linux-scsi@vger.kernel.org;
> linux-ker...@vger.kernel.org
> Subject: Re: scsi-mq V2
>
> On Fri, Jul 11, 2014 at 06:02:03AM +, Elliott, Robert (Server Storage)
> wrote:
> > Allowing longer run times before declaring success, the problem
> > does appear in all of the bisect trees. I just let fio
> > continue to run for many minutes - no ^Cs necessary.
> >
> > no-rebase: good for > 45 minutes (I will leave that running for
> > 8 more hours)
>
> Ok, thanks. If it's still running tomorrow morning let's look into the
> aio reverts again.
That ran 9 total hours with no problem.
Rather than revert in the bisect trees, I added just this single additional
patch to the no-rebase tree, and the problem appeared:
48a2e94154177286b3bcbed25ea802232527fa7c
aio: fix aio request leak when events are reaped by userspace
diff --git a/fs/aio.c b/fs/aio.c
index 4f078c0..e59bba8 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1021,6 +1021,7 @@ void aio_complete(struct kiocb *iocb, long res, long res2)
/* everything turned out well, dispose of the aiocb. */
kiocb_free(iocb);
+ put_reqs_available(ctx, 1); /* added by patch f8567 */
/*
* We have to order our ring_info tail store above and test
@@ -1101,7 +1102,7 @@ static long aio_read_events_ring(struct kioctx *ctx,
pr_debug("%li h%u t%u\n", ret, head, tail);
- put_reqs_available(ctx, ret);
+ /* put_reqs_available(ctx, ret); removed by patch f8567 */
out:
mutex_unlock(&ctx->ring_lock);
---
Rob ElliottHP Server Storage
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 6/8] Drivers: scsi: storvsc: Implement an abort handler
> -Original Message-
> From: Hannes Reinecke [mailto:h...@suse.de]
> Sent: Friday, July 11, 2014 2:53 AM
> To: KY Srinivasan; Christoph Hellwig
> Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org;
> oher...@suse.com; jbottom...@parallels.com; jasow...@redhat.com;
> a...@canonical.com; linux-scsi@vger.kernel.org
> Subject: Re: [PATCH 6/8] Drivers: scsi: storvsc: Implement an abort handler
>
> On 07/11/2014 12:26 AM, KY Srinivasan wrote:
> >
> >
> >> -Original Message-
> >> From: Christoph Hellwig [mailto:h...@infradead.org]
> >> Sent: Thursday, July 10, 2014 3:13 AM
> >> To: KY Srinivasan
> >> Cc: Christoph Hellwig; linux-ker...@vger.kernel.org;
> >> de...@linuxdriverproject.org; oher...@suse.com;
> >> jbottom...@parallels.com; jasow...@redhat.com; a...@canonical.com;
> >> linux-scsi@vger.kernel.org
> >> Subject: Re: [PATCH 6/8] Drivers: scsi: storvsc: Implement an abort
> >> handler
> >>
> >>
> >> Note that you could increase the timeout and/or implement an
> >> eh_timed_out handler that just returns BLK_EH_RESET_TIMER, but if the
> >> completion takes too long the expectation is that a command will
> >> eventually finish instead of beeing delayed by an unmound amount.
> >
> > I like this idea; I will implement a eh_timed_out_handler.
> >
> Something like this should be sufficient:
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index
> e71a0d7..630ae81 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1468,6 +1468,12 @@ static int storvsc_get_chs(struct scsi_device
> *sdev, stru ct block_device * bdev,
> return 0;
> }
>
> +static enum blk_eh_timer_return
> +storvsc_timed_out_handler(struct scsi_cmnd *scmd) {
> + return BLK_EH_RESET_TIMER;
> +}
> +
> static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd)
> {
> struct hv_host_device *host_dev = shost_priv(scmnd->device->host);
> @@ -1687,6 +1693,7 @@ static struct scsi_host_template scsi_driver = {
> .name = "storvsc_host_t",
> .bios_param = storvsc_get_chs,
> .queuecommand = storvsc_queuecommand,
> + .eh_timed_out = storvsc_timed_out_handler,
> .eh_host_reset_handler =storvsc_host_reset_handler,
> .slave_alloc = storvsc_device_alloc,
> .slave_destroy =storvsc_device_destroy,
>
> Cheers,
Thanks Hannes. Based on Christoph's feedback I have implemented exactly this
patch. It is under test on Azure.
I will be re-spinning and posting the patches shortly.
K. Y
>
> Hannes
> --
> Dr. Hannes Reinecke zSeries & Storage
> h...@suse.de+49 911 74053 688
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: scsi-mq V2
On Fri, Jul 11, 2014 at 02:33:12PM +, Elliott, Robert (Server Storage)
wrote:
> That ran 9 total hours with no problem.
>
> Rather than revert in the bisect trees, I added just this single additional
> patch to the no-rebase tree, and the problem appeared:
Can you try the below totally untested patch instead? It looks like
put_reqs_available() is not irq-safe.
-ben
--
"Thought is the essence of where you are now."
diff --git a/fs/aio.c b/fs/aio.c
index 955947e..4b97180 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -830,16 +830,20 @@ void exit_aio(struct mm_struct *mm)
static void put_reqs_available(struct kioctx *ctx, unsigned nr)
{
struct kioctx_cpu *kcpu;
+ unsigned long flags;
preempt_disable();
kcpu = this_cpu_ptr(ctx->cpu);
+ local_irq_save(flags);
kcpu->reqs_available += nr;
+
while (kcpu->reqs_available >= ctx->req_batch * 2) {
kcpu->reqs_available -= ctx->req_batch;
atomic_add(ctx->req_batch, &ctx->reqs_available);
}
+ local_irq_restore(flags);
preempt_enable();
}
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/10] sd: don't use scsi_setup_blk_pc_cmnd for write same requests
>> -rq->__data_len = sdp->sector_size; >> +rq->__data_len = sdp->sector_size; >> +ret = scsi_init_io(cmd, GFP_ATOMIC); >> +rq->__data_len = nr_bytes; >> return ret; >> } >> > Hmm? __data_len is the amount of data written _on the target_. > Do we actually care about it? > And if so, why didn't it break with the original version? > In either case a short description in the patch would be nice. The drivers care about it, and scsi_init_io uses it as transfer size, thus we have to set it to the tranfer length before the scsi_init_io call, and to the full number of bytes to be written after it. We already do this before the patch, I just moved the first assginment next to the call to scsi_init_io so that it's more obvious. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/10] sd: don't use scsi_setup_blk_pc_cmnd for discard requests
On Fri, Jul 11, 2014 at 02:26:24PM +0200, Hannes Reinecke wrote: >> blk_add_request_payload(rq, page, len); >> -ret = scsi_setup_blk_pc_cmnd(sdp, rq); >> + >> +cmd->transfersize = len; >> +cmd->allowed = rq->retries; >> + >> +ret = scsi_init_io(cmd, GFP_ATOMIC); >> rq->__data_len = nr_bytes; >> >> out: > Ah. Here it's done properly. blk_add_request_payload also sets up rq->__data_len.. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi: Remove some false sharing in Scsi_Host / Scsi_Device
From: Andi Kleen
These data structures are accessed by different CPUs and have some
fields which are mostly read only and others which are frequently
written. Separate some common ones into separate cache line
to minimize false sharing while submitting a command.
This allows scsi_dispatch_cmd to do more work with shared clean
cache lines.
- Move the cmd_serial_number to the end before the host data
- Separate write common fields from read mostly fields in
the scsi device
Signed-off-by: Andi Kleen
---
include/scsi/scsi_device.h | 4
include/scsi/scsi_host.h | 15 ++-
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 5853c91..2064d47 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -98,6 +98,10 @@ struct scsi_device {
unsigned long last_queue_ramp_up; /* last queue ramp up time */
+ /* Mostly read fields below. Move to own cache line to avoid false
+* sharing */
+ char align1 __attribute__((aligned(SMP_CACHE_BYTES)));
+
unsigned int id, lun, channel;
unsigned int manufacturer; /* Manufacturer of device, for using
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 94844fc..a9371b9 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -654,11 +654,7 @@ struct Scsi_Host {
short unsigned int sg_prot_tablesize;
short unsigned int max_sectors;
unsigned long dma_boundary;
- /*
-* Used to assign serial numbers to the cmds.
-* Protected by the host lock.
-*/
- unsigned long cmd_serial_number;
+
unsigned active_mode:2;
unsigned unchecked_isa_dma:1;
@@ -761,6 +757,15 @@ struct Scsi_Host {
struct device *dma_dev;
/*
+* Used to assign serial numbers to the cmds.
+* Protected by the host lock.
+* Hot cache line, keep separate from read only fields.
+*/
+ unsigned long cmd_serial_number
+ __attribute__((aligned(SMP_CACHE_BYTES)));
+ char pad[SMP_CACHE_BYTES - sizeof(unsigned long)];
+
+ /*
* We should ensure that this is aligned, both for better performance
* and also because some compilers (m68k) don't automatically force
* alignment to a long boundary.
--
1.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 7/8] drivers: scsi: storvsc: Set srb_flags in all cases
> -Original Message- > From: Christoph Hellwig [mailto:h...@infradead.org] > Sent: Thursday, July 10, 2014 3:19 AM > To: KY Srinivasan > Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org; > oher...@suse.com; jbottom...@parallels.com; jasow...@redhat.com; > a...@canonical.com; linux-scsi@vger.kernel.org; sta...@vger.kernel.org > Subject: Re: [PATCH 7/8] drivers: scsi: storvsc: Set srb_flags in all cases > > > default: > > vm_srb->data_in = UNKNOWN_TYPE; > > - vm_srb->win8_extension.srb_flags = 0; > > + vm_srb->win8_extension.srb_flags |= > (SRB_FLAGS_DATA_IN | > > +SRB_FLAGS_DATA_OUT); > > This would usually be a command that doesn't transfer data (e.g. > TEST_UNIT_READY or SYNCHRONIZE_CACHE), do you really want to set the in > and out flags here? On the host, before they forward the command to the native driver stack, I am told they validate that the flags be correctly set because of some bugs in the Emulex driver. K. Y -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uas - kernel panic on drive connection
Hans, Sure enough, the enclosure worked flawlessly with uas on a machine with an Intel xhci controller (2012 MacBook Air running Linux with kernel 3.15.3). I look forward to learning your results with the Etron controller. Best, Jonathan On Fri, 11 Jul 2014 11:36:28 +0200 Hans de Goede wrote: > Etron, that is the first time I've heard of them. So that is the 5th > manufacturer > which is making xhci controllers now (the others are nec, intel, via and > designware). > > I've managed to find a pci-e add-on card using the same Etron chipset as you > have, > and I've ordered one, so that I can try and reproduce this problem. It will be > aprox. 3 weeks before I get it though. > > In the mean time, can you try the usb disk enclosure on a machine with a nec > or > intel xhci controller ? So that we can confirm that this is indeed a problem > specific to the Etron controller? > > Regards, > > Hans -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/8] Drivers: scsi: storvsc: Filter WRITE_SAME_16
> -Original Message-
> From: h...@infradead.org [mailto:h...@infradead.org]
> Sent: Thursday, July 10, 2014 11:32 PM
> To: James Bottomley
> Cc: KY Srinivasan; linux-ker...@vger.kernel.org; m...@mkp.net;
> h...@infradead.org; de...@linuxdriverproject.org; a...@canonical.com;
> sta...@vger.kernel.org; linux-scsi@vger.kernel.org; oher...@suse.com;
> jasow...@redhat.com
> Subject: Re: [PATCH 4/8] Drivers: scsi: storvsc: Filter WRITE_SAME_16
>
> On Wed, Jul 09, 2014 at 10:27:24PM +, James Bottomley wrote:
> > If we fix it at source, why would there be any need to filter? That's
> > the reason the no_write_same flag was introduced. If we can find and
> > fix the bug, it can go back into the stable trees as a bug fix, hence
> > nothing should ever emit write_same(10 or 16) and additional driver
> > code is redundant (and counter productive, since if this ever breaks
> > again you're our best canary).
> >
> > This looks like it might be the problem but Martin should confirm (I
> > think the problem comes to us from the RC16 code which unconditionally
> > sets WS16).
>
> I think the problem is a differnet one. If we have the logical provisioning
> EVPD it configures what method to use, but if we don't have one we simply
> check for a max unmap blocks field, and if that's not present use WRITE
> SAME.
>
> The patch checks the no_write_same flag before doing that, for which we
> also have to do the write_same setup before the discard setup in
> sd_revalidate_disk.
>
> Ky: does hyperv support UNMAP? If so any idea why it doesn't set the
> maximum unmap block count field in the EVPD?
Windows hosts do support UNMAP and set the field in the EVPD. However, since
the host
advertises SPC-2 compliance, Linux does not even query the VPD page.
>
> If we want to enable UNMAP in this case I'd prefer a blacklist entry than
> trying UNMAP despite the device not advertising it.
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index ba756b1..fbccfd2
> 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2614,9 +2614,10 @@ static void sd_read_block_limits(struct scsi_disk
> *sdkp)
>
> if (sdkp->max_unmap_blocks)
> sd_config_discard(sdkp, SD_LBP_UNMAP);
> - else
> + else if (!sdkp->device->no_write_same)
> sd_config_discard(sdkp, SD_LBP_WS16);
> -
> + else
> + sd_config_discard(sdkp, SD_LBP_DISABLE);
> } else {/* LBP VPD page tells us what to use */
>
> if (sdkp->lbpu && sdkp->max_unmap_blocks) @@ -
> 2766,6 +2767,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
>*/
> if (sdkp->media_present) {
> sd_read_capacity(sdkp, buffer);
> + sd_read_write_same(sdkp, buffer);
>
> if (sd_try_extended_inquiry(sdp)) {
> sd_read_block_provisioning(sdkp);
> @@ -2776,7 +2778,6 @@ static int sd_revalidate_disk(struct gendisk *disk)
> sd_read_write_protect_flag(sdkp, buffer);
> sd_read_cache_type(sdkp, buffer);
> sd_read_app_tag_own(sdkp, buffer);
> - sd_read_write_same(sdkp, buffer);
> }
>
> sdkp->first_scan = 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/8] Drivers: scsi: storvsc: Filter WRITE_SAME_16
> -Original Message- > From: Martin K. Petersen [mailto:martin.peter...@oracle.com] > Sent: Friday, July 11, 2014 5:54 AM > To: h...@infradead.org > Cc: James Bottomley; KY Srinivasan; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; a...@canonical.com; sta...@vger.kernel.org; > linux-scsi@vger.kernel.org; oher...@suse.com; jasow...@redhat.com > Subject: Re: [PATCH 4/8] Drivers: scsi: storvsc: Filter WRITE_SAME_16 > > > "hch" == hch@infradead org writes: > > (Back from vacation: Bear with me while I'm catching up on two weeks of > linux-scsi stuff...) > > hch> I think the problem is a differnet one. If we have the logical > hch> provisioning EVPD it configures what method to use, but if we don't > hch> have one we simply check for a max unmap blocks field, and if > hch> that's not present use WRITE SAME. > > Yeah, that was done to accommodate the devices out there that predate the > LBP VPD. There sadly are several still. And it's hard to motivate people to > update their storage array firmware even when updates are readily available. > > hch> The patch checks the no_write_same flag before doing that, for > hch> which we also have to do the write_same setup before the discard > hch> setup in sd_revalidate_disk. > > The no_write_same was introduced to disable the REQ_WRITE_SAME use > case where we have no INQUIRY/READ CAPACITY/VPD flags to key off of to > determine whether it is safe to send the commands. > > no_write_same does not preclude the REQ_DISCARD variants of > WRITE_SAME. Those are gated by the discard heuristics exclusively. > > So first of all I'd like to know whether it's a WRITE SAME(16) or a WRITE > SAME(16) with the UNMAP bit set that's coming down. > > hch> Ky: does hyperv support UNMAP? If so any idea why it doesn't set > hch> the maximum unmap block count field in the EVPD? > > We shouldn't be sending down WRITE SAME(16) with the UNMAP bit set > unless the device sets LBPME=1 in the READ CAPACITY(16) response. > > So what does the storsvc report as its thin provisioning capabilities? Given that our current Host (ws2012 r2) advertises SPC-2 compliance, Linux does not even query this page. We support UNMAP. We just prototyped a host where we advertised SPC-3 compliance and Linux queries the EVPD page and does not use WRITE_SAME_16 K. Y -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: mpt2sas stuck installing
> -Original Message- > From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- > ow...@vger.kernel.org] On Behalf Of Joe Lawrence ... > In your crash stack trace, the scsi error handler has issued a host > reset, but then crashed in mpt2sas_base_get_iocstate. Reading through > _scsih_shutdown, I don't believe that the mpt2sas .shutdown path takes > any precaution before heading down mpt2sas_base_detach to free adapter > resources. The ordinary .remove path looks similar, though it does > call sas_remove_host before freeing resources, *then* scsi_remove_host > and scsi_host_put. > > I wonder if this ordering needs to be reversed (and added to > _scsih_shutdown) to properly de-register from the SCSI midlayer prior > to removing the controller instance. > > Regards, > > -- Joe Nagalakshmi was working on an mpt3sas patch for the scsi-mq tree to do just that. I don't recall if the patch has made it into the scsi-mq tree yet. Apparently it's also needed for non-mq and mpt2sas. It is making this change: > sas_remove_host(shost); > + scsi_remove_host(shost); > mpt3sas_base_detach(ioc); > list_del(&ioc->list); > - scsi_remove_host(shost); > scsi_host_put(shost); We are making a similar change in hpsa. Doing so led to a general protection fault, which unveiled that we also needed to change cancel_delayed_work() calls to cancel_delayed_work_sync() to ensure there are no worker functions still active after the scsi_host structure is unregistered. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: eata - issue appeared in Linus git master in last 24-48 hours
Christoph Hellwig wrote, on 11/07/14 18:50:
On Mon, Jun 30, 2014 at 04:31:33AM +0930, Arthur Marsh wrote:
Hi, I haven't had time to do a git bisect yet, but just saw this after
rebuilding the kernel in the last day or so:
It seems like some of the routines called during the driver
initialization may sleep while the driver_lock is held and irqs are
disabled.
As eata2x_detect is only called during module load the lock seems
entirely pointless and should be removed, like in the patch below:
diff --git a/drivers/scsi/eata.c b/drivers/scsi/eata.c
index 03372cf..980898e 100644
--- a/drivers/scsi/eata.c
+++ b/drivers/scsi/eata.c
@@ -837,7 +837,6 @@ struct hostdata {
static struct Scsi_Host *sh[MAX_BOARDS];
static const char *driver_name = "EATA";
static char sha[MAX_BOARDS];
-static DEFINE_SPINLOCK(driver_lock);
/* Initialize num_boards so that ihdlr can work while detect is in progress */
static unsigned int num_boards = MAX_BOARDS;
@@ -1097,8 +1096,6 @@ static int port_detect(unsigned long port_base, unsigned
int j,
goto fail;
}
- spin_lock_irq(&driver_lock);
-
if (do_dma(port_base, 0, READ_CONFIG_PIO)) {
#if defined(DEBUG_DETECT)
printk("%s: detect, do_dma failed at 0x%03lx.\n", name,
@@ -1265,10 +1262,7 @@ static int port_detect(unsigned long port_base, unsigned
int j,
}
#endif
- spin_unlock_irq(&driver_lock);
sh[j] = shost = scsi_register(tpnt, sizeof(struct hostdata));
- spin_lock_irq(&driver_lock);
-
if (shost == NULL) {
printk("%s: unable to register host, detaching.\n", name);
goto freedma;
@@ -1345,8 +1339,6 @@ static int port_detect(unsigned long port_base, unsigned
int j,
else
sprintf(dma_name, "DMA %u", dma_channel);
- spin_unlock_irq(&driver_lock);
-
for (i = 0; i < shost->can_queue; i++)
ha->cp[i].cp_dma_addr = pci_map_single(ha->pdev,
&ha->cp[i],
@@ -1439,7 +1431,6 @@ static int port_detect(unsigned long port_base, unsigned
int j,
freeirq:
free_irq(irq, &sha[j]);
freelock:
- spin_unlock_irq(&driver_lock);
release_region(port_base, REGION_SIZE);
fail:
return 0;
Thanks, I've rebuilt the kernel with this patch applied and running the
rebuilt kernel fine using a DPT 2044W SCSI adaptor:
$ lspci|grep DPT
00:0c.0 SCSI storage controller: Adaptec (formerly DPT) SmartCache/Raid
I-IV Controller (rev 02)
$ dmesg|grep -i eata
[1.038968] EATA0: warning, DMA protocol support not asserted.
[1.039041] EATA0: IRQ 11 mapped to IO-APIC IRQ 16.
[1.040801] EATA/DMA 2.0x: Copyright (C) 1994-2003 Dario Ballabio.
[1.040861] EATA config options -> tm:1, lc:y, mq:16, rs:y, et:n,
ip:n, ep:n, pp:y.
[1.040922] EATA0: 2.0C, PCI 0x7410, IRQ 16, BMST, SG 122, MB 64.
[1.040973] EATA0: wide SCSI support enabled, max_id 16, max_lun 8.
[1.041025] EATA0: SCSI channel 0 enabled, host target ID 7.
[1.041095] scsi2 : EATA/DMA 2.0x rev. 8.10.00
Arthur.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
