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=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 EF3F2C432BE for ; Tue, 3 Aug 2021 23:54:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D722C61037 for ; Tue, 3 Aug 2021 23:54:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234734AbhHCXzC (ORCPT ); Tue, 3 Aug 2021 19:55:02 -0400 Received: from Galois.linutronix.de ([193.142.43.55]:60438 "EHLO galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233784AbhHCXzB (ORCPT ); Tue, 3 Aug 2021 19:55:01 -0400 From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1628034888; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=uXPzQ106Gzz/QfQUvV21NZBY82K/Xa1WUotPKxNKs/s=; b=g2tVyGLVgyvvt51y4phFCb4e/IRo+TkCrR0P7rBCXrlqqV0FKAVjAnQJdSyb3vhaHN1GmM JWSCfIw6Z9h71HVqL2Fd4i8fs59RyB65mscfupaiU1TtrZDpqx3291G6iYN3YYjdcB612K kI/o+QLxBFdJZICfP3jZdqsMg89RjInVaGpfpF/JObzZDzA4zvMVZtaCB1dE5rrFiOP1xF Nxv2UQEayZUGtfN2y6y5P/vjiQq5C5KmuFshIrlqWHrLBchL/6WfwSceA6k0dw2XmFQ7jq Ws+K18sAjWTdCQXhV73P9SYp3dTLo8YxiEVUOfXDtxbs8Il6oHwvbssnPGYTBQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1628034888; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=uXPzQ106Gzz/QfQUvV21NZBY82K/Xa1WUotPKxNKs/s=; b=ZHq7Hf3ihfyizoxoVi0V4ONzM+2F6n6diNdxjaMuOUZ9CkkB7CtkKmdUo9JFoivEPz8Nhv K7baBhsWJ/AmalCw== To: Mel Gorman , Andrew Morton Cc: Ingo Molnar , Vlastimil Babka , Hugh Dickins , Linux-MM , Linux-RT-Users , LKML , Mel Gorman , Peter Zijlstra Subject: Re: [PATCH 2/2] mm/vmstat: Protect per cpu variables with preempt disable on RT In-Reply-To: <20210723100034.13353-3-mgorman@techsingularity.net> References: <20210723100034.13353-1-mgorman@techsingularity.net> <20210723100034.13353-3-mgorman@techsingularity.net> Date: Wed, 04 Aug 2021 01:54:47 +0200 Message-ID: <87czqu2iew.ffs@tglx> MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Mel! On Fri, Jul 23 2021 at 11:00, Mel Gorman wrote: > From: Ingo Molnar > > Disable preemption on -RT for the vmstat code. On vanila the code runs > in IRQ-off regions while on -RT it may not when stats are updated under > a local_lock. "preempt_disable" ensures that the same resources is not > updated in parallel due to preemption. > > This patch differs from the preempt-rt version where __count_vm_event and > __count_vm_events are also protected. The counters are explicitly "allowed > to be to be racy" so there is no need to protect them from preemption. Only > the accurate page stats that are updated by a read-modify-write need > protection. > > Signed-off-by: Ingo Molnar > Signed-off-by: Thomas Gleixner > Signed-off-by: Mel Gorman > --- > mm/vmstat.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/mm/vmstat.c b/mm/vmstat.c > index b0534e068166..d06332c221b1 100644 > --- a/mm/vmstat.c > +++ b/mm/vmstat.c > @@ -319,6 +319,7 @@ void __mod_zone_page_state(struct zone *zone, enum zone_stat_item item, > long x; > long t; > > + preempt_disable_rt(); Yes, this is smart to some extent. But in reality it's a bandaid simply because nobody can tell which item of vmstat requires which protection. If you go back in RT history then you will figure out that we were able to eliminate _all_ occurences of preempt_disable_rt() except for this one. Even mm developers are wary about this: so in vmstat.c there is this magic comment: * For use when we know that interrupts are disabled * or when we know that preemption is disabled and that * particular counter cannot be updated from interrupt context. how can I know which counters need what? I don't think there's a list, one would have to check on counter to counter basis :/ and of course there is nothing which validates that, right? exactly Brilliant stuff which prevents you to do any validation on this. Over the years there have been several issues where callers had to be fixed by analysing bug reports instead of having a proper instrumentation in that code which would have told the developer that he got it wrong. Of course on RT kernels the preempt_disable_rt() will serialize everything correctly, but as we have learned over the years just slapping _if_rt() or if_not_rt() variants of things around is most of the time papering over the underlying problem of badly defined protection scopes. Let's not proliferate that. As I said in the above IRC conversation: I fundamentally hate this preempt_disable_rt() muck Thanks, tglx 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=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 E43C9C43214 for ; Tue, 3 Aug 2021 23:54:52 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 6290461106 for ; Tue, 3 Aug 2021 23:54:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 6290461106 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linutronix.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 78A798D0011; Tue, 3 Aug 2021 19:54:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7135D8D000F; Tue, 3 Aug 2021 19:54:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5B41D8D0011; Tue, 3 Aug 2021 19:54:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0193.hostedemail.com [216.40.44.193]) by kanga.kvack.org (Postfix) with ESMTP id 3E30A8D000F for ; Tue, 3 Aug 2021 19:54:51 -0400 (EDT) Received: from smtpin01.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id D7B9D8249980 for ; Tue, 3 Aug 2021 23:54:50 +0000 (UTC) X-FDA: 78435427140.01.21463C8 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by imf12.hostedemail.com (Postfix) with ESMTP id 1D4FE10055A2 for ; Tue, 3 Aug 2021 23:54:49 +0000 (UTC) From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1628034888; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=uXPzQ106Gzz/QfQUvV21NZBY82K/Xa1WUotPKxNKs/s=; b=g2tVyGLVgyvvt51y4phFCb4e/IRo+TkCrR0P7rBCXrlqqV0FKAVjAnQJdSyb3vhaHN1GmM JWSCfIw6Z9h71HVqL2Fd4i8fs59RyB65mscfupaiU1TtrZDpqx3291G6iYN3YYjdcB612K kI/o+QLxBFdJZICfP3jZdqsMg89RjInVaGpfpF/JObzZDzA4zvMVZtaCB1dE5rrFiOP1xF Nxv2UQEayZUGtfN2y6y5P/vjiQq5C5KmuFshIrlqWHrLBchL/6WfwSceA6k0dw2XmFQ7jq Ws+K18sAjWTdCQXhV73P9SYp3dTLo8YxiEVUOfXDtxbs8Il6oHwvbssnPGYTBQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1628034888; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=uXPzQ106Gzz/QfQUvV21NZBY82K/Xa1WUotPKxNKs/s=; b=ZHq7Hf3ihfyizoxoVi0V4ONzM+2F6n6diNdxjaMuOUZ9CkkB7CtkKmdUo9JFoivEPz8Nhv K7baBhsWJ/AmalCw== To: Mel Gorman , Andrew Morton Cc: Ingo Molnar , Vlastimil Babka , Hugh Dickins , Linux-MM , Linux-RT-Users , LKML , Mel Gorman ,Peter Zijlstra Subject: Re: [PATCH 2/2] mm/vmstat: Protect per cpu variables with preempt disable on RT In-Reply-To: <20210723100034.13353-3-mgorman@techsingularity.net> References: <20210723100034.13353-1-mgorman@techsingularity.net> <20210723100034.13353-3-mgorman@techsingularity.net> Date: Wed, 04 Aug 2021 01:54:47 +0200 Message-ID: <87czqu2iew.ffs@tglx> MIME-Version: 1.0 Content-Type: text/plain X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 1D4FE10055A2 Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=linutronix.de header.s=2020 header.b=g2tVyGLV; dkim=pass header.d=linutronix.de header.s=2020e header.b=ZHq7Hf3i; dmarc=pass (policy=none) header.from=linutronix.de; spf=pass (imf12.hostedemail.com: domain of tglx@linutronix.de designates 193.142.43.55 as permitted sender) smtp.mailfrom=tglx@linutronix.de X-Stat-Signature: t8b5yq97seznr6wun8bbh91sc39gnwre X-HE-Tag: 1628034889-627777 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Mel! On Fri, Jul 23 2021 at 11:00, Mel Gorman wrote: > From: Ingo Molnar > > Disable preemption on -RT for the vmstat code. On vanila the code runs > in IRQ-off regions while on -RT it may not when stats are updated under > a local_lock. "preempt_disable" ensures that the same resources is not > updated in parallel due to preemption. > > This patch differs from the preempt-rt version where __count_vm_event and > __count_vm_events are also protected. The counters are explicitly "allowed > to be to be racy" so there is no need to protect them from preemption. Only > the accurate page stats that are updated by a read-modify-write need > protection. > > Signed-off-by: Ingo Molnar > Signed-off-by: Thomas Gleixner > Signed-off-by: Mel Gorman > --- > mm/vmstat.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/mm/vmstat.c b/mm/vmstat.c > index b0534e068166..d06332c221b1 100644 > --- a/mm/vmstat.c > +++ b/mm/vmstat.c > @@ -319,6 +319,7 @@ void __mod_zone_page_state(struct zone *zone, enum zone_stat_item item, > long x; > long t; > > + preempt_disable_rt(); Yes, this is smart to some extent. But in reality it's a bandaid simply because nobody can tell which item of vmstat requires which protection. If you go back in RT history then you will figure out that we were able to eliminate _all_ occurences of preempt_disable_rt() except for this one. Even mm developers are wary about this: so in vmstat.c there is this magic comment: * For use when we know that interrupts are disabled * or when we know that preemption is disabled and that * particular counter cannot be updated from interrupt context. how can I know which counters need what? I don't think there's a list, one would have to check on counter to counter basis :/ and of course there is nothing which validates that, right? exactly Brilliant stuff which prevents you to do any validation on this. Over the years there have been several issues where callers had to be fixed by analysing bug reports instead of having a proper instrumentation in that code which would have told the developer that he got it wrong. Of course on RT kernels the preempt_disable_rt() will serialize everything correctly, but as we have learned over the years just slapping _if_rt() or if_not_rt() variants of things around is most of the time papering over the underlying problem of badly defined protection scopes. Let's not proliferate that. As I said in the above IRC conversation: I fundamentally hate this preempt_disable_rt() muck Thanks, tglx