linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Aaron Lu <aaron.lu@intel.com>
To: Jeff Garzik <jgarzik@pobox.com>,
	James Bottomley <James.Bottomley@hansenpartnership.com>,
	"Rafael J. Wysocki" <rjw@sisk.pl>,
	Alan Stern <stern@rowland.harvard.edu>, Tejun Heo <tj@kernel.org>
Cc: Jeff Wu <jeff.wu@amd.com>, Aaron Lu <aaron.lwe@gmail.com>,
	linux-ide@vger.kernel.org, linux-pm@vger.kernel.org,
	linux-scsi@vger.kernel.org, linux-acpi@vger.kernel.org,
	Aaron Lu <aaron.lu@intel.com>
Subject: [PATCH v9 07/10] block: add a new interface to block events
Date: Fri,  9 Nov 2012 14:51:59 +0800	[thread overview]
Message-ID: <1352443922-13734-8-git-send-email-aaron.lu@intel.com> (raw)
In-Reply-To: <1352443922-13734-1-git-send-email-aaron.lu@intel.com>

A new interface to block disk events is added, this interface is
meant to eliminate a race between PM runtime callback and disk events
checking.

Suppose the following device tree:
device_sata_port  (the parent)
  device_ODD      (the child)

When ODD is runtime suspended, sata port will have a chance to enter
runtime suspended state. And in sata port's runtime suspend callback,
it will check if it is OK to omit the power of the ODD. And if yes, the
periodically running events checking work will be stopped, as the ODD
will be waken up by that check and cancel it can make the ODD stay in
zero power state much longer(no worry about how the ODD gets media
change event in ZPODD's case).

I used disk_block_events to do the events blocking, but there is a race
and can lead to a deadlock: when I call disk_block_events in sata port's
runtime suspend callback, the events checking work may already be running
and it will resume the ODD synchronously, and PM core will try to resume
its parent, the sata port first. The PM core makes sure that runtime
resume callback does not run concurrently with runtime suspend callback,
and if the runtime suspend callback is running, the runtime resume
callback will wait for it done. So the events checking work will wait
for sata port's runtime suspend callback, while the sata port's runtime
suspend callback is waiting for the disk events work to finish. Deadlock.

ODD_suspend                        disk_events_workfn
  ata_port_suspend                   check_events
    disk_block_events                  resume ODD
      cancel_delayed_work_sync           resume parent
      (waiting for disk_events_workfn)   (waiting for suspend callback)

So a new function disk_try_block_events is added, it will try to
cancel the delayed work if it is pending. If succeed, disk_block_events
will be called and we are done; if failed, false is returned without
doing anything. In this way, the race can be avoided.

The newly added interface and the disk_unblock_events are exported, as
sr driver will need to use them to block/unblock disk events.

Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
 block/genhd.c         | 26 ++++++++++++++++++++++++++
 include/linux/genhd.h |  1 +
 2 files changed, 27 insertions(+)

diff --git a/block/genhd.c b/block/genhd.c
index 6cace66..8632fd3 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1469,6 +1469,31 @@ void disk_block_events(struct gendisk *disk)
 	mutex_unlock(&ev->block_mutex);
 }
 
+/*
+ * Under some circumstances, there is a race between the calling thread
+ * of disk_block_events and the events checking function. To avoid such a race,
+ * this function will check if the delayed work is pending. If not, it means
+ * the work is either not queued or is already running, false is returned.
+ * And if yes, try to cancel the delayed work. If succedded, disk_block_events
+ * will be called and there is no worry that cancel_delayed_work_sync will
+ * deadlock the events checking function. And if failed, false is returned.
+ */
+bool disk_try_block_events(struct gendisk *disk)
+{
+	struct disk_events *ev = disk->ev;
+
+	if (!ev)
+		return false;
+
+	if (cancel_delayed_work(&disk->ev->dwork)) {
+		disk_block_events(disk);
+		return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL(disk_try_block_events);
+
 static void __disk_unblock_events(struct gendisk *disk, bool check_now)
 {
 	struct disk_events *ev = disk->ev;
@@ -1512,6 +1537,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/include/linux/genhd.h b/include/linux/genhd.h
index 4f440b3..b67247f 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -420,6 +420,7 @@ static inline int get_disk_ro(struct gendisk *disk)
 }
 
 extern void disk_block_events(struct gendisk *disk);
+extern bool disk_try_block_events(struct gendisk *disk);
 extern void disk_unblock_events(struct gendisk *disk);
 extern void disk_flush_events(struct gendisk *disk, unsigned int mask);
 extern unsigned int disk_clear_events(struct gendisk *disk, unsigned int mask);
-- 
1.7.12.4


  parent reply	other threads:[~2012-11-09  6:51 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-09  6:51 [PATCH v9 00/10] ZPODD Patches Aaron Lu
2012-11-09  6:51 ` [PATCH v9 01/10] scsi: sr: support runtime pm Aaron Lu
2012-11-09  6:51 ` [PATCH v9 02/10] ata: zpodd: Add CONFIG_SATA_ZPODD Aaron Lu
2012-11-09  6:51 ` [PATCH v9 03/10] ata: zpodd: identify and init ZPODD devices Aaron Lu
2012-11-12 18:53   ` Tejun Heo
2012-11-14  1:32     ` Aaron Lu
2012-11-18 14:38       ` Tejun Heo
2012-11-19  2:15         ` Aaron Lu
2012-11-09  6:51 ` [PATCH v9 04/10] libata: acpi: move acpi notification code to zpodd Aaron Lu
2012-11-12 18:55   ` Tejun Heo
2012-11-14  1:36     ` Aaron Lu
2012-11-09  6:51 ` [PATCH v9 05/10] libata: separate ATAPI code Aaron Lu
2012-11-12 18:57   ` Tejun Heo
2012-11-13 12:49     ` Aaron Lu
2012-11-18 15:01       ` Tejun Heo
2012-11-19  2:21         ` Aaron Lu
2012-11-19 14:51           ` Tejun Heo
2012-11-09  6:51 ` [PATCH v9 06/10] ata: zpodd: check zero power ready status Aaron Lu
2012-11-12 19:13   ` Tejun Heo
2012-11-13 13:20     ` Aaron Lu
2012-11-14  2:18     ` Aaron Lu
2012-11-18 15:00       ` Tejun Heo
2012-11-19  3:09         ` Aaron Lu
2012-11-19 14:56           ` Tejun Heo
2012-11-19 15:06             ` James Bottomley
2012-11-26  0:33               ` Rafael J. Wysocki
2012-11-26  0:45                 ` Aaron Lu
2012-11-26  5:03                 ` James Bottomley
2012-11-26  5:09                   ` Aaron Lu
2012-11-26  7:32                     ` James Bottomley
2012-11-26  8:27                       ` Aaron Lu
2012-11-26 13:17                         ` James Bottomley
2012-11-26 16:21                           ` Alan Stern
2012-11-26 19:15                             ` James Bottomley
2012-11-27  1:41                           ` Aaron Lu
2012-11-28  0:51                   ` Rafael J. Wysocki
2012-11-28  1:39                     ` Tejun Heo
2012-11-28  2:24                       ` Aaron Lu
2012-11-28  8:56                       ` James Bottomley
2012-12-03  8:13                         ` Aaron Lu
2012-12-03  8:25                           ` James Bottomley
2012-12-03  8:59                             ` Aaron Lu
2012-12-03 16:23                             ` Tejun Heo
2012-12-03 18:56                               ` Jeff Garzik
2012-12-04  5:04                                 ` Aaron Lu
2012-12-04 12:11                               ` James Bottomley
2012-12-07  6:13                                 ` Aaron Lu
2012-12-10  3:26                                   ` Aaron Lu
2012-12-11  5:10                                     ` Tejun Heo
2012-12-18  8:30                                       ` Aaron Lu
2012-12-20  6:07                               ` Aaron Lu
2012-12-25 17:17                                 ` Tejun Heo
2012-12-26  1:42                                   ` Aaron Lu
2012-12-28 21:16                                     ` Tejun Heo
2013-01-04  1:04                                       ` Aaron Lu
2012-11-30  8:55                       ` Aaron Lu
2012-11-30 11:15                         ` Rafael J. Wysocki
2012-11-20  6:00             ` Aaron Lu
2012-11-20  8:59               ` Aaron Lu
2012-11-26  0:50                 ` Rafael J. Wysocki
2012-11-26  0:48                   ` Aaron Lu
2012-11-26  1:03                     ` Rafael J. Wysocki
2012-11-26  1:05                       ` Aaron Lu
2012-11-26  1:11                         ` Rafael J. Wysocki
2012-11-26  1:09                           ` Aaron Lu
2012-11-26  1:22                             ` Rafael J. Wysocki
2012-11-26  1:22                               ` Aaron Lu
2012-11-26  1:17                           ` Aaron Lu
2012-11-09  6:51 ` Aaron Lu [this message]
2012-11-12 19:14   ` [PATCH v9 07/10] block: add a new interface to block events Tejun Heo
2012-11-12 19:18     ` Alan Stern
2012-11-12 19:21       ` Tejun Heo
2012-11-12 19:34         ` Alan Stern
2012-11-18 15:05           ` Tejun Heo
2012-11-18 17:41             ` Alan Stern
2012-11-18 21:56               ` Tejun Heo
2012-11-18 21:58                 ` Tejun Heo
2012-11-18 23:28                 ` Alan Stern
2012-11-18 23:35                   ` Tejun Heo
2012-11-19  2:07                     ` Alan Stern
2012-11-19  3:21                       ` Aaron Lu
2012-11-19 14:50                       ` Tejun Heo
2012-11-09  6:52 ` [PATCH v9 08/10] scsi: sr: support (un)block events Aaron Lu
2012-11-09  6:52 ` [PATCH v9 09/10] ata: zpodd: handle power transition of ODD Aaron Lu
2012-11-09  6:52 ` [PATCH v9 10/10] ata: expose pm qos flags to user space for ata device Aaron Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1352443922-13734-8-git-send-email-aaron.lu@intel.com \
    --to=aaron.lu@intel.com \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=aaron.lwe@gmail.com \
    --cc=jeff.wu@amd.com \
    --cc=jgarzik@pobox.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=rjw@sisk.pl \
    --cc=stern@rowland.harvard.edu \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).