[PATCH v2 4/7] scsi: sr: runtime pm when ODD is open/closed

2012-07-24 Thread Aaron Lu
The ODD can either be runtime resumed by the user or by a software
request. And for the latter part, we only support runtime resume the ODD
when the eject request is received. We did this in sr's block ioctl
function, this looks ugly.

Change this by runtime resuming the ODD in its open function and runtime
suspending it in its release function.

The downside of this approach is that in old distros with old udisk
daemon, the ODD will be polled by udisk daemon so open/close will
constantly be called, which will cause the ODD frequently resume from
suspend state, breaking the effect of power saving. User with such a
distro is advised to issue a udisk command to inhibit polling of the
disk like this:
$ udisks --inhibit-polling /dev/sr0
And since newer kernel has in kernel polling, there is no problem
regarding ODD's event report.

Signed-off-by: Aaron Lu aaron...@amd.com
---
 drivers/scsi/sr.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index e54945a..2aa50c3 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -152,8 +152,12 @@ static inline struct scsi_cd *scsi_cd_get(struct gendisk 
*disk)
kref_get(cd-kref);
if (scsi_device_get(cd-device))
goto out_put;
+   if (scsi_autopm_get_device(cd-device))
+   goto out_pm;
goto out;
 
+ out_pm:
+   scsi_device_put(cd-device);
  out_put:
kref_put(cd-kref, sr_kref_release);
cd = NULL;
@@ -169,6 +173,7 @@ static void scsi_cd_put(struct scsi_cd *cd)
mutex_lock(sr_ref_mutex);
kref_put(cd-kref, sr_kref_release);
scsi_device_put(sdev);
+   scsi_autopm_put_device(sdev);
mutex_unlock(sr_ref_mutex);
 }
 
@@ -652,10 +657,6 @@ static int sr_block_ioctl(struct block_device *bdev, 
fmode_t mode, unsigned cmd,
void __user *argp = (void __user *)arg;
int ret;
 
-   /* Make sure the ODD is not suspended */
-   if (scsi_autopm_get_device(sdev))
-   return -EACCES;
-
mutex_lock(sr_mutex);
 
/*
@@ -687,7 +688,6 @@ static int sr_block_ioctl(struct block_device *bdev, 
fmode_t mode, unsigned cmd,
 
 out:
mutex_unlock(sr_mutex);
-   scsi_autopm_put_device(sdev);
return ret;
 }
 
-- 
1.7.11.3


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


[PATCH v2 5/7] scsi: sr: block events when runtime suspended

2012-07-24 Thread Aaron Lu
When the ODD is runtime suspended, there is no need to poll it for
events, so block events poll for it and unblock when resumed.

Signed-off-by: Aaron Lu aaron...@amd.com
---
 block/genhd.c | 2 ++
 drivers/scsi/sr.c | 7 ---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 9cf5583..bdb3682 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1458,6 +1458,7 @@ void disk_block_events(struct gendisk *disk)
 
mutex_unlock(ev-block_mutex);
 }
+EXPORT_SYMBOL(disk_block_events);
 
 static void __disk_unblock_events(struct gendisk *disk, bool check_now)
 {
@@ -1502,6 +1503,7 @@ void disk_unblock_events(struct gendisk *disk)
if (disk-ev)
__disk_unblock_events(disk, false);
 }
+EXPORT_SYMBOL(disk_unblock_events);
 
 /**
  * disk_flush_events - schedule immediate event checking and flushing
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 2aa50c3..d6c0574 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -204,6 +204,8 @@ static int sr_suspend(struct device *dev, pm_message_t msg)
return -EBUSY;
}
 
+   disk_block_events(cd-disk);
+
return 0;
 }
 
@@ -226,6 +228,8 @@ static int sr_resume(struct device *dev)
atomic_set(cd-suspend_count, 1);
}
 
+   disk_unblock_events(cd-disk);
+
return 0;
 }
 
@@ -314,9 +318,6 @@ static unsigned int sr_check_events(struct 
cdrom_device_info *cdi,
if (CDSL_CURRENT != slot)
return 0;
 
-   if (pm_runtime_suspended(cd-device-sdev_gendev))
-   return 0;
-
/* if the logical unit just finished loading/unloading, do a TUR */
if (cd-device-can_power_off  cd-dbml  sr_unit_load_done(cd)) {
events = 0;
-- 
1.7.11.3


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


[PATCH v2 0/7] Fix for ZPODD

2012-07-24 Thread Aaron Lu
V2:
Bug fix for V1;
Use scsi_autopm_* in sr driver instead of pm_runtime_*;

V1:
Here are some patches to make ZPODD easier to use for end users and
a fix for using ZPODD with system suspend.

Aaron Lu (7):
  scsi: sr: fix for sr suspend and resume
  scsi: pm: use autosuspend if device supports it
  scsi: sr: use scsi layer autopm in sr driver
  scsi: sr: runtime pm when ODD is open/closed
  scsi: sr: block events when runtime suspended
  scsi: pm: use runtime resume callback if available
  block: genhd: add an interface to set disk's poll interval

 block/genhd.c  | 25 --
 drivers/scsi/scsi_pm.c | 22 +--
 drivers/scsi/sr.c  | 57 --
 include/linux/genhd.h  |  1 +
 4 files changed, 73 insertions(+), 32 deletions(-)

-- 
1.7.11.3


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


[PATCH v2 6/7] scsi: pm: use runtime resume callback if available

2012-07-24 Thread Aaron Lu
When runtime resume a scsi device, if the device's driver has
implemented runtime resume callback, use that instead of the resume
callback.

sr driver needs this to properly do different things for system resume
and runtime resume.

Signed-off-by: Aaron Lu aaron...@amd.com
---
V2:
Check if the resume callback is NULL before calling it as pointed by
Sergei Shtylyov.

 drivers/scsi/scsi_pm.c | 15 ++-
 drivers/scsi/sr.c  | 21 +
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index a002fbf..717d2c1 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -34,14 +34,19 @@ static int scsi_dev_type_suspend(struct device *dev, 
pm_message_t msg)
return err;
 }
 
-static int scsi_dev_type_resume(struct device *dev)
+static int scsi_dev_type_resume(struct device *dev, bool runtime)
 {
struct device_driver *drv;
int err = 0;
+   int (*resume)(struct device *);
 
drv = dev-driver;
-   if (drv  drv-resume)
-   err = drv-resume(dev);
+   if (runtime  drv  drv-pm  drv-pm-runtime_resume)
+   resume = drv-pm-runtime_resume;
+   else
+   resume = drv ? drv-resume : NULL;
+   if (resume)
+   err = resume(dev);
scsi_device_resume(to_scsi_device(dev));
dev_dbg(dev, scsi resume: %d\n, err);
return err;
@@ -84,7 +89,7 @@ static int scsi_bus_resume_common(struct device *dev)
 * Resume it on behalf of child.
 */
pm_runtime_get_sync(dev-parent);
-   err = scsi_dev_type_resume(dev);
+   err = scsi_dev_type_resume(dev, false);
pm_runtime_put_sync(dev-parent);
}
 
@@ -159,7 +164,7 @@ static int scsi_runtime_resume(struct device *dev)
 
dev_dbg(dev, scsi_runtime_resume\n);
if (scsi_is_sdev_device(dev))
-   err = scsi_dev_type_resume(dev);
+   err = scsi_dev_type_resume(dev, true);
 
/* Insert hooks here for targets, hosts, and transport classes */
 
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index d6c0574..466ff8e 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -82,6 +82,11 @@ static int sr_remove(struct device *);
 static int sr_done(struct scsi_cmnd *);
 static int sr_suspend(struct device *dev, pm_message_t msg);
 static int sr_resume(struct device *dev);
+static int sr_runtime_resume(struct device *dev);
+
+static struct dev_pm_ops sr_pm_ops = {
+   .runtime_resume = sr_runtime_resume,
+};
 
 static struct scsi_driver sr_template = {
.owner  = THIS_MODULE,
@@ -91,6 +96,7 @@ static struct scsi_driver sr_template = {
.remove = sr_remove,
.suspend= sr_suspend,
.resume = sr_resume,
+   .pm = sr_pm_ops,
},
.done   = sr_done,
 };
@@ -211,6 +217,21 @@ static int sr_suspend(struct device *dev, pm_message_t msg)
 
 static int sr_resume(struct device *dev)
 {
+   struct scsi_cd *cd = dev_get_drvdata(dev);
+
+   /*
+* If ODD is runtime suspended before system pm, unblock disk
+* events now since on system resume, we will fully resume it
+* and set its rumtime status to active.
+*/
+   if (pm_runtime_suspended(dev))
+   disk_unblock_events(cd-disk);
+
+   return 0;
+}
+
+static int sr_runtime_resume(struct device *dev)
+{
struct scsi_cd *cd;
struct scsi_sense_hdr sshdr;
 
-- 
1.7.11.3


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


[PATCH v2 1/7] scsi: sr: fix for sr suspend and resume

2012-07-24 Thread Aaron Lu
In sr_suspend, we do not need to do anything if it is not a runtime pm
request, so just return by checking the PM_IS_AUTO macro.
And in sr_resume, only reset the suspend_count back to 1 if the ODD is
waken up by the user, or the usage count of the scsi device will not
balance.

Signed-off-by: Aaron Lu aaron...@amd.com
---
V2:
Use PMSG_IS_AUTO as suggested by Alan Stern.

 drivers/scsi/sr.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 3da0879..225848c 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -178,8 +178,8 @@ static int sr_suspend(struct device *dev, pm_message_t msg)
struct scsi_sense_hdr sshdr;
struct scsi_cd *cd = dev_get_drvdata(dev);
 
-   /* no action for system suspend */
-   if (msg.event == PM_EVENT_SUSPEND)
+   /* no action for system pm operations */
+   if (!PMSG_IS_AUTO(msg))
return 0;
 
/* do another TUR to see if the ODD is still ready to be powered off */
@@ -217,9 +217,9 @@ static int sr_resume(struct device *dev)
cd-device-wakeup_by_user = 0;
if (!(cd-cdi.mask  CDC_CLOSE_TRAY))
sr_tray_move(cd-cdi, 1);
-   }
 
-   atomic_set(cd-suspend_count, 1);
+   atomic_set(cd-suspend_count, 1);
+   }
 
return 0;
 }
-- 
1.7.11.3


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


[PATCH v2 3/7] scsi: sr: use scsi layer autopm in sr driver

2012-07-24 Thread Aaron Lu
Use scsi_autopm_* whenever possible instead of the pm_runtime_* in sr
driver as suggested by Alan Stern.

The reason pm_runtime_get_noresume in sr_suspend is used is that we
can't do sync resume call in suspend callback, or rpm_resume will wait
forever for the suspend request to finish.

Signed-off-by: Aaron Lu aaron...@amd.com
---
V2:
Use scsi_autopm_* instead of pm_runtime_* in sr driver as suggested
by Alan Stern.

 drivers/scsi/sr.c | 16 +---
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 225848c..e54945a 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -377,10 +377,8 @@ do_tur:
 * thread (in-kernel polling) and old versions of udisks,
 * to avoid put the device twice, an atomic operation is used.
 */
-   if (poweroff  atomic_add_unless(cd-suspend_count, -1, 0)) {
-   pm_runtime_mark_last_busy(cd-device-sdev_gendev);
-   pm_runtime_put_autosuspend(cd-device-sdev_gendev);
-   }
+   if (poweroff  atomic_add_unless(cd-suspend_count, -1, 0))
+   scsi_autopm_put_device(cd-device);
}
 
if (cd-ignore_get_event)
@@ -655,11 +653,8 @@ static int sr_block_ioctl(struct block_device *bdev, 
fmode_t mode, unsigned cmd,
int ret;
 
/* Make sure the ODD is not suspended */
-   ret = pm_runtime_get_sync(sdev-sdev_gendev);
-   if (ret  0) {
-   pm_runtime_put_noidle(sdev-sdev_gendev);
+   if (scsi_autopm_get_device(sdev))
return -EACCES;
-   }
 
mutex_lock(sr_mutex);
 
@@ -692,8 +687,7 @@ static int sr_block_ioctl(struct block_device *bdev, 
fmode_t mode, unsigned cmd,
 
 out:
mutex_unlock(sr_mutex);
-   pm_runtime_mark_last_busy(cd-device-sdev_gendev);
-   pm_runtime_put_autosuspend(cd-device-sdev_gendev);
+   scsi_autopm_put_device(sdev);
return ret;
 }
 
@@ -1121,7 +1115,7 @@ static int sr_remove(struct device *dev)
 
/* disable runtime pm and possibly resume the device */
if (!atomic_dec_and_test(cd-suspend_count))
-   pm_runtime_get_sync(dev);
+   scsi_autopm_get_device(cd-device);
 
blk_queue_prep_rq(cd-device-request_queue, scsi_prep_fn);
del_gendisk(cd-disk);
-- 
1.7.11.3


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


[PATCH v2 2/7] scsi: pm: use autosuspend if device supports it

2012-07-24 Thread Aaron Lu
If the device is using autosuspend, when scsi_autopm_put_device is
called for it, use autosuspend runtime pm calls instead of the sync
call.

Signed-off-by: Aaron Lu aaron...@amd.com
---
 drivers/scsi/scsi_pm.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index d4201de..a002fbf 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -196,7 +196,12 @@ EXPORT_SYMBOL_GPL(scsi_autopm_get_device);
 
 void scsi_autopm_put_device(struct scsi_device *sdev)
 {
-   pm_runtime_put_sync(sdev-sdev_gendev);
+   if (sdev-sdev_gendev.power.use_autosuspend) {
+   pm_runtime_mark_last_busy(sdev-sdev_gendev);
+   pm_runtime_put_autosuspend(sdev-sdev_gendev);
+   } else {
+   pm_runtime_put_sync(sdev-sdev_gendev);
+   }
 }
 EXPORT_SYMBOL_GPL(scsi_autopm_put_device);
 
-- 
1.7.11.3


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


Re: [PATCH] tcm_vhost: Expose ABI version via VHOST_SCSI_GET_ABI_VERSION

2012-07-24 Thread Greg Kroah-Hartman
On Tue, Jul 24, 2012 at 01:26:20AM +, Nicholas A. Bellinger wrote:
 From: Nicholas Bellinger n...@linux-iscsi.org
 
 As requested by Anthony, here is a patch against target-pending/for-next-merge
 to expose an ABI version to userspace via a new VHOST_SCSI_GET_ABI_VERSION
 ioctl operation.
 
 As mentioned in the comment, ABI Rev 0 is for pre 2012 out-of-tree code, and
 ABI Rev 1 (the current rev) is for current WIP v3.6 kernel merge candiate 
 code.
 
 I think this is what you had in mind, and hopefully it will make MST happy 
 too.
 The incremental vhost-scsi patches against Zhi's QEMU are going out shortly 
 ahead
 of cutting a new vhost-scsi RFC over the next days.
 
 Please have a look and let me know if you have any concerns here.
 
 Thanks!
 
 Reported-by: Anthony Liguori aligu...@us.ibm.com
 Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Paolo Bonzini pbonz...@redhat.com
 Cc: Zhi Yong Wu wu...@linux.vnet.ibm.com
 Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
 ---
  drivers/vhost/tcm_vhost.c |9 +
  drivers/vhost/tcm_vhost.h |   11 +++
  2 files changed, 20 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
 index dc7e024..3f04169 100644
 --- a/drivers/vhost/tcm_vhost.c
 +++ b/drivers/vhost/tcm_vhost.c
 @@ -977,6 +977,15 @@ static long vhost_scsi_ioctl(struct file *f, unsigned 
 int ioctl,
   return -EFAULT;
  
   return vhost_scsi_clear_endpoint(vs, backend);
 + case VHOST_SCSI_GET_ABI_VERSION:
 + if (copy_from_user(backend, argp, sizeof backend))
 + return -EFAULT;
 +
 + backend.abi_version = VHOST_SCSI_ABI_VERSION;
 +
 + if (copy_to_user(argp, backend, sizeof backend))
 + return -EFAULT;
 + return 0;
   case VHOST_GET_FEATURES:
   features = VHOST_FEATURES;
   if (copy_to_user(featurep, features, sizeof features))
 diff --git a/drivers/vhost/tcm_vhost.h b/drivers/vhost/tcm_vhost.h
 index e942df9..3d5378f 100644
 --- a/drivers/vhost/tcm_vhost.h
 +++ b/drivers/vhost/tcm_vhost.h
 @@ -80,7 +80,17 @@ struct tcm_vhost_tport {
  
  #include linux/vhost.h
  
 +/*
 + * Used by QEMU userspace to ensure a consistent vhost-scsi ABI.
 + *
 + * ABI Rev 0: All pre 2012 revisions used by prototype out-of-tree code
 + * ABI Rev 1: 2012 version for v3.6 kernel merge candiate
 + */
 +
 +#define VHOST_SCSI_ABI_VERSION   1
 +
  struct vhost_scsi_target {
 + int abi_version;
   unsigned char vhost_wwpn[TRANSPORT_IQN_LEN];
   unsigned short vhost_tpgt;
  };
 @@ -88,3 +98,4 @@ struct vhost_scsi_target {
  /* VHOST_SCSI specific defines */
  #define VHOST_SCSI_SET_ENDPOINT _IOW(VHOST_VIRTIO, 0x40, struct 
 vhost_scsi_target)
  #define VHOST_SCSI_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x41, struct 
 vhost_scsi_target)
 +#define VHOST_SCSI_GET_ABI_VERSION _IOW(VHOST_VIRTIO, 0x42, struct 
 vhost_scsi_target)

No, you just broke the ABI for version 0 here, that's not how you do
this at all.

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: scsi_requeue_command and dropping reference to sdev_gendev

2012-07-24 Thread Ani Sinha

 commit 940f5d47e2f2e1fa00443921a0abf4822335b54d
 Author: Bart Van Assche bvanass...@acm.org
 Date:   Fri Jun 29 15:34:26 2012 +

 [SCSI] Avoid dangling pointer in scsi_requeue_command()

 James


Thanks James. We have been only looking at Torvalds' tree and I think this
fix hasn't been pulled in yet. Your pointer is much appreciated.

Cheers,
Ani

--
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/5] block: genhd: add an interface to set disk's poll interval

2012-07-24 Thread Betty Dall
Hi Aaron,

On Tue, 2012-07-24 at 07:52 +0800, Aaron Lu wrote:
 On Mon, Jul 23, 2012 at 12:43:34PM -0600, Betty Dall wrote:
  Hi Aaron,
 
 Hi,
 
  
  On Mon, 2012-07-23 at 14:49 +0800, Aaron Lu wrote:
   Set the ODD's in kernel poll interval to 2s for the user in case the
   user is using an old distro on which udev will not set the system wide
   block parameter events_dfl_poll_msecs.
  Why did you pick 2 seconds? 
 
 Just a random pick, no special meaning here.
 On newer distros, udev will also pick 2s for the events_dfl_poll_msecs
 parameter, so I just followed that :-)
 Do you see any problem with this setting?

No problem, and I was curious as to why 2s, and the fact that is it used
in udev for events_dfl_poll_msecs is a good reason.

  
   
   Signed-off-by: Aaron Lu aaron...@amd.com
   ---
block/genhd.c | 23 +--
drivers/scsi/sr.c |  1 +
include/linux/genhd.h |  1 +
3 files changed, 19 insertions(+), 6 deletions(-)
   
   diff --git a/block/genhd.c b/block/genhd.c
   index bdb3682..de9b9d9 100644
   --- a/block/genhd.c
   +++ b/block/genhd.c
   @@ -1619,6 +1619,19 @@ static void disk_events_workfn(struct work_struct 
   *work)
 kobject_uevent_env(disk_to_dev(disk)-kobj, KOBJ_CHANGE, envp);
}

   +int disk_events_set_poll_msecs(struct gendisk *disk, long intv)
   +{
   + if (intv  0  intv != -1)
   + return -EINVAL;
   +
   + disk_block_events(disk);
   + disk-ev-poll_msecs = intv;
   + __disk_unblock_events(disk, true);
   +
   + return 0;
   +}
   +EXPORT_SYMBOL(disk_events_set_poll_msecs);
   +
/*
 * A disk events enabled device has the following sysfs nodes under
 * its /sys/block/X/ directory.
   @@ -1675,16 +1688,14 @@ static ssize_t 
   disk_events_poll_msecs_store(struct device *dev,
{
 struct gendisk *disk = dev_to_disk(dev);
 long intv;
   + int ret;

 if (!count || !sscanf(buf, %ld, intv))
 return -EINVAL;

   - if (intv  0  intv != -1)
   - return -EINVAL;
   -
   - disk_block_events(disk);
   - disk-ev-poll_msecs = intv;
   - __disk_unblock_events(disk, true);
   + ret = disk_events_set_poll_msecs(disk, intv);
   + if (ret)
   + return ret;

 return count;
}
   diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
   index 2f159aa..78c4226 100644
   --- a/drivers/scsi/sr.c
   +++ b/drivers/scsi/sr.c
   @@ -869,6 +869,7 @@ static int sr_probe(struct device *dev)
 dev_set_drvdata(dev, cd);
 disk-flags |= GENHD_FL_REMOVABLE;
 add_disk(disk);
   + disk_events_set_poll_msecs(disk, 2000);
  
  Could you check that disk event's poll_msecs is the default (-1) before
  setting it to 2s? I am thinking of a case when the probe happens after
  the call to disk_events_poll_msecs_store() and this code would overwrite
  the user specified value.
 
 The block device sr0 is created by this driver in this probe function,
 so the user should not be able to set the poll interval before probe,
 right?

The add_disk() call happens immediately before the new
disk_events_set_poll_msecs() call. add_disk() is what eventually creates
the sysfs files and calls your new disk_events_set_poll_msecs(). It
makes more sense to me to have the new call to
disk_events_set_poll_msecs(disk, 2000) before the call to add_disk().
That way add_disk()could override the 2 second default based on user
input. If a distro doesn't support udev setting events_dfl_poll_msecs,
the add_disk() won't ever make a call to disk_events_set_poll_msecs()
and the 2 second default will stand.

-Betty


--
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] tcm_vhost: Expose ABI version via VHOST_SCSI_GET_ABI_VERSION

2012-07-24 Thread Nicholas A. Bellinger
On Mon, 2012-07-23 at 18:56 -0700, Greg Kroah-Hartman wrote:
 On Tue, Jul 24, 2012 at 01:26:20AM +, Nicholas A. Bellinger wrote:
  From: Nicholas Bellinger n...@linux-iscsi.org
  
  As requested by Anthony, here is a patch against 
  target-pending/for-next-merge
  to expose an ABI version to userspace via a new VHOST_SCSI_GET_ABI_VERSION
  ioctl operation.
  
  As mentioned in the comment, ABI Rev 0 is for pre 2012 out-of-tree code, and
  ABI Rev 1 (the current rev) is for current WIP v3.6 kernel merge candiate 
  code.
  
  I think this is what you had in mind, and hopefully it will make MST happy 
  too.
  The incremental vhost-scsi patches against Zhi's QEMU are going out shortly 
  ahead
  of cutting a new vhost-scsi RFC over the next days.
  
  Please have a look and let me know if you have any concerns here.
  
  Thanks!
  
  Reported-by: Anthony Liguori aligu...@us.ibm.com
  Cc: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
  Cc: Michael S. Tsirkin m...@redhat.com
  Cc: Paolo Bonzini pbonz...@redhat.com
  Cc: Zhi Yong Wu wu...@linux.vnet.ibm.com
  Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
  ---
   drivers/vhost/tcm_vhost.c |9 +
   drivers/vhost/tcm_vhost.h |   11 +++
   2 files changed, 20 insertions(+), 0 deletions(-)
  

SNIP

  diff --git a/drivers/vhost/tcm_vhost.h b/drivers/vhost/tcm_vhost.h
  index e942df9..3d5378f 100644
  --- a/drivers/vhost/tcm_vhost.h
  +++ b/drivers/vhost/tcm_vhost.h
  @@ -80,7 +80,17 @@ struct tcm_vhost_tport {
   
   #include linux/vhost.h
   
  +/*
  + * Used by QEMU userspace to ensure a consistent vhost-scsi ABI.
  + *
  + * ABI Rev 0: All pre 2012 revisions used by prototype out-of-tree code
  + * ABI Rev 1: 2012 version for v3.6 kernel merge candiate
  + */
  +
  +#define VHOST_SCSI_ABI_VERSION 1
  +
   struct vhost_scsi_target {
  +   int abi_version;
  unsigned char vhost_wwpn[TRANSPORT_IQN_LEN];
  unsigned short vhost_tpgt;
   };
  @@ -88,3 +98,4 @@ struct vhost_scsi_target {
   /* VHOST_SCSI specific defines */
   #define VHOST_SCSI_SET_ENDPOINT _IOW(VHOST_VIRTIO, 0x40, struct 
  vhost_scsi_target)
   #define VHOST_SCSI_CLEAR_ENDPOINT _IOW(VHOST_VIRTIO, 0x41, struct 
  vhost_scsi_target)
  +#define VHOST_SCSI_GET_ABI_VERSION _IOW(VHOST_VIRTIO, 0x42, struct 
  vhost_scsi_target)
 
 No, you just broke the ABI for version 0 here, that's not how you do
 this at all.
 

