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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id DB17CC678D4 for ; Thu, 2 Mar 2023 19:18:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 70EEF6B0071; Thu, 2 Mar 2023 14:18:24 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 6BF646B0073; Thu, 2 Mar 2023 14:18:24 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5871B6B0074; Thu, 2 Mar 2023 14:18:24 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 47C1F6B0071 for ; Thu, 2 Mar 2023 14:18:24 -0500 (EST) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 1C6191C67E0 for ; Thu, 2 Mar 2023 19:18:24 +0000 (UTC) X-FDA: 80524919328.04.2F5DD3E Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf03.hostedemail.com (Postfix) with ESMTP id DAB2B2000A for ; Thu, 2 Mar 2023 19:18:21 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=gadJn4w+; spf=pass (imf03.hostedemail.com: domain of mtosatti@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=mtosatti@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1677784702; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=NE0DZXa/zvRZJ6eaL4Afj3nXFIuAOckIe9J5zIxESJk=; b=xXJrLN0qsfTn/37qUNZSTmAz1VxK4f+pShEo8cTFj0q8mDEsfQtSvAn2rPmnYItSAEnvxu axncfh9FhkRdOSlaPsYCJlsh/OFnYnK9soiRlfUOwleQoXxSbhxaiXRk+2Ed4AVmwiVAZw /1mBs43eGGunmsc1hF/U0aBQBlH8sTk= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=gadJn4w+; spf=pass (imf03.hostedemail.com: domain of mtosatti@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=mtosatti@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1677784702; a=rsa-sha256; cv=none; b=GmOsuYhC5X4bpN5028Yli1KvtUc8H64E0QAaWJcSlxtg7Q7142CE9mIGX65X1Jk/ZUErW9 g8IxCztkm8JiYKDH7AXujWonbUMIfnBRigp8TNjuUDhZ80p1ypGcKzyhc6IA5ynle+0BEC iYA2Qzg0j4hXvb1S0dhfVh/4/CbpSnc= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1677784701; 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=NE0DZXa/zvRZJ6eaL4Afj3nXFIuAOckIe9J5zIxESJk=; b=gadJn4w+FYYHK98ArPSS9omP4mILEab58HETNPrwGEQXegxmOqCCHcTCtcFHibGyTXzIUe stZaznZDYAxEbf9fX+9nv64pcSCvx7xy1Vlm4cP1xxDbmzK+PhT/QNDxFtWG2tY1ZIOan0 Z1FB9WpEPgshe4QOH1V27fNGkmbbAoE= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-561-_vBAMve-Phi6lQUXRGFG-A-1; Thu, 02 Mar 2023 14:18:19 -0500 X-MC-Unique: _vBAMve-Phi6lQUXRGFG-A-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 39D93811E9C; Thu, 2 Mar 2023 19:18:19 +0000 (UTC) Received: from tpad.localdomain (ovpn-112-2.gru2.redhat.com [10.97.112.2]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E7FF9440D8; Thu, 2 Mar 2023 19:18:18 +0000 (UTC) Received: by tpad.localdomain (Postfix, from userid 1000) id B93BA403F7F24; Thu, 2 Mar 2023 16:11:32 -0300 (-03) Date: Thu, 2 Mar 2023 16:11:32 -0300 From: Marcelo Tosatti To: Peter Xu Cc: David Hildenbrand , Christoph Lameter , Aaron Tomlin , Frederic Weisbecker , Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v2 08/11] mm/vmstat: switch counter modification to cmpxchg Message-ID: References: <20230209150150.380060673@redhat.com> <20230209153204.846239718@redhat.com> <3331790c-95a1-6ab9-2667-86aae3d28d7d@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Scanned-By: MIMEDefang 3.1 on 10.11.54.5 X-Stat-Signature: wj9swqzaoscasroz3jd4pares9hxj8f4 X-Rspam-User: X-Rspamd-Queue-Id: DAB2B2000A X-Rspamd-Server: rspam06 X-HE-Tag: 1677784701-777979 X-HE-Meta: U2FsdGVkX1+P6ub2fSN4Qktbyo8xoKeyyMcxnWDCpOWxrDBhLAwBSDrAqejDiV+ZGIuqA4S3l/dHg0JDqVIRkKZ41ZMOaK6yJ67AeUlPTshlwn3+1F98FHY/hz83fLOxFN4bNNopE6YQHMVYPG1HSTSkHSYj6vb4LehuPFePhx6SGz8YkkN2dz52DmertQG4sOL4n7Qbn4cURIt+QOdOMMbn1NM7t45iiZwibqVdqpemTu9iiiI8LT2hkkMUSns6ykY0CbZkPi60pYpcZXW1H6qHmsRCFPjXgQcesxaL1+9yRvgHJrOfPPVRtw+9wrYWo0VrkY/SBLu9feJWCLpVsG2i5xXHS26VX377Gh4RTKlDdRdrjCwejahz1V0Rh7yUwZaGM145gl7A0oY7GCAY5ygQ5L0zEX+zcusI/u6VYT9d452XMwewET3RqTgZmqJp2vbSq9e2wlz6xgyET6ztNhtx6pUuHueSSNMsshcarjg6rxS51iDIIbX4bmKZ+0HNwjhjox4tAkKmL7ItM3tFX91uwNlN6k9+bBoAQddWBW+bYTkafQ8mHyG2bveb5V67lSVdd8NCwAyAmWyluS1qEvjnx2AqqlQO6eVs1wGeIr4+QxM/OTRjDUeId3XCvG2vwtpK3dgzGCAQ30uoiuqn2R7dvFgqY7pEkcv53S1GjoQDZfKbSF+SaS8dS8HHmq0GcUb4Fl46+N/1nINIFe9zlS+Uo1uvstT8DobiuxS4mY9GVVOuBJx7DNybW/XOZyHaNo8j1Y71XXYJJ5aBZgF21NsvQWvJHtza1tschPntfpgD+JkEdgotx7Pa8uNr7BOWlRRmz6cjUgvp3StknMfdsSXgkF6Dd8y+Pp2VvGsPWYevQmq3uCmrVplOG55Z6vEyM9w91d5HisEO4RSswraWV0K46TkM35ruMm4PVRoJrL8Mv1kEcrtUrgUOqcWRD/8CCOZfWf0D6kqoi4wt+Ch UzyEGGjz a6iedSwHNIGOk6vy56dAxo2cX/wVwUqQhlMOhw9/MJW18oiLm/3ifLXcJZEujNlWvofPZQgyX4xG+55vKJEnOwMBaOoV0grj/IuA6jpYTbuXyhHvDexlFu2KO/IjYk4tQbrUdv0k3hWUeAsMb26M1YFRXbQ6h+o8AtAgnYvoyGzbkTxY= 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: On Thu, Mar 02, 2023 at 11:20:49AM -0500, Peter Xu wrote: > On Thu, Mar 02, 2023 at 11:47:35AM -0300, Marcelo Tosatti wrote: > > So it will be: > > > > #ifdef CONFIG_HAVE_CMPXCHG_LOCAL > > mod_zone_page_state > > inc_zone_page_state > > dec_zone_page_state > > mod_node_page_state > > inc_node_page_state > > dec_node_page_state > > __mod_zone_page_state (new function, calls mod_zone_page_state). > > __mod_node_page_state (new function, calls mod_node_page_state). > > __inc_zone_page_state > > __inc_node_page_state > > __dec_zone_page_state > > __dec_node_page_state > > #else > > __mod_zone_page_state (old, shared function for both CONFIG_HAVE_CMPXCHG_LOCAL and not) > > __mod_node_page_state > > __inc_zone_page_state > > __inc_node_page_state > > __dec_zone_page_state > > __dec_node_page_state > > mod_zone_page_state > > inc_zone_page_state > > dec_zone_page_state > > mod_node_page_state > > inc_node_page_state > > dec_node_page_state > > #endif > > > > Any suggestion on how to split this into multiple patchsets for easier > > reviewing? (can't think of anything obvious). > > I figured this out before saw this, but it did take me some time to read > carefully into the code base.. maybe it'll be a good idea to mention > something like above in the commit message to ease future reviewers (and > more likelyhood to attract the experts to start chim in)? > > One fundamental (but maybe another naive.. :) question on this code piece > (so not directly related to the changeset but maybe it is still..): > > AFAICT CONFIG_HAVE_CMPXCHG_LOCAL only means we can do cmpxchg() without > locking memory bus, CONFIG_HAVE_CMPXCHG_LOCAL means cmpxchg_local is implemented (that is cmpxchg which is atomic with respect to local CPU). LOCK cmpxchg is necessary for cmpxchg to be atomic on SMP. > however when !CONFIG_HAVE_CMPXCHG_LOCAL here we're not > using non-local version but using preempt_disable_nested() to make sure the > read is atomic. Does it really worth it? What happens if we use cmpxchg() > unconditionally, but just use local (e.g. no "LOCK" prefix) version when > CONFIG_HAVE_CMPXCHG_LOCAL? Can't use local version of cmpxchg because the vmstat counters are supposed to be accessed from different CPUs simultaneously (this is the objective of the patchset): CPU-0 CPU-1 vmstat_shepherd mod_zone_page_state xchg location LOCK cmpxchg location xchg locks memory bus implicitly. Is this what you are thinking about or did i misunderstood what you mean?