So I take everyone's latest and greatest product and injudiciously type the

above command.  The result five minutes later is at

http://userweb.kernel.org/~akpm/borkage.jpg.  See if you can count all the bugs.
Sorry, but I've had it with this stuff and I'm tired of fixing everyone else's

stuff.  I'm just going to ship it.  Good luck.

--

Like I said in another thread, please try without

pm-acquire-device-locks-on-suspend-rev-3.patch and without git-acpi.patch.

If that still doesn't work, it's not my fault. ;-)

--

Heh.  Laptop suspend to anything has been so broken for so long in the

-mm series on my Compaq R3000 that I didn't even know it was ever

supposed to work.

--=20

Zan Lynx <zlynx@acm.org>

It gets broken more often than anything else.  I do test each release on

two laptops and I get to do a lot of bisection searching and

grumpygramming as a result.
Probably it would be more efficient to have the people who wrote the code

also test it.

--

Big fat ACK from here. Suspend issues in past few -mms were *very* hard (and

time consuming) to track down.
Looking forward to test suspend of -rc8-mm1 :),

--js

--

it's really all a matter of reducing latencies. Testing suspend/resume

is manual work currently (it either needs me hit a key on the laptop or

necessiates the use of a test-script that might or might not work

depending on whether the new /dev/rtc driver is enabled). So few people

besides those that rely on it will do it. The more a patch that breaks

suspend is out in the open unidentified, the more damage it does: it

gets into more trees, gets harder to bisect, etc.
So please give us overworked maintainers an easy to use .config option

dependent on CONFIG_DEBUG_KERNEL=y that automatically triggers a simple

suspend+resume sequence 60 seconds after bootup. It would be godsent.

(dont worry about proper gx resume) I compile and boot up every patch i

add to x86.git, so this would catch crap the moment we add it to the

tree.
The other, more long-term trick is to make rarely used functionality

more widely used. Consolidate code. Try to merge as much of

suspend/resume with bootup/module-insert/shutdown sequences as possible.

Suspend unused devices more agressively - such as non-mounted block

devices or downed networking ports. Try create more network effects with

other functionality, suspend and resume is not just about suspending

laptops, it can/could be used for so much more stuff. Try to get Pavel's

"Sleepy Linux" concept to work reasonably well - so that more people

(including developers) would use it in a daily basis.
Test coverage of a given piece of code is a direct function of its

utility and of its ease of testing. Decreeing "this is important" really

wont get more testing done. What you should realize i think is that this

is not a social/mindset problem (so no need to get frustrated about it),

this is a mostly technology problem: you can gradually _code_ your way

into people's test efforts.
	Ingo

--

Well, that would certainly help.
I do test all of my patches and generally all of the patches I sign off, but

surely that's not enough.

--

please add a .config option dependent on CONFIG_DEBUG_KERNEL=y [and

default-disabled] that auto-tests suspend/resume functionality 60

seconds after hitting user-space (the suspend/resume cycle kept small

via a small RTC timeout) and s2ram correctness will be tested _a lot_

more.
(it doesnt matter if graphics does not resume fine - at least for my

tests)
kprobes had similar problems and it now has a few simple smoke-tests -

which i just saw trigger on a patch that i did not notice would break

kprobes. I think this should be done for all functionality that is not

regularly triggered by a normal distro bootup (and which is easy to

overlook in testing).
	Ingo

--

Seeing as we're so lame about being able to distribute userspace stuff:

create a shell script in /proc/rc.kernel and start teaching initscripts to

run it.  Then we can modify it at will.
I hate me.

--

would be fine to me. The problem isnt just distribution and the tests

getting out of sync with the kernel (and its capabilities enabled in the

.config, etc.), the other problem is unintrusive testing: i for example

use unmodified images of various distributions, with only a new bzImage

plopped in. That is an intentionally minimal impact test vector, any

userspace side changes are discouraged. And that's how many people test

new kernels, they just plop it in. The moment we require any userlevel

changes, the testing barrier increases significantly.
[ and a Friday rant: that's why the objection against my

  make-relatime-actually-useful-to-people patch is so stupid,

  shortsighted, self-defeating and misplaced. The "Dont put policy into

  the kernel, use a new mount option and update your /etc/fstab"

  stupidity that people have been repeating for 10 years without

  thinking about it is a move that scares away 90% of our testers and

  99% of the distros. See:
    http://people.redhat.com/mingo/relatime-patches/improve-relatime.patch
  Trivial improvement, it's 6 months and still not upstream. We are just

  so good at shooting in our feet by creating artificial barriers and

  making it as hard as possible for people to actually use our kernel

  features ;-) ]
	Ingo

