From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 06F3DC43381 for ; Fri, 22 Mar 2019 09:56:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C4B7A2190A for ; Fri, 22 Mar 2019 09:56:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1553248572; bh=afa2nvnyHWP8+3FySA0JVnwQVrQ+UqIMEl/R1ucrSAE=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-ID:From; b=q9GYDFU7OqfjOQ8Vo5/HdHj/9c9mP35hxXGOvGSe4Cj85jfWVYunT4gsM8xwmcX1G +rrWWknV2Ja0Jose8nVnU2qDIBWZ8e9uDXFop+auUsEqrAPIKcOkVMuwkF3XlhL63N kVpGQiP4qEWIXNLwfxJjtMFGw0RKtXhHhpZRGShQ= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727962AbfCVJ4L (ORCPT ); Fri, 22 Mar 2019 05:56:11 -0400 Received: from mail-oi1-f193.google.com ([209.85.167.193]:43505 "EHLO mail-oi1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727865AbfCVJ4L (ORCPT ); Fri, 22 Mar 2019 05:56:11 -0400 Received: by mail-oi1-f193.google.com with SMTP id 67so1242296oif.10; Fri, 22 Mar 2019 02:56:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=R769lZEppttus9KqpwL+hUjXXVn/tU1JC7ZECyzBamc=; b=tccTtVworl4Kduy/7XS93kLHH1cxVaa0wGCNH30p4Ne0JWVoMBavBVvIam/SmbwoIv 9LRjBzJ8CcnEjYCL5KD5QPqrhrDAu9+4FrohqiVq5NHf4t/exyJ3ND/BibDFgK2CDucN gZVBZ1bQxgNsUEMjkmXj8DQB923XI2W4Zu9+rf7wYZ4dm0nx8E64nbm0Zvf54lw3FnVM HG4ztqkstVc7PU7/fTR7/KM7VPLnyKaVV4PKUcyfZeGANvqZpBe7lfqvzjC21u7LMb1O ftNGikxBK44KE9wbHg1g217zlMGeJ3jXKyGMVqwufsmBfhXKWfbFqQ7yq3oERN2q/U3T zD+A== X-Gm-Message-State: APjAAAXyP2hBS0lUHn+o1zl1B+anO7S644UIKibBBAxE5Jj7majnyQot 6lUgaBm2/ppyvN6F1LMSpguKcZ2qLzfsHjPLXK8= X-Google-Smtp-Source: APXvYqzfYWEJ9k1BlLeuYHyhGI8XIj2c5DvqpphS6zVQFJIHX5sO4ZexCNDvS1JR5BgncxePxdpvZwhzD60J6/IIFOE= X-Received: by 2002:aca:8d3:: with SMTP id 202mr1242844oii.76.1553248570097; Fri, 22 Mar 2019 02:56:10 -0700 (PDT) MIME-Version: 1.0 References: <1eb27e3bbcbb2c67e6eadc0893c9b41e5d76894b.1553057341.git.viresh.kumar@linaro.org> <20190322062744.efpl4itnqtny7txf@vireshk-i7> In-Reply-To: <20190322062744.efpl4itnqtny7txf@vireshk-i7> From: "Rafael J. Wysocki" Date: Fri, 22 Mar 2019 10:55:59 +0100 Message-ID: Subject: Re: [PATCH V3] cpufreq: Call transition notifier only once for each policy To: Viresh Kumar Cc: Thomas Gleixner , Rafael Wysocki , Peter Zijlstra , Russell King , "David S. Miller" , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , "the arch/x86 maintainers" , Paolo Bonzini , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Linux PM , Vincent Guittot , Linux ARM , Linux Kernel Mailing List , sparclinux@vger.kernel.org, kvm@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Mar 22, 2019 at 7:28 AM Viresh Kumar wrote: > > On 21-03-19, 16:49, Thomas Gleixner wrote: > > On Wed, 20 Mar 2019, Viresh Kumar wrote: > > > > > > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c > > > index 3fae23834069..b2fe665878f7 100644 > > > --- a/arch/x86/kernel/tsc.c > > > +++ b/arch/x86/kernel/tsc.c > > > @@ -958,10 +958,15 @@ static int time_cpufreq_notifier(struct notifier_block *nb, unsigned long val, > > > struct cpufreq_freqs *freq = data; > > > unsigned long *lpj; > > > > > > + if (WARN_ON_ONCE(cpumask_weight(freq->policy->related_cpus) != 1)) { > > > + mark_tsc_unstable("cpufreq changes: related CPUs affected"); > > > + return 0; > > > + } > > > > You might add a check which ensures that policy->cpu == smp_processor_id() > > because if this is not the case .... > > How about something like this ? > > if (WARN_ON_ONCE(cpumask_weight(freq->policy->related_cpus) != 1 || > freq->policy->cpu != smp_processor_id())) { > mark_tsc_unstable("cpufreq changes: related CPUs affected"); > return 0; > } > > > Thanks for your feedback. Peter suggested something like this IIRC. Anyway, I'm still concerned that this approach in general fundamentally doesn't work on SMP with frequency synchronization, which is the case for the platforms affected by the problem it attempts to overcome. The frequency has just been changed on one CPU, presumably to the requested value (so this cannot work when turbo is enabled anyway), but then it also has changed for all of the other CPUs in the system (or at least in the package), so it is not sufficient to update the single CPU here as it is only a messenger, so to speak. However, updating the other CPUs from here would be fundamentally racy AFAICS. From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Date: Fri, 22 Mar 2019 09:55:59 +0000 Subject: Re: [PATCH V3] cpufreq: Call transition notifier only once for each policy Message-Id: List-Id: References: <1eb27e3bbcbb2c67e6eadc0893c9b41e5d76894b.1553057341.git.viresh.kumar@linaro.org> <20190322062744.efpl4itnqtny7txf@vireshk-i7> In-Reply-To: <20190322062744.efpl4itnqtny7txf@vireshk-i7> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Viresh Kumar Cc: Thomas Gleixner , Rafael Wysocki , Peter Zijlstra , Russell King , "David S. Miller" , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , the arch/x86 maintainers , Paolo Bonzini , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Linux PM , Vincent Guittot , Linux ARM , Linux Kernel Mailing List , sparclinux@vger.kernel.org, kvm@vger.kernel.org On Fri, Mar 22, 2019 at 7:28 AM Viresh Kumar wrote: > > On 21-03-19, 16:49, Thomas Gleixner wrote: > > On Wed, 20 Mar 2019, Viresh Kumar wrote: > > > > > > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c > > > index 3fae23834069..b2fe665878f7 100644 > > > --- a/arch/x86/kernel/tsc.c > > > +++ b/arch/x86/kernel/tsc.c > > > @@ -958,10 +958,15 @@ static int time_cpufreq_notifier(struct notifier_block *nb, unsigned long val, > > > struct cpufreq_freqs *freq = data; > > > unsigned long *lpj; > > > > > > + if (WARN_ON_ONCE(cpumask_weight(freq->policy->related_cpus) != 1)) { > > > + mark_tsc_unstable("cpufreq changes: related CPUs affected"); > > > + return 0; > > > + } > > > > You might add a check which ensures that policy->cpu = smp_processor_id() > > because if this is not the case .... > > How about something like this ? > > if (WARN_ON_ONCE(cpumask_weight(freq->policy->related_cpus) != 1 || > freq->policy->cpu != smp_processor_id())) { > mark_tsc_unstable("cpufreq changes: related CPUs affected"); > return 0; > } > > > Thanks for your feedback. Peter suggested something like this IIRC. Anyway, I'm still concerned that this approach in general fundamentally doesn't work on SMP with frequency synchronization, which is the case for the platforms affected by the problem it attempts to overcome. The frequency has just been changed on one CPU, presumably to the requested value (so this cannot work when turbo is enabled anyway), but then it also has changed for all of the other CPUs in the system (or at least in the package), so it is not sufficient to update the single CPU here as it is only a messenger, so to speak. However, updating the other CPUs from here would be fundamentally racy AFAICS. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 65346C43381 for ; Fri, 22 Mar 2019 09:56:20 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 33FDC21874 for ; Fri, 22 Mar 2019 09:56:20 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="CXvfAKwX" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 33FDC21874 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=jwGGyjDfw0ZiuRL/YWtX7CRT75GBVGFP2mHnbk5u4Z0=; b=CXvfAKwXlqlRxt vtZB8auEWd3mGTKAGt18YPNwWzydjcJm8vGo0RwibxE2xiGinO76vsdxMjkM34hO0Yj/6MffI/pjO v+M1qPyCyJ6Xf7ywAFMyHS7tGZFMUjiImOfHj2MTP82tOmjmaNLNPEnDcM3qf5CXyUXSpseOxLGmo XKXNS7Ysnmrgc5owtMnhZVukOYvm+UkK6KBJ+AZzjNNK5H3Lr+h9AXluavgBns5zHzQa3j6mWZfxP fbgaYSQYjGNFtTZVMjKGsMI7ljxzpfuMKBaxjIp21aInjIjs/b4aTsC56/Vf04zyfhCDWWUtPb05z lTTnKEYsjM2DfC4FzVVQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1h7GuM-0004OI-Vy; Fri, 22 Mar 2019 09:56:14 +0000 Received: from mail-oi1-f194.google.com ([209.85.167.194]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1h7GuK-0004NU-2B for linux-arm-kernel@lists.infradead.org; Fri, 22 Mar 2019 09:56:13 +0000 Received: by mail-oi1-f194.google.com with SMTP id w139so1249756oie.9 for ; Fri, 22 Mar 2019 02:56:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=R769lZEppttus9KqpwL+hUjXXVn/tU1JC7ZECyzBamc=; b=QzH+j6qhTQSYsMO8apddaacvdoLKcTwd8vvDM6LmdoXvfkLp9gtmZZBCqwyQ3tTPnc o1RP8HrX4P5Vbhn6b7VZNRxNrQgZzxvj0iAt1mFVGyNDlUOzWh50P9n98mlYligo8sgD rEXkzT35LZSxWVSfskDyEh6m/d5yaUHM8W5NXQ3Twe4DDmQwV2/NTcKF7e26EJy6z1oE QloneXHnrQrSqjPUdT5KHTWIS0EFLXmVmo+3YTf0MYsFAWvnrHoAa0sfF1a8FqoLxXi7 DoRRRnWEEKLWlB7ah1+oVSF0loAtzZ8xdh8lw04cKdCoO5poVe1ZUNathwOcGcxJbEO4 Z5zg== X-Gm-Message-State: APjAAAXZ8WQ0I5c4wxpwMWb7n/786tf1ZSDq249DMiDGgWX2bHqChiYg 7XacWAlko/MNdEcI5YYnZIFTyYxfqOkQLOn7QZ8= X-Google-Smtp-Source: APXvYqzfYWEJ9k1BlLeuYHyhGI8XIj2c5DvqpphS6zVQFJIHX5sO4ZexCNDvS1JR5BgncxePxdpvZwhzD60J6/IIFOE= X-Received: by 2002:aca:8d3:: with SMTP id 202mr1242844oii.76.1553248570097; Fri, 22 Mar 2019 02:56:10 -0700 (PDT) MIME-Version: 1.0 References: <1eb27e3bbcbb2c67e6eadc0893c9b41e5d76894b.1553057341.git.viresh.kumar@linaro.org> <20190322062744.efpl4itnqtny7txf@vireshk-i7> In-Reply-To: <20190322062744.efpl4itnqtny7txf@vireshk-i7> From: "Rafael J. Wysocki" Date: Fri, 22 Mar 2019 10:55:59 +0100 Message-ID: Subject: Re: [PATCH V3] cpufreq: Call transition notifier only once for each policy To: Viresh Kumar X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190322_025612_108066_E473F7B5 X-CRM114-Status: GOOD ( 17.76 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Vincent Guittot , kvm@vger.kernel.org, =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Peter Zijlstra , Linux PM , the arch/x86 maintainers , Rafael Wysocki , Russell King , Linux Kernel Mailing List , Ingo Molnar , Borislav Petkov , "H. Peter Anvin" , sparclinux@vger.kernel.org, Paolo Bonzini , Thomas Gleixner , "David S. Miller" , Linux ARM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Mar 22, 2019 at 7:28 AM Viresh Kumar wrote: > > On 21-03-19, 16:49, Thomas Gleixner wrote: > > On Wed, 20 Mar 2019, Viresh Kumar wrote: > > > > > > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c > > > index 3fae23834069..b2fe665878f7 100644 > > > --- a/arch/x86/kernel/tsc.c > > > +++ b/arch/x86/kernel/tsc.c > > > @@ -958,10 +958,15 @@ static int time_cpufreq_notifier(struct notifier_block *nb, unsigned long val, > > > struct cpufreq_freqs *freq = data; > > > unsigned long *lpj; > > > > > > + if (WARN_ON_ONCE(cpumask_weight(freq->policy->related_cpus) != 1)) { > > > + mark_tsc_unstable("cpufreq changes: related CPUs affected"); > > > + return 0; > > > + } > > > > You might add a check which ensures that policy->cpu == smp_processor_id() > > because if this is not the case .... > > How about something like this ? > > if (WARN_ON_ONCE(cpumask_weight(freq->policy->related_cpus) != 1 || > freq->policy->cpu != smp_processor_id())) { > mark_tsc_unstable("cpufreq changes: related CPUs affected"); > return 0; > } > > > Thanks for your feedback. Peter suggested something like this IIRC. Anyway, I'm still concerned that this approach in general fundamentally doesn't work on SMP with frequency synchronization, which is the case for the platforms affected by the problem it attempts to overcome. The frequency has just been changed on one CPU, presumably to the requested value (so this cannot work when turbo is enabled anyway), but then it also has changed for all of the other CPUs in the system (or at least in the package), so it is not sufficient to update the single CPU here as it is only a messenger, so to speak. However, updating the other CPUs from here would be fundamentally racy AFAICS. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel