linux-kernel - Re: [PATCH v2 4/4] ata: add ata port runtime PM callbacks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 14 Nov 2011 10:31:45 -0500 (EST)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Lin Ming <ming.m.lin@...el.com>
cc:	lkml <linux-kernel@...r.kernel.org>, <linux-ide@...r.kernel.org>,
	<linux-scsi@...r.kernel.org>, <linux-pm@...r.kernel.org>,
	Jeff Garzik <jgarzik@...ox.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	James Bottomley <JBottomley@...allels.com>,
	Tejun Heo <tj@...nel.org>, Huang Ying <ying.huang@...el.com>,
	Zhang Rui <rui.zhang@...el.com>
Subject: Re: [PATCH v2 4/4] ata: add ata port runtime PM callbacks

On Mon, 14 Nov 2011, Lin Ming wrote:

> Current patches has a bug that system suspend fails if ata port was
> runtime suspended.
> 
> disk suspend issues sync cache and stop device commands that obviously
> need ata port to be active. So we need to runtime resume ata port first.

This is wrong.  If the port is already suspended then so are all the 
drives below the port.  Hence there is no need to sync the cache or 
stop the device.

> Alan, Tejun
> 
> How about below fix?
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index fa3a591..ebb87fbf 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -50,6 +50,7 @@
>  #include <linux/string_helpers.h>
>  #include <linux/async.h>
>  #include <linux/slab.h>
> +#include <linux/pm_runtime.h>
>  #include <asm/uaccess.h>
>  #include <asm/unaligned.h>
>  
> @@ -2762,6 +2763,14 @@ static int sd_suspend(struct device *dev, pm_message_t mesg)
>  	if (!sdkp)
>  		return 0;	/* this can happen */
>  
> +	/*
> +	 * Resume parent device to handle sync cache and
> +	 * stop device commands
> +	 */
> +	ret = pm_runtime_get_sync(dev->parent);
> +	if (ret)
> +		goto exit;
> +
>  	if (sdkp->WCE) {
>  		sd_printk(KERN_NOTICE, sdkp, "Synchronizing SCSI cache\n");
>  		ret = sd_sync_cache(sdkp);

This is not the right approach.  You should look instead at 
scsi_dev_type_suspend() in scsi_pm.c.  If the device is already runtime 
suspended then the routine should return immediately.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/