From owner-freebsd-stable@freebsd.org Sat Jul 25 22:54:41 2015 Return-Path: Delivered-To: freebsd-stable@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 4EFF99AA208 for ; Sat, 25 Jul 2015 22:54:41 +0000 (UTC) (envelope-from kob6558@gmail.com) Received: from mail-oi0-x233.google.com (mail-oi0-x233.google.com [IPv6:2607:f8b0:4003:c06::233]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 0A18CE16; Sat, 25 Jul 2015 22:54:41 +0000 (UTC) (envelope-from kob6558@gmail.com) Received: by oigd21 with SMTP id d21so35698124oig.1; Sat, 25 Jul 2015 15:54:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=scCHRaBu/C69H1QzD38gRf00TC+uEGlIrnhX7LQ4wHs=; b=mkziP6GXW8AdPgVWnjHPJCkMPN/OPNLbaSA/3us5FOyOdE4wRt+VhbOaeNrB1+tChY DOvkBF4w1kXbbyzwNfWzawCS2eLfFw8W+PZpvRWUyUMEOpqj8ledkztr3TrasBBzUdWS 7lT0SauVz8m7n+Z8gHhAzI/cr+fhhlYWVtOM9pHFQDvsbYvlDwOtjujeO9OW8fzw2b+t i+MEa2YxqtSBZfOMMyPtOLxcjJrhMu+kfSlUYBsbfmNzbCi4QJ02FfAbUPdw1gaSzWpg Cw20JA2gcPy1HWu+Qgu0XAykE2QffD23QS55lr4jxUFGA/ABxGUeq++6+uJLOOvSOPm1 20Xw== MIME-Version: 1.0 X-Received: by 10.202.169.212 with SMTP id s203mr7891812oie.71.1437864880272; Sat, 25 Jul 2015 15:54:40 -0700 (PDT) Sender: kob6558@gmail.com Received: by 10.202.221.69 with HTTP; Sat, 25 Jul 2015 15:54:40 -0700 (PDT) In-Reply-To: References: <86oak289hv.fsf@gly.ftfl.ca> <86oaj9dnbo.fsf@gly.ftfl.ca> <12509399.h3RdpFfE1l@ralph.baldwin.cx> Date: Sat, 25 Jul 2015 15:54:40 -0700 X-Google-Sender-Auth: rNtxnBJWFQkM1h-KxRVsENjkDaE Message-ID: Subject: Re: suspend/resume regression From: Kevin Oberman To: John Baldwin Cc: Joseph Mingrone , "Brandon J. Wandersee" , Adrian Chadd , FreeBSD-STABLE Mailing List Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.20 X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 25 Jul 2015 22:54:41 -0000 John, I'm concerned that two issues may be getting conflated. The issue I thought we were looking at was the failure of some systems (T520, X220, T430) to resume after a number of PCI enhancements were MFCed. This is completely unrelated to the USB issue I was experiencing when trying to test the problem on HEAD. The more I think about it, the more I think that the USB "issue" is just how things need to work. Specifically, if you are booting a full, multi-user system from a USB connected drive, suspending and resuming will leave the system in an untenable condition that will force a panic. At least I don't see how the OS can determine that the disk present on resume is unchanged from that present when the system was suspended. Modern disk IDs greatly improve the situation, but I am unaware of any way to be sure that a removable drive (such as a USB) has not been removed and plugged into some other system that might have written to it. My knowledge of such things is very dated, going back to my days doing kernel programming about 25-30 year ago on VMS, so someone may have resolved the issue, but I don't understand exactly how. I guess that the risk might be low enough to just go ahead and pray that nobody did something really, really stupid like unplugging the drive, plugging it in elsewhere, and writing to it. The real issue is just resuming the system after r281874 was MFCed as a part of 284034. No USB connected file systems are involved. I m happy to see that it has been reverted for 10.2, but clearly, these changes are needed down the line and I hope the issue can be resolved well before 11.0. (This assumes a 10.3 before 11.0 happens next year.) Thanks for the time you have spent on this and I'll be happy to help out with testing in the future. Things will be easier now that I have a disk with head on it. I wasted way too much time trying to get HEAD to work in a USB drive and with related issues. Kevin Oberman, Network Engineer, Retired E-mail: rkoberman@gmail.com PGP Fingerprint: D03FB98AFA78E3B78C1694B318AB39EF1B055683 On Wed, Jul 22, 2015 at 10:46 PM, Kevin Oberman wrote: > On Tue, Jul 21, 2015 at 3:56 PM, John Baldwin wrote: > >> On Saturday, July 18, 2015 10:22:33 PM Kevin Oberman wrote: >> > I just confirmed that my system resumes on HEAD of July 16 but fails on >> > 10.2-BETA2. So the problem limited to 10. I'm guessing that some other >> > change made to pci that has not been MFCed is the cause, but it is only >> > causing a problem on some hardware. I have seen no reports about systems >> > other than Lenovo systems. >> >> So my x220 does fail with a USB disk on 10, but I also get a weird >> behavior >> where it seems to wake up (disk lights up) and then goes back to sleep and >> never resumes again. I'm not sure if this is due to using a USB disk or >> not. I get the same result when I disable power management during suspend >> which was reported to fix other laptops IIRC. >> >> Please try this: >> >> Index: sys/dev/acpica/acpi.c >> =================================================================== >> --- sys/dev/acpica/acpi.c (revision 285761) >> +++ sys/dev/acpica/acpi.c (working copy) >> @@ -691,7 +691,7 @@ >> static void >> acpi_set_power_children(device_t dev, int state) >> { >> - device_t child, parent; >> + device_t child; >> device_t *devlist; >> struct pci_devinfo *dinfo; >> int dstate, i, numdevs; >> @@ -703,13 +703,12 @@ >> * Retrieve and set D-state for the sleep state if _SxD is >> present. >> * Skip children who aren't attached since they are handled >> separately. >> */ >> - parent = device_get_parent(dev); >> for (i = 0; i < numdevs; i++) { >> child = devlist[i]; >> dinfo = device_get_ivars(child); >> dstate = state; >> if (device_is_attached(child) && >> - acpi_device_pwr_for_sleep(parent, dev, &dstate) == 0) >> + acpi_device_pwr_for_sleep(dev, child, &dstate) == 0) >> acpi_set_powerstate(child, dstate); >> } >> free(devlist, M_TEMP); >> Index: sys/dev/pci/pci.c >> =================================================================== >> --- sys/dev/pci/pci.c (revision 285761) >> +++ sys/dev/pci/pci.c (working copy) >> @@ -3671,7 +3671,7 @@ >> child = devlist[i]; >> dstate = state; >> if (device_is_attached(child) && >> - PCIB_POWER_FOR_SLEEP(pcib, dev, &dstate) == 0) >> + PCIB_POWER_FOR_SLEEP(pcib, child, &dstate) == 0) >> pci_set_powerstate(child, dstate); >> } >> } >> Index: . >> =================================================================== >> --- . (revision 285761) >> +++ . (working copy) >> >> Property changes on: . >> ___________________________________________________________________ >> Modified: svn:mergeinfo >> Merged /head:r274386,274397 >> >> >> -- >> John Baldwin >> > > Tried both sysctls and the patch. Nothing worked. Ticket updated with the > information. > -- > Kevin Oberman, Network Engineer, Retired > E-mail: rkoberman@gmail.com > PGP Fingerprint: D03FB98AFA78E3B78C1694B318AB39EF1B055683 > >