From: "Bjørn Mork" <bjorn@mork.no>
To: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
Cc: Viresh Kumar <viresh.kumar@linaro.org>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
Linux PM mailing list <linux-pm@vger.kernel.org>,
cpufreq <cpufreq@vger.kernel.org>
Subject: Re: [PATCH] cpufreq: fix garbage kobj on errors during suspend/resume
Date: Thu, 05 Dec 2013 14:21:55 +0100 [thread overview]
Message-ID: <87zjofv3f0.fsf@nemi.mork.no> (raw)
In-Reply-To: <52A0746F.9010705@linux.vnet.ibm.com> (Srivatsa S. Bhat's message of "Thu, 05 Dec 2013 18:11:19 +0530")
"Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com> writes:
> You seem to be getting confused as to what the *actual* userspace
> expectation was, in that mail thread. The expectation was that suspend/
> resume is a kernel operation that brings back the system to the same
> state (as much as possible) at the end of resume, as it was before
> suspend. And that is a perfectly valid expectation, and it is something
> that the kernel has to try its level best to honor.
Our understanding of the thread is obviously biased by our respective
views of the matter ;-)
> And in this particular case, the specific expectation was that the sysfs
> file permissions set by the user for cpufreq files will remain as it is
> even after a suspend/resume cycle. That's it. There is _absolutely_ _no_
> talk about CPU hotplug here.
This is not a cpufreq problem. It is a CPU problem. You are only
complicating things by trying to solve it in cpufreq. What about
permissions on the other CPU sysfs attributes?
> Robert _happened_ to dig this further and observe that suspend/resume
> actually does offline/online of CPUs, and thought that he should have
> also seen the associated udev events as well. But we have purposefully
> not exposed the fact that suspend/resume involves CPU hotplug.
Well, that is a bug in my opinion. You are adding lots of kernel code
to avoid a very simple userspace configuration. Seems very backwards.
This is mostly a documentation problem, if a problem at all. I do note
that Robert did try the udev way before concluding that this was a
kernel bug.
> Today,
> suspend/resume uses CPU hotplug internally because we don't have any
> other better alternative. The very concept/semantic of suspend/resume
> _does_ _not_ imply CPU hotplug - it is just an implementation detail
> that userspace should not need to care about or rely on.
>
> Moreover, cpufreq is not the only subsystem that participates in suspend/
> resume and CPU hotplug. And fundamentally, regular CPU hotplug has _very_
> different semantics and guarantees than the hotplug done in suspend/resume.
> For example, if you offline CPUs normally, the cpusets associated with
> those CPUs will get destroyed, potentially in ways that _won't_ bring
> them back to the same state when you online those exact same CPUs!
> And this would have been totally unacceptable to a user innocuously
> using suspend/resume. Look at commit d35be8bab9 (CPU hotplug, cpusets,
> suspend: Don't modify cpusets during suspend/resume), for more details
> on how we fixed that problem.
Yes, I do see that point. The special suspend/resume handling is
definitely necessary in this case.
> So, to summarize, suspend/resume should mean one simple thing to
> userspace - "the kernel will transparently (to the extent possible)
> perform suspend/resume and bring back the system during resume, to a
> state as close as possible compared to how it was before suspend".
> Any implementation challenges must be handled in the kernel (as far as
> possible), and we should avoid burdening userspace with extraneous
> events etc.
OK, I think we may have different views on how much kernel
implementation details userspace need to know :-)
That's fine, and it really was not my intention to question the way
cpufreq handled this. It all just seemed so meaningless to me. I now
understand that it has a meaning to you, which of course is what
matters.
I am glad I could help removing the most obvious bugs.
Bj√∏rn
next prev parent reply other threads:[~2013-12-05 13:21 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-03 11:14 [PATCH] cpufreq: fix garbage kobj on errors during suspend/resume Bjørn Mork
2013-12-03 21:45 ` Rafael J. Wysocki
2013-12-04 6:23 ` Srivatsa S. Bhat
2013-12-24 9:46 ` Jarzmik, Robert
2013-12-04 10:32 ` viresh kumar
2013-12-04 12:08 ` Bjørn Mork
2013-12-04 14:41 ` Viresh Kumar
2013-12-04 15:41 ` Bjørn Mork
[not found] ` <CAKohponu3Fu=WaBHXP1iBJM87V9g=+hDPe=M168U_weODenZdQ@mail.gmail.com>
[not found] ` <878uvzyecg.fsf@nemi.mork.no>
2013-12-05 12:41 ` Srivatsa S. Bhat
2013-12-05 13:21 ` Bjørn Mork [this message]
2013-12-05 22:29 ` Rafael J. Wysocki
2013-12-06 5:23 ` Srivatsa S. Bhat
2013-12-07 1:17 ` Rafael J. Wysocki
2013-12-05 1:29 ` Rafael J. Wysocki
2013-12-09 2:59 ` Lan Tianyu
2013-12-09 6:48 ` Srivatsa S. Bhat
2013-12-09 10:04 ` Bjørn Mork
2013-12-12 1:59 ` Rafael J. Wysocki
2013-12-12 8:52 ` Bjørn Mork
2013-12-09 11:24 ` Martin Ziegler
2013-12-09 11:53 ` Bjørn Mork
2013-12-10 16:02 ` Martin Ziegler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zjofv3f0.fsf@nemi.mork.no \
--to=bjorn@mork.no \
--cc=cpufreq@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rjw@rjwysocki.net \
--cc=srivatsa.bhat@linux.vnet.ibm.com \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox