Re: [PATCH ] drivers/base: cacheinfo: remove warning in resume

[Sudeep Holla]

On 02/09/16 13:58, Sumit Gupta wrote:
> Hi Sudeep,
>
> Thank you for your comments.
> > I understand the warning we get but the patch is completely wrong.
> > One it removes the feature of adding/removing the cache devices on
> > cpu hotplug events. Have you tested your patch with simple cpu
> > hotplug and seen no change before and after this change ?
>
> I referred other node "cpufreq" under "cpu" node and that was also
> not getting deleted when core goes down. So, thought the behavior
> should be similar as entries are only used to read data and it won't
> change after boot.

I agree, but for caches it has been like this and in a way make sense.

> > On 29/08/16 08:20, Sumit Gupta wrote:
> > > CPU notifier is present for creating device entries for child
> > > node "cache" under parent node "cpu" as per DT.
> >
> > Again DT is only on few architectures not on all(e.g. x86)
> >
> > > During resume from suspend, while booting all non-boot CPU's,
> > > this notifier for adding cache device gets called before cpu
> > > device is added by device_resume. Because of this warning message
> > > of "parent should not be sleeping" comes during resume.
> >
> > Yes that's correct and needs to be fixed. I have seen this but
> > haven't spent much time to check in detail. It's harmless warning
> > IMO. See the comment in the code too:"This is a fib. But we'll
> > allow new children to be added below a resumed device, even if the
> > device hasn't been completed yet"
> >
> > CPU devices are special and they have separate hotplug paths. So we
> > need to consider that for cpu devices and set is_prepared quite
> > early.
> >
> > > Removing the notifier to explicitly add/remove cache device as
> > > CPU and cache device get added/removed anyway as part of normal
> > > suspend resume sequence. dpm_resume_end - > dpm_resume ->
> > > device_resume
> >
> > Yes, but: 1. the caches objects are visible even when the cpu is
> > offline 2. how is this handled for normal cpu-hotplug events ? This
> > patch breaks the existing feature.
>
> Compared with x86 now and there even cpufreq is getting deleted. As
> per the comments, right behavior seems to be where cache entries
> should be created during core on and removed during core off in
> hotplug. I will try to find other way of doing it. Also will see why
> cpufreq is not getting deleted as in x86.

IIUC you are saying cpufreq sysfs entries are deleted on x86 while not
on ARM{32,64} ? If so are you running same kernel version on both x86
and ARM ? I remember some recent change around cpufreq sysfs entries, it
could be result of that.


--
Regards,
Sudeep

--
Regards,
Sudeep