From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754223AbcIBPCQ (ORCPT ); Fri, 2 Sep 2016 11:02:16 -0400 Received: from foss.arm.com ([217.140.101.70]:53186 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754174AbcIBPCN (ORCPT ); Fri, 2 Sep 2016 11:02:13 -0400 Subject: Re: [PATCH ] drivers/base: cacheinfo: remove warning in resume To: Sumit Gupta References: <1472455254-27692-1-git-send-email-sumitg@nvidia.com> <32ada52a-4d47-282a-ef10-33d3e32fbd52@arm.com> <8413522b34f64829a4e601015c76ee45@BGMAIL104.nvidia.com> Cc: Sudeep Holla , "gregkh@linuxfoundation.org" , Bibek Basu , "linux-tegra@vger.kernel.org" , "linux-kernel@vger.kernel.org" From: Sudeep Holla Organization: ARM Message-ID: <4937b653-6b54-fa6e-10aa-e5b8e567c257@arm.com> Date: Fri, 2 Sep 2016 16:02:09 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <8413522b34f64829a4e601015c76ee45@BGMAIL104.nvidia.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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