The intention of this patch is use ABI=1 as a starting point for
tcm_vhost moving forward, with no back-wards compat for the ABI=0
prototype userspace code because:

- It's based on a slightly older version of QEMU (updating the QEMU series now)
- It does not have an GET_ABI_VERSION ioctl cmd (that starts with ABI=1)
- It has a small user-base of target + virtio-scsi developers

So I did consider just starting from ABI=0, but figured this would help
reduce the confusion for QEMU userspace wrt to the vhost-scsi code
that's been floating around out-of-tree for the last 2 years.

Stefan  Co folks, do you have a preference for a starting point..?

Thanks,

--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: 'Device not ready' issue on mpt2sas since 3.1.10

2012-07-24 Thread Matthias Prager
Hello everyone,

I retested with a new firmware (P14 - released today), since it contains
a bunch of sata and SATL fixes (according to the changelog).
Unfortunately the observed behavior is unchanged (tested on a 3.4.5 kernel).

Just wanted to let everyone know.

Cheers
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/5] block: genhd: add an interface to set disk's poll interval

2012-07-24 Thread Aaron Lu
Hi Betty,

On Tue, Jul 24, 2012 at 12:55:06PM -0600, Betty Dall wrote:
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 2f159aa..78c4226 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -869,6 +869,7 @@ static int sr_probe(struct device *dev)
dev_set_drvdata(dev, cd);
disk-flags |= GENHD_FL_REMOVABLE;
add_disk(disk);
+   disk_events_set_poll_msecs(disk, 2000);
   
   Could you check that disk event's poll_msecs is the default (-1) before
   setting it to 2s? I am thinking of a case when the probe happens after
   the call to disk_events_poll_msecs_store() and this code would overwrite
   the user specified value.
  
  The block device sr0 is created by this driver in this probe function,
  so the user should not be able to set the poll interval before probe,
  right?
 
 The add_disk() call happens immediately before the new
 disk_events_set_poll_msecs() call. add_disk() is what eventually creates
 the sysfs files

Right, and it's disk_add_events in add_disk that adds these sysfs files.

 and calls your new disk_events_set_poll_msecs().

No... there is no call to disk_events_set_poll_msecs in add_disk, when
the events for the disk is created by disk_alloc_events, the poll_msecs
of the event is initialized to the default value -1. And then
disk_add_events will create the sysfs files and add_disk will return,
and I'll change the default value of -1 to 2000 with the new function
I've made.

 It makes more sense to me to have the new call to
 disk_events_set_poll_msecs(disk, 2000) before the call to add_disk().

This is too early, since the events of the disk is not allocated yet.

I hope I've explained this clearly, if you see a problem, please let me
know, thanks.

-Aaron


--
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: [resend PATCH 1/5] libata: reset once

2012-07-24 Thread Jeff Garzik

On 07/10/2012 12:05 AM, Dan Williams wrote:

Hotplug testing with libsas currently encounters a 55 second wait for
link recovery to give up.  In the case where the user trusts the
response time of their devices permit the recovery attempts to be
limited to one.

Signed-off-by: Dan Williams dan.j.willi...@intel.com


Acked-by: Jeff Garzik jgar...@redhat.com

--
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: [resend PATCH 2/5] libata: export ata_port suspend/resume infrastructure for sas

2012-07-24 Thread Jeff Garzik

On 07/10/2012 12:05 AM, Dan Williams wrote:

Reuse ata_port_{suspend|resume}_common for sas.  This path is chosen
over adding coordination between ata-tranport and sas-transport because
libsas wants to revalidate the domain at resume-time at the host level.
It can not validate links have resumed properly until libata has had a
chance to perform its revalidation, and any sane placing of an ata_port
in the sas-transport model would delay it's resumption until after the
host.

Export the common portion of port suspend/resume (bypass pm_runtime),
and allow sas to perform these operations asynchronously (similar to the
libsas async-ata probe implmentation).  Async operation is determined by
having an external, rather than stack based, location for storing the
result of the operation.

Reviewed-by: Jacek Danecki jacek.dane...@intel.com
Signed-off-by: Dan Williams dan.j.willi...@intel.com


Acked-by: Jeff Garzik jgar...@redhat.com




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