--

Hi!
Ingo wanted simple sleep self-test... Here's something.
set_alarm needs to move into rtc-cmos, and I guess I should boottest

it...
			Pavel ~be careful what you wish for~ Machek
diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c

index 29cf145..d830ed2 100644

--- a/drivers/rtc/rtc-cmos.c

+++ b/drivers/rtc/rtc-cmos.c

@@ -78,7 +78,7 @@ static inline int is_intr(u8 rtc_intr)
 /*----------------------------------------------------------------*/
-static int cmos_read_time(struct device *dev, struct rtc_time *t)

+int cmos_read_time(struct device *dev, struct rtc_time *t)

 {

 	/* REVISIT:  if the clock has a "century" register, use

 	 * that instead of the heuristic in get_rtc_time().

@@ -170,7 +170,7 @@ static int cmos_read_alarm(struct device

 	return 0;

 }
-static int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t)

+int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t)

 {

 	struct cmos_rtc	*cmos = dev_get_drvdata(dev);

 	unsigned char	mon, mday, hrs, min, sec;

@@ -394,6 +394,7 @@ static const struct rtc_class_ops cmos_r

 /*----------------------------------------------------------------*/
 static struct cmos_rtc	cmos_rtc;

+struct device *pc_rtc_device;
 static irqreturn_t cmos_interrupt(int irq, void *p)

 {

@@ -431,6 +432,8 @@ cmos_do_probe(struct device *dev, struct

 	if (cmos_rtc.dev)

 		return -EBUSY;
+	pc_rtc_device = dev;

+

 	if (!ports)

 		return -ENODEV;
@@ -546,7 +549,7 @@ cleanup0:
 static void cmos_do_shutdown(void)

 {

-	unsigned char	rtc_control;

+	unsigned char rtc_control;
 	spin_lock_irq(&rtc_lock);

 	rtc_control = CMOS_READ(RTC_CONTROL);
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig

index 8e186c6..2886056 100644

--- a/kernel/power/Kconfig

+++ b/kernel/power/Kconfig

@@ -53,14 +53,20 @@ config PM_TRACE

 	RTC across reboots, so that you can debug a machine that just hangs

 	during suspend (or more commonly, during resume).
-	To use this debugging feature you should attempt...
[ message continues ]

thanks :)
threw it into auto-qa, and it got thrown back with:
dione:~/linux/linux> ./err

kernel/power/sleepy.c: In function 'set_alarm':

kernel/power/sleepy.c:50: warning: passing argument 1 of 'cmos_set_alarm' from incompatible pointer type

kernel/power/sleepy.c:56: warning: control reaches end of non-void function

kernel/built-in.o: In function `set_alarm':
config attached.
	Ingo

Ok... it needs to depend on cmos_rtc... Plus timeout needs to be

longer, because if it fails to suspend within timeout, it will hang.
I actually tested this version... and it has some problems with

nohz. I have to press shift (probably any key will do) to make it

suspend :-(.
How big is your autotest farm, btw?

								Pavel
diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c

index 29cf145..d830ed2 100644

--- a/drivers/rtc/rtc-cmos.c

+++ b/drivers/rtc/rtc-cmos.c

@@ -78,7 +78,7 @@ static inline int is_intr(u8 rtc_intr)
 /*----------------------------------------------------------------*/
-static int cmos_read_time(struct device *dev, struct rtc_time *t)

+int cmos_read_time(struct device *dev, struct rtc_time *t)

 {

 	/* REVISIT:  if the clock has a "century" register, use

 	 * that instead of the heuristic in get_rtc_time().

@@ -170,7 +170,7 @@ static int cmos_read_alarm(struct device

 	return 0;

 }
-static int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t)

+int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t)

 {

 	struct cmos_rtc	*cmos = dev_get_drvdata(dev);

 	unsigned char	mon, mday, hrs, min, sec;

@@ -394,6 +394,7 @@ static const struct rtc_class_ops cmos_r

 /*----------------------------------------------------------------*/
 static struct cmos_rtc	cmos_rtc;

+struct device *pc_rtc_device;
 static irqreturn_t cmos_interrupt(int irq, void *p)

 {

@@ -431,6 +432,8 @@ cmos_do_probe(struct device *dev, struct

 	if (cmos_rtc.dev)

 		return -EBUSY;
+	pc_rtc_device = dev;

+

 	if (!ports)

 		return -ENODEV;
@@ -546,7 +549,7 @@ cleanup0:
 static void cmos_do_shutdown(void)

 {

-	unsigned char	rtc_control;

+	unsigned char rtc_control;
 	spin_lock_irq(&rtc_lock);

 	rtc_control = CMOS_READ(RTC_CONTROL);

diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig

index 8e186c6..b719fd3 100644

--- a/kernel/power/Kconfig

+++ b/kernel/power/Kconfig

@@ -53,14 +53,20 @@ config PM_TRACE

 	RTC across reboots, so that ...
[ message continues ]

Hi,
Why compiling it uncoditionally?  What would be wrong with
obj-$(CONFIG_PM_SLEEPY_TEST		+= sleepy.o
	Hannes

--

Fixed.
Thanks,

									Pavel
--

(english) http://www.livejournal.com/~pavelmachek

(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

--

any updated patch i could try? Thanks,
	Ingo

--

Here you go, but notice that it needs nohz=off here. (Or press shift

few times, that makes it proceed). idle=poll also works around that

specific problem, but provokes deadlock in sysfs instead :-(.
diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c

index 29cf145..5bbdb70 100644

--- a/drivers/rtc/rtc-cmos.c

+++ b/drivers/rtc/rtc-cmos.c

@@ -78,7 +78,7 @@ static inline int is_intr(u8 rtc_intr)
 /*----------------------------------------------------------------*/
-static int cmos_read_time(struct device *dev, struct rtc_time *t)

+int cmos_read_time(struct device *dev, struct rtc_time *t)

 {

 	/* REVISIT:  if the clock has a "century" register, use

 	 * that instead of the heuristic in get_rtc_time().

@@ -170,7 +170,7 @@ static int cmos_read_alarm(struct device

 	return 0;

 }
-static int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t)

+int cmos_set_alarm(struct device *dev, struct rtc_wkalrm *t)

 {

 	struct cmos_rtc	*cmos = dev_get_drvdata(dev);

 	unsigned char	mon, mday, hrs, min, sec;

@@ -394,6 +394,33 @@ static const struct rtc_class_ops cmos_r

 /*----------------------------------------------------------------*/
 static struct cmos_rtc	cmos_rtc;

+static struct device *pc_rtc_device;

+

+int set_alarm(int length)

+{

+	ssize_t retval;

+	unsigned long now, alarm;

+	struct rtc_wkalrm alm;

+

+	retval = cmos_read_time(pc_rtc_device, &alm.time);

+	if (retval < 0) {

+		printk("Auto sleep: can't get time?\n");

+		return retval;

+	}

+	rtc_tm_to_time(&alm.time, &now);

+	printk("Auto sleep: Now %ld\n", now);

+

+	alarm = now+length;

+	rtc_time_to_tm(alarm, &alm.time);

+

+	retval = cmos_set_alarm(pc_rtc_device, &alm);

+	if (retval < 0) {

+		printk("Auto sleep: can't set alarm.\n");

+		return retval;

+	}

+	printk("Auto sleep: Alarm set\n");

+	return 0;

+}
 static irqreturn_t cmos_interrupt(int irq, void *p)

 {

@@ -431,6 +458,8 @@ cmos_do_probe(struct device *dev, struct

 	if (cmos_rtc.dev)

 		return -EBUSY;
...
[ message continues ]

Very true.
Greetings,

Rafael

--

With all the discussion lately about boot-time smoketests and self-tests

maybe this kind of stuff would be a good first candidate for useful

new early-userspace functionality.  Then the kernel build could be

taught about building an initramfs that runs a bunch of tests

and leaves the user in a shell letting them know if it passed or not.
This would be a great way for increasing the number of testers, just

ask them to build with that option and a _known_ testsuite could

be reported as working or not.
Or maybe I'm out to lunch....
Harvey
--

Hi.
It is far too easy to take a cursory glance, say 'That looks okay' and

move on to the next thing, isn't it? I was horrified when I saw the list

of acks etc (including me) on the commit with the helper_unlock issue we

just fixed. It's truly scary to think that none of us looked closely

enough to pick that up at the time.
Nigel

--

Well, the code in question was not in the patch, so if you didn't actually look

at the _patched_ kmod.c, you would have a little chance to spot it.  I

overlooked it, even though I did look at the patched file ...
Still, this particular problem hasn't been triggering it testing for quite some

time.
Greetings,

Rafael

--