Linux IDE/ATA development

Aaron Lu | 23 Jul 2012 08:49

Picon

[PATCH 0/5] Fix for ZPODD

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

Aaron Lu (5):
  scsi: sr: fix for sr suspend and resume
  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 | 14 ++++++++-----
 drivers/scsi/sr.c      | 53 ++++++++++++++++++++++++++++++++++++--------------
 include/linux/genhd.h  |  1 +
 4 files changed, 67 insertions(+), 26 deletions(-)

--

-- 
1.7.11.3

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

Aaron Lu | 23 Jul 2012 08:49

Picon

[PATCH 2/5] scsi: sr: runtime pm when ODD is open/closed

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.lu <at> amd.com>
---
 drivers/scsi/sr.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 5f4d19a..f7a7635 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
 <at>  <at>  -152,8 +152,15  <at>  <at>  static inline struct scsi_cd *scsi_cd_get(struct gendisk *disk)
 	kref_get(&cd->kref);
 	if (scsi_device_get(cd->device))

(Continue reading)

Alan Stern | 23 Jul 2012 16:57

Picon

Re: [PATCH 2/5] scsi: sr: runtime pm when ODD is open/closed

On Mon, 23 Jul 2012, Aaron Lu wrote:

> 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.lu <at> amd.com>
> ---
>  drivers/scsi/sr.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
> index 5f4d19a..f7a7635 100644
> --- a/drivers/scsi/sr.c
> +++ b/drivers/scsi/sr.c
>  <at>  <at>  -152,8 +152,15  <at>  <at>  static inline struct scsi_cd *scsi_cd_get(struct gendisk *disk)

(Continue reading)

Aaron Lu | 23 Jul 2012 08:49

Picon

[PATCH 4/5] scsi: pm: use runtime resume callback if available

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.lu <at> amd.com>
---
 drivers/scsi/scsi_pm.c | 14 +++++++++-----
 drivers/scsi/sr.c      | 21 +++++++++++++++++++++
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index d4201de..19bba47 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
 <at>  <at>  -34,14 +34,18  <at>  <at>  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)

(Continue reading)

Sergei Shtylyov | 23 Jul 2012 16:36

Re: [PATCH 4/5] scsi: pm: use runtime resume callback if available

Hello.

On 07/23/2012 10:49 AM, Aaron Lu wrote:

> 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.lu <at> amd.com>
> ---
>  drivers/scsi/scsi_pm.c | 14 +++++++++-----
>  drivers/scsi/sr.c      | 21 +++++++++++++++++++++
>  2 files changed, 30 insertions(+), 5 deletions(-)

> diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
> index d4201de..19bba47 100644
> --- a/drivers/scsi/scsi_pm.c
> +++ b/drivers/scsi/scsi_pm.c
>  <at>  <at>  -34,14 +34,18  <at>  <at>  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;

(Continue reading)

Aaron Lu | 24 Jul 2012 01:30

Picon

Re: [PATCH 4/5] scsi: pm: use runtime resume callback if available

Hi,

On Mon, Jul 23, 2012 at 06:36:12PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 07/23/2012 10:49 AM, Aaron Lu wrote:
> 
> > 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.lu <at> amd.com>
> > ---
> >  drivers/scsi/scsi_pm.c | 14 +++++++++-----
> >  drivers/scsi/sr.c      | 21 +++++++++++++++++++++
> >  2 files changed, 30 insertions(+), 5 deletions(-)
> 
> > diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
> > index d4201de..19bba47 100644
> > --- a/drivers/scsi/scsi_pm.c
> > +++ b/drivers/scsi/scsi_pm.c
> >  <at>  <at>  -34,14 +34,18  <at>  <at>  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)

(Continue reading)

Aaron Lu | 23 Jul 2012 08:49

Picon

[PATCH 5/5] block: genhd: add an interface to set disk's poll interval

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.

Signed-off-by: Aaron Lu <aaron.lu <at> 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
 <at>  <at>  -1619,6 +1619,19  <at>  <at>  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;
+}

(Continue reading)

Betty Dall | 23 Jul 2012 20:43

Picon

Re: [PATCH 5/5] block: genhd: add an interface to set disk's poll interval

Hi Aaron,

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? 

> 
> Signed-off-by: Aaron Lu <aaron.lu <at> 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
>  <at>  <at>  -1619,6 +1619,19  <at>  <at>  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);

(Continue reading)

Aaron Lu | 24 Jul 2012 01:52

Picon

Re: [PATCH 5/5] block: genhd: add an interface to set disk's poll interval

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?

> 
> > 
> > Signed-off-by: Aaron Lu <aaron.lu <at> 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

(Continue reading)

Betty Dall | 24 Jul 2012 20:55

Picon

Re: [PATCH 5/5] block: genhd: add an interface to set disk's poll interval

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.lu <at> amd.com>
> > > ---
> > >  block/genhd.c         | 23 +++++++++++++++++------
> > >  drivers/scsi/sr.c     |  1 +
> > >  include/linux/genhd.h |  1 +

(Continue reading)

Aaron Lu | 25 Jul 2012 04:47

Picon

Re: [PATCH 5/5] block: genhd: add an interface to set disk's poll interval

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
> > > >  <at>  <at>  -869,6 +869,7  <at>  <at>  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().

(Continue reading)

Aaron Lu | 23 Jul 2012 08:49

Picon

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

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_EVENT_AUTO flag.
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.lu <at> amd.com>
---
 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..5f4d19a 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
 <at>  <at>  -178,8 +178,8  <at>  <at>  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 (!(msg.event & PM_EVENT_AUTO))
 		return 0;

 	/* do another TUR to see if the ODD is still ready to be powered off */
 <at>  <at>  -217,9 +217,9  <at>  <at>  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);

(Continue reading)

Alan Stern | 23 Jul 2012 16:50

Picon

Re: [PATCH 1/5] scsi: sr: fix for sr suspend and resume

On Mon, 23 Jul 2012, Aaron Lu wrote:

> 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_EVENT_AUTO flag.
> 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.lu <at> amd.com>
> ---
>  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..5f4d19a 100644
> --- a/drivers/scsi/sr.c
> +++ b/drivers/scsi/sr.c
>  <at>  <at>  -178,8 +178,8  <at>  <at>  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 (!(msg.event & PM_EVENT_AUTO))

Please use the PMSG_IS_AUTO macro.

Alan Stern

(Continue reading)

Aaron Lu | 23 Jul 2012 08:49

Picon

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

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.lu <at> 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
 <at>  <at>  -1458,6 +1458,7  <at>  <at>  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)
 {
 <at>  <at>  -1502,6 +1503,7  <at>  <at>  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

(Continue reading)

Return

Return to gmane.linux.ide.

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane