From patchwork Mon Oct 29 09:01:36 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aaron Lu X-Patchwork-Id: 1662191 Return-Path: X-Original-To: patchwork-linux-pm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id A93553FDDA for ; Mon, 29 Oct 2012 09:02:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758452Ab2J2JCK (ORCPT ); Mon, 29 Oct 2012 05:02:10 -0400 Received: from mga14.intel.com ([143.182.124.37]:28484 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758441Ab2J2JCJ (ORCPT ); Mon, 29 Oct 2012 05:02:09 -0400 Received: from azsmga002.ch.intel.com ([10.2.17.35]) by azsmga102.ch.intel.com with ESMTP; 29 Oct 2012 02:02:08 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.80,669,1344236400"; d="scan'208";a="161810545" Received: from aaronlu.sh.intel.com ([10.239.36.69]) by AZSMGA002.ch.intel.com with ESMTP; 29 Oct 2012 02:02:05 -0700 From: Aaron Lu To: Jeff Garzik , "Rafael J. Wysocki" , James Bottomley , Alan Stern , Tejun Heo , Oliver Neukum Cc: Jeff Wu , Aaron Lu , Shane Huang , linux-ide@vger.kernel.org, linux-pm@vger.kernel.org, linux-scsi@vger.kernel.org, linux-acpi@vger.kernel.org Subject: [PATCH v8 09/11] block: add a new interface to block events Date: Mon, 29 Oct 2012 17:01:36 +0800 Message-Id: <1351501298-3716-10-git-send-email-aaron.lu@intel.com> X-Mailer: git-send-email 1.7.12.4 In-Reply-To: <1351501298-3716-1-git-send-email-aaron.lu@intel.com> References: <1351501298-3716-1-git-send-email-aaron.lu@intel.com> Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org 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 the work is currently pending, which means it has been queued but the timer is not expired yet. If the work is not pending, we return false, as we do not know if the task is already running or not queued at all. And if the queued work is successfully cancelled, disk_block_events will be called and we are done. But if we failed to cancel the work, false will be returned, and the calling thread knows that it's not safe to block events at the moment and should act accordingly. In ZPODD's case, poweroff will not be attempted. Note: ODD means: Optical Disc Drive. ZPODD means: Zero Power ODD. It's a mechnism to place the ODD into zero power state without user's awareness when the system is at S0 system state. Signed-off-by: Aaron Lu --- 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 cac7366..c6af755 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -1469,6 +1469,32 @@ 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 (delayed_work_pending(&ev->dwork)) { + if (cancel_delayed_work(&disk->ev->dwork)) { + disk_block_events(disk); + return true; + } + } + + return false; +} + static void __disk_unblock_events(struct gendisk *disk, bool check_now) { struct disk_events *ev = disk->ev; 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);