Messages in this thread | | | Date | Mon, 1 Sep 2003 17:52:35 -0700 (PDT) | From | Patrick Mochel <> | Subject | Re: Fix up power managment in 2.6 |
| |
> 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
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |