Re: [PATCH] cpufreq: fix garbage kobj on errors during suspend/resume

Rafael J. Wysocki
2013-12-07T01:17:50Z

On Friday, December 06, 2013 10:53:16 AM Srivatsa S. Bhat wrote:
> On 12/06/2013 03:59 AM, Rafael J. Wysocki wrote:
> > On Thursday, December 05, 2013 06:11:19 PM Srivatsa S. Bhat wrote:
> >> On 12/05/2013 12:27 PM, BjÃ¸rn Mork wrote:
> >>> Viresh Kumar <viresh.kumar@linaro.org> writes:
> >>>
> >>>> Sending from phone.. html.. so left list.
> >>>>
> >>>> Here is the old thread where we discussed this.. see if this helps..
> >>>>
> >>>> http://marc.info/?t=136845016900003&r=1&w=2
> >>>
> >>> Thanks. That helped a lot.
> >>>
> >>> Unless I miss something, it looks like the permission problem *started*
> >>> with fallout from special suspend code - surprising the user by not
> >>> creating any offline/online event on suspend/resume. Quoting from
> >>> http://marc.info/?l=linux-pm&m=136847781510358 :
> >>>
> >>> (And yes, even code-wise, we use a slightly different
> >>> path in the S3 code, to initiate hotplug. That's why the uevents
> >>> are by-passed.)
> >>>
> >>
> >> I hope you didn't miss the main idea I was trying to convey in that
> >> reply:
> >> "IMHO, using CPU hotplug (offline/online of CPUs) in the S3 path is
> >> supposed to be totally internal to the suspend/resume code. It is not
> >> intended for userspace to know that we are internally offlining/
> >> onlining CPUs."
> >
> > By the way, in the meantime I discussed this with Viresh in the context of
> > a different (although related) fix and I suggested a different approach.
> >
> > Namely, to split the CPU offline/online code into "core" and "add-ons"
> > parts, where the core part will do just whatever is needed to offline/online
> > CPU cores cleanly and the "add-ons" part will carry out the rest (e.g.
> > removal/addition of sysfs attributes and so on).
> >
> > Then, the system suspend/resume code path will only run the "core" part
> > and whatever else CPU handling is needed during suspend/resume will be
> > carried out by the device suspend/resume code (via driver callbacks or
> > stuff similar to cpufreq_suspend() and cpufreq_resume() recently proposed
> > by Viresh).
> >
> > In turn, the "runtime" CPU offline/online will carry out both the core
> > and the add-ons parts as it does today.
> >
> > In my view this should address the problems we have with sysfs attributes,
> > governors start/stop etc. during system suspend/resume.
> >
>
> Hmm, yes that sounds like a good idea. Are you suggesting this "core" and
> "add-on" split only for the cpufreq parts of CPU hotplug or for everything
> involved in CPU hotplug code? IIUC you are suggesting the latter, which is
> likely to be a significant undertaking, but very well worth it in the long
> run, since it gives us an elegant solution for all these problems.

Yeah, and I'd like that to be done.

> I guess the *_FROZEN CPU hotplug notifications were originally introduced
> to provide us an infrastructure to do something like this, but obviously
> that hasn't worked out very well.

That's been a workaround to be honest.

> So I agree that a fundamental restructuring is in order, to cure all the
> innumerable problems.

Yes.

Thanks!

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.