Re: 'Device not ready' issue on mpt2sas since 3.1.10

2012-07-21 Thread Matthias Prager
Am 17.07.2012 22:01, schrieb Tejun Heo:
> 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?
> 

While reading the linux-scsi mailing list I stumbled upon

'[Bug 16070] Fail to issue Start/Stop Unit'

(bugtracker: )

which lead me to trying to enable the 'allow_restart' flag for my disks.
With this workaround a vanilla kernel 3.4.5 does not exhibit the i/o
errors on sleeping sata disks hanging off sas controllers.


I'm currently running one of my systems with a

'echo 1 | tee /sys/block/sd?/device/scsi_disk/*/allow_restart >/dev/null'

line added to the init scripts. This way I can use the untouched kernel
sources and still get around the i/o errors. But I reckon this is no
solution.


I'm no expert on scsi/sas/ata internals, so please take the following
thoughts with a grain of salt:

As far as I can see (and Tejun confirmed that - I think) Tejun commit
85ef06d1d252f6a2e73b678591ab71caad4667bb somehow exposes a bug, which
lies deeper in the sas/ata code. The 'sas_slave_configure()' function in
'drivers/scsi/libsas/sas_scsi_host.c' sets the 'allow_restart' flag for
sas disks hanging off sas controllers. But if it encounters a sata disk
it calls 'ata_sas_slave_configure()' in 'drivers/ata/libata_scsi.c'
instead and returns without enabling the 'allow_restart' flag. A simple
fix would be to set allow_restart=1 after having called
'ata_sas_slave_configure()' but before returning (in
'sas_slave_configure()').

Now I'm not sure this isn't taping over another bug. Which leads me to
my question: What is the correct behavior?

#1 Issuing a separate spin-up command (START UNIT?) prior to sending i/o
by setting allow_restart=1 for sata disks on sas controllers

or

#2 Teaching the sas drivers they do not need spin-up commands and can
simply start issuing i/o to sata disks

--
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


[GIT PULL] target updates for v3.6-rc1 (round 1)

2012-07-21 Thread Nicholas A. Bellinger
Hello Linus,

The following is the first round of target core + fabric driver updates
for the freshly opened v3.6 merge window.  Please go ahead and pull
from:

  git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git for-next

This series is based on target-pending/master commit 1765fe5edcb83f53f

So I ended up needing to rebase for-next last week to pick up fixes
in master that for-next ended up conflicting with, but the bulk of the
series has been in linux-next a number of weeks and has getting a good
amount of testing.

There have been lots of work in a number of areas this past round.  The
highlights include:

*) Break out target_core_cdb.c emulation into SPC/SBC ops (hch)
*) Add a parse_cdb method to target backend drivers (hch)
*) Move sync_cache + write_same + unmap into spc_ops (hch)
*) Use target_execute_cmd for WRITEs in iscsi_target + srpt (hch)
*) Offload WRITE I/O backend submission in tcm_qla2xxx + tcm_fc (hch + nab)
*) Refactor core_update_device_list_for_node() into enable/disable funcs 
(agrover)
*) Replace the TCM processing thread with a TMR work queue (hch)
*) Fix regression in transport_add_device_to_core_hba from TMR conversion (DanC)
*) Remove racy, now-redundant check of sess_tearing_down with qla2xxx (roland)
*) Add range checking, fix reading of data len + possible underflow in UNMAP 
(roland)
*) Allow for target_submit_cmd() returning errors + convert fabrics (roland + 
nab)
*) Drop bogus struct file usage for iSCSI/SCTP (viro)

Thanks again to everyone who has contributed!

On the new target fabric module front for-3.6, tcm_vhost is currently in
RFC-v4 on the lists + queued up + waiting for ACKs from MST & Co here:

http://marc.info/?l=kvm&m=134285417314176&w=2

At this point I think the vhost folks are pretty agreeable for an
initial merge of this kernel code, but the vhost-scsi bits in upstream
QEMU userspace code will still need to be shorted out.  So because of
this fact, MST has requested to mark this driver as STAGING o indicate
the userspace ABI is not (yet) set in stone.

Currently the last item being discussed for initial merge is if this
code can just be marked as STAGING + live in drivers/vhost/, or if it
should actually reside under drivers/staging/ and go upstream via the
staging tree.  The last -v4 series of the code keeps the initial merge
in drivers/vhost/, which is easier than getting staging to depending on
vhost + target-pending..

So the current plan is to send a [GIT PULL] next weekend, in order to
give MST a chance to review + ACK as he returns from holiday.  Otherwise
if folks object, I'll move this code to drivers/staginig/tcm_vhost/ and
let Greg-KH decide if he's OK to take it via staging for this round.

Thank you!

--nab

Al Viro (1):
  iscsi-target: Drop bogus struct file usage for iSCSI/SCTP

Andy Grover (7):
  target: Do not special-case loop and iscsi fabric module loads
  target/iblock: Add parameter to specify read-only devices
  target: Remove unneeded double parentheses
  target: Remove hba param from core_dev_add_lun
  target: Misc retval cleanups
  target: Eliminate else using boolean logic
  target: refactor core_update_device_list_for_node()

Christoph Hellwig (26):
  target: move unrelated code out of transport_generic_cmd_sequencer
  target: remove control CDB flags
  target: split overflow and underflow checks into a helper
  target: split parsing of SPC commands into a separate helper
  target: add a parse_cdb method to the backend drivers
  target: move code for CDB emulation
  target: move transport_generic_prepare_cdb into pscsi
  target: remove the execute list
  target: move ref_cmd from the generic se_tmr_req into iscsi code
  target: remove dead SCF_ flags
  target: add struct spc_ops + initial ->execute_rw pointer usage
  target: move sync_cache to struct spc_ops
  target: move write_same to struct spc_ops
  target: move unmap to struct spc_ops
  target: split transport_cmd_check_stop
  target: remove transport_generic_process_write
  target: call transport_check_aborted_status from target_execute_cmd
  target: merge transport_generic_write_pending into
transport_generic_new_cmd
  iscsit: use target_execute_cmd for WRITEs
  srpt: use target_execute_cmd for WRITEs in srpt_handle_rdma_comp
  tcm_qla2xxx: Offload WRITE I/O backend submission to tcm_qla2xxx wq
  tcm_fc: Offload WRITE I/O backend submission to tpg workqueue
  target: remove transport_generic_handle_data
  target: simply fabric driver queue full processing
  target: remove transport_generic_handle_cdb_map
  target: replace the processing thread with a TMR work queue

Dan Carpenter (1):
  target: NULL dereference on error path

Nicholas Bellinger (4):
  target: Move MAINTENANCE_[IN,OUT] from pscsi_parse_cdb ->
spc_parse_cdb
  target/pscsi: Only emulate REPORT_LUNS for passthrough
  Revert "target: Do not special-case loop and iscsi fabric module
loads"
  target: Make core_disable_device_list_for_node use pre-refactoring
lock ordering

Rol

Re: [RFC-v4 1/3] vhost: Separate vhost-net features from vhost features

2012-07-21 Thread Michael S. Tsirkin
On Sat, Jul 21, 2012 at 06:55:36AM +, Nicholas A. Bellinger wrote:
> 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 

Applied to vhost-next.

> ---
>  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


Re: [RFC-v4 2/3] vhost: make vhost work queue visible

2012-07-21 Thread Michael S. Tsirkin
On Sat, Jul 21, 2012 at 06:55:37AM +, Nicholas A. Bellinger wrote:
> 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 

Applied to vhost-next.

> ---
>  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


Re: [RFC-v4 3/3] tcm_vhost: Initial merge for vhost level target fabric driver

2012-07-21 Thread Michael S. Tsirkin
On Sat, Jul 21, 2012 at 06:55:38AM +, Nicholas A. Bellinger wrote:
> 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 v3 -> v4:
> 
>   Rename vhost_vring_target -> vhost_scsi_target (mst + nab)
>   Use TRANSPORT_IQN_LEN in vhost_scsi_target->vhost_wwpn[] def (nab)
>   Move back to drivers/vhost/, and just use drivers/vhost/Kconfig.tcm (mst)
>   Move TCM_VHOST related ioctl defines from include/linux/vhost.h ->
>   drivers/vhost/tcm_vhost.h as requested by MST (nab)
>   Move Kbuild.tcm include from drivers/staging -> drivers/vhost/, and
>   just use 'if STAGING' around 'source drivers/vhost/Kbuild.tcm'
> 
> 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: Michael S. Tsirkin 
> Cc: Stefan Hajnoczi 
> Cc: Zhi Yong Wu 
> Cc: Paolo Bonzini 
> Cc: Christoph Hellwig 
> Cc: Hannes Reinecke 
> Cc: Jens Axboe 
> Signed-off-by: Nicholas Bellinger 

There are some comments that I sent that have not been addressed yet,
in particular it's easy for userspace to flood the host log.
But I think it's in an OK state for a staging driver.

I don't know if the dependency on CONFIG_STAGING is enough:
it's easy for someone to miss, if it was in the menu with other
stagung drivers it woul be more obvious.  If the way to do this is to
move it to drivers/staging/tcp_vhost it might be worth it,  I'm fine
either way.  Anyway, I think we both agree it should go in through
Greg's tree so he's the boss.

I put the dependency patches 1 and 2 on my tree
git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-net-next
and sending pull request to dave ASAP.

> ---
>  drivers/vhost/Kconfig |3 +
>  drivers/vhost/Kconfig.tcm |6 +
>  drivers/vhost/Makefile|2 +
>  drivers/vhost/tcm_vhost.c | 1611 
> +
>  drivers/vhost/tcm_vhost.h |   90 +++
>  5 files changed, 1712 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/vhost/Kconfig.tcm
>  create mode 100644 drivers/vhost/tcm_vhost.c
>  create mode 100644 drivers/vhost/tcm_vhost.h
> 
> diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
> index e4e2fd1..202bba6 100644
> --- a/drivers/vhost/Kconfig
> +++ b/drivers/vhost/Kconfig
> @@ -9,3 +9,6 @@ config VHOST_NET
> To compile this driver as a module, choose M here: the module will
> be called vhost_net.
>  
> +if STAGING
> +source "drivers/vhost/Kconfig.tcm"
> +endif
> diff --git a/drivers/vhost/Kconfig.tcm b/drivers/vhost/Kconfig.tcm
> new file mode 100644
> index 000..a9c6f76
> --- /dev/null
> +++ b/drivers/vhost/Kconfig.tcm
> @@ -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/Makefile b/drivers/vhost/Makefile
> index 72dd020..a27b053 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_vhost.o
> diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
> new file mode 100644
> index 000..dc7e024
> --- /dev/null
> +++ b/drivers/vhost/tcm_vhost.c
> @@ -0,0 +1,1611 @@
> +/***
> + * Vhost kernel TCM fabric driver for virtio SCSI initiators
> + *
> + * (C) Copyrigh