linux-kernel - Re: Suspend to ram regression (2.6.24-rc1-git)

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]

Message-ID: <20071101120141.GF5037@kernel.dk>
Date:	Thu, 1 Nov 2007 13:01:41 +0100
From:	Jens Axboe <jens.axboe@...cle.com>
To:	Jeff Garzik <jeff@...zik.org>
Cc:	linux-kernel@...r.kernel.org, kristen.c.accardi@...el.com,
	linux-ide@...r.kernel.org
Subject: Re: Suspend to ram regression (2.6.24-rc1-git)

On Thu, Nov 01 2007, Jeff Garzik wrote:
> Jens Axboe wrote:
>> On Thu, Nov 01 2007, Jeff Garzik wrote:
>>> Jens Axboe wrote:
>>>> Reverting just the default AHCI flags makes it work again. IOW, with the
>>>> below patch I can suspend properly with current -git.
>>>> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
>>>> index ed9b407..77f7631 100644
>>>> --- a/drivers/ata/ahci.c
>>>> +++ b/drivers/ata/ahci.c
>>>> @@ -190,8 +190,7 @@ enum {
>>>>   	AHCI_FLAG_COMMON		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
>>>>  					  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
>>>> -					  ATA_FLAG_ACPI_SATA | ATA_FLAG_AN |
>>>> -					  ATA_FLAG_IPM,
>>>> +					  ATA_FLAG_ACPI_SATA | ATA_FLAG_AN,
>>>>  	AHCI_LFLAG_COMMON		= ATA_LFLAG_SKIP_D2H_BSY,
>>>
>>> sounds like the easy thing to do, in light of this breakage, might be to 
>>> default it to off, add a module parameter turning it on by setting that 
>>> flag.
>> Wouldn't it be better to just get this bug fixed? IOW, is there a reason
>> for disabling ALPM if it's Bug Free?
>> I'd suggest committing the patch disabling IPM, then Kristen can debug
>> the thing in piece in quiet. Once confident it works with ahci again, we
>> can revert that commit.
>
> Right -- if you are going to commit a patch "disabling" it, it is best to 
> do so via a simple module option, which allows users to easily try the 
> feature in parallel with Intel's debugging.

OK, so you just want the option to be temporary? In that case I think a
config option is better, since you don't risk breaking peoples setups
later when removing the option. That can be quite annoying. Ala the
below.

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index ba63619..e276ab6 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -48,6 +48,14 @@ config SATA_AHCI
 
 	  If unsure, say N.
 
+config SATA_AHCI_IPM
+	bool "AHCI power management"
+	depends on EXPERIMENTAL && SATA_AHCI
+	help
+	  This option adds support for AHCI power management. It current
+	  breaks suspend on some laptops. This option is temporary and will
+	  go away once those issues are fully resolved.
+
 config SATA_SVW
 	tristate "ServerWorks Frodo / Apple K2 SATA support"
 	depends on PCI
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index ed9b407..37266ce 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -190,8 +190,11 @@ enum {
 
 	AHCI_FLAG_COMMON		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 					  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
-					  ATA_FLAG_ACPI_SATA | ATA_FLAG_AN |
-					  ATA_FLAG_IPM,
+					  ATA_FLAG_ACPI_SATA | ATA_FLAG_AN
+#ifdef CONFIG_SATA_AHCI_IPM
+					  | ATA_FLAG_IPM
+#endif
+					,
 	AHCI_LFLAG_COMMON		= ATA_LFLAG_SKIP_D2H_BSY,
 };
 

-- 
Jens Axboe

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