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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <32ada52a-4d47-282a-ef10-33d3e32fbd52@arm.com>
Date:   Tue, 30 Aug 2016 11:20:18 +0100
From:   Sudeep Holla <sudeep.holla@....com>
To:     Sumit Gupta <sumitg@...dia.com>, linux-kernel@...r.kernel.org
Cc:     gregkh@...uxfoundation.org, Sudeep Holla <sudeep.holla@....com>,
        bbasu@...dia.com, linux-tegra@...r.kernel.org
Subject: Re: [PATCH ] drivers/base: cacheinfo: remove warning in resume

Hi Sumit,

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 ?

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.

> Signed-off-by: Sumit Gupta <sumitg@...dia.com>
> ---
>  drivers/base/cacheinfo.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
> index e9fd32e91668..17d9c051a16f 100644
> --- a/drivers/base/cacheinfo.c
> +++ b/drivers/base/cacheinfo.c
> @@ -544,8 +544,6 @@ static int __init cacheinfo_sysfs_init(void)
>  			goto out;
>  		}
>  	}
> -	__hotcpu_notifier(cacheinfo_cpu_callback, 0);
> -
>  out:
>  	cpu_notifier_register_done();
>  	return rc;
>

-- 
Regards,
Sudeep