[MINI SUMMIT] SCSI core performance
Hi KS-PCs, I'd like to propose a SCSI performance mini-summit to see how interested folks are in helping address the long-term issues that SCSI core is currently facing wrt to multi-lun per host and heavy small block random I/O workloads. I know this would probably be better suited for LSF (for the record it was proposed this year) but now that we've acknowledge there is a problem with SCSI LLDs vs. raw block drivers vs. other SCSI subsystems, it would be useful to get the storage folks into a single room at some point during KS/LPC to figure out what is actually going on with SCSI core. As mentioned in the recent tcm_vhost thread, there are a number of cases where drivers/target/ code can demonstrate this limitation pretty vividly now. This includes the following scenarios using raw block flash export with target_core_mod + target_core_iblock export and the same small block (4k) mixed random I/O workload with fio: *) tcm_loop local SCSI LLD performance is an order of magnitude slower than the same local raw block flash backend. *) tcm_qla2xxx performs better using MSFT Server hosts than Linux v3.x based hosts using 2x socket Nehalem hardware w/ PCI-e Gen2 HBAs *) ib_srpt performs better using MSFT Server host than RHEL 6.x .32 based hosts using 2x socket Romley hardware w/ PCI-e Gen3 HCAs *) Raw block IBLOCK export into KVM guest v3.5-rc w/ virtio-scsi is behind in performance vs. raw local block flash. (cmwq on the host is helping here, but still need to with MSFT SCSI mini-port) Also, with 1M IOPs into a single VM guest now being done by other non Linux based hypervisors, the virtualization bit for high performance KVM SCSI based storage is quickly coming on.. So all of that said, I'd like to at least have a discussion with the key SCSI + block folks who will be present in San Diego on path forward to address these without having to wait until LSF-2013 + hope for a topic slot to materialize then. Thank you for your consideration, --nab -- 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 RESEND 0/5] Add vhost-blk support
On 07/17/2012 11:09 PM, Michael S. Tsirkin wrote: On Fri, Jul 13, 2012 at 04:55:06PM +0800, Asias He wrote: Hi folks, [I am resending to fix the broken thread in the previous one.] This patchset adds vhost-blk support. vhost-blk is a in kernel virito-blk device accelerator. Compared to userspace virtio-blk implementation, vhost-blk gives about 5% to 15% performance improvement. Same thing as tcm_host comment: It seems not 100% clear whether this driver will have major userspace using it. And if not, it would be very hard to support a driver when recent userspace does not use it in the end. I think a good idea for 3.6 would be to make it depend on CONFIG_STAGING. Then we don't commit to an ABI. For this, you can add a separate Kconfig and source it from drivers/staging/Kconfig. Maybe it needs to be in a separate directory drivers/vhost/staging/Kconfig. OK. I Cc'd the list of tcm_host in the hope that you can cooperate on this. Sure. -- Asias -- 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 5/5] megaraid_sas: Version and Changelog update
James/linux-scsi, The following patch for megaraid_sas updates the driver version and Documentation/scsi/ChangeLog.megaraid_sas files. Signed-off-by: Adam Radford diff -Naur scsi/Documentation/scsi/ChangeLog.megaraid_sas scsi.new/Documentation/scsi/ChangeLog.megaraid_sas --- scsi/Documentation/scsi/ChangeLog.megaraid_sas 2012-07-17 14:06:21.0 -0700 +++ scsi.new/Documentation/scsi/ChangeLog.megaraid_sas 2012-07-17 15:07:27.368388750 -0700 @@ -1,3 +1,13 @@ +Release Date: Tue. Jun 17, 2012 17:00:00 PST 2012 - + (emaild-id:megaraidli...@lsi.com) + Adam Radford/Kashyap Desai +Current Version : 00.00.06.18-rc1 +Old Version : 00.00.06.15-rc1 +1. Fix Copyright dates. +2. Add throttlequeuedepth module parameter. +3. Add resetwaittime module parameter. +4. Move poll_aen_lock initializer. +--- Release Date: Mon. Mar 19, 2012 17:00:00 PST 2012 - (emaild-id:megaraidli...@lsi.com) Adam Radford diff -Naur scsi/drivers/scsi/megaraid/megaraid_sas_base.c scsi.new/drivers/scsi/megaraid/megaraid_sas_base.c --- scsi/drivers/scsi/megaraid/megaraid_sas_base.c 2012-07-17 15:01:15.136294781 -0700 +++ scsi.new/drivers/scsi/megaraid/megaraid_sas_base.c 2012-07-17 15:01:50.296168198 -0700 @@ -18,7 +18,7 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA * * FILE: megaraid_sas_base.c - * Version : v00.00.06.15-rc1 + * Version : v00.00.06.18-rc1 * * Authors: LSI Corporation * Sreenivas Bagalkote diff -Naur scsi/drivers/scsi/megaraid/megaraid_sas.h scsi.new/drivers/scsi/megaraid/megaraid_sas.h --- scsi/drivers/scsi/megaraid/megaraid_sas.h 2012-07-17 14:44:04.342294076 -0700 +++ scsi.new/drivers/scsi/megaraid/megaraid_sas.h 2012-07-17 15:02:19.715232696 -0700 @@ -33,9 +33,9 @@ /* * MegaRAID SAS Driver meta data */ -#define MEGASAS_VERSION"00.00.06.15-rc1" -#define MEGASAS_RELDATE"Mar. 19, 2012" -#define MEGASAS_EXT_VERSION"Mon. Mar. 19 17:00:00 PDT 2012" +#define MEGASAS_VERSION"00.00.06.18-rc1" +#define MEGASAS_RELDATE"Jun. 17, 2012" +#define MEGASAS_EXT_VERSION"Tue. Jun. 17 17:00:00 PDT 2012" /* * Device IDs megaraid_sas.patch5 Description: Binary data
[PATCH 4/5] megaraid_sas: Move poll_aen_lock initializer
Cc: stable James/linux-scsi, The following patch from Kashyap Desai for megaraid_sas moves the poll_aen_lock initializer from megasas_probe_one() to megasas_init(). This prevents a crash when a user loads the driver and tries to issue a poll() system call on the ioctl interface with no adapters present. Signed-off-by: Kashyap Desai Signed-off-by: Adam Radford diff -Naur scsi/drivers/scsi/megaraid/megaraid_sas_base.c scsi.new/drivers/scsi/megaraid/megaraid_sas_base.c --- scsi/drivers/scsi/megaraid/megaraid_sas_base.c 2012-07-17 14:57:32.890231627 -0700 +++ scsi.new/drivers/scsi/megaraid/megaraid_sas_base.c 2012-07-17 14:59:37.285232167 -0700 @@ -4095,7 +4095,6 @@ spin_lock_init(&instance->cmd_pool_lock); spin_lock_init(&instance->hba_lock); spin_lock_init(&instance->completion_lock); - spin_lock_init(&poll_aen_lock); mutex_init(&instance->aen_mutex); mutex_init(&instance->reset_mutex); @@ -5421,6 +5420,8 @@ printk(KERN_INFO "megasas: %s %s\n", MEGASAS_VERSION, MEGASAS_EXT_VERSION); + spin_lock_init(&poll_aen_lock); + support_poll_for_event = 2; support_device_change = 1; megaraid_sas.patch4 Description: Binary data
[PATCH 3/5] megaraid_sas: Add resetwaittime module parameter
James/linux-scsi, The following patch for megaraid_sas adds support for a resetwaittime module parameter. This allows a user to adjust the wait time in seconds after I/O timeout before resetting the adapter. Signed-off-by: Adam Radford diff -Naur scsi/drivers/scsi/megaraid/megaraid_sas_base.c scsi.new/drivers/scsi/megaraid/megaraid_sas_base.c --- scsi/drivers/scsi/megaraid/megaraid_sas_base.c 2012-07-17 14:44:04.341294660 -0700 +++ scsi.new/drivers/scsi/megaraid/megaraid_sas_base.c 2012-07-17 14:50:41.503232158 -0700 @@ -76,6 +76,11 @@ MODULE_PARM_DESC(throttlequeuedepth, "Adapter queue depth when throttled due to I/O timeout. Default: 16"); +int resetwaittime = MEGASAS_RESET_WAIT_TIME; +module_param(resetwaittime, int, S_IRUGO); +MODULE_PARM_DESC(resetwaittime, "Wait time in seconds after I/O timeout " +"before resetting adapter. Default: 180"); + MODULE_LICENSE("GPL"); MODULE_VERSION(MEGASAS_VERSION); MODULE_AUTHOR("megaraidli...@lsi.com"); @@ -1778,7 +1783,7 @@ return SUCCESS; } - for (i = 0; i < wait_time; i++) { + for (i = 0; i < resetwaittime; i++) { int outstanding = atomic_read(&instance->fw_outstanding); diff -Naur scsi/drivers/scsi/megaraid/megaraid_sas_fusion.c scsi.new/drivers/scsi/megaraid/megaraid_sas_fusion.c --- scsi/drivers/scsi/megaraid/megaraid_sas_fusion.c2012-07-17 14:33:50.682231505 -0700 +++ scsi.new/drivers/scsi/megaraid/megaraid_sas_fusion.c2012-07-17 14:54:56.487358768 -0700 @@ -94,6 +94,7 @@ void megaraid_sas_kill_hba(struct megasas_instance *instance); extern u32 megasas_dbg_lvl; +extern int resetwaittime; /** * megasas_enable_intr_fusion -Enables interrupts @@ -2063,9 +2064,9 @@ int megasas_wait_for_outstanding_fusion(struct megasas_instance *instance) { int i, outstanding, retval = 0; - u32 fw_state, wait_time = MEGASAS_RESET_WAIT_TIME; + u32 fw_state; - for (i = 0; i < wait_time; i++) { + for (i = 0; i < resetwaittime; i++) { /* Check if firmware is in fault state */ fw_state = instance->instancet->read_fw_status_reg( instance->reg_set) & MFI_STATE_MASK; megaraid_sas.patch3 Description: Binary data
[PATCH 2/5] megaraid_sas: Add throttlequeuedepth module parameter
James/linux-scsi, The following patch for megaraid_sas adds a throttlequeuedepth module parameter. This allows a user to adjust the queue depth of the adapter when throttled due to I/O timeout. Signed-off-by: Adam Radford diff -Naur scsi/drivers/scsi/megaraid/megaraid_sas_base.c scsi.new/drivers/scsi/megaraid/megaraid_sas_base.c --- scsi/drivers/scsi/megaraid/megaraid_sas_base.c 2012-07-17 14:33:50.681233390 -0700 +++ scsi.new/drivers/scsi/megaraid/megaraid_sas_base.c 2012-07-17 14:40:59.507233383 -0700 @@ -71,6 +71,11 @@ module_param(msix_disable, int, S_IRUGO); MODULE_PARM_DESC(msix_disable, "Disable MSI-X interrupt handling. Default: 0"); +static int throttlequeuedepth = MEGASAS_THROTTLE_QUEUE_DEPTH; +module_param(throttlequeuedepth, int, S_IRUGO); +MODULE_PARM_DESC(throttlequeuedepth, + "Adapter queue depth when throttled due to I/O timeout. Default: 16"); + MODULE_LICENSE("GPL"); MODULE_VERSION(MEGASAS_VERSION); MODULE_AUTHOR("megaraidli...@lsi.com"); @@ -1595,8 +1600,9 @@ { unsigned long flags; if (instance->flag & MEGASAS_FW_BUSY - && time_after(jiffies, instance->last_time + 5 * HZ) - && atomic_read(&instance->fw_outstanding) < 17) { + && time_after(jiffies, instance->last_time + 5 * HZ) + && atomic_read(&instance->fw_outstanding) < + instance->throttlequeuedepth + 1) { spin_lock_irqsave(instance->host->host_lock, flags); instance->flag &= ~MEGASAS_FW_BUSY; @@ -1914,7 +1920,7 @@ /* FW is busy, throttle IO */ spin_lock_irqsave(instance->host->host_lock, flags); - instance->host->can_queue = 16; + instance->host->can_queue = instance->throttlequeuedepth; instance->last_time = jiffies; instance->flag |= MEGASAS_FW_BUSY; @@ -3577,6 +3583,24 @@ kfree(ctrl_info); + /* Check for valid throttlequeuedepth module parameter */ + if (instance->pdev->device == PCI_DEVICE_ID_LSI_SAS0073SKINNY || + instance->pdev->device == PCI_DEVICE_ID_LSI_SAS0071SKINNY) { + if (throttlequeuedepth > (instance->max_fw_cmds - + MEGASAS_SKINNY_INT_CMDS)) + instance->throttlequeuedepth = + MEGASAS_THROTTLE_QUEUE_DEPTH; + else + instance->throttlequeuedepth = throttlequeuedepth; + } else { + if (throttlequeuedepth > (instance->max_fw_cmds - + MEGASAS_INT_CMDS)) + instance->throttlequeuedepth = + MEGASAS_THROTTLE_QUEUE_DEPTH; + else + instance->throttlequeuedepth = throttlequeuedepth; + } + /* * Setup tasklet for cmd completion */ diff -Naur scsi/drivers/scsi/megaraid/megaraid_sas.h scsi.new/drivers/scsi/megaraid/megaraid_sas.h --- scsi/drivers/scsi/megaraid/megaraid_sas.h 2012-07-17 14:33:50.683231556 -0700 +++ scsi.new/drivers/scsi/megaraid/megaraid_sas.h 2012-07-17 14:42:44.066172702 -0700 @@ -747,6 +747,7 @@ #defineMEGASAS_RESET_NOTICE_INTERVAL 5 #define MEGASAS_IOCTL_CMD 0 #define MEGASAS_DEFAULT_CMD_TIMEOUT90 +#define MEGASAS_THROTTLE_QUEUE_DEPTH 16 /* * FW reports the maximum of number of commands that it can accept (maximum @@ -1364,6 +1365,7 @@ unsigned long bar; long reset_flags; struct mutex reset_mutex; + int throttlequeuedepth; }; enum { megaraid_sas.patch2 Description: Binary data
[PATCH 1/5] megaraid_sas: Fix Copyright dates
James/linux-scsi, The following patch for megaraid_sas fixes the Copyright dates. Signed-off-by: Adam Radford diff -Naur scsi/drivers/scsi/megaraid/megaraid_sas_base.c scsi.new/drivers/scsi/megaraid/megaraid_sas_base.c --- scsi/drivers/scsi/megaraid/megaraid_sas_base.c 2012-07-17 14:06:30.0 -0700 +++ scsi.new/drivers/scsi/megaraid/megaraid_sas_base.c 2012-07-17 14:30:14.102231138 -0700 @@ -1,7 +1,7 @@ /* * Linux MegaRAID driver for SAS based RAID controllers * - * Copyright (c) 2009-2011 LSI Corporation. + * Copyright (c) 2003-2012 LSI Corporation. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License diff -Naur scsi/drivers/scsi/megaraid/megaraid_sas_fp.c scsi.new/drivers/scsi/megaraid/megaraid_sas_fp.c --- scsi/drivers/scsi/megaraid/megaraid_sas_fp.c2012-07-17 14:06:30.0 -0700 +++ scsi.new/drivers/scsi/megaraid/megaraid_sas_fp.c2012-07-17 14:30:31.471232644 -0700 @@ -1,7 +1,7 @@ /* * Linux MegaRAID driver for SAS based RAID controllers * - * Copyright (c) 2009-2011 LSI Corporation. + * Copyright (c) 2009-2012 LSI Corporation. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License diff -Naur scsi/drivers/scsi/megaraid/megaraid_sas_fusion.c scsi.new/drivers/scsi/megaraid/megaraid_sas_fusion.c --- scsi/drivers/scsi/megaraid/megaraid_sas_fusion.c2012-07-17 14:06:30.0 -0700 +++ scsi.new/drivers/scsi/megaraid/megaraid_sas_fusion.c2012-07-17 14:30:46.026232550 -0700 @@ -1,7 +1,7 @@ /* * Linux MegaRAID driver for SAS based RAID controllers * - * Copyright (c) 2009-2011 LSI Corporation. + * Copyright (c) 2009-2012 LSI Corporation. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License diff -Naur scsi/drivers/scsi/megaraid/megaraid_sas_fusion.h scsi.new/drivers/scsi/megaraid/megaraid_sas_fusion.h --- scsi/drivers/scsi/megaraid/megaraid_sas_fusion.h2012-07-17 14:06:30.0 -0700 +++ scsi.new/drivers/scsi/megaraid/megaraid_sas_fusion.h2012-07-17 14:31:04.978232551 -0700 @@ -1,7 +1,7 @@ /* * Linux MegaRAID driver for SAS based RAID controllers * - * Copyright (c) 2009-2011 LSI Corporation. + * Copyright (c) 2009-2012 LSI Corporation. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License diff -Naur scsi/drivers/scsi/megaraid/megaraid_sas.h scsi.new/drivers/scsi/megaraid/megaraid_sas.h --- scsi/drivers/scsi/megaraid/megaraid_sas.h 2012-07-17 14:06:30.0 -0700 +++ scsi.new/drivers/scsi/megaraid/megaraid_sas.h 2012-07-17 14:31:19.255388683 -0700 @@ -1,7 +1,7 @@ /* * Linux MegaRAID driver for SAS based RAID controllers * - * Copyright (c) 2009-2011 LSI Corporation. + * Copyright (c) 2003-2012 LSI Corporation. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License megaraid_sas.patch1 Description: Binary data
[PATCH 0/5] megaraid_sas: Updates for scsi for-next
James/linux-scsi, The following patch series for megaraid_sas brings the driver up to v6.18-rc1: 1. Fix Copyright dates. 2. Add throttlequeuedepth module parameter. 3. Add resetwaittime module parameter. 4. Move poll_aen_lock initializer. 5. Version and Changelog update. -Adam -- 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-v3 4/4] tcm_vhost: Initial merge for vhost level target fabric driver
From: Nicholas Bellinger This patch adds the initial code for tcm_vhost, a Vhost level TCM fabric driver for virtio SCSI initiators into KVM guest. This code is currently up and running on v3.5-rc2 host+guest along with the virtio-scsi vdev->scan() patch to allow a proper scsi_scan_host() to occur once the tcm_vhost nexus has been established by the paravirtualized virtio-scsi client here: virtio-scsi: Add vdrv->scan for post VIRTIO_CONFIG_S_DRIVER_OK LUN scanning http://marc.info/?l=linux-scsi&m=134160609212542&w=2 Using tcm_vhost requires Zhi's -> Stefan's qemu vhost-scsi tree here: https://github.com/wuzhy/qemu/tree/vhost-scsi along with the recent QEMU patch to hw/virtio-scsi.c to set max_target=0 during vhost-scsi operation. Changelog v2 -> v3: Unlock on error in tcm_vhost_drop_nexus() (DanC) Fix strlen() doesn't count the terminator (DanC) Call kfree() on an error path (DanC) Convert tcm_vhost_write_pending to use target_execute_cmd (hch + nab) Fix another strlen() off by one in tcm_vhost_make_tport (DanC) Add option under drivers/staging/Kconfig, and move to drivers/vhost/tcm/ as requested by MST (nab) Changelog v1 -> v2: Fix tv_cmd completion -> release SGL memory leak (nab) Fix sparse warnings for static variable usage ((Fengguang Wu) Fix sparse warnings for min() typing + printk format specs (Fengguang Wu) Convert to cmwq submission for I/O dispatch (nab + hch) Changelog v0 -> v1: Merge into single source + header file, and move to drivers/vhost/ Cc: Stefan Hajnoczi Cc: Zhi Yong Wu Cc: Michael S. Tsirkin Cc: Paolo Bonzini Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Jens Axboe Signed-off-by: Nicholas Bellinger --- drivers/staging/Kconfig |2 + drivers/vhost/Makefile|2 + drivers/vhost/tcm/Kconfig |6 + drivers/vhost/tcm/Makefile|1 + drivers/vhost/tcm/tcm_vhost.c | 1611 + drivers/vhost/tcm/tcm_vhost.h | 74 ++ 6 files changed, 1696 insertions(+), 0 deletions(-) create mode 100644 drivers/vhost/tcm/Kconfig create mode 100644 drivers/vhost/tcm/Makefile create mode 100644 drivers/vhost/tcm/tcm_vhost.c create mode 100644 drivers/vhost/tcm/tcm_vhost.h diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig index 05e33c7..8d1a627 100644 --- a/drivers/staging/Kconfig +++ b/drivers/staging/Kconfig @@ -132,4 +132,6 @@ source "drivers/staging/ipack/Kconfig" source "drivers/staging/gdm72xx/Kconfig" +source "drivers/vhost/tcm/Kconfig" + endif # STAGING diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile index 72dd020..3408bea 100644 --- a/drivers/vhost/Makefile +++ b/drivers/vhost/Makefile @@ -1,2 +1,4 @@ obj-$(CONFIG_VHOST_NET) += vhost_net.o vhost_net-y := vhost.o net.o + +obj-$(CONFIG_TCM_VHOST) += tcm/ diff --git a/drivers/vhost/tcm/Kconfig b/drivers/vhost/tcm/Kconfig new file mode 100644 index 000..a9c6f76 --- /dev/null +++ b/drivers/vhost/tcm/Kconfig @@ -0,0 +1,6 @@ +config TCM_VHOST + tristate "TCM_VHOST fabric module (EXPERIMENTAL)" + depends on TARGET_CORE && EVENTFD && EXPERIMENTAL && m + default n + ---help--- + Say M here to enable the TCM_VHOST fabric module for use with virtio-scsi guests diff --git a/drivers/vhost/tcm/Makefile b/drivers/vhost/tcm/Makefile new file mode 100644 index 000..54b0ea6 --- /dev/null +++ b/drivers/vhost/tcm/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_TCM_VHOST) += tcm_vhost.o diff --git a/drivers/vhost/tcm/tcm_vhost.c b/drivers/vhost/tcm/tcm_vhost.c new file mode 100644 index 000..0ee4046 --- /dev/null +++ b/drivers/vhost/tcm/tcm_vhost.c @@ -0,0 +1,1611 @@ +/*** + * Vhost kernel TCM fabric driver for virtio SCSI initiators + * + * (C) Copyright 2010-2012 RisingTide Systems LLC. + * (C) Copyright 2010-2012 IBM Corp. + * + * Licensed to the Linux Foundation under the General Public License (GPL) version 2. + * + * Authors: Nicholas A. Bellinger + * Stefan Hajnoczi + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + / + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include /* TODO vhost.h currently depends on this */ +#include + +#in
[RFC-v3 3/4] vhost: Add vhost_scsi specific defines
From: Nicholas Bellinger This patch adds the initial vhost_scsi_ioctl() callers for VHOST_SCSI_SET_ENDPOINT and VHOST_SCSI_CLEAR_ENDPOINT respectively, and also adds struct vhost_vring_target that is used by tcm_vhost code when locating target ports during qemu setup. Signed-off-by: Stefan Hajnoczi Cc: Zhi Yong Wu Cc: Michael S. Tsirkin Cc: Paolo Bonzini , Signed-off-by: Nicholas A. Bellinger --- include/linux/vhost.h |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/include/linux/vhost.h b/include/linux/vhost.h index e847f1e..33b313b 100644 --- a/include/linux/vhost.h +++ b/include/linux/vhost.h @@ -24,7 +24,11 @@ struct vhost_vring_state { struct vhost_vring_file { unsigned int index; int fd; /* Pass -1 to unbind from file. */ +}; +struct vhost_vring_target { + unsigned char vhost_wwpn[224]; + unsigned short vhost_tpgt; }; struct vhost_vring_addr { @@ -121,6 +125,11 @@ struct vhost_memory { * device. This can be used to stop the ring (e.g. for migration). */ #define VHOST_NET_SET_BACKEND _IOW(VHOST_VIRTIO, 0x30, struct vhost_vring_file) +/* VHOST_SCSI specific defines */ + +#define VHOST_SCSI_SET_ENDPOINT _IOW(VHOST_VIRTIO, 0x40, struct vhost_vring_target) +#define VHOST_SCSI_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x41, struct vhost_vring_target) + /* Feature bits */ /* Log all write descriptors. Can be changed while device is active. */ #define VHOST_F_LOG_ALL 26 -- 1.7.2.5 -- 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-v3 2/4] vhost: make vhost work queue visible
From: Stefan Hajnoczi The vhost work queue allows processing to be done in vhost worker thread context, which uses the owner process mm. Access to the vring and guest memory is typically only possible from vhost worker context so it is useful to allow work to be queued directly by users. Currently vhost_net only uses the poll wrappers which do not expose the work queue functions. However, for tcm_vhost (vhost_scsi) it will be necessary to queue custom work. Signed-off-by: Stefan Hajnoczi Cc: Zhi Yong Wu Cc: Michael S. Tsirkin Cc: Paolo Bonzini Signed-off-by: Nicholas Bellinger --- drivers/vhost/vhost.c |5 ++--- drivers/vhost/vhost.h |3 +++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 94dbd25..1aab08b 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -64,7 +64,7 @@ static int vhost_poll_wakeup(wait_queue_t *wait, unsigned mode, int sync, return 0; } -static void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn) +void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn) { INIT_LIST_HEAD(&work->node); work->fn = fn; @@ -137,8 +137,7 @@ void vhost_poll_flush(struct vhost_poll *poll) vhost_work_flush(poll->dev, &poll->work); } -static inline void vhost_work_queue(struct vhost_dev *dev, - struct vhost_work *work) +void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work) { unsigned long flags; diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 07b9763..1125af3 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -43,6 +43,9 @@ struct vhost_poll { struct vhost_dev *dev; }; +void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn); +void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work); + void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn, unsigned long mask, struct vhost_dev *dev); void vhost_poll_start(struct vhost_poll *poll, struct file *file); -- 1.7.2.5 -- 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-v3 1/4] vhost: Separate vhost-net features from vhost features
From: Stefan Hajnoczi In order for other vhost devices to use the VHOST_FEATURES bits the vhost-net specific bits need to be moved to their own VHOST_NET_FEATURES constant. (Asias: Update drivers/vhost/test.c to use VHOST_NET_FEATURES) Signed-off-by: Stefan Hajnoczi Cc: Zhi Yong Wu Cc: Michael S. Tsirkin Cc: Paolo Bonzini Cc: Asias He Signed-off-by: Nicholas A. Bellinger --- drivers/vhost/net.c |4 ++-- drivers/vhost/test.c |4 ++-- drivers/vhost/vhost.h |3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index f82a739..072cbba 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -823,14 +823,14 @@ static long vhost_net_ioctl(struct file *f, unsigned int ioctl, return -EFAULT; return vhost_net_set_backend(n, backend.index, backend.fd); case VHOST_GET_FEATURES: - features = VHOST_FEATURES; + features = VHOST_NET_FEATURES; if (copy_to_user(featurep, &features, sizeof features)) return -EFAULT; return 0; case VHOST_SET_FEATURES: if (copy_from_user(&features, featurep, sizeof features)) return -EFAULT; - if (features & ~VHOST_FEATURES) + if (features & ~VHOST_NET_FEATURES) return -EOPNOTSUPP; return vhost_net_set_features(n, features); case VHOST_RESET_OWNER: diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c index 3de00d9..91d6f06 100644 --- a/drivers/vhost/test.c +++ b/drivers/vhost/test.c @@ -261,14 +261,14 @@ static long vhost_test_ioctl(struct file *f, unsigned int ioctl, return -EFAULT; return vhost_test_run(n, test); case VHOST_GET_FEATURES: - features = VHOST_FEATURES; + features = VHOST_NET_FEATURES; if (copy_to_user(featurep, &features, sizeof features)) return -EFAULT; return 0; case VHOST_SET_FEATURES: if (copy_from_user(&features, featurep, sizeof features)) return -EFAULT; - if (features & ~VHOST_FEATURES) + if (features & ~VHOST_NET_FEATURES) return -EOPNOTSUPP; return vhost_test_set_features(n, features); case VHOST_RESET_OWNER: diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h index 8de1fd5..07b9763 100644 --- a/drivers/vhost/vhost.h +++ b/drivers/vhost/vhost.h @@ -201,7 +201,8 @@ enum { VHOST_FEATURES = (1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) | (1ULL << VIRTIO_RING_F_INDIRECT_DESC) | (1ULL << VIRTIO_RING_F_EVENT_IDX) | -(1ULL << VHOST_F_LOG_ALL) | +(1ULL << VHOST_F_LOG_ALL), + VHOST_NET_FEATURES = VHOST_FEATURES | (1ULL << VHOST_NET_F_VIRTIO_NET_HDR) | (1ULL << VIRTIO_NET_F_MRG_RXBUF), }; -- 1.7.2.5 -- 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-v3 0/4] tcm_vhost+cmwq fabric driver code for-3.6
From: Nicholas Bellinger Hi folks, The following is the RFC-v3 series of tcm_vhost target fabric driver code currently in-flight for-3.6 mainline code. With the merge window opening soon, the tcm_vhost code has started seeing time in linux-next. The v2 -> v3 changelog from the last week is currently looking like: *) Unlock on error in tcm_vhost_drop_nexus() (DanC) *) Fix strlen() doesn't count the terminator (DanC) *) Call kfree() on an error path (DanC) *) Convert tcm_vhost_write_pending to use target_execute_cmd (hch + nab) *) Fix another strlen() off by one in tcm_vhost_make_tport (DanC) *) Add option under drivers/staging/Kconfig, and move to drivers/vhost/tcm/ as requested by MST (nab) Thanks to Dan Carpenter for his smatch fixes this past round. Also as requested by MST, the code has been moved to a seperate tcm/ subdirectory under drivers/vhost/ so that it can be included under staging's config options until we can settle on the necessary userspace bits for QEMU and kvm-tool. The updated series will be going out shortly to target-pending/for-next-merge. Please have another look and let us know if you have any concerned. Thanks! Nicholas Bellinger (2): vhost: Add vhost_scsi specific defines tcm_vhost: Initial merge for vhost level target fabric driver Stefan Hajnoczi (2): vhost: Separate vhost-net features from vhost features vhost: make vhost work queue visible drivers/staging/Kconfig |2 + drivers/vhost/Makefile|2 + drivers/vhost/net.c |4 +- drivers/vhost/tcm/Kconfig |6 + drivers/vhost/tcm/Makefile|1 + drivers/vhost/tcm/tcm_vhost.c | 1611 + drivers/vhost/tcm/tcm_vhost.h | 74 ++ drivers/vhost/test.c |4 +- drivers/vhost/vhost.c |5 +- drivers/vhost/vhost.h |6 +- include/linux/vhost.h |9 + 11 files changed, 1716 insertions(+), 8 deletions(-) create mode 100644 drivers/vhost/tcm/Kconfig create mode 100644 drivers/vhost/tcm/Makefile create mode 100644 drivers/vhost/tcm/tcm_vhost.c create mode 100644 drivers/vhost/tcm/tcm_vhost.h -- 1.7.2.5 -- 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-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
On Wed, 2012-07-18 at 02:11 +0300, Michael S. Tsirkin wrote: > On Tue, Jul 17, 2012 at 03:37:20PM -0700, Nicholas A. Bellinger wrote: > > On Wed, 2012-07-18 at 01:18 +0300, Michael S. Tsirkin wrote: > > > On Tue, Jul 17, 2012 at 03:02:08PM -0700, Nicholas A. Bellinger wrote: > > > > On Wed, 2012-07-18 at 00:34 +0300, Michael S. Tsirkin wrote: > > > > > > I don't see how it helps. The driver is either a guaranteed ABI or not. > > > I'd prefer not to have vhost users outside drivers/vhost/ since it is > > > harder for me to keep track of them. > > > > > > What's the problem with staging proposal? It's just another hoop to jump > > > through to enable it? > > > > > > > Yeah, I'm OK with just adding a CONFIG_STAGING tag is a reasonable step > > forward for-3.6. > > > > Adding the following patch into target-pending/for-next-merge now: > > > > diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig > > index ccbeb6f..2cd7135 100644 > > --- a/drivers/vhost/Kconfig > > +++ b/drivers/vhost/Kconfig > > @@ -11,7 +11,7 @@ config VHOST_NET > > > > config TCM_VHOST > > tristate "TCM_VHOST fabric module (EXPERIMENTAL)" > > - depends on TARGET_CORE && EVENTFD && EXPERIMENTAL && m > > + depends on TARGET_CORE && EVENTFD && EXPERIMENTAL && STAGING && m > > default n > > ---help--- > > Say M here to enable the TCM_VHOST fabric module for use with > > virtio-scsi guests > > > > > > Hmm that's not explicit enough, someone might enable CONFIG_STAGING for > some other reason and won't notice the dependency. > We need it to appear with other staging drivers in the menu, > so there needs to be a Kconfig that is included from > drivers/staging/Kconfig. > M, I am sensing a linux-next merge conflict with staging-next and/or another for-next-merge rebase coming on.. > For example, we can create > drivers/vhost/staging/Kconfig or drivers/vhost/tcm/Kconfig and include > it from drivers/staging/Kconfig. nouveau did something like this for a > while, see f3c93cbde7eab38671ae085cb1027b08f5f36757. > > No need to move the rest of the code. > OK, lets use drivers/vhost/tcm/Kconfig and I'll post a incremental patch to make it appear under staging it shortly. (CC'ing Greg-KH for good measure.) -- 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] target: Allow for target_submit_cmd() returning errors
On Tue, 2012-07-17 at 23:37 +, Nicholas A. Bellinger wrote: > From: Roland Dreier > > We want it to be possible for target_submit_cmd() to return errors up > to its fabric module callers. For now just update the prototype to > return an int, and update all callers to handle non-zero return values > as an error. > > This is immediately useful for tcm_qla2xxx to fix a long-standing active > I/O session shutdown race, but tcm_fc, usb-gadget, and sbp-target the > fabric maintainers need to check + ACK that handling a target_submit_cmd() > failure due to session shutdown does not introduce regressions > > (nab: Respin against for-next after initial NACK + update docbook comment) > + trimming CC to USB folks: > index 02ace18..9ddf315 100644 > --- a/drivers/usb/gadget/tcm_usb_gadget.c > +++ b/drivers/usb/gadget/tcm_usb_gadget.c > @@ -1060,7 +1060,10 @@ static void usbg_cmd_work(struct work_struct *work) > tpg = cmd->fu->tpg; > tv_nexus = tpg->tpg_nexus; > dir = get_cmd_dir(cmd->cmd_buf); > - if (dir < 0) { > + if (dir < 0 || > + target_submit_cmd(se_cmd, tv_nexus->tvn_se_sess, > + cmd->cmd_buf, cmd->sense_iu.sense, > cmd->unpacked_lun, > + 0, cmd->prio_attr, dir, TARGET_SCF_UNKNOWN_SIZE)) > { > transport_init_se_cmd(se_cmd, > tv_nexus->tvn_se_sess->se_tpg->se_tpg_tfo, > tv_nexus->tvn_se_sess, cmd->data_len, DMA_NONE, > @@ -1069,12 +1072,7 @@ static void usbg_cmd_work(struct work_struct *work) > transport_send_check_condition_and_sense(se_cmd, > TCM_UNSUPPORTED_SCSI_OPCODE, 1); > usbg_cleanup_cmd(cmd); > - return; > } > - > - target_submit_cmd(se_cmd, tv_nexus->tvn_se_sess, > - cmd->cmd_buf, cmd->sense_iu.sense, cmd->unpacked_lun, > - 0, cmd->prio_attr, dir, TARGET_SCF_UNKNOWN_SIZE); > } > > static int usbg_submit_command(struct f_uas *fu, > @@ -1172,7 +1170,10 @@ static void bot_cmd_work(struct work_struct *work) > tpg = cmd->fu->tpg; > tv_nexus = tpg->tpg_nexus; > dir = get_cmd_dir(cmd->cmd_buf); > - if (dir < 0) { > + if (dir < 0 || > + target_submit_cmd(se_cmd, tv_nexus->tvn_se_sess, > + cmd->cmd_buf, cmd->sense_iu.sense, > cmd->unpacked_lun, > + cmd->data_len, cmd->prio_attr, dir, 0)) { > transport_init_se_cmd(se_cmd, > tv_nexus->tvn_se_sess->se_tpg->se_tpg_tfo, > tv_nexus->tvn_se_sess, cmd->data_len, DMA_NONE, > @@ -1181,12 +1182,7 @@ static void bot_cmd_work(struct work_struct *work) > transport_send_check_condition_and_sense(se_cmd, > TCM_UNSUPPORTED_SCSI_OPCODE, 1); > usbg_cleanup_cmd(cmd); > - return; > } > - > - target_submit_cmd(se_cmd, tv_nexus->tvn_se_sess, > - cmd->cmd_buf, cmd->sense_iu.sense, cmd->unpacked_lun, > - cmd->data_len, cmd->prio_attr, dir, 0); > } > Looking at this part again target_submit_cmd() has been moved ahead of target_init_se_cmd() in the exception path, which ends up getting called twice during a target_get_sess_cmd() failure.. It looks like usbg_cmd_work() + bot_cmd_work() will need some work if they intends to use a non zero failure to signal exception here, without invoking a CHECK_CONDITION and sense. Actually, I'm not even sure sending a CHECK_CONDITION here after the target_submit_cmd() is going to work for other fabrics drivers, but it appears to work with usb-gadget. (Sebastian..?) So for the moment, lets fix the double se_cmd init and make things a little easier to read.. Sebastian, please have a look and double check that the (dir < 0) + target_submit_cmd() failures cases are both going to work as expected whilst sending a CHECK_CONDITION response. Thanks! --nab diff --git a/drivers/usb/gadget/tcm_usb_gadget.c b/drivers/usb/gadget/tcm_usb_gadget.c index 9ddf315..5444866 100644 --- a/drivers/usb/gadget/tcm_usb_gadget.c +++ b/drivers/usb/gadget/tcm_usb_gadget.c @@ -1060,19 +1060,25 @@ static void usbg_cmd_work(struct work_struct *work) tpg = cmd->fu->tpg; tv_nexus = tpg->tpg_nexus; dir = get_cmd_dir(cmd->cmd_buf); - if (dir < 0 || - target_submit_cmd(se_cmd, tv_nexus->tvn_se_sess, - cmd->cmd_buf, cmd->sense_iu.sense, cmd->unpacked_lun, - 0, cmd->prio_attr, dir, TARGET_SCF_UNKNOWN_SIZE)) { + if (dir < 0) { transport_init_se_cmd(se_cmd, tv_nexus->tvn_se_sess->se_tpg->se_tpg_tfo, tv_nexus->tvn_se_sess, cmd->data_len, DMA_NONE, cmd->prio_attr, cmd->sense_iu.sense); - -
[PATCH] target: Allow for target_submit_cmd() returning errors
From: Roland Dreier We want it to be possible for target_submit_cmd() to return errors up to its fabric module callers. For now just update the prototype to return an int, and update all callers to handle non-zero return values as an error. This is immediately useful for tcm_qla2xxx to fix a long-standing active I/O session shutdown race, but tcm_fc, usb-gadget, and sbp-target the fabric maintainers need to check + ACK that handling a target_submit_cmd() failure due to session shutdown does not introduce regressions (nab: Respin against for-next after initial NACK + update docbook comment) Cc: Chad Dupuis Cc: Arun Easi Cc: Chris Boot Cc: Stefan Richter Cc: Mark Rustad Cc: Sebastian Andrzej Siewior Cc: Felipe Balbi Cc: Andy Grover Signed-off-by: Roland Dreier Signed-off-by: Nicholas Bellinger --- drivers/scsi/qla2xxx/tcm_qla2xxx.c |3 +-- drivers/target/sbp/sbp_target.c|8 +--- drivers/target/target_core_transport.c | 14 +- drivers/target/tcm_fc/tfc_cmd.c|8 +--- drivers/usb/gadget/tcm_usb_gadget.c| 20 include/target/target_core_fabric.h|2 +- 6 files changed, 29 insertions(+), 26 deletions(-) diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c index 65a7ed9..4752f65 100644 --- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c +++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c @@ -597,10 +597,9 @@ static int tcm_qla2xxx_handle_cmd(scsi_qla_host_t *vha, struct qla_tgt_cmd *cmd, return -EINVAL; } - target_submit_cmd(se_cmd, se_sess, cdb, &cmd->sense_buffer[0], + return target_submit_cmd(se_cmd, se_sess, cdb, &cmd->sense_buffer[0], cmd->unpacked_lun, data_length, fcp_task_attr, data_dir, flags); - return 0; } static void tcm_qla2xxx_handle_data_work(struct work_struct *work) diff --git a/drivers/target/sbp/sbp_target.c b/drivers/target/sbp/sbp_target.c index e10e622..39ddba5 100644 --- a/drivers/target/sbp/sbp_target.c +++ b/drivers/target/sbp/sbp_target.c @@ -1235,9 +1235,11 @@ static void sbp_handle_command(struct sbp_target_request *req) pr_debug("sbp_handle_command ORB:0x%llx unpacked_lun:%d data_len:%d data_dir:%d\n", req->orb_pointer, unpacked_lun, data_length, data_dir); - target_submit_cmd(&req->se_cmd, sess->se_sess, req->cmd_buf, - req->sense_buf, unpacked_lun, data_length, - MSG_SIMPLE_TAG, data_dir, 0); + if (target_submit_cmd(&req->se_cmd, sess->se_sess, req->cmd_buf, + req->sense_buf, unpacked_lun, data_length, + MSG_SIMPLE_TAG, data_dir, 0)) + goto err; + return; err: diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 14e54b4..7647eca 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -1447,10 +1447,14 @@ EXPORT_SYMBOL(transport_handle_cdb_direct); * @data_dir: DMA data direction * @flags: flags for command submission from target_sc_flags_tables * + * Returns non zero to signal active I/O shutdown failure. All other + * setup exceptions will be returned as a SCSI CHECK_CONDITION response, + * but still return zero here. + * * This may only be called from process context, and also currently * assumes internal allocation of fabric payload buffer by target-core. **/ -void target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess, +int target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess, unsigned char *cdb, unsigned char *sense, u32 unpacked_lun, u32 data_length, int task_attr, int data_dir, int flags) { @@ -1478,7 +1482,7 @@ void target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess, */ rc = target_get_sess_cmd(se_sess, se_cmd, (flags & TARGET_SCF_ACK_KREF)); if (rc) - return; + return rc; /* * Signal bidirectional data payloads to target-core */ @@ -1491,13 +1495,13 @@ void target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess, transport_send_check_condition_and_sense(se_cmd, se_cmd->scsi_sense_reason, 0); target_put_sess_cmd(se_sess, se_cmd); - return; + return 0; } rc = target_setup_cmd_from_cdb(se_cmd, cdb); if (rc != 0) { transport_generic_request_failure(se_cmd); - return; + return 0; } /* @@ -1507,7 +1511,7 @@ void target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess, core_alua_check_nonop_delay(se_cmd); transport_handle_cdb_direct(se_cmd); - return; + return 0; } EXPORT_SYMBOL(target_submit_cmd
Re: [PATCH 5/7] target: Check sess_tearing_down in target_get_sess_cmd()
On Tue, 2012-07-17 at 09:34 -0700, Roland Dreier wrote: > On Mon, Jul 16, 2012 at 6:56 PM, Nicholas A. Bellinger > wrote: > >> Do you have a plan for how to handle this? Do we really want to plumb > >> through another callback to tell the fabric driver to free the command > >> in this case? > > > I need to think more about this ahead of changing it back again for-3.6 > > now that other fabrics have the assumption of target_submit_cmd() would > > not fail. > > > There is a clear case with qla2xxx for just changing it back (we might > > end up doing that just yet) but I wanted to get the other important bits > > into for-next into place first.. > > Sleeping on things, I now feel pretty strongly that having target_submit_cmd > return an error value for "immediate" errors where the command does not > make it into the target core is the right approach. > > Freeing commands is already one of the most confusing and complex parts > of the target code, with "check_stop_free," "cmd_wait_comp" and > "SCF_ACK_KREF." Adding another code flow back to the fabric driver > with yet another set of semantics around freeing a command seems like > it's making things even harder to maintain. > I wasn't talking about adding another release path. I was thinking more about a TFO call to optionally abort HW/SW exchange if we can't accept the command in question. Not to mention, this could end up being useful for other purposes beyond the initial target_submit_cmd() failure case due to active I/O shutdown. (aborts + active I/O shutdown with active SRP WRITEs with ib_srpt come to mind needing something like this..) > On the other hand, every caller of target_submit_cmd() in the tree already > has an error path where it handles problems it detects right before calling > target_submit_cmd(), and so it's trivial to share that for problems detected > inside target_submit_cmd(). If you look at patch 4/7, pretty much the > only changes are like going from > > target_submit_cmd(...); > > to > > if (target_submit_cmd(...)) > goto err; > > and in fact the ->handle_cmd() wrapper that qla_target uses to call > target_submit_cmd via tcm_qla2xxx already has a return value that > qla_target handled properly! > Sure, there is no doubting tcm_qla2xxx's usage of this return value back into qla_target.c code.. > I guess another approach would be to split target_submit_cmd() into > the target_get_sess_cmd() part and the rest of it, and have fabric > drivers handle errors queueing commands but not have to worry about > the submit cmd part failing. But I'm not sure that's any better. > > Andy, any feelings about this one way or another? Christoph? > Ok, so for-3.6 it makes the most sense to just convert this back to propagate up the return code, but only for target_get_sess_cmd() failure case.. That said, I'll go ahead and respin patch #4 on top of for-next, and post the updated patch shortly with the same CC's and ask for ACKs from the necessary folks. --nab -- 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-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
On Tue, Jul 17, 2012 at 03:37:20PM -0700, Nicholas A. Bellinger wrote: > On Wed, 2012-07-18 at 01:18 +0300, Michael S. Tsirkin wrote: > > On Tue, Jul 17, 2012 at 03:02:08PM -0700, Nicholas A. Bellinger wrote: > > > On Wed, 2012-07-18 at 00:34 +0300, Michael S. Tsirkin wrote: > > > > On Tue, Jul 17, 2012 at 02:17:22PM -0700, Nicholas A. Bellinger wrote: > > > > > On Tue, 2012-07-17 at 18:05 +0300, Michael S. Tsirkin wrote: > > > > > > On Wed, Jul 11, 2012 at 09:15:00PM +, Nicholas A. Bellinger > > > > > > wrote: > > > > > > > From: Nicholas Bellinger > > > > > > As mentioned in the response to Anthony, we are using the same generic > > > fabric ABI in drivers/target/target_core_fabric_configfs.c since .38. > > > > > > That part is not going to change, and it has not changed for any of the > > > other 7 target fabric modules we've merged into mainline since then. > > > > > > > > Also, tcm_vhost has been marked as Experimental following virtio-scsi. > > > > > I'd much rather leave it at Experimental until we merge upstream > > > > > userspace support. If userspace support never ends up materializing, > > > > > I'm fine with dropping it all together. > > > > > > > > Once it's in kernel you never know who will use this driver. > > > > Experimental does not mean driver can be dropped, staging does. > > > > > > > > > > Yes, that's the point of being in mainline. People using the code, > > > right..? > > > > Exactly. I am just worried about in the end being no major users and we > > are being stuck with a niche driver that as a result is very hard to test. > > And the reason for the fear is the initial negative reaction from the > > qemu side. And no if it's there we can't just drop it. > > > > That is certainly a reasonable concern.. > > > > > > However at this point given that there is a 3x performance gap between > > > > > virtio-scsi-raw + virtio-scsi+tcm_vhost for random mixed small block > > > > > I/O, and we still need the latter to do proper SCSI CDB passthrough > > > > > for > > > > > non TYPE_DISK devices I'm hoping that we can agree on userspace bits > > > > > once tcm_vhost is merged. > > > > > > > > > > --nab > > > > > > > > I do think upstream kernel would help you nail userspace issues too > > > > but at this point it looks like either staging meterial or 3.6 is too > > > > early. > > > > > > > > > > I think for-3.6 is just the right time for this kernel code. Seriously, > > > The basic ABI fabric layout for /sys/kernel/config/target/vhost/ is > > > going to be the same now for-3.6, the same for-3.7, and the same for .38 > > > code. > > > > > > I'd be happy to move tcm_vhost back to drivers/target/ for now, and we > > > move it to drivers/vhost/ once the userspace bits are worked out..? > > > > > > Would that be a reasonable compromise to move forward..? > > > > > > --nab > > > > I don't see how it helps. The driver is either a guaranteed ABI or not. > > I'd prefer not to have vhost users outside drivers/vhost/ since it is > > harder for me to keep track of them. > > > > What's the problem with staging proposal? It's just another hoop to jump > > through to enable it? > > > > Yeah, I'm OK with just adding a CONFIG_STAGING tag is a reasonable step > forward for-3.6. > > Adding the following patch into target-pending/for-next-merge now: > > diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig > index ccbeb6f..2cd7135 100644 > --- a/drivers/vhost/Kconfig > +++ b/drivers/vhost/Kconfig > @@ -11,7 +11,7 @@ config VHOST_NET > > config TCM_VHOST > tristate "TCM_VHOST fabric module (EXPERIMENTAL)" > - depends on TARGET_CORE && EVENTFD && EXPERIMENTAL && m > + depends on TARGET_CORE && EVENTFD && EXPERIMENTAL && STAGING && m > default n > ---help--- > Say M here to enable the TCM_VHOST fabric module for use with > virtio-scsi guests > > Hmm that's not explicit enough, someone might enable CONFIG_STAGING for some other reason and won't notice the dependency. We need it to appear with other staging drivers in the menu, so there needs to be a Kconfig that is included from drivers/staging/Kconfig. For example, we can create drivers/vhost/staging/Kconfig or drivers/vhost/tcm/Kconfig and include it from drivers/staging/Kconfig. nouveau did something like this for a while, see f3c93cbde7eab38671ae085cb1027b08f5f36757. No need to move the rest of the code. -- MST -- 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-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
On Wed, 2012-07-18 at 01:18 +0300, Michael S. Tsirkin wrote: > On Tue, Jul 17, 2012 at 03:02:08PM -0700, Nicholas A. Bellinger wrote: > > On Wed, 2012-07-18 at 00:34 +0300, Michael S. Tsirkin wrote: > > > On Tue, Jul 17, 2012 at 02:17:22PM -0700, Nicholas A. Bellinger wrote: > > > > On Tue, 2012-07-17 at 18:05 +0300, Michael S. Tsirkin wrote: > > > > > On Wed, Jul 11, 2012 at 09:15:00PM +, Nicholas A. Bellinger wrote: > > > > > > From: Nicholas Bellinger > > As mentioned in the response to Anthony, we are using the same generic > > fabric ABI in drivers/target/target_core_fabric_configfs.c since .38. > > > > That part is not going to change, and it has not changed for any of the > > other 7 target fabric modules we've merged into mainline since then. > > > > > > Also, tcm_vhost has been marked as Experimental following virtio-scsi. > > > > I'd much rather leave it at Experimental until we merge upstream > > > > userspace support. If userspace support never ends up materializing, > > > > I'm fine with dropping it all together. > > > > > > Once it's in kernel you never know who will use this driver. > > > Experimental does not mean driver can be dropped, staging does. > > > > > > > Yes, that's the point of being in mainline. People using the code, > > right..? > > Exactly. I am just worried about in the end being no major users and we > are being stuck with a niche driver that as a result is very hard to test. > And the reason for the fear is the initial negative reaction from the > qemu side. And no if it's there we can't just drop it. > That is certainly a reasonable concern.. > > > > However at this point given that there is a 3x performance gap between > > > > virtio-scsi-raw + virtio-scsi+tcm_vhost for random mixed small block > > > > I/O, and we still need the latter to do proper SCSI CDB passthrough for > > > > non TYPE_DISK devices I'm hoping that we can agree on userspace bits > > > > once tcm_vhost is merged. > > > > > > > > --nab > > > > > > I do think upstream kernel would help you nail userspace issues too > > > but at this point it looks like either staging meterial or 3.6 is too > > > early. > > > > > > > I think for-3.6 is just the right time for this kernel code. Seriously, > > The basic ABI fabric layout for /sys/kernel/config/target/vhost/ is > > going to be the same now for-3.6, the same for-3.7, and the same for .38 > > code. > > > > I'd be happy to move tcm_vhost back to drivers/target/ for now, and we > > move it to drivers/vhost/ once the userspace bits are worked out..? > > > > Would that be a reasonable compromise to move forward..? > > > > --nab > > I don't see how it helps. The driver is either a guaranteed ABI or not. > I'd prefer not to have vhost users outside drivers/vhost/ since it is > harder for me to keep track of them. > > What's the problem with staging proposal? It's just another hoop to jump > through to enable it? > Yeah, I'm OK with just adding a CONFIG_STAGING tag is a reasonable step forward for-3.6. Adding the following patch into target-pending/for-next-merge now: diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig index ccbeb6f..2cd7135 100644 --- a/drivers/vhost/Kconfig +++ b/drivers/vhost/Kconfig @@ -11,7 +11,7 @@ config VHOST_NET config TCM_VHOST tristate "TCM_VHOST fabric module (EXPERIMENTAL)" - depends on TARGET_CORE && EVENTFD && EXPERIMENTAL && m + depends on TARGET_CORE && EVENTFD && EXPERIMENTAL && STAGING && m default n ---help--- Say M here to enable the TCM_VHOST fabric module for use with virtio-scsi guests -- 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: [Bug 44771] [REGRESSION] a7a20d103994fd760766e6c9d494daa569cbfe06 makes kernel 3.5 unbootable on an Intel chipset based motherboard
On Tue, 2012-07-17 at 12:51 -0700, Linus Torvalds wrote: > Wrong people cc'd, it looks like. > > Guys, commit a7a20d103994 ("sd: limit the scope of the async probe > domain") is causing boot problems. It's timing-dependent and > apparently sometimes works, which makes sense with that commit. > > However, it *should* have been fixed by commit 43a8d39d0137 ("fix > async probe regression"), but Artem seems to report the problem even > in -rc7. > > Comments? As far as I can tell, the fix should have worked. However, there are a lot of assumptions in the async stuff that end up not being true in the presence of separate async domains. We should be fixing it all here: http://git.kernel.org/?p=linux/kernel/git/jejb/scsi.git;a=shortlog;h=refs/heads/async I've got to say, I don't understand the bug report. all of those commits were about probing for devices. However, the screen shot https://bugzilla.kernel.org/attachment.cgi?id=75351 shows the devices were found, it's the partition tables that weren't. For us to see the message about sda's capacity, we're already in the async code the commits were trying to synchronise with. However, there are some missing messages: there's no partition table print and no final sd_printk(KERN_NOTICE, sdkp, "Attached SCSI %sdisk\n", sdp->removable ? "removable " : ""); So sd_probe_async() got stuck somewhere after the first sd_revalidata_disk(). James > Artem, can you put a working dmesg from the last good kernel in there > as plain text? > >Linus > > On Tue, Jul 17, 2012 at 12:15 PM, > wrote: > > https://bugzilla.kernel.org/show_bug.cgi?id=44771 > > > > > > > > > > > > --- Comment #9 from Artem S. Tashkinov 2012-07-17 > > 19:15:46 --- > > Wow, I'm not the first person to hit this problem: > > > > http://help.lockergnome.com/linux/SCSI-sd-limit-scope-async-probe-domain-breaks-booting--ftopict555976.html > > > >> Dudes, > > > >> so I've been testing latest linus > >> (731a7378b81c2f5fa88ca1ae20b83d548d5613dc) here and my box fails booting > >> because it can't find the root partition, see message below. > >> > >> I did a bisect run (also below) and pointed me to the first bad commit > >> (see below too). > >> > >> Reverting the commit in question fixes booting. > >> > >> Let me know what other info you'd need. > >> > >> Posted: Wed May 30, 2012 2:10 pm > >> > >> Borislav Petkov > > > > Yet another person: > > > > https://lkml.org/lkml/2012/5/25/271 > > > >> Dan Williams > >> > >> Ok, I'll take a look. > >> > >> Thanks for the help! > > > > And no fix whatsoever. > > > > -- > > Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email > > --- You are receiving this mail because: --- > > You are on the CC list for the bug.
Re: [RFC-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
On Tue, Jul 17, 2012 at 03:02:08PM -0700, Nicholas A. Bellinger wrote: > On Wed, 2012-07-18 at 00:34 +0300, Michael S. Tsirkin wrote: > > On Tue, Jul 17, 2012 at 02:17:22PM -0700, Nicholas A. Bellinger wrote: > > > On Tue, 2012-07-17 at 18:05 +0300, Michael S. Tsirkin wrote: > > > > On Wed, Jul 11, 2012 at 09:15:00PM +, Nicholas A. Bellinger wrote: > > > > > From: Nicholas Bellinger > > > > > > I'm happy to commit to working with QEMU + kvm-tool folks to get to a > > > series that can (eventually) see vhost-scsi support merged into upstream > > > userspace code. > > > > > > It took roughly 2 years to get the megasas HBA emulation from Dr. Hannes > > > merged, but certainly vhost-scsi has alot less moving pieces and > > > hopefully alot less controversial bits than the buffer -> SGL > > > conversion.. The key word being here 'hopefully'.. ;) > > > > > > > I think a good idea for 3.6 would be to make it depend on > > > > CONFIG_STAGING. > > > > Then we don't commit to an ABI. > > > > For this, you can add a separate Kconfig and source it from > > > > drivers/staging/Kconfig. > > > > Maybe it needs to be in a separate directory > > > > drivers/vhost/staging/Kconfig. > > > > > > > > > > So tcm_vhost has been marked as Experimental following virtio-scsi. > > > > > > Wrt to staging, I'd like to avoid mucking with staging because: > > > > > > *) The code has been posted for review > > > *) The code has been converted to use the latest target-core primitives > > > *) The code does not require cleanups between staging -> merge > > > *) The code has been stable the last 7 days since RFC-v2 with heavy > > > > staging is not just for code that needs cleanups. > > It's for anything that does not guarantee ABI stability yet. > > And I think it's a bit early to guarantee ABI stability - 7 days > > is not all that long. > > > > I was talking about the new I/O path has been running for 7 days. > > > See for example Anthony's comments that raise exactly the ABI > > issues. > > > > As mentioned in the response to Anthony, we are using the same generic > fabric ABI in drivers/target/target_core_fabric_configfs.c since .38. > > That part is not going to change, and it has not changed for any of the > other 7 target fabric modules we've merged into mainline since then. > > > > Also, tcm_vhost has been marked as Experimental following virtio-scsi. > > > I'd much rather leave it at Experimental until we merge upstream > > > userspace support. If userspace support never ends up materializing, > > > I'm fine with dropping it all together. > > > > Once it's in kernel you never know who will use this driver. > > Experimental does not mean driver can be dropped, staging does. > > > > Yes, that's the point of being in mainline. People using the code, > right..? Exactly. I am just worried about in the end being no major users and we are being stuck with a niche driver that as a result is very hard to test. And the reason for the fear is the initial negative reaction from the qemu side. And no if it's there we can't just drop it. > > > However at this point given that there is a 3x performance gap between > > > virtio-scsi-raw + virtio-scsi+tcm_vhost for random mixed small block > > > I/O, and we still need the latter to do proper SCSI CDB passthrough for > > > non TYPE_DISK devices I'm hoping that we can agree on userspace bits > > > once tcm_vhost is merged. > > > > > > --nab > > > > I do think upstream kernel would help you nail userspace issues too > > but at this point it looks like either staging meterial or 3.6 is too > > early. > > > > I think for-3.6 is just the right time for this kernel code. Seriously, > The basic ABI fabric layout for /sys/kernel/config/target/vhost/ is > going to be the same now for-3.6, the same for-3.7, and the same for .38 > code. > > I'd be happy to move tcm_vhost back to drivers/target/ for now, and we > move it to drivers/vhost/ once the userspace bits are worked out..? > > Would that be a reasonable compromise to move forward..? > > --nab I don't see how it helps. The driver is either a guaranteed ABI or not. I'd prefer not to have vhost users outside drivers/vhost/ since it is harder for me to keep track of them. What's the problem with staging proposal? It's just another hoop to jump through to enable it? -- MST -- 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-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
On Wed, 2012-07-18 at 00:58 +0300, Michael S. Tsirkin wrote: > On Tue, Jul 17, 2012 at 02:17:22PM -0700, Nicholas A. Bellinger wrote: > > Wrt to staging, I'd like to avoid mucking with staging because: > > > > *) The code has been posted for review > > *) The code has been converted to use the latest target-core primitives > > *) The code does not require cleanups between staging -> merge > > *) The code has been stable the last 7 days since RFC-v2 with heavy > > BTW I don't suggest putting code itself in staging. Just the config > flag to enable it. Once we are more or less sure multiple userspaces > are using this driver, we'll move the config hopefully already in 3.7. > What's the downside? > Ahh, sorry I managed to miss that part.. ;) If it's just a CONFIG_STAGING flag for a release or two until we work out the userspace bits, I don't have an objection doing something like that if it helps getting the code exposed to a large set of eyes in mainline. --nab -- 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-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
On Wed, 2012-07-18 at 00:34 +0300, Michael S. Tsirkin wrote: > On Tue, Jul 17, 2012 at 02:17:22PM -0700, Nicholas A. Bellinger wrote: > > On Tue, 2012-07-17 at 18:05 +0300, Michael S. Tsirkin wrote: > > > On Wed, Jul 11, 2012 at 09:15:00PM +, Nicholas A. Bellinger wrote: > > > > From: Nicholas Bellinger > > I'm happy to commit to working with QEMU + kvm-tool folks to get to a > > series that can (eventually) see vhost-scsi support merged into upstream > > userspace code. > > > > It took roughly 2 years to get the megasas HBA emulation from Dr. Hannes > > merged, but certainly vhost-scsi has alot less moving pieces and > > hopefully alot less controversial bits than the buffer -> SGL > > conversion.. The key word being here 'hopefully'.. ;) > > > > > I think a good idea for 3.6 would be to make it depend on CONFIG_STAGING. > > > Then we don't commit to an ABI. > > > For this, you can add a separate Kconfig and source it from > > > drivers/staging/Kconfig. > > > Maybe it needs to be in a separate directory > > > drivers/vhost/staging/Kconfig. > > > > > > > So tcm_vhost has been marked as Experimental following virtio-scsi. > > > > Wrt to staging, I'd like to avoid mucking with staging because: > > > > *) The code has been posted for review > > *) The code has been converted to use the latest target-core primitives > > *) The code does not require cleanups between staging -> merge > > *) The code has been stable the last 7 days since RFC-v2 with heavy > > staging is not just for code that needs cleanups. > It's for anything that does not guarantee ABI stability yet. > And I think it's a bit early to guarantee ABI stability - 7 days > is not all that long. > I was talking about the new I/O path has been running for 7 days. > See for example Anthony's comments that raise exactly the ABI > issues. > As mentioned in the response to Anthony, we are using the same generic fabric ABI in drivers/target/target_core_fabric_configfs.c since .38. That part is not going to change, and it has not changed for any of the other 7 target fabric modules we've merged into mainline since then. > > Also, tcm_vhost has been marked as Experimental following virtio-scsi. > > I'd much rather leave it at Experimental until we merge upstream > > userspace support. If userspace support never ends up materializing, > > I'm fine with dropping it all together. > > Once it's in kernel you never know who will use this driver. > Experimental does not mean driver can be dropped, staging does. > Yes, that's the point of being in mainline. People using the code, right..? > > However at this point given that there is a 3x performance gap between > > virtio-scsi-raw + virtio-scsi+tcm_vhost for random mixed small block > > I/O, and we still need the latter to do proper SCSI CDB passthrough for > > non TYPE_DISK devices I'm hoping that we can agree on userspace bits > > once tcm_vhost is merged. > > > > --nab > > I do think upstream kernel would help you nail userspace issues too > but at this point it looks like either staging meterial or 3.6 is too > early. > I think for-3.6 is just the right time for this kernel code. Seriously, The basic ABI fabric layout for /sys/kernel/config/target/vhost/ is going to be the same now for-3.6, the same for-3.7, and the same for .38 code. I'd be happy to move tcm_vhost back to drivers/target/ for now, and we move it to drivers/vhost/ once the userspace bits are worked out..? Would that be a reasonable compromise to move forward..? --nab -- 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] sd: do not set changed flag on all unit attention conditions
On Tue, 2012-07-17 at 12:36 -0400, Christoph Hellwig wrote: > On Tue, Jul 17, 2012 at 10:11:57AM +0100, James Bottomley wrote: > > There's no such thing in the market today as a removable disk that's > > resizeable. Removable disks are for things like backup cartridges and > > ageing jazz drives. Worse: most removeable devices today are USB card > > readers whose standards compliance varies from iffy to non existent. > > Resizeable disks are currently the province of storage arrays. > > The virtual disks exported by aacraid are both marked removable and > can be resized. So what are properties of these things? ... or is this just an instance of a RAID manufacturer hacking around a problem by adding a removable flag? James -- 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-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
On Tue, Jul 17, 2012 at 02:17:22PM -0700, Nicholas A. Bellinger wrote: > Wrt to staging, I'd like to avoid mucking with staging because: > > *) The code has been posted for review > *) The code has been converted to use the latest target-core primitives > *) The code does not require cleanups between staging -> merge > *) The code has been stable the last 7 days since RFC-v2 with heavy BTW I don't suggest putting code itself in staging. Just the config flag to enable it. Once we are more or less sure multiple userspaces are using this driver, we'll move the config hopefully already in 3.7. What's the downside? -- MST -- 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-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
On Tue, 2012-07-17 at 13:55 -0500, Anthony Liguori wrote: > On 07/17/2012 10:05 AM, Michael S. Tsirkin wrote: > > On Wed, Jul 11, 2012 at 09:15:00PM +, Nicholas A. Bellinger wrote: > > > > It still seems not 100% clear whether this driver will have major > > userspace using it. And if not, it would be very hard to support a driver > > when recent userspace does not use it in the end. > > I don't think this is a good reason to exclude something from the kernel. > However, there are good reasons why this doesn't make sense for something > like > QEMU--specifically because we have a large number of features in our block > layer > that tcm_vhost would bypass. > I can definitely appreciate your concern here as the QEMU maintainer. > But perhaps it makes sense for something like native kvm tool. And if it did > go > into the kernel, we would certainly support it in QEMU. > ... > But I do think the kernel should carefully consider whether it wants to > support > an interface like this. This an extremely complicated ABI with a lot of > subtle > details around state and compatibility. > > Are you absolutely confident that you can support a userspace application > that > expects to get exactly the same response from all possible commands in 20 > kernel > versions from now? Virtualization requires absolutely precise compatibility > in > terms of bugs and features. This is probably not something the TCM stack has > had to consider yet. > We most certainly have thought about long term userspace compatibility with TCM. Our userspace code (that's now available in all major distros) is completely forward-compatible with new fabric modules such as tcm_vhost. No update required. Also, by virtue of the fact that we are using configfs + rtslib (python object library) on top, it's very easy to keep any type of compatibility logic around in python code. With rtslib, we are able to hide configfs ABI changes from higher level apps. So far we've had a track record of 100% userspace ABI compatibility in mainline since .38, and I don't intend to merge a patch that breaks this any time soon. But if that ever happens, apps using rtslib are not going to be effected. > > I think a good idea for 3.6 would be to make it depend on CONFIG_STAGING. > > Then we don't commit to an ABI. > > I think this is a good idea. Even if it goes in, a really clear policy would > be > needed wrt the userspace ABI. > > While tcm_vhost is probably more useful than vhost_blk, it's a much more > complex > ABI to maintain. > As far as I am concerned, the kernel API (eg: configfs directory layout) as it is now in sys/kernel/config/target/vhost/ is not going to change. It's based on the same drivers/target/target_core_fabric_configfs.c generic layout that we've had since .38. The basic functional fabric layout in configfs is identical (with fabric dependent WWPN naming of course) regardless of fabric driver, and by virtue of being generic it means we can add things like fabric dependent attributes + parameters in the future for existing fabrics without breaking userspace. So while I agree the ABI is more complex than vhost-blk, the logic in target_core_fabric_configfs.c is a basic ABI fabric definition that we are enforcing across all fabric modules in mainline for long term compatibility. --nab -- 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-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
On Tue, Jul 17, 2012 at 02:17:22PM -0700, Nicholas A. Bellinger wrote: > On Tue, 2012-07-17 at 18:05 +0300, Michael S. Tsirkin wrote: > > On Wed, Jul 11, 2012 at 09:15:00PM +, Nicholas A. Bellinger wrote: > > > From: Nicholas Bellinger > > > > > > Hi folks, > > > > > > The following is a RFC-v2 series of tcm_vhost target fabric driver code > > > currently in-flight for-3.6 mainline code. > > > > > > After last week's developments along with the help of some new folks, the > > > changelog v1 -> v2 so far looks like: > > > > > > *) Fix drivers/vhost/test.c to use VHOST_NET_FEATURES in patch #1 (Asias > > > He) > > > *) Fix tv_cmd completion -> release SGL memory leak (nab) > > > *) Fix sparse warnings for static variable usage (Fengguang Wu) > > > *) Fix sparse warnings for min() typing + printk format specs (Fengguang > > > Wu) > > > *) Convert to cmwq submission for I/O dispatch (nab + hch) > > > > > > Also following Paolo's request, a patch for hw/virtio-scsi.c that sets > > > scsi_host->max_target=0 that removes the need for virtio-scsi LLD to > > > hardcode > > > VirtIOSCSIConfig->max_id=1 in order to function with tcm_vhost. > > > > > > Note this series has been pushed into target-pending.git/for-next-merge, > > > and > > > should be getting picked up for tomorrow's linux-next build. > > > > > > Please let us know if you have any concerns and/or additional review > > > feedback. > > > > > > Thank you! > > > > > > It still seems not 100% clear whether this driver will have major > > userspace using it. And if not, it would be very hard to support a driver > > when recent userspace does not use it in the end. > > > > I'm happy to commit to working with QEMU + kvm-tool folks to get to a > series that can (eventually) see vhost-scsi support merged into upstream > userspace code. > > It took roughly 2 years to get the megasas HBA emulation from Dr. Hannes > merged, but certainly vhost-scsi has alot less moving pieces and > hopefully alot less controversial bits than the buffer -> SGL > conversion.. The key word being here 'hopefully'.. ;) > > > I think a good idea for 3.6 would be to make it depend on CONFIG_STAGING. > > Then we don't commit to an ABI. > > For this, you can add a separate Kconfig and source it from > > drivers/staging/Kconfig. > > Maybe it needs to be in a separate directory drivers/vhost/staging/Kconfig. > > > > So tcm_vhost has been marked as Experimental following virtio-scsi. > > Wrt to staging, I'd like to avoid mucking with staging because: > > *) The code has been posted for review > *) The code has been converted to use the latest target-core primitives > *) The code does not require cleanups between staging -> merge > *) The code has been stable the last 7 days since RFC-v2 with heavy staging is not just for code that needs cleanups. It's for anything that does not guarantee ABI stability yet. And I think it's a bit early to guarantee ABI stability - 7 days is not all that long. See for example Anthony's comments that raise exactly the ABI issues. > Also, tcm_vhost has been marked as Experimental following virtio-scsi. > I'd much rather leave it at Experimental until we merge upstream > userspace support. If userspace support never ends up materializing, > I'm fine with dropping it all together. Once it's in kernel you never know who will use this driver. Experimental does not mean driver can be dropped, staging does. > However at this point given that there is a 3x performance gap between > virtio-scsi-raw + virtio-scsi+tcm_vhost for random mixed small block > I/O, and we still need the latter to do proper SCSI CDB passthrough for > non TYPE_DISK devices I'm hoping that we can agree on userspace bits > once tcm_vhost is merged. > > --nab I do think upstream kernel would help you nail userspace issues too but at this point it looks like either staging meterial or 3.6 is too early. -- MST -- 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-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
On Tue, 2012-07-17 at 18:05 +0300, Michael S. Tsirkin wrote: > On Wed, Jul 11, 2012 at 09:15:00PM +, Nicholas A. Bellinger wrote: > > From: Nicholas Bellinger > > > > Hi folks, > > > > The following is a RFC-v2 series of tcm_vhost target fabric driver code > > currently in-flight for-3.6 mainline code. > > > > After last week's developments along with the help of some new folks, the > > changelog v1 -> v2 so far looks like: > > > > *) Fix drivers/vhost/test.c to use VHOST_NET_FEATURES in patch #1 (Asias He) > > *) Fix tv_cmd completion -> release SGL memory leak (nab) > > *) Fix sparse warnings for static variable usage (Fengguang Wu) > > *) Fix sparse warnings for min() typing + printk format specs (Fengguang Wu) > > *) Convert to cmwq submission for I/O dispatch (nab + hch) > > > > Also following Paolo's request, a patch for hw/virtio-scsi.c that sets > > scsi_host->max_target=0 that removes the need for virtio-scsi LLD to > > hardcode > > VirtIOSCSIConfig->max_id=1 in order to function with tcm_vhost. > > > > Note this series has been pushed into target-pending.git/for-next-merge, and > > should be getting picked up for tomorrow's linux-next build. > > > > Please let us know if you have any concerns and/or additional review > > feedback. > > > > Thank you! > > > It still seems not 100% clear whether this driver will have major > userspace using it. And if not, it would be very hard to support a driver > when recent userspace does not use it in the end. > I'm happy to commit to working with QEMU + kvm-tool folks to get to a series that can (eventually) see vhost-scsi support merged into upstream userspace code. It took roughly 2 years to get the megasas HBA emulation from Dr. Hannes merged, but certainly vhost-scsi has alot less moving pieces and hopefully alot less controversial bits than the buffer -> SGL conversion.. The key word being here 'hopefully'.. ;) > I think a good idea for 3.6 would be to make it depend on CONFIG_STAGING. > Then we don't commit to an ABI. > For this, you can add a separate Kconfig and source it from > drivers/staging/Kconfig. > Maybe it needs to be in a separate directory drivers/vhost/staging/Kconfig. > So tcm_vhost has been marked as Experimental following virtio-scsi. Wrt to staging, I'd like to avoid mucking with staging because: *) The code has been posted for review *) The code has been converted to use the latest target-core primitives *) The code does not require cleanups between staging -> merge *) The code has been stable the last 7 days since RFC-v2 with heavy Also, tcm_vhost has been marked as Experimental following virtio-scsi. I'd much rather leave it at Experimental until we merge upstream userspace support. If userspace support never ends up materializing, I'm fine with dropping it all together. However at this point given that there is a 3x performance gap between virtio-scsi-raw + virtio-scsi+tcm_vhost for random mixed small block I/O, and we still need the latter to do proper SCSI CDB passthrough for non TYPE_DISK devices I'm hoping that we can agree on userspace bits once tcm_vhost is merged. --nab -- 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] sd: do not set changed flag on all unit attention conditions
Il 17/07/2012 20:49, Mike Christie ha scritto: > > Not sure if we are talking about the same thing. > > > > So can virtio-scsi send a UA with asc/ascq that indicates the lun > > changed size? Other drivers do this. I updated Hannes's patches the > > other day to support UAs like those in userspace. > > > > I just saw the code in the patch where virtio-scsi gets that event. > Was not done. I meant I saw that patch where virtio-scsi gets that > virtio_scsi_event and kicks of a rescan based off of that. Yes, it sends both (event + UA). Right now Linux ignores the UA, and I wanted virtio-scsi to match real hardware as much as possible so I copied what aacraid does. The event also has the advantage over UA that it the revalidate is done immediately, not the next time the unit is accessed. Paolo -- 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: 'Device not ready' issue on mpt2sas since 3.1.10
Hello, On Tue, Jul 17, 2012 at 09:39:41PM +0200, Matthias Prager wrote: > I could not however reproduce the issue on any other device than a LSI > SAS controller (using SATA disks) - on a regular ICH10 using AHCI and a > SATA drive I don't see these i/o errors. But since I'm experiencing > these issues on two different systems (both with lsi controllers while > running vmware-guests on them) and Robert sees them on his > (non-virtualized) system with the same lsi controller (9211-8i), I'm > inclined to make the following assumptions: > Either it is an issue which is limited to this controller and possibly > sata disks hanging off it or it is a more general issue with sas > controllers and sata disks (again it could well affect sas disks too). > Lacking other controllers or sas disks I can't be sure. So, nothing in the libata stack generates NOT_READY - "initializing command required". I suppose it's LSI firmware / driver translating TUR to CHECK_POWER_MODE and generating NOT_READY. I don't know what SAT says about this but this can't be correct. An ATA device in standby mode is ready to process any commands. It should be able to come back to full operation on demand as necessary and that's why it can be transparently enabled from device side. Eric? Thanks. -- tejun -- 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: 'Device not ready' issue on mpt2sas since 3.1.10
Hello Tejun, Am 17.07.2012 20:09, schrieb Tejun Heo: > Hello, > > On Wed, Jul 11, 2012 at 03:48:00PM +0200, Matthias Prager wrote: >> I'm trying to understand why this commit leads to the issue of i/o >> failing on spun down drives, in hopes of being able to fix it. Meanwhile >> maybe Tejun Heo (author of the commit) or Jens Axboe (the committer) are >> able to shed some light on this (I've included them in the CC list). > > Nothing rings a bell for me. How does it fail? The only thing it > change is when and which media check commands are issued. I will try to describe the issue as best as I can (please feel free to point me to more helpful debugging steps or guides): Whenever I put a drive to sleep (either via 'hdparm -y ...' or by letting it run into standby timeout) and issue i/o's afterwards (like with the help of 'fdisk -l') I get back i/o errors (along the lines of 'end_request: I/O error, ...' - see previous posts in this thread) and the drive remains in standby (instead of waking up). Robert (who also saw these errors) bisected the issue down to your patch. And without it kernels 3.1.10 + 3.4.4 run smoothly for him and me. I could not however reproduce the issue on any other device than a LSI SAS controller (using SATA disks) - on a regular ICH10 using AHCI and a SATA drive I don't see these i/o errors. But since I'm experiencing these issues on two different systems (both with lsi controllers while running vmware-guests on them) and Robert sees them on his (non-virtualized) system with the same lsi controller (9211-8i), I'm inclined to make the following assumptions: Either it is an issue which is limited to this controller and possibly sata disks hanging off it or it is a more general issue with sas controllers and sata disks (again it could well affect sas disks too). Lacking other controllers or sas disks I can't be sure. Thank you for taking the time to look into this - it's much appreciated Matthias -- 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 5/7] target: Check sess_tearing_down in target_get_sess_cmd()
On 07/17/2012 09:39 AM, Christoph Hellwig wrote: > On Tue, Jul 17, 2012 at 09:34:49AM -0700, Roland Dreier wrote: >> Sleeping on things, I now feel pretty strongly that having target_submit_cmd >> return an error value for "immediate" errors where the command does not >> make it into the target core is the right approach. > > I think it is. When I tried to convert other drivers to > target_submit_cmd while doing the target processing thread removal that > would have simplified a lot of things. > >> Freeing commands is already one of the most confusing and complex parts >> of the target code, with "check_stop_free," "cmd_wait_comp" and >> "SCF_ACK_KREF." Adding another code flow back to the fabric driver >> with yet another set of semantics around freeing a command seems like >> it's making things even harder to maintain. > > True. And the above mess really needs to be simplified, too. I'm ok with submit_cmd returning a value for now. Maybe in the end it doesn't, but getting the code working comes first. This is one of those areas that needs a complete rewrite, so who cares if there's some dirt on the floor when the whole joint is due for remodeling. -- Andy -- 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-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
On Tue, Jul 17, 2012 at 01:55:42PM -0500, Anthony Liguori wrote: > On 07/17/2012 10:05 AM, Michael S. Tsirkin wrote: > >On Wed, Jul 11, 2012 at 09:15:00PM +, Nicholas A. Bellinger wrote: > >>From: Nicholas Bellinger > >> > >>Hi folks, > >> > >>The following is a RFC-v2 series of tcm_vhost target fabric driver code > >>currently in-flight for-3.6 mainline code. > >> > >>After last week's developments along with the help of some new folks, the > >>changelog v1 -> v2 so far looks like: > >> > >>*) Fix drivers/vhost/test.c to use VHOST_NET_FEATURES in patch #1 (Asias He) > >>*) Fix tv_cmd completion -> release SGL memory leak (nab) > >>*) Fix sparse warnings for static variable usage (Fengguang Wu) > >>*) Fix sparse warnings for min() typing + printk format specs (Fengguang Wu) > >>*) Convert to cmwq submission for I/O dispatch (nab + hch) > >> > >>Also following Paolo's request, a patch for hw/virtio-scsi.c that sets > >>scsi_host->max_target=0 that removes the need for virtio-scsi LLD to > >>hardcode > >>VirtIOSCSIConfig->max_id=1 in order to function with tcm_vhost. > >> > >>Note this series has been pushed into target-pending.git/for-next-merge, and > >>should be getting picked up for tomorrow's linux-next build. > >> > >>Please let us know if you have any concerns and/or additional review > >>feedback. > >> > >>Thank you! > > > > > >It still seems not 100% clear whether this driver will have major > >userspace using it. And if not, it would be very hard to support a driver > >when recent userspace does not use it in the end. > > I don't think this is a good reason to exclude something from the > kernel. However, there are good reasons why this doesn't make sense > for something like QEMU--specifically because we have a large number > of features in our block layer that tcm_vhost would bypass. > > But perhaps it makes sense for something like native kvm tool. And > if it did go into the kernel, we would certainly support it in QEMU. > > But I do think the kernel should carefully consider whether it wants > to support an interface like this. This an extremely complicated > ABI with a lot of subtle details around state and compatibility. > > Are you absolutely confident that you can support a userspace > application that expects to get exactly the same response from all > possible commands in 20 kernel versions from now? Virtualization > requires absolutely precise compatibility in terms of bugs and > features. This is probably not something the TCM stack has had to > consider yet. > > >I think a good idea for 3.6 would be to make it depend on CONFIG_STAGING. > >Then we don't commit to an ABI. > > I think this is a good idea. Even if it goes in, a really clear > policy would be needed wrt the userspace ABI. > > While tcm_vhost is probably more useful than vhost_blk, it's a much > more complex ABI to maintain. > > Regards, > > Anthony Liguori Maybe something like a whitelist of features will help? Might even be a good idea to make it user controllable. > >For this, you can add a separate Kconfig and source it from > >drivers/staging/Kconfig. > >Maybe it needs to be in a separate directory drivers/vhost/staging/Kconfig. > > > > > >>Nicholas Bellinger (2): > >> vhost: Add vhost_scsi specific defines > >> tcm_vhost: Initial merge for vhost level target fabric driver > >> > >>Stefan Hajnoczi (2): > >> vhost: Separate vhost-net features from vhost features > >> vhost: make vhost work queue visible > >> > >> drivers/vhost/Kconfig |6 + > >> drivers/vhost/Makefile|1 + > >> drivers/vhost/net.c |4 +- > >> drivers/vhost/tcm_vhost.c | 1609 > >> + > >> drivers/vhost/tcm_vhost.h | 74 ++ > >> drivers/vhost/test.c |4 +- > >> drivers/vhost/vhost.c |5 +- > >> drivers/vhost/vhost.h |6 +- > >> include/linux/vhost.h |9 + > >> 9 files changed, 1710 insertions(+), 8 deletions(-) > >> create mode 100644 drivers/vhost/tcm_vhost.c > >> create mode 100644 drivers/vhost/tcm_vhost.h > >> > >>-- > >>1.7.2.5 > > -- 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-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
On 07/17/2012 10:05 AM, Michael S. Tsirkin wrote: On Wed, Jul 11, 2012 at 09:15:00PM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger Hi folks, The following is a RFC-v2 series of tcm_vhost target fabric driver code currently in-flight for-3.6 mainline code. After last week's developments along with the help of some new folks, the changelog v1 -> v2 so far looks like: *) Fix drivers/vhost/test.c to use VHOST_NET_FEATURES in patch #1 (Asias He) *) Fix tv_cmd completion -> release SGL memory leak (nab) *) Fix sparse warnings for static variable usage (Fengguang Wu) *) Fix sparse warnings for min() typing + printk format specs (Fengguang Wu) *) Convert to cmwq submission for I/O dispatch (nab + hch) Also following Paolo's request, a patch for hw/virtio-scsi.c that sets scsi_host->max_target=0 that removes the need for virtio-scsi LLD to hardcode VirtIOSCSIConfig->max_id=1 in order to function with tcm_vhost. Note this series has been pushed into target-pending.git/for-next-merge, and should be getting picked up for tomorrow's linux-next build. Please let us know if you have any concerns and/or additional review feedback. Thank you! It still seems not 100% clear whether this driver will have major userspace using it. And if not, it would be very hard to support a driver when recent userspace does not use it in the end. I don't think this is a good reason to exclude something from the kernel. However, there are good reasons why this doesn't make sense for something like QEMU--specifically because we have a large number of features in our block layer that tcm_vhost would bypass. But perhaps it makes sense for something like native kvm tool. And if it did go into the kernel, we would certainly support it in QEMU. But I do think the kernel should carefully consider whether it wants to support an interface like this. This an extremely complicated ABI with a lot of subtle details around state and compatibility. Are you absolutely confident that you can support a userspace application that expects to get exactly the same response from all possible commands in 20 kernel versions from now? Virtualization requires absolutely precise compatibility in terms of bugs and features. This is probably not something the TCM stack has had to consider yet. I think a good idea for 3.6 would be to make it depend on CONFIG_STAGING. Then we don't commit to an ABI. I think this is a good idea. Even if it goes in, a really clear policy would be needed wrt the userspace ABI. While tcm_vhost is probably more useful than vhost_blk, it's a much more complex ABI to maintain. Regards, Anthony Liguori For this, you can add a separate Kconfig and source it from drivers/staging/Kconfig. Maybe it needs to be in a separate directory drivers/vhost/staging/Kconfig. Nicholas Bellinger (2): vhost: Add vhost_scsi specific defines tcm_vhost: Initial merge for vhost level target fabric driver Stefan Hajnoczi (2): vhost: Separate vhost-net features from vhost features vhost: make vhost work queue visible drivers/vhost/Kconfig |6 + drivers/vhost/Makefile|1 + drivers/vhost/net.c |4 +- drivers/vhost/tcm_vhost.c | 1609 + drivers/vhost/tcm_vhost.h | 74 ++ drivers/vhost/test.c |4 +- drivers/vhost/vhost.c |5 +- drivers/vhost/vhost.h |6 +- include/linux/vhost.h |9 + 9 files changed, 1710 insertions(+), 8 deletions(-) create mode 100644 drivers/vhost/tcm_vhost.c create mode 100644 drivers/vhost/tcm_vhost.h -- 1.7.2.5 -- 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] sd: do not set changed flag on all unit attention conditions
On 07/17/2012 12:45 PM, Mike Christie wrote: > On 07/17/2012 10:47 AM, Paolo Bonzini wrote: >> Il 17/07/2012 18:36, Christoph Hellwig ha scritto: > There's no such thing in the market today as a removable disk that's > resizeable. Removable disks are for things like backup cartridges and > ageing jazz drives. Worse: most removeable devices today are USB card > readers whose standards compliance varies from iffy to non existent. > Resizeable disks are currently the province of storage arrays. >>> The virtual disks exported by aacraid are both marked removable and >>> can be resized. >> >> Do they report resizing via unit attention? I can skip this part on >> removable virtio-scsi disks if that's what real hardware does, it would >> also work. >> > > Not sure if we are talking about the same thing. > > So can virtio-scsi send a UA with asc/ascq that indicates the lun > changed size? Other drivers do this. I updated Hannes's patches the > other day to support UAs like those in userspace. > > I just saw the code in the patch where virtio-scsi gets that event. Was not done. I meant I saw that patch where virtio-scsi gets that virtio_scsi_event and kicks of a rescan based off of that. -- 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] sd: do not set changed flag on all unit attention conditions
On 07/17/2012 10:47 AM, Paolo Bonzini wrote: > Il 17/07/2012 18:36, Christoph Hellwig ha scritto: There's no such thing in the market today as a removable disk that's resizeable. Removable disks are for things like backup cartridges and ageing jazz drives. Worse: most removeable devices today are USB card readers whose standards compliance varies from iffy to non existent. Resizeable disks are currently the province of storage arrays. >> The virtual disks exported by aacraid are both marked removable and >> can be resized. > > Do they report resizing via unit attention? I can skip this part on > removable virtio-scsi disks if that's what real hardware does, it would > also work. > Not sure if we are talking about the same thing. So can virtio-scsi send a UA with asc/ascq that indicates the lun changed size? Other drivers do this. I updated Hannes's patches the other day to support UAs like those in userspace. I just saw the code in the patch where virtio-scsi gets that event. -- 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: 'Device not ready' issue on mpt2sas since 3.1.10
Hello, On Wed, Jul 11, 2012 at 03:48:00PM +0200, Matthias Prager wrote: > I just tested kernel version 3.4.4 without commit > 85ef06d1d252f6a2e73b678591ab71caad4667bb and it also works fine (beware > of commit 62d3c5439c534b0e6c653fc63e6d8c67be3a57b1 as it conflicts with > reverting 85ef06d1d252f6a2e73b678591ab71caad4667bb). > > I'm trying to understand why this commit leads to the issue of i/o > failing on spun down drives, in hopes of being able to fix it. Meanwhile > maybe Tejun Heo (author of the commit) or Jens Axboe (the committer) are > able to shed some light on this (I've included them in the CC list). Nothing rings a bell for me. How does it fail? The only thing it change is when and which media check commands are issued. Thanks. -- tejun -- 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] sd: do not set changed flag on all unit attention conditions
On Tue, Jul 17, 2012 at 06:47:15PM +0200, Paolo Bonzini wrote: > Il 17/07/2012 18:36, Christoph Hellwig ha scritto: > >> > There's no such thing in the market today as a removable disk that's > >> > resizeable. Removable disks are for things like backup cartridges and > >> > ageing jazz drives. Worse: most removeable devices today are USB card > >> > readers whose standards compliance varies from iffy to non existent. > >> > Resizeable disks are currently the province of storage arrays. > > The virtual disks exported by aacraid are both marked removable and > > can be resized. > > Do they report resizing via unit attention? I can skip this part on > removable virtio-scsi disks if that's what real hardware does, it would > also work. I don't have access to aacraid hardware currently. -- 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] sd: do not set changed flag on all unit attention conditions
Il 17/07/2012 18:36, Christoph Hellwig ha scritto: >> > There's no such thing in the market today as a removable disk that's >> > resizeable. Removable disks are for things like backup cartridges and >> > ageing jazz drives. Worse: most removeable devices today are USB card >> > readers whose standards compliance varies from iffy to non existent. >> > Resizeable disks are currently the province of storage arrays. > The virtual disks exported by aacraid are both marked removable and > can be resized. Do they report resizing via unit attention? I can skip this part on removable virtio-scsi disks if that's what real hardware does, it would also work. Paolo -- 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 5/7] target: Check sess_tearing_down in target_get_sess_cmd()
On Tue, Jul 17, 2012 at 09:34:49AM -0700, Roland Dreier wrote: > Sleeping on things, I now feel pretty strongly that having target_submit_cmd > return an error value for "immediate" errors where the command does not > make it into the target core is the right approach. I think it is. When I tried to convert other drivers to target_submit_cmd while doing the target processing thread removal that would have simplified a lot of things. > Freeing commands is already one of the most confusing and complex parts > of the target code, with "check_stop_free," "cmd_wait_comp" and > "SCF_ACK_KREF." Adding another code flow back to the fabric driver > with yet another set of semantics around freeing a command seems like > it's making things even harder to maintain. True. And the above mess really needs to be simplified, too. -- 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] sd: do not set changed flag on all unit attention conditions
On Tue, Jul 17, 2012 at 10:11:57AM +0100, James Bottomley wrote: > There's no such thing in the market today as a removable disk that's > resizeable. Removable disks are for things like backup cartridges and > ageing jazz drives. Worse: most removeable devices today are USB card > readers whose standards compliance varies from iffy to non existent. > Resizeable disks are currently the province of storage arrays. The virtual disks exported by aacraid are both marked removable and can be resized. -- 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 5/7] target: Check sess_tearing_down in target_get_sess_cmd()
On Mon, Jul 16, 2012 at 6:56 PM, Nicholas A. Bellinger wrote: >> Do you have a plan for how to handle this? Do we really want to plumb >> through another callback to tell the fabric driver to free the command >> in this case? > I need to think more about this ahead of changing it back again for-3.6 > now that other fabrics have the assumption of target_submit_cmd() would > not fail. > There is a clear case with qla2xxx for just changing it back (we might > end up doing that just yet) but I wanted to get the other important bits > into for-next into place first.. Sleeping on things, I now feel pretty strongly that having target_submit_cmd return an error value for "immediate" errors where the command does not make it into the target core is the right approach. Freeing commands is already one of the most confusing and complex parts of the target code, with "check_stop_free," "cmd_wait_comp" and "SCF_ACK_KREF." Adding another code flow back to the fabric driver with yet another set of semantics around freeing a command seems like it's making things even harder to maintain. On the other hand, every caller of target_submit_cmd() in the tree already has an error path where it handles problems it detects right before calling target_submit_cmd(), and so it's trivial to share that for problems detected inside target_submit_cmd(). If you look at patch 4/7, pretty much the only changes are like going from target_submit_cmd(...); to if (target_submit_cmd(...)) goto err; and in fact the ->handle_cmd() wrapper that qla_target uses to call target_submit_cmd via tcm_qla2xxx already has a return value that qla_target handled properly! I guess another approach would be to split target_submit_cmd() into the target_get_sess_cmd() part and the rest of it, and have fabric drivers handle errors queueing commands but not have to worry about the submit cmd part failing. But I'm not sure that's any better. Andy, any feelings about this one way or another? Christoph? Thanks, Roland -- 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 RESEND 0/3] scsi: fix internal write cache issue on usb hdd.
On Tue, Jul 17, 2012 at 08:19:14AM +0100, James Bottomley wrote: > On Mon, 2012-07-16 at 16:48 -0700, Greg KH wrote: > > On Sat, Jul 07, 2012 at 11:04:45PM -0400, Namjae Jeon wrote: > > > From: Namjae Jeon > > > > > > The numbers of USB HDDs(All USB HDD I checked) does not respond > > > correctly to scsi mode sense command for retrieving the write cache > > > page status. Even though write cache is enabled by default, due to > > > scsi driver assume that cache is not enabled which in turn might lead > > > to loss of data since data still will be in cache. > > > This result that all filesystems is not stable on USB HDD when the > > > device is unplugged abruptly, even though these are having journaling > > > feature. Our first trying is that scsi driver send ATA command > > > (ATA Pass through, #85) to USB HDD after failure from normal routine to > > > know write cache enable. > > > We have known it is dangerous after testing several USB HDD. some of > > > HDD is stalled by this command(A-DATA HDD). So we tried to make the > > > patch James Bottomley's suggestion(usb quirk) on version 2 that add > > > product ID and verdor ID of USB HDD to USB quirk list after checking > > > write cache. > > > All filesystem will be stable on USB HDD registered in quirk list. > > > And it will be updated continuously. > > > > Now applied to the usb-next branch. > > It's been in scsi#misc for ten days with no problems. Lets leave it > there rather than create merge and rebase issues. No need for rebasing. It's already in my tree, so we can handle the merge when this hits Linus's tree. thanks, greg k-h -- 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 RESEND 0/5] Add vhost-blk support
On Fri, Jul 13, 2012 at 04:55:06PM +0800, Asias He wrote: > > Hi folks, > > [I am resending to fix the broken thread in the previous one.] > > This patchset adds vhost-blk support. vhost-blk is a in kernel virito-blk > device accelerator. Compared to userspace virtio-blk implementation, vhost-blk > gives about 5% to 15% performance improvement. Same thing as tcm_host comment: It seems not 100% clear whether this driver will have major userspace using it. And if not, it would be very hard to support a driver when recent userspace does not use it in the end. I think a good idea for 3.6 would be to make it depend on CONFIG_STAGING. Then we don't commit to an ABI. For this, you can add a separate Kconfig and source it from drivers/staging/Kconfig. Maybe it needs to be in a separate directory drivers/vhost/staging/Kconfig. I Cc'd the list of tcm_host in the hope that you can cooperate on this. -- MST -- 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-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6
On Wed, Jul 11, 2012 at 09:15:00PM +, Nicholas A. Bellinger wrote: > From: Nicholas Bellinger > > Hi folks, > > The following is a RFC-v2 series of tcm_vhost target fabric driver code > currently in-flight for-3.6 mainline code. > > After last week's developments along with the help of some new folks, the > changelog v1 -> v2 so far looks like: > > *) Fix drivers/vhost/test.c to use VHOST_NET_FEATURES in patch #1 (Asias He) > *) Fix tv_cmd completion -> release SGL memory leak (nab) > *) Fix sparse warnings for static variable usage (Fengguang Wu) > *) Fix sparse warnings for min() typing + printk format specs (Fengguang Wu) > *) Convert to cmwq submission for I/O dispatch (nab + hch) > > Also following Paolo's request, a patch for hw/virtio-scsi.c that sets > scsi_host->max_target=0 that removes the need for virtio-scsi LLD to hardcode > VirtIOSCSIConfig->max_id=1 in order to function with tcm_vhost. > > Note this series has been pushed into target-pending.git/for-next-merge, and > should be getting picked up for tomorrow's linux-next build. > > Please let us know if you have any concerns and/or additional review feedback. > > Thank you! It still seems not 100% clear whether this driver will have major userspace using it. And if not, it would be very hard to support a driver when recent userspace does not use it in the end. I think a good idea for 3.6 would be to make it depend on CONFIG_STAGING. Then we don't commit to an ABI. For this, you can add a separate Kconfig and source it from drivers/staging/Kconfig. Maybe it needs to be in a separate directory drivers/vhost/staging/Kconfig. > Nicholas Bellinger (2): > vhost: Add vhost_scsi specific defines > tcm_vhost: Initial merge for vhost level target fabric driver > > Stefan Hajnoczi (2): > vhost: Separate vhost-net features from vhost features > vhost: make vhost work queue visible > > drivers/vhost/Kconfig |6 + > drivers/vhost/Makefile|1 + > drivers/vhost/net.c |4 +- > drivers/vhost/tcm_vhost.c | 1609 > + > drivers/vhost/tcm_vhost.h | 74 ++ > drivers/vhost/test.c |4 +- > drivers/vhost/vhost.c |5 +- > drivers/vhost/vhost.h |6 +- > include/linux/vhost.h |9 + > 9 files changed, 1710 insertions(+), 8 deletions(-) > create mode 100644 drivers/vhost/tcm_vhost.c > create mode 100644 drivers/vhost/tcm_vhost.h > > -- > 1.7.2.5 -- 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] sd: do not set changed flag on all unit attention conditions
On Tue, 2012-07-17 at 14:31 +0200, Paolo Bonzini wrote: > Il 17/07/2012 14:21, James Bottomley ha scritto: > >> Yes, I realize failing only on specific sense codes as I did it in the > >> patch is not going to work. However, the other way round is not > >> problematic (explicitly allow some sense codes, fail on all others). > > > > Heh, I once thought that, but there's no end to the strange ideas USB > > manufacturers get. > > :) > > >> > Another example is "target operating conditions have changed". QEMU > >> > cannot report such changes because scsi_error prints a warning (fine) > >> > and then passes the unit attention upwards. With removable drives, this > >> > has the same problem as resizing. > > Why would a removable device ever use any of the codes under this ASC > > when the medium hasn't changed? They're all for arrays (well except > > 0x10 and 0x11 ... and they're only supposed to apply to tape changers > > with autoload support declared in the control mode page). > > There are also a couple of generic ones such as "reported luns have > changed". I also considered briefly using "inquiry data has changed" to > reload the unmap parameters after live snapshot or storage migration. I > dropped the idea, please don't put me in the same bin as the USB > manufacturers. I'm not ... but at the same time removable functionality in sd.c is special cased in quite a few areas which makes it fragile but exercised. Its fragility is somewhat mitigated by the fact that a lot of the special casing separates the removable from the non-removable paths. I don't want to increase that fragility by entangling the removable and non-removable paths without a good reason for doing so. James -- 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 17/18] srp_transport: Add transport layer recovery support
> On Mon, 2012-07-16 at 18:07 -0400, Mike Christie wrote: >> On 01/14/2012 05:56 AM, Bart Van Assche wrote: >> > Add the necessary functions in the SRP transport module to allow >> > an SRP initiator driver to implement transport layer recovery. >> >> I was updating my iscsi dev loss patch when I saw this is still not >> merged. > > Yes, I got part way into doing a rework of Bart's code before I got > sidetracked on another project. Cognizant of the looming merge window, > I'm desperately trying to get back to it. I'll post the latest version of the patch series next Monday (I'm traveling now). If you have any suggestions for improvements about the patches themselves or the implemented algorithms, these are welcome. Bart. -- 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
[PATCHv2 3/3] ipr: Driver version 2.5.4
Bump driver version. Signed-off-by: Brian King --- drivers/scsi/ipr.h |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff -puN drivers/scsi/ipr.h~ipr_version_2_5_4 drivers/scsi/ipr.h --- linux-2.6/drivers/scsi/ipr.h~ipr_version_2_5_4 2012-07-11 16:40:34.0 -0500 +++ linux-2.6-bjking1/drivers/scsi/ipr.h2012-07-11 16:40:34.0 -0500 @@ -38,8 +38,8 @@ /* * Literals */ -#define IPR_DRIVER_VERSION "2.5.3" -#define IPR_DRIVER_DATE "(March 10, 2012)" +#define IPR_DRIVER_VERSION "2.5.4" +#define IPR_DRIVER_DATE "(July 11, 2012)" /* * IPR_MAX_CMD_PER_LUN: This defines the maximum number of outstanding _ -- 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 13/15] ib_srp: Allow SRP disconnect through sysfs
> On 03/25/2012 09:01 AM, Bart Van Assche wrote: >> +static void srp_rport_delete(struct srp_rport *rport) >> +{ >> +struct srp_target_port *target = rport->lld_data; >> + >> +BUG_ON(!target); >> + > > I don't think this null check is needed, because below you set the > lld_data before you call srp_rport_add which does the transport driver > model/sysfs file addition for the rport. So I think the rport delete > sysfs file won't show up before you set the lld_data. Agreed. I'm fine with leaving that check out. Bart. -- 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
[PATCHv2 2/3] ipr: Reduce interrupt lock time
Reduce the amount of time the host lock is held in the interrupt handler for improved performance. Signed-off-by: Brian King --- drivers/scsi/ipr.c | 60 - drivers/scsi/ipr.h |1 2 files changed, 47 insertions(+), 14 deletions(-) diff -puN drivers/scsi/ipr.h~ipr_reduce_isr_lock2 drivers/scsi/ipr.h --- linux-2.6/drivers/scsi/ipr.h~ipr_reduce_isr_lock2 2012-07-17 08:05:25.0 -0500 +++ linux-2.6-bjking1/drivers/scsi/ipr.h2012-07-17 08:05:25.0 -0500 @@ -1525,6 +1525,7 @@ struct ipr_cmnd { struct ata_queued_cmd *qc; struct completion completion; struct timer_list timer; + void (*fast_done) (struct ipr_cmnd *); void (*done) (struct ipr_cmnd *); int (*job_step) (struct ipr_cmnd *); int (*job_step_failed) (struct ipr_cmnd *); diff -puN drivers/scsi/ipr.c~ipr_reduce_isr_lock2 drivers/scsi/ipr.c --- linux-2.6/drivers/scsi/ipr.c~ipr_reduce_isr_lock2 2012-07-17 08:05:25.0 -0500 +++ linux-2.6-bjking1/drivers/scsi/ipr.c2012-07-17 08:06:27.0 -0500 @@ -566,6 +566,23 @@ static void ipr_trc_hook(struct ipr_cmnd #endif /** + * ipr_lock_and_done - Acquire lock and complete command + * @ipr_cmd: ipr command struct + * + * Return value: + * none + **/ +static void ipr_lock_and_done(struct ipr_cmnd *ipr_cmd) +{ + unsigned long lock_flags; + struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg; + + spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags); + ipr_cmd->done(ipr_cmd); + spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags); +} + +/** * ipr_reinit_ipr_cmnd - Re-initialize an IPR Cmnd block for reuse * @ipr_cmd: ipr command struct * @@ -611,11 +628,13 @@ static void ipr_reinit_ipr_cmnd(struct i * Return value: * none **/ -static void ipr_init_ipr_cmnd(struct ipr_cmnd *ipr_cmd) +static void ipr_init_ipr_cmnd(struct ipr_cmnd *ipr_cmd, + void (*fast_done) (struct ipr_cmnd *)) { ipr_reinit_ipr_cmnd(ipr_cmd); ipr_cmd->u.scratch = 0; ipr_cmd->sibling = NULL; + ipr_cmd->fast_done = fast_done; init_timer(&ipr_cmd->timer); } @@ -648,7 +667,7 @@ static struct ipr_cmnd *ipr_get_free_ipr_cmnd(struct ipr_ioa_cfg *ioa_cfg) { struct ipr_cmnd *ipr_cmd = __ipr_get_free_ipr_cmnd(ioa_cfg); - ipr_init_ipr_cmnd(ipr_cmd); + ipr_init_ipr_cmnd(ipr_cmd, ipr_lock_and_done); return ipr_cmd; } @@ -5130,8 +5149,9 @@ static irqreturn_t ipr_isr(int irq, void u16 cmd_index; int num_hrrq = 0; int irq_none = 0; - struct ipr_cmnd *ipr_cmd; + struct ipr_cmnd *ipr_cmd, *temp; irqreturn_t rc = IRQ_NONE; + LIST_HEAD(doneq); spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags); @@ -5152,8 +5172,8 @@ static irqreturn_t ipr_isr(int irq, void if (unlikely(cmd_index >= IPR_NUM_CMD_BLKS)) { ipr_isr_eh(ioa_cfg, "Invalid response handle from IOA"); - spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags); - return IRQ_HANDLED; + rc = IRQ_HANDLED; + goto unlock_out; } ipr_cmd = ioa_cfg->ipr_cmnd_list[cmd_index]; @@ -5162,9 +5182,7 @@ static irqreturn_t ipr_isr(int irq, void ipr_trc_hook(ipr_cmd, IPR_TRACE_FINISH, ioasc); - list_del(&ipr_cmd->queue); - del_timer(&ipr_cmd->timer); - ipr_cmd->done(ipr_cmd); + list_move_tail(&ipr_cmd->queue, &doneq); rc = IRQ_HANDLED; @@ -5194,8 +5212,8 @@ static irqreturn_t ipr_isr(int irq, void } else if (num_hrrq == IPR_MAX_HRRQ_RETRIES && int_reg & IPR_PCII_HRRQ_UPDATED) { ipr_isr_eh(ioa_cfg, "Error clearing HRRQ"); - spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags); - return IRQ_HANDLED; + rc = IRQ_HANDLED; + goto unlock_out; } else break; } @@ -5203,7 +5221,14 @@ static irqreturn_t ipr_isr(int irq, void if (unlikely(rc == IRQ_NONE)) rc = ipr_handle_other_interrupt(ioa_cfg, int_reg); +unlock_out: spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags); + list_for_each_entry_safe(ipr_cmd, temp, &doneq, queue) { + list_del(&ipr_cmd->queue); + del_timer(&ipr_cmd->timer); + ipr_cmd->fast_done(ipr_cmd); + } + return rc; } @@ -5784,15 +5809,22 @@ static void ipr_scsi_done(struct ipr_cmn struct ipr_ioa_cfg *ioa_cfg = ipr_cmd->ioa_cfg; struct sc
[PATCHv2 1/3] ipr: Reduce queuecommand lock time
Reduce the amount of time the host lock is held in queuecommand for improved performance. Signed-off-by: Brian King --- drivers/scsi/ipr.c | 90 + 1 file changed, 63 insertions(+), 27 deletions(-) diff -puN drivers/scsi/ipr.c~ipr_reduce_lock_contention drivers/scsi/ipr.c --- linux-2.6/drivers/scsi/ipr.c~ipr_reduce_lock_contention 2012-07-16 15:33:32.0 -0500 +++ linux-2.6-bjking1/drivers/scsi/ipr.c2012-07-17 08:04:57.0 -0500 @@ -620,25 +620,39 @@ static void ipr_init_ipr_cmnd(struct ipr } /** - * ipr_get_free_ipr_cmnd - Get a free IPR Cmnd block + * __ipr_get_free_ipr_cmnd - Get a free IPR Cmnd block * @ioa_cfg: ioa config struct * * Return value: * pointer to ipr command struct **/ static -struct ipr_cmnd *ipr_get_free_ipr_cmnd(struct ipr_ioa_cfg *ioa_cfg) +struct ipr_cmnd *__ipr_get_free_ipr_cmnd(struct ipr_ioa_cfg *ioa_cfg) { struct ipr_cmnd *ipr_cmd; ipr_cmd = list_entry(ioa_cfg->free_q.next, struct ipr_cmnd, queue); list_del(&ipr_cmd->queue); - ipr_init_ipr_cmnd(ipr_cmd); return ipr_cmd; } /** + * ipr_get_free_ipr_cmnd - Get a free IPR Cmnd block and initialize it + * @ioa_cfg: ioa config struct + * + * Return value: + * pointer to ipr command struct + **/ +static +struct ipr_cmnd *ipr_get_free_ipr_cmnd(struct ipr_ioa_cfg *ioa_cfg) +{ + struct ipr_cmnd *ipr_cmd = __ipr_get_free_ipr_cmnd(ioa_cfg); + ipr_init_ipr_cmnd(ipr_cmd); + return ipr_cmd; +} + +/** * ipr_mask_and_clear_interrupts - Mask all and clear specified interrupts * @ioa_cfg: ioa config struct * @clr_ints: interrupts to clear @@ -5783,8 +5797,8 @@ static void ipr_scsi_done(struct ipr_cmn /** * ipr_queuecommand - Queue a mid-layer request + * @shost: scsi host struct * @scsi_cmd: scsi command struct - * @done: done function * * This function queues a request generated by the mid-layer. * @@ -5793,61 +5807,58 @@ static void ipr_scsi_done(struct ipr_cmn * SCSI_MLQUEUE_DEVICE_BUSY if device is busy * SCSI_MLQUEUE_HOST_BUSY if host is busy **/ -static int ipr_queuecommand_lck(struct scsi_cmnd *scsi_cmd, - void (*done) (struct scsi_cmnd *)) +static int ipr_queuecommand(struct Scsi_Host *shost, + struct scsi_cmnd *scsi_cmd) { struct ipr_ioa_cfg *ioa_cfg; struct ipr_resource_entry *res; struct ipr_ioarcb *ioarcb; struct ipr_cmnd *ipr_cmd; + unsigned long lock_flags; int rc = 0; - scsi_cmd->scsi_done = done; - ioa_cfg = (struct ipr_ioa_cfg *)scsi_cmd->device->host->hostdata; - res = scsi_cmd->device->hostdata; + ioa_cfg = (struct ipr_ioa_cfg *)shost->hostdata; + + spin_lock_irqsave(shost->host_lock, lock_flags); scsi_cmd->result = (DID_OK << 16); + res = scsi_cmd->device->hostdata; /* * We are currently blocking all devices due to a host reset * We have told the host to stop giving us new requests, but * ERP ops don't count. FIXME */ - if (unlikely(!ioa_cfg->allow_cmds && !ioa_cfg->ioa_is_dead)) + if (unlikely(!ioa_cfg->allow_cmds && !ioa_cfg->ioa_is_dead)) { + spin_unlock_irqrestore(shost->host_lock, lock_flags); return SCSI_MLQUEUE_HOST_BUSY; + } /* * FIXME - Create scsi_set_host_offline interface * and the ioa_is_dead check can be removed */ if (unlikely(ioa_cfg->ioa_is_dead || !res)) { - memset(scsi_cmd->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE); - scsi_cmd->result = (DID_NO_CONNECT << 16); - scsi_cmd->scsi_done(scsi_cmd); - return 0; + spin_unlock_irqrestore(shost->host_lock, lock_flags); + goto err_nodev; } if (ipr_is_gata(res) && res->sata_port) return ata_sas_queuecmd(scsi_cmd, res->sata_port->ap); - ipr_cmd = ipr_get_free_ipr_cmnd(ioa_cfg); + ipr_cmd = __ipr_get_free_ipr_cmnd(ioa_cfg); + spin_unlock_irqrestore(shost->host_lock, lock_flags); + + ipr_init_ipr_cmnd(ipr_cmd); ioarcb = &ipr_cmd->ioarcb; - list_add_tail(&ipr_cmd->queue, &ioa_cfg->pending_q); memcpy(ioarcb->cmd_pkt.cdb, scsi_cmd->cmnd, scsi_cmd->cmd_len); ipr_cmd->scsi_cmd = scsi_cmd; - ioarcb->res_handle = res->res_handle; ipr_cmd->done = ipr_scsi_done; - ipr_trc_hook(ipr_cmd, IPR_TRACE_START, IPR_GET_RES_PHYS_LOC(res)); if (ipr_is_gscsi(res) || ipr_is_vset_device(res)) { if (scsi_cmd->underflow == 0) ioarcb->cmd_pkt.flags_hi |= IPR_FLAGS_HI_NO_ULEN_CHK; - if (res->needs_sync_complete) { - ioarcb->cmd_pkt.flags_hi |= IPR_FLAGS_HI_SYNC_COMPLETE; - res->needs_sy
Re: [PATCH 1/3] ipr: Reduce queuecommand lock time
On 07/16/2012 09:03 PM, Matthew Wilcox wrote: > On Mon, Jul 16, 2012 at 03:48:08PM -0500, Brian King wrote: >> +static int ipr_queuecommand(struct Scsi_Host *shost, >> +struct scsi_cmnd *scsi_cmd) >> { >> struct ipr_ioa_cfg *ioa_cfg; >> struct ipr_resource_entry *res; >> struct ipr_ioarcb *ioarcb; >> struct ipr_cmnd *ipr_cmd; >> +unsigned long lock_flags = 0; > > You don't need to initialise lock_flags. > > Looking at the rest of the code, you drop the lock in the middle, > then re-acquire it. That'll help with hold time, but I'm not convinced > it'll help with performance. Have you done performance testing with > these changes? I seem to remember we used an eight-socket box to show > host_lock problems in the past. We've done performance testing of these patches and they provided roughly a 25% increase in the number of IOPs we are able to push through an adapter on Power. This was running on an 8 socket box with 4 way SMT, so 32 separate hardware threads. One of the main things these patches do is to get the dma map/unmap call from underneath the host lock. On Power, these calls have more overhead than on some other platforms, since they end up resulting in a hypervisor call, which can significantly increase host lock hold times. I'll resend with the change to not initialize the lock flags. Thanks, Brian -- Brian King Power Linux I/O IBM Linux Technology Center -- 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] sd: do not set changed flag on all unit attention conditions
Il 17/07/2012 14:21, James Bottomley ha scritto: >> Yes, I realize failing only on specific sense codes as I did it in the >> patch is not going to work. However, the other way round is not >> problematic (explicitly allow some sense codes, fail on all others). > > Heh, I once thought that, but there's no end to the strange ideas USB > manufacturers get. :) >> > Another example is "target operating conditions have changed". QEMU >> > cannot report such changes because scsi_error prints a warning (fine) >> > and then passes the unit attention upwards. With removable drives, this >> > has the same problem as resizing. > Why would a removable device ever use any of the codes under this ASC > when the medium hasn't changed? They're all for arrays (well except > 0x10 and 0x11 ... and they're only supposed to apply to tape changers > with autoload support declared in the control mode page). There are also a couple of generic ones such as "reported luns have changed". I also considered briefly using "inquiry data has changed" to reload the unmap parameters after live snapshot or storage migration. I dropped the idea, please don't put me in the same bin as the USB manufacturers. Paolo -- 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] sd: do not set changed flag on all unit attention conditions
On Tue, 2012-07-17 at 11:28 +0200, Paolo Bonzini wrote: > Il 17/07/2012 11:11, James Bottomley ha scritto: > > We don't do stuff just because the standards allows it; just the > > opposite: we try to use the smallest implementations from the standards > > we can get away with just because the more things we do, the more > > exceptions and broken devices we come across. > > Yes, I realize failing only on specific sense codes as I did it in the > patch is not going to work. However, the other way round is not > problematic (explicitly allow some sense codes, fail on all others). Heh, I once thought that, but there's no end to the strange ideas USB manufacturers get. > Another example is "target operating conditions have changed". QEMU > cannot report such changes because scsi_error prints a warning (fine) > and then passes the unit attention upwards. With removable drives, this > has the same problem as resizing. Why would a removable device ever use any of the codes under this ASC when the medium hasn't changed? They're all for arrays (well except 0x10 and 0x11 ... and they're only supposed to apply to tape changers with autoload support declared in the control mode page). The unanswered point is still that there's no use case for a device that's both removable and requires array like sense code support. James -- 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 03/09] [SCSI] mpt2sas : Fix for Driver oops, when loading driver with max_queue_depth command line option to a very small value
If the specified max_queue_depth setting is less than the expected number of internal commands, then driver will calculate the queue depth size to a negitive number. This negitive number is actually a very large number because variable is unsigned 16bit integer. So, the driver will ask for a very large amount of memory for message frames and resulting into oops as memory allocation routines will not able to handle such a large request. So, in order to limit this kind of oops, The driver need to set the max_queue_depth to a scsi mid layer's can_queue value. Then the overall message frames required for IO is minimum of either (max_queue_depth plus internal commands) or the IOC global credits. Signed-off-by: Sreekanth Reddy Cc: --- diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c index ffa32ad..f7c1394 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_base.c +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c @@ -2424,10 +2424,13 @@ _base_allocate_memory_pools(struct MPT2SAS_ADAPTER *ioc, int sleep_flag) } /* command line tunables for max controller queue depth */ - if (max_queue_depth != -1) - max_request_credit = (max_queue_depth < facts->RequestCredit) - ? max_queue_depth : facts->RequestCredit; - else + if (max_queue_depth != -1 && max_queue_depth != 0) { + max_request_credit = min_t(u16, max_queue_depth + + ioc->hi_priority_depth + ioc->internal_depth, + facts->RequestCredit); + if (max_request_credit > MAX_HBA_QUEUE_DEPTH) + max_request_credit = MAX_HBA_QUEUE_DEPTH; + } else max_request_credit = min_t(u16, facts->RequestCredit, MAX_HBA_QUEUE_DEPTH); @@ -2502,7 +2505,7 @@ _base_allocate_memory_pools(struct MPT2SAS_ADAPTER *ioc, int sleep_flag) /* set the scsi host can_queue depth * with some internal commands that could be outstanding */ - ioc->shost->can_queue = ioc->scsiio_depth - (2); + ioc->shost->can_queue = ioc->scsiio_depth; dinitprintk(ioc, printk(MPT2SAS_INFO_FMT "scsi host: " "can_queue depth (%d)\n", ioc->name, ioc->shost->can_queue)); -- 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 09/09] [SCSI] mpt2sas : Bump driver vesion to 14.100.00.00
Bump driver version to 14.100.00.00 Signed-off-by: Sreekanth Reddy --- diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.h b/drivers/scsi/mpt2sas/mpt2sas_base.h index dd05d24..e7e517a 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_base.h +++ b/drivers/scsi/mpt2sas/mpt2sas_base.h @@ -69,8 +69,8 @@ #define MPT2SAS_DRIVER_NAME"mpt2sas" #define MPT2SAS_AUTHOR "LSI Corporation " #define MPT2SAS_DESCRIPTION"LSI MPT Fusion SAS 2.0 Device Driver" -#define MPT2SAS_DRIVER_VERSION "13.100.00.00" -#define MPT2SAS_MAJOR_VERSION 13 +#define MPT2SAS_DRIVER_VERSION "14.100.00.00" +#define MPT2SAS_MAJOR_VERSION 14 #define MPT2SAS_MINOR_VERSION 100 #define MPT2SAS_BUILD_VERSION 00 #define MPT2SAS_RELEASE_VERSION00 -- 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 08/09] [SCSI] mpt2sas : Fix for With post diag reset same set of device gets added, removed and then again gets added with new target ids
When device discovery is disabled during driver load time using module parameter "disable_discovery=1" and when diag reset is issued then from logs, it is observed that the devices get added, removed and then added with new target ids. So, inorder to limit this turn-off the code which is deleting and devices across host reset when the disable_discovery module parameter is turned on. Signed-off-by: Sreekanth Reddy --- diff --git a/drivers/scsi/mpt2sas/mpt2sas_scsih.c b/drivers/scsi/mpt2sas/mpt2sas_scsih.c index 7a69a5c..7efefb7 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_scsih.c +++ b/drivers/scsi/mpt2sas/mpt2sas_scsih.c @@ -7264,7 +7264,8 @@ mpt2sas_scsih_reset_handler(struct MPT2SAS_ADAPTER *ioc, int reset_phase) _scsih_search_responding_sas_devices(ioc); _scsih_search_responding_raid_devices(ioc); _scsih_search_responding_expanders(ioc); - if (!ioc->is_driver_loading) { + if ((!ioc->is_driver_loading) && !(disable_discovery > 0 && + !ioc->sas_hba.num_phys)) { _scsih_prep_device_scan(ioc); _scsih_search_responding_sas_devices(ioc); _scsih_search_responding_raid_devices(ioc); -- 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 06/09] [SCSI] mpt2sas : MPI next revision header update
MPI 2.0 Rev V(2.0.14) specification Changeset in MPI 2.0 Rev V(2.0.14) specification 1) Bumped MPI2_HEADER_VERSION_UNIT. 2) Added a product specific range to event values. 3) Added clarification to Direct-Attached SAS PHY Power condition. 4) Updated timing requirements for performing Hard Reset. Signed-off-by: Sreekanth Reddy --- diff --git a/drivers/scsi/mpt2sas/mpi/mpi2.h b/drivers/scsi/mpt2sas/mpi/mpi2.h index a80f322..e960f96 100644 --- a/drivers/scsi/mpt2sas/mpi/mpi2.h +++ b/drivers/scsi/mpt2sas/mpi/mpi2.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000-2011 LSI Corporation. + * Copyright (c) 2000-2012 LSI Corporation. * * * Name: mpi2.h @@ -8,7 +8,7 @@ * scatter/gather formats. * Creation Date: June 21, 2006 * - * mpi2.h Version: 02.00.23 + * mpi2.h Version: 02.00.25 * * Version History * --- @@ -72,6 +72,9 @@ * 05-25-11 02.00.21 Bumped MPI2_HEADER_VERSION_UNIT. * 08-24-11 02.00.22 Bumped MPI2_HEADER_VERSION_UNIT. * 11-18-11 02.00.23 Bumped MPI2_HEADER_VERSION_UNIT. + * 02-06-12 02.00.24 Bumped MPI2_HEADER_VERSION_UNIT. + * 03-29-12 02.00.25 Bumped MPI2_HEADER_VERSION_UNIT. + * Added Hard Reset delay timings. * -- */ @@ -97,7 +100,7 @@ #define MPI2_VERSION_02_00 (0x0200) /* versioning for this MPI header set */ -#define MPI2_HEADER_VERSION_UNIT(0x17) +#define MPI2_HEADER_VERSION_UNIT(0x19) #define MPI2_HEADER_VERSION_DEV (0x00) #define MPI2_HEADER_VERSION_UNIT_MASK (0xFF00) #define MPI2_HEADER_VERSION_UNIT_SHIFT (8) @@ -275,6 +278,11 @@ typedef volatile struct _MPI2_SYSTEM_INTERFACE_REGS #define MPI2_REQUEST_DESCRIPTOR_POST_HIGH_OFFSET(0x00C4) +/* Hard Reset delay timings */ +#define MPI2_HARD_RESET_PCIE_FIRST_READ_DELAY_MICRO_SEC (5) +#define MPI2_HARD_RESET_PCIE_RESET_READ_WINDOW_MICRO_SEC(255000) +#define MPI2_HARD_RESET_PCIE_SECOND_READ_DELAY_MICRO_SEC(256000) + /* * *Message Descriptors diff --git a/drivers/scsi/mpt2sas/mpi/mpi2_init.h b/drivers/scsi/mpt2sas/mpi/mpi2_init.h index de90162..38c5da3 100644 --- a/drivers/scsi/mpt2sas/mpi/mpi2_init.h +++ b/drivers/scsi/mpt2sas/mpi/mpi2_init.h @@ -1,12 +1,12 @@ /* - * Copyright (c) 2000-2010 LSI Corporation. + * Copyright (c) 2000-2012 LSI Corporation. * * * Name: mpi2_init.h * Title: MPI SCSI initiator mode messages and structures * Creation Date: June 23, 2006 * - *mpi2_init.h Version: 02.00.11 + *mpi2_init.h Version: 02.00.13 * * Version History * --- @@ -34,6 +34,8 @@ * 02-10-10 02.00.09 Removed unused structure that had "#if 0" around it. * 05-12-10 02.00.10 Added optional vendor-unique region to SCSI IO Request. * 11-10-10 02.00.11 Added MPI2_SCSIIO_NUM_SGLOFFSETS define. + * 02-06-12 02.00.13 Added alternate defines for Task Priority / Command + * Priority to match SAM-4. * -- */ @@ -194,6 +196,9 @@ typedef struct _MPI2_SCSI_IO_REQUEST #define MPI2_SCSIIO_CONTROL_TASKPRI_MASK(0x7800) #define MPI2_SCSIIO_CONTROL_TASKPRI_SHIFT (11) +/* alternate name for the previous field; called Command Priority in SAM-4 */ +#define MPI2_SCSIIO_CONTROL_CMDPRI_MASK (0x7800) +#define MPI2_SCSIIO_CONTROL_CMDPRI_SHIFT(11) #define MPI2_SCSIIO_CONTROL_TASKATTRIBUTE_MASK (0x0700) #define MPI2_SCSIIO_CONTROL_SIMPLEQ (0x) diff --git a/drivers/scsi/mpt2sas/mpi/mpi2_ioc.h b/drivers/scsi/mpt2sas/mpi/mpi2_ioc.h index 9a925c0..b0d4760 100644 --- a/drivers/scsi/mpt2sas/mpi/mpi2_ioc.h +++ b/drivers/scsi/mpt2sas/mpi/mpi2_ioc.h @@ -1,12 +1,12 @@ /* - * Copyright (c) 2000-2011 LSI Corporation. + * Copyright (c) 2000-2012 LSI Corporation. * * * Name: mpi2_ioc.h * Title: MPI IOC, Port, Event, FW Download, and FW Upload messages * Creation Date: October 11, 2006 * - * mpi2_ioc.h Version: 02.00.19 + * mpi2_ioc.h Version: 02.00.21 * * Version History * --- @@ -117,6 +117,7 @@ * 08-24-11 02.00.19 Added PhysicalPort field to * MPI2_EVENT_DATA_SAS_DEVICE_STATUS_CHANGE structure. * Marked MPI2_PM_CONTROL_FEATURE_PCIE_LINK as obsolete. + * 03-29-12 02.00.21 Added a product specific range to event values. * -- */ @@ -492,7 +493,8 @@ typedef struct _MPI2_EVENT_NOTIFICATION_REPLY #define MPI2_EVENT_SAS_NOTIFY_PRIMITIVE (0x0026) #define MPI2_EVENT_TEMP_THRESHOLD (0x0027) #define MPI2_EVENT_HOST_MESSAGE
[PATCH 07/09] [SCSI] mpt2sas : Fix for staged device discovery functionality of driver not working
This patch provides a command line option to disable "Port enable" during the driver load. The objective of this command line option is to load the driver and do all the necessary initialization excluding port enable(i.e. delay device discovery) Signed-off-by: Sreekanth Reddy --- diff --git a/drivers/scsi/mpt2sas/mpt2sas_scsih.c b/drivers/scsi/mpt2sas/mpt2sas_scsih.c index 3e231a7..7a69a5c 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_scsih.c +++ b/drivers/scsi/mpt2sas/mpt2sas_scsih.c @@ -119,6 +119,10 @@ module_param(diag_buffer_enable, int, 0); MODULE_PARM_DESC(diag_buffer_enable, " post diag buffers " "(TRACE=1/SNAPSHOT=2/EXTENDED=4/default=0)"); +static int disable_discovery = -1; +module_param(disable_discovery, int, 0); +MODULE_PARM_DESC(disable_discovery, " disable discovery "); + /** * struct sense_info - common structure for obtaining sense keys * @skey: sense key @@ -5973,8 +5977,14 @@ _scsih_sas_discovery_event(struct MPT2SAS_ADAPTER *ioc, #endif if (event_data->ReasonCode == MPI2_EVENT_SAS_DISC_RC_STARTED && - !ioc->sas_hba.num_phys) + !ioc->sas_hba.num_phys) { + if (disable_discovery > 0 && ioc->shost_recovery) { + /* Wait for the reset to complete */ + while (ioc->shost_recovery) + ssleep(1); + } _scsih_sas_host_add(ioc); + } } /** @@ -7929,6 +7939,9 @@ _scsih_scan_start(struct Scsi_Host *shost) if (diag_buffer_enable != -1 && diag_buffer_enable != 0) mpt2sas_enable_diag_buffer(ioc, diag_buffer_enable); + if (disable_discovery > 0) + return; + ioc->start_scan = 1; rc = mpt2sas_port_enable(ioc); @@ -7950,6 +7963,12 @@ _scsih_scan_finished(struct Scsi_Host *shost, unsigned long time) { struct MPT2SAS_ADAPTER *ioc = shost_priv(shost); + if (disable_discovery > 0) { + ioc->is_driver_loading = 0; + ioc->wait_for_discovery_to_complete = 0; + return 1; + } + if (time >= (300 * HZ)) { ioc->base_cmds.status = MPT2_CMD_NOT_USED; printk(MPT2SAS_INFO_FMT "port enable: FAILED with timeout " -- 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 05/09] [SCSI] mpt2sas : Fix for max_sectors warning message is stating the incorrect range
When specifying the command line option "max_sectors" less than 64, then warning message should provide correct upper boundary value 32767 instead of 8192. Signed-off-by: Sreekanth Reddy --- diff --git a/drivers/scsi/mpt2sas/mpt2sas_scsih.c b/drivers/scsi/mpt2sas/mpt2sas_scsih.c index 0f4f971..4a09ace 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_scsih.c +++ b/drivers/scsi/mpt2sas/mpt2sas_scsih.c @@ -8055,8 +8055,8 @@ _scsih_probe(struct pci_dev *pdev, const struct pci_device_id *id) if (max_sectors != 0x) { if (max_sectors < 64) { shost->max_sectors = 64; - printk(MPT2SAS_WARN_FMT "Invalid value %d passed " - "for max_sectors, range is 64 to 8192. Assigning " + printk(MPT2SAS_WARN_FMT "Invalid value %d passed "\ + "for max_sectors, range is 64 to 32767. Assigning "\ "value of 64.\n", ioc->name, max_sectors); } else if (max_sectors > 32767) { shost->max_sectors = 32767; -- 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 04/09] [SCSI] mpt2sas : Provide sysfs attribute to report Backup Rail Monitor Status
A new sysfs shost attribute called "BMR_status" is implemented to report Backup Rail Monitor status. This attribute is located in the path /sys/class/scsi_host/host#/BMR_status when reading this adapter attribute, then driver will output the state of GPIO[24]. It returns "0" if BMR is healthy and it returns "1" for failure. if it returns an empty string then it means that there was an error while obtaining the BMR status. Then check dmesg for what error has occured. Signed-off-by: Sreekanth Reddy --- diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.h b/drivers/scsi/mpt2sas/mpt2sas_base.h index 401c8ac..dd05d24 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_base.h +++ b/drivers/scsi/mpt2sas/mpt2sas_base.h @@ -1096,6 +1096,8 @@ int mpt2sas_config_get_iounit_pg1(struct MPT2SAS_ADAPTER *ioc, Mpi2ConfigReply_t *mpi_reply, Mpi2IOUnitPage1_t *config_page); int mpt2sas_config_set_iounit_pg1(struct MPT2SAS_ADAPTER *ioc, Mpi2ConfigReply_t *mpi_reply, Mpi2IOUnitPage1_t *config_page); +int mpt2sas_config_get_iounit_pg3(struct MPT2SAS_ADAPTER *ioc, + Mpi2ConfigReply_t *mpi_reply, Mpi2IOUnitPage3_t *config_page, u16 sz); int mpt2sas_config_get_sas_iounit_pg1(struct MPT2SAS_ADAPTER *ioc, Mpi2ConfigReply_t *mpi_reply, Mpi2SasIOUnitPage1_t *config_page, u16 sz); int mpt2sas_config_set_sas_iounit_pg1(struct MPT2SAS_ADAPTER *ioc, diff --git a/drivers/scsi/mpt2sas/mpt2sas_config.c b/drivers/scsi/mpt2sas/mpt2sas_config.c index 323309b..8637780 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_config.c +++ b/drivers/scsi/mpt2sas/mpt2sas_config.c @@ -683,6 +683,42 @@ mpt2sas_config_set_iounit_pg1(struct MPT2SAS_ADAPTER *ioc, } /** + * mpt2sas_config_get_iounit_pg3 - obtain iounit page 3 + * @ioc: per adapter object + * @mpi_reply: reply mf payload returned from firmware + * @config_page: contents of the config page + * @sz: size of buffer passed in config_page + * Context: sleep. + * + * Returns 0 for success, non-zero for failure. + */ +int +mpt2sas_config_get_iounit_pg3(struct MPT2SAS_ADAPTER *ioc, + Mpi2ConfigReply_t *mpi_reply, Mpi2IOUnitPage3_t *config_page, u16 sz) +{ + Mpi2ConfigRequest_t mpi_request; + int r; + + memset(&mpi_request, 0, sizeof(Mpi2ConfigRequest_t)); + mpi_request.Function = MPI2_FUNCTION_CONFIG; + mpi_request.Action = MPI2_CONFIG_ACTION_PAGE_HEADER; + mpi_request.Header.PageType = MPI2_CONFIG_PAGETYPE_IO_UNIT; + mpi_request.Header.PageNumber = 3; + mpi_request.Header.PageVersion = MPI2_IOUNITPAGE3_PAGEVERSION; + mpt2sas_base_build_zero_len_sge(ioc, &mpi_request.PageBufferSGE); + r = _config_request(ioc, &mpi_request, mpi_reply, + MPT2_CONFIG_PAGE_DEFAULT_TIMEOUT, NULL, 0); + if (r) + goto out; + + mpi_request.Action = MPI2_CONFIG_ACTION_PAGE_READ_CURRENT; + r = _config_request(ioc, &mpi_request, mpi_reply, + MPT2_CONFIG_PAGE_DEFAULT_TIMEOUT, config_page, sz); + out: + return r; +} + +/** * mpt2sas_config_get_ioc_pg8 - obtain ioc page 8 * @ioc: per adapter object * @mpi_reply: reply mf payload returned from firmware diff --git a/drivers/scsi/mpt2sas/mpt2sas_ctl.c b/drivers/scsi/mpt2sas/mpt2sas_ctl.c index 6da4a40..6425441 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_ctl.c +++ b/drivers/scsi/mpt2sas/mpt2sas_ctl.c @@ -2690,6 +2690,75 @@ _ctl_ioc_reply_queue_count_show(struct device *cdev, static DEVICE_ATTR(reply_queue_count, S_IRUGO, _ctl_ioc_reply_queue_count_show, NULL); +/** + * _ctl_BRM_status_show - Backup Rail Monitor Status + * @cdev - pointer to embedded class device + * @buf - the buffer returned + * + * This is number of reply queues + * + * A sysfs 'read-only' shost attribute. + */ +static ssize_t +_ctl_BRM_status_show(struct device *cdev, struct device_attribute *attr, + char *buf) +{ + struct Scsi_Host *shost = class_to_shost(cdev); + struct MPT2SAS_ADAPTER *ioc = shost_priv(shost); + Mpi2IOUnitPage3_t *io_unit_pg3 = NULL; + Mpi2ConfigReply_t mpi_reply; + u16 backup_rail_monitor_status = 0; + u16 ioc_status; + int sz; + ssize_t rc = 0; + + if (!ioc->is_warpdrive) { + printk(MPT2SAS_ERR_FMT "%s: BRM attribute is only for"\ + "warpdrive\n", ioc->name, __func__); + goto out; + } + + /* allocate upto GPIOVal 36 entries */ + sz = offsetof(Mpi2IOUnitPage3_t, GPIOVal) + (sizeof(u16) * 36); + io_unit_pg3 = kzalloc(sz, GFP_KERNEL); + if (!io_unit_pg3) { + printk(MPT2SAS_ERR_FMT "%s: failed allocating memory"\ + "for iounit_pg3: (%d) bytes\n", ioc->name, __func__, sz); + goto out; + } + + if (mpt2sas_config_get_iounit_pg3(ioc, &mpi_reply, io_unit_pg3, sz) != + 0) { + printk(MPT2SAS_ERR_FMT + "%s: failed reading iounit_pg3\n", ioc->name, + __func__); + goto out; + }
[PATCH 02/09] [SCSI] mpt2sas : To include more Intel Branding
Updating the customer branding string for "SSD 910 Series" controller Signed-off-by: Sreekanth Reddy --- diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c index e68deff..ffa32ad 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_base.c +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c @@ -1971,9 +1971,9 @@ _base_display_intel_branding(struct MPT2SAS_ADAPTER *ioc) printk(MPT2SAS_INFO_FMT "%s\n", ioc->name, MPT2SAS_INTEL_RMS2LL040_BRANDING); break; - case MPT2SAS_INTEL_RAMSDALE_SSDID: + case MPT2SAS_INTEL_SSD910_SSDID: printk(MPT2SAS_INFO_FMT "%s\n", ioc->name, - MPT2SAS_INTEL_RAMSDALE_BRANDING); + MPT2SAS_INTEL_SSD910_BRANDING); break; default: break; diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.h b/drivers/scsi/mpt2sas/mpt2sas_base.h index 64ac0c6..401c8ac 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_base.h +++ b/drivers/scsi/mpt2sas/mpt2sas_base.h @@ -171,8 +171,8 @@ "Intel Integrated RAID Module RMS2LL040" #define MPT2SAS_INTEL_RS25GB008_BRANDING \ "Intel(R) RAID Controller RS25GB008" -#define MPT2SAS_INTEL_RAMSDALE_BRANDING\ - "Intel 720 Series SSD" +#define MPT2SAS_INTEL_SSD910_BRANDING \ + "Intel(R) SSD 910 Series" /* * Intel HBA SSDIDs */ @@ -183,7 +183,7 @@ #define MPT2SAS_INTEL_RMS2LL080_SSDID 0x350E #define MPT2SAS_INTEL_RMS2LL040_SSDID 0x350F #define MPT2SAS_INTEL_RS25GB008_SSDID 0x3000 -#define MPT2SAS_INTEL_RAMSDALE_SSDID 0x3700 +#define MPT2SAS_INTEL_SSD910_SSDID 0x3700 /* * HP HBA branding -- 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 01/09] [SCSI] mpt2sas : 2012 source code copyright
2012 source code copyright - The Copyright String in all the drivers sources were changed to 2012 Signed-off-by: Sreekanth Reddy --- diff --git a/drivers/scsi/mpt2sas/Kconfig b/drivers/scsi/mpt2sas/Kconfig index bbb7e4b..39f08dd 100644 --- a/drivers/scsi/mpt2sas/Kconfig +++ b/drivers/scsi/mpt2sas/Kconfig @@ -2,7 +2,7 @@ # Kernel configuration file for the MPT2SAS # # This code is based on drivers/scsi/mpt2sas/Kconfig -# Copyright (C) 2007-2010 LSI Corporation +# Copyright (C) 2007-2012 LSI Corporation # (mailto:dl-mptfusionli...@lsi.com) # This program is free software; you can redistribute it and/or diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c index 6102ef2..e68deff 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_base.c +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c @@ -3,7 +3,7 @@ * for access to MPT (Message Passing Technology) firmware. * * This code is based on drivers/scsi/mpt2sas/mpt2_base.c - * Copyright (C) 2007-2010 LSI Corporation + * Copyright (C) 2007-2012 LSI Corporation * (mailto:dl-mptfusionli...@lsi.com) * * This program is free software; you can redistribute it and/or diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.h b/drivers/scsi/mpt2sas/mpt2sas_base.h index b6dd3a5..64ac0c6 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_base.h +++ b/drivers/scsi/mpt2sas/mpt2sas_base.h @@ -3,7 +3,7 @@ * for access to MPT (Message Passing Technology) firmware. * * This code is based on drivers/scsi/mpt2sas/mpt2_base.h - * Copyright (C) 2007-2010 LSI Corporation + * Copyright (C) 2007-2012 LSI Corporation * (mailto:dl-mptfusionli...@lsi.com) * * This program is free software; you can redistribute it and/or diff --git a/drivers/scsi/mpt2sas/mpt2sas_config.c b/drivers/scsi/mpt2sas/mpt2sas_config.c index 2b4d376..323309b 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_config.c +++ b/drivers/scsi/mpt2sas/mpt2sas_config.c @@ -2,7 +2,7 @@ * This module provides common API for accessing firmware configuration pages * * This code is based on drivers/scsi/mpt2sas/mpt2_base.c - * Copyright (C) 2007-2010 LSI Corporation + * Copyright (C) 2007-2012 LSI Corporation * (mailto:dl-mptfusionli...@lsi.com) * * This program is free software; you can redistribute it and/or diff --git a/drivers/scsi/mpt2sas/mpt2sas_ctl.c b/drivers/scsi/mpt2sas/mpt2sas_ctl.c index 49bdd2d..6da4a40 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_ctl.c +++ b/drivers/scsi/mpt2sas/mpt2sas_ctl.c @@ -3,7 +3,7 @@ * controllers * * This code is based on drivers/scsi/mpt2sas/mpt2_ctl.c - * Copyright (C) 2007-2010 LSI Corporation + * Copyright (C) 2007-2012 LSI Corporation * (mailto:dl-mptfusionli...@lsi.com) * * This program is free software; you can redistribute it and/or diff --git a/drivers/scsi/mpt2sas/mpt2sas_ctl.h b/drivers/scsi/mpt2sas/mpt2sas_ctl.h index 11ff1d5..b5eb0d1 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_ctl.h +++ b/drivers/scsi/mpt2sas/mpt2sas_ctl.h @@ -3,7 +3,7 @@ * controllers * * This code is based on drivers/scsi/mpt2sas/mpt2_ctl.h - * Copyright (C) 2007-2010 LSI Corporation + * Copyright (C) 2007-2012 LSI Corporation * (mailto:dl-mptfusionli...@lsi.com) * * This program is free software; you can redistribute it and/or diff --git a/drivers/scsi/mpt2sas/mpt2sas_debug.h b/drivers/scsi/mpt2sas/mpt2sas_debug.h index 9731f8e..69cc7d0 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_debug.h +++ b/drivers/scsi/mpt2sas/mpt2sas_debug.h @@ -2,7 +2,7 @@ * Logging Support for MPT (Message Passing Technology) based controllers * * This code is based on drivers/scsi/mpt2sas/mpt2_debug.c - * Copyright (C) 2007-2010 LSI Corporation + * Copyright (C) 2007-2012 LSI Corporation * (mailto:dl-mptfusionli...@lsi.com) * * This program is free software; you can redistribute it and/or diff --git a/drivers/scsi/mpt2sas/mpt2sas_scsih.c b/drivers/scsi/mpt2sas/mpt2sas_scsih.c index 76973e8..0f4f971 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_scsih.c +++ b/drivers/scsi/mpt2sas/mpt2sas_scsih.c @@ -2,7 +2,7 @@ * Scsi Host Layer for MPT (Message Passing Technology) based controllers * * This code is based on drivers/scsi/mpt2sas/mpt2_scsih.c - * Copyright (C) 2007-2010 LSI Corporation + * Copyright (C) 2007-2012 LSI Corporation * (mailto:dl-mptfusionli...@lsi.com) * * This program is free software; you can redistribute it and/or diff --git a/drivers/scsi/mpt2sas/mpt2sas_transport.c b/drivers/scsi/mpt2sas/mpt2sas_transport.c index c6cf20f..8c2ffbe 100644 --- a/drivers/scsi/mpt2sas/mpt2sas_transport.c +++ b/drivers/scsi/mpt2sas/mpt2sas_transport.c @@ -2,7 +2,7 @@ * SAS Transport Layer for MPT (Message Passing Technology) based controllers * * This code is based on drivers/scsi/mpt2sas/mpt2_transport.c - * Copyright (C) 2007-2010 LSI Corporation + * Copyright (C) 2007-2012 LSI Corporation * (mailto:dl-mptfusionli...@lsi.com) * * This program is free software; you can redistribute it and/or -- To unsubscribe from thi
[PATCH] [scsi] mpt2sas: Description Patch
Please consider this patch set for next kernel release. Signed-off-by: Sreekanth Reddy --- [PATCH 01/09] [SCSI] mpt2sas : 2012 source code copyright. [PATCH 02/09] [SCSI] mpt2sas : To include more Intel Branding. [PATCH 03/09] [SCSI] mpt2sas : Fix for Driver oops, when loading driver with max_queue_depth command line option to a very small value. [PATCH 04/09] [SCSI] mpt2sas : Provide sysfs attribute to report Backup Rail Monitor Status. [PATCH 05/09] [SCSI] mpt2sas : Fix for max_sectors warning message is stating the incorrect range. [PATCH 06/09] [SCSI] mpt2sas : MPI next revision header update. [PATCH 07/09] [SCSI] mpt2sas : Fix for staged device discovery functionality of driver not working. [PATCH 08/09] [SCSI] mpt2sas : Fix for With post diag reset same set of device gets added, removed and then again gets added with new target ids. [PATCH 09/09] [SCSI] mpt2sas : Bump driver vesion to 14.100.00.00. -- 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] sd: do not set changed flag on all unit attention conditions
Il 17/07/2012 11:11, James Bottomley ha scritto: > We don't do stuff just because the standards allows it; just the > opposite: we try to use the smallest implementations from the standards > we can get away with just because the more things we do, the more > exceptions and broken devices we come across. Yes, I realize failing only on specific sense codes as I did it in the patch is not going to work. However, the other way round is not problematic (explicitly allow some sense codes, fail on all others). Another example is "target operating conditions have changed". QEMU cannot report such changes because scsi_error prints a warning (fine) and then passes the unit attention upwards. With removable drives, this has the same problem as resizing. Paolo -- 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] sd: do not set changed flag on all unit attention conditions
On Tue, 2012-07-17 at 10:54 +0200, Paolo Bonzini wrote: > Il 17/07/2012 10:40, James Bottomley ha scritto: > >> > > >> > It's not specific to virtio-scsi, in fact I expect that virtio-scsi will > >> > be almost always used with non-removable disks. > >> > > >> > However, QEMU's SCSI target is not used just for virtio-scsi (for > >> > example it can be used for USB storage), and it lets you mark a disk as > >> > removable---why? because there exists real hardware that presents itself > >> > as an SBC removable disk. The only thing that is specific to > >> > virtualization, is support for online resizing (which generates a unit > >> > attention condition CAPACITY DATA HAS CHANGED). > > So what's the problem? If you're doing pass through of a physical disk, > > we pick up removable from its inquiry string ... a physical removable > > device doesn't get resized. If you have a virtual disk you want to > > resize, you don't set the removable flag in the inquiry data. > > In practice people will do what you said, and it's not a problem. > > However, there's nothing that prevents you from running qemu with a > removable SCSI disk, and then resizing it. I would like this to work, > because SBC allows it and there's no reason why it shouldn't. There's no such thing in the market today as a removable disk that's resizeable. Removable disks are for things like backup cartridges and ageing jazz drives. Worse: most removeable devices today are USB card readers whose standards compliance varies from iffy to non existent. Resizeable disks are currently the province of storage arrays. We don't do stuff just because the standards allows it; just the opposite: we try to use the smallest implementations from the standards we can get away with just because the more things we do, the more exceptions and broken devices we come across. James James -- 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] sd: do not set changed flag on all unit attention conditions
Il 17/07/2012 10:40, James Bottomley ha scritto: >> > >> > It's not specific to virtio-scsi, in fact I expect that virtio-scsi will >> > be almost always used with non-removable disks. >> > >> > However, QEMU's SCSI target is not used just for virtio-scsi (for >> > example it can be used for USB storage), and it lets you mark a disk as >> > removable---why? because there exists real hardware that presents itself >> > as an SBC removable disk. The only thing that is specific to >> > virtualization, is support for online resizing (which generates a unit >> > attention condition CAPACITY DATA HAS CHANGED). > So what's the problem? If you're doing pass through of a physical disk, > we pick up removable from its inquiry string ... a physical removable > device doesn't get resized. If you have a virtual disk you want to > resize, you don't set the removable flag in the inquiry data. In practice people will do what you said, and it's not a problem. However, there's nothing that prevents you from running qemu with a removable SCSI disk, and then resizing it. I would like this to work, because SBC allows it and there's no reason why it shouldn't. Paolo -- 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] sd: do not set changed flag on all unit attention conditions
On Tue, 2012-07-17 at 10:34 +0200, Paolo Bonzini wrote: > Il 17/07/2012 09:45, James Bottomley ha scritto: > > On Mon, 2012-07-16 at 19:20 +0200, Paolo Bonzini wrote: > >> Il 16/07/2012 18:18, James Bottomley ha scritto: > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > > index b583277..6d8ca08 100644 > > --- a/drivers/scsi/scsi_lib.c > > +++ b/drivers/scsi/scsi_lib.c > > @@ -843,8 +843,11 @@ void scsi_io_completion(struct scsi_cmnd *cmd, > > unsigned int good_bytes) > > } else if (sense_valid && !sense_deferred) { > > switch (sshdr.sense_key) { > > case UNIT_ATTENTION: > > - if (cmd->device->removable) { > > - /* Detected disc change. Set a bit > > + if (cmd->device->removable && > > + (sshdr.asc == 0x3a || > > +(sshdr.asc == 0x28 && sshdr.ascq == > > 0x00))) { > > + /* "No medium" or "Medium may have > > changed." > > +* This means a disc change. Set a bit > >>> This type of change would likely cause a huge cascade of errors in real > >>> removable media devices. Under the MMC standards, which a lot of the > >>> older removable discs seem to follow, UNIT ATTENTION indicates either > >>> medium change or device reset (which we check for and eat lower down); > >>> we can't rely on them giving proper SBC-2 sense codes. If you want to > >>> pretend to be removable media, you have to conform to its standards. > >> > >> Would you accept a patch doing the opposite, i.e. passing some sense > >> codes such as PARAMETERS CHANGED and TARGET OPERATING CONDITIONS HAVE > >> CHANGED? > > > > Could you explain what the problem actually is? It looks like you had a > > reason to mark virtio-scsi as removable, even though it isn't, and now > > you want to add further hacks because being removable doesn't quite > > work. > > It's not specific to virtio-scsi, in fact I expect that virtio-scsi will > be almost always used with non-removable disks. > > However, QEMU's SCSI target is not used just for virtio-scsi (for > example it can be used for USB storage), and it lets you mark a disk as > removable---why? because there exists real hardware that presents itself > as an SBC removable disk. The only thing that is specific to > virtualization, is support for online resizing (which generates a unit > attention condition CAPACITY DATA HAS CHANGED). So what's the problem? If you're doing pass through of a physical disk, we pick up removable from its inquiry string ... a physical removable device doesn't get resized. If you have a virtual disk you want to resize, you don't set the removable flag in the inquiry data. James -- 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] sd: do not set changed flag on all unit attention conditions
Il 17/07/2012 09:45, James Bottomley ha scritto: > On Mon, 2012-07-16 at 19:20 +0200, Paolo Bonzini wrote: >> Il 16/07/2012 18:18, James Bottomley ha scritto: > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > index b583277..6d8ca08 100644 > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -843,8 +843,11 @@ void scsi_io_completion(struct scsi_cmnd *cmd, > unsigned int good_bytes) > } else if (sense_valid && !sense_deferred) { > switch (sshdr.sense_key) { > case UNIT_ATTENTION: > - if (cmd->device->removable) { > - /* Detected disc change. Set a bit > + if (cmd->device->removable && > + (sshdr.asc == 0x3a || > + (sshdr.asc == 0x28 && sshdr.ascq == 0x00))) { > + /* "No medium" or "Medium may have changed." > + * This means a disc change. Set a bit >>> This type of change would likely cause a huge cascade of errors in real >>> removable media devices. Under the MMC standards, which a lot of the >>> older removable discs seem to follow, UNIT ATTENTION indicates either >>> medium change or device reset (which we check for and eat lower down); >>> we can't rely on them giving proper SBC-2 sense codes. If you want to >>> pretend to be removable media, you have to conform to its standards. >> >> Would you accept a patch doing the opposite, i.e. passing some sense >> codes such as PARAMETERS CHANGED and TARGET OPERATING CONDITIONS HAVE >> CHANGED? > > Could you explain what the problem actually is? It looks like you had a > reason to mark virtio-scsi as removable, even though it isn't, and now > you want to add further hacks because being removable doesn't quite > work. It's not specific to virtio-scsi, in fact I expect that virtio-scsi will be almost always used with non-removable disks. However, QEMU's SCSI target is not used just for virtio-scsi (for example it can be used for USB storage), and it lets you mark a disk as removable---why? because there exists real hardware that presents itself as an SBC removable disk. The only thing that is specific to virtualization, is support for online resizing (which generates a unit attention condition CAPACITY DATA HAS CHANGED). Paolo -- 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] sd: do not set changed flag on all unit attention conditions
On Mon, 2012-07-16 at 19:20 +0200, Paolo Bonzini wrote: > Il 16/07/2012 18:18, James Bottomley ha scritto: > >> > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > >> > index b583277..6d8ca08 100644 > >> > --- a/drivers/scsi/scsi_lib.c > >> > +++ b/drivers/scsi/scsi_lib.c > >> > @@ -843,8 +843,11 @@ void scsi_io_completion(struct scsi_cmnd *cmd, > >> > unsigned int good_bytes) > >> > } else if (sense_valid && !sense_deferred) { > >> > switch (sshdr.sense_key) { > >> > case UNIT_ATTENTION: > >> > -if (cmd->device->removable) { > >> > -/* Detected disc change. Set a bit > >> > +if (cmd->device->removable && > >> > +(sshdr.asc == 0x3a || > >> > + (sshdr.asc == 0x28 && sshdr.ascq == > >> > 0x00))) { > >> > +/* "No medium" or "Medium may have > >> > changed." > >> > + * This means a disc change. Set a bit > > This type of change would likely cause a huge cascade of errors in real > > removable media devices. Under the MMC standards, which a lot of the > > older removable discs seem to follow, UNIT ATTENTION indicates either > > medium change or device reset (which we check for and eat lower down); > > we can't rely on them giving proper SBC-2 sense codes. If you want to > > pretend to be removable media, you have to conform to its standards. > > Would you accept a patch doing the opposite, i.e. passing some sense > codes such as PARAMETERS CHANGED and TARGET OPERATING CONDITIONS HAVE > CHANGED? Could you explain what the problem actually is? It looks like you had a reason to mark virtio-scsi as removable, even though it isn't, and now you want to add further hacks because being removable doesn't quite work. Lets go back and see if there's a more correct way to do whatever it is you want to do. James -- 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 RESEND 0/3] scsi: fix internal write cache issue on usb hdd.
On Mon, 2012-07-16 at 16:48 -0700, Greg KH wrote: > On Sat, Jul 07, 2012 at 11:04:45PM -0400, Namjae Jeon wrote: > > From: Namjae Jeon > > > > The numbers of USB HDDs(All USB HDD I checked) does not respond > > correctly to scsi mode sense command for retrieving the write cache > > page status. Even though write cache is enabled by default, due to > > scsi driver assume that cache is not enabled which in turn might lead > > to loss of data since data still will be in cache. > > This result that all filesystems is not stable on USB HDD when the > > device is unplugged abruptly, even though these are having journaling > > feature. Our first trying is that scsi driver send ATA command > > (ATA Pass through, #85) to USB HDD after failure from normal routine to > > know write cache enable. > > We have known it is dangerous after testing several USB HDD. some of > > HDD is stalled by this command(A-DATA HDD). So we tried to make the > > patch James Bottomley's suggestion(usb quirk) on version 2 that add > > product ID and verdor ID of USB HDD to USB quirk list after checking > > write cache. > > All filesystem will be stable on USB HDD registered in quirk list. > > And it will be updated continuously. > > Now applied to the usb-next branch. It's been in scsi#misc for ten days with no problems. Lets leave it there rather than create merge and rebase issues. Thanks, James -- 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