Re: Fix up power managment in 2.6

Patrick Mochel
2003-09-02T00:46:39Z

> I've been reviewing your code, see your mailbox. Unfortunately due to
> you renaming functions and moving files around, it is very hard to review.

I've tried to make each changeset do one conceptual item. And, each patch
that I posted, besides obviously the cumulative one, represents one
changeset.

> [Attached is patch "not changing core functionality". How did you
> expect me to verify that? And it was you who protested on killing 3
> printks.]

What is the problem with it? Why is it not better than what was there
before?

> And managed to call sleeping functions with interrupts disabled and
> break x86-64 somewhere in the process.

The first is unintentional and not something I see here. Will investigate
further.

Have you confirmed that x86-64 is broken, or are you simply trying to
raise more accusations? If it is broken, please tell exactly what the
problem is and I will fix it.

> Hmm, and because you killed
> BUG_ON(in_atomic()), you did not realize that you were breaking
> that.

That was in software_suspend() itself, which was completely bogus. For
one, you should review how you're getting called and realize that neither
places were atomic contexts. So, it was useless.

Now, you did export software_suspend() for some unknown reason, and that
is simply bogus. Why would a module call you?

Finally, you BUG()'d when you could simply return an error. That's
completely unfriendly to the user. Just return an error, like every other
sane code path.

> And I do not think you actually tested those "panic" codepaths
> to make sure you are not corrupting data, right?

panic() is not a valid replacement for sane error handling. Every single
panic() in swsusp can be replaced by proper error handling. You should
have done that a long time ago. Calling panic() is just lazy.

> > I will also restore swsusp to whatever state you like - either -test1,
> > -test3 or -test4 state, or keep it the way it currently is in my patches.
> > But note that doing so will result in a large amount of duplicated code
> > which you will be responsible for either merging or removing.
>
> Good, please return it to -test3 state. If you can leave your split-up
> patches on some public ftp site, that would be good; when dm is back
> working so I can actually test it, I'll do some cherry-picking.

They will remain at the URL I posted the other day. As will the patches
to convert it back to the state you please. I will patch against the
current tree and continue to work from there. Note that this involves the
contents of kernel/swsusp.c only.

And, the driver model is working fine. I don't know what you're
complaining about now, but a sane bug report would be helpful. So would
some patches -- you've been maintaining swsusp for two years now, and
you've not help convert one driver to the new model (even before it
changed).

Thanks,

Pat