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 08ABBC77B75 for ; Tue, 18 Apr 2023 17:17:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5DD3F8E0002; Tue, 18 Apr 2023 13:17:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 58DA78E0001; Tue, 18 Apr 2023 13:17:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 455AB8E0002; Tue, 18 Apr 2023 13:17:52 -0400 (EDT) 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 37E7D8E0001 for ; Tue, 18 Apr 2023 13:17:52 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id F34851C62E2 for ; Tue, 18 Apr 2023 17:17:51 +0000 (UTC) X-FDA: 80695169142.16.DC94228 Received: from mail-wr1-f47.google.com (mail-wr1-f47.google.com [209.85.221.47]) by imf03.hostedemail.com (Postfix) with ESMTP id 19BA220023 for ; Tue, 18 Apr 2023 17:17:48 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b="ajCU/xzh"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf03.hostedemail.com: domain of surenb@google.com designates 209.85.221.47 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1681838269; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=oi2l1IFP5mrz8GQJ7hLf0B4pdQ6HkkMS+3cH5HJ4cAI=; b=mBjSw5Uk0FriEhGeBc9LHdM0aI1c8Y0LnDkp9Z8Wr7hiQLwfrmpFamwdCbEIAS8MAt1FGN Z0wr+5PwONfeqa8erWMZCNkbsigQENGGnLmivkF32vIeRrvy715I2XNVbZ9Csg1M2lheJO uti98RXT+W0BUsmmzk8bPFYxRhVC+/U= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b="ajCU/xzh"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf03.hostedemail.com: domain of surenb@google.com designates 209.85.221.47 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1681838269; a=rsa-sha256; cv=none; b=geZAHCTWh/WHMI8DHmKAoli4WxnMwU5XtPzAd4DqiYIsEp+sln0TQNcc/hFE/WrIVwQTtM xRe9VAt37NHpxmQT6Dm+U5Lvb+io12H7x3c/0xm/OM0WHhomzOTePLHE0OXazxrFgLlLYx 8HzqloizDEYSl0jsRRFTC3E2oUdwUU4= Received: by mail-wr1-f47.google.com with SMTP id ffacd0b85a97d-2f4214b430aso1672975f8f.0 for ; Tue, 18 Apr 2023 10:17:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1681838267; x=1684430267; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=oi2l1IFP5mrz8GQJ7hLf0B4pdQ6HkkMS+3cH5HJ4cAI=; b=ajCU/xzhoS4z16YqqHR/ZNLIcpmqGdDt1Pe16Ah9zpCuOEkjMd/25w7dCfjCg3+c3Z bh8dNPADdxDNGjqSyO0e0AOclZ/QY/birlElvua4pVKaPNxjBWO/JgFeJF8FYtYEowTg ly3DYfXXF6kZUZ2pmBgl4ikTYkzhzkFS82+MUloioFOF6+aLh1ALx/mq1ySMVXSf2jXO 6ywfH9fthxppwF+JOcXFci96w74/cKGHRFkaoGXGE1b+d2CnwEPmPpWouDGaexdXmoKa 7XKETqjqI6Tb8dHZE3ANo+GyCD7lVpSDvuxp6ZyO+N0ISNgt1QB/ddq9e6zeK66tCVoy 7bkA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681838267; x=1684430267; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=oi2l1IFP5mrz8GQJ7hLf0B4pdQ6HkkMS+3cH5HJ4cAI=; b=P/THoUrHzpcHCyL2iqVXln1seOWHGh2vm8/lo9prNFiE53B5kO7pfcvOLdMU25kZz9 mmcvfx6MB3lgsLp3p/3V+K5PgGfiRMj3ZhWxTEbvk2Q96WoWkRlANl8Er30JsanNcagy z8qf021/fA9wPRCtBVPlMsU35SYyLfhcUyjAocWpBHldyh4/sxkkp6ocbK2GJEfMsFtW GEr7x0Pfsi/iXcp4upR5hejuZKQ1Kt6f0eDRFI0OPYSPzMQqvDALetcD72GsNms+iBgi Xqj4Q2qNP0RZFPPH4bww5IiOzeRkPmzAjh95sxsHS4grsTCTaFO0onXAReRFSvHV4045 1ZVA== X-Gm-Message-State: AAQBX9fYCXvXrvrkidjhXRyX8Lh9MFHkCvlgfZ4Cq0TP4SYIvazJt7AL HLYMt8/PYOmNR+EU2Vu7dZ0cs8XyZ1t1pHp0AHdl/Q== X-Google-Smtp-Source: AKy350bpC+2179a4iS4QE7SJGekbc1AdMviwe2WIOYBXhv0y8rJVWkdMfujaLS44ciSAoFq0VDfFKsuoIzyfXk1FO8w= X-Received: by 2002:a5d:4ecd:0:b0:2f5:8e8b:572c with SMTP id s13-20020a5d4ecd000000b002f58e8b572cmr2457342wrv.49.1681838267464; Tue, 18 Apr 2023 10:17:47 -0700 (PDT) MIME-Version: 1.0 References: <20230415000818.1955007-1-surenb@google.com> In-Reply-To: From: Suren Baghdasaryan Date: Tue, 18 Apr 2023 10:17:35 -0700 Message-ID: Subject: Re: [PATCH v2 1/1] mm: do not increment pgfault stats when page fault handler retries To: Peter Xu Cc: Matthew Wilcox , akpm@linux-foundation.org, hannes@cmpxchg.org, mhocko@suse.com, josef@toxicpanda.com, jack@suse.cz, ldufour@linux.ibm.com, laurent.dufour@fr.ibm.com, michel@lespinasse.org, liam.howlett@oracle.com, jglisse@google.com, vbabka@suse.cz, minchan@google.com, dave@stgolabs.net, punit.agrawal@bytedance.com, lstoakes@gmail.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@android.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 19BA220023 X-Rspamd-Server: rspam09 X-Rspam-User: X-Stat-Signature: okem66ybzi5tzqmjagmdpxwqypn5bnej X-HE-Tag: 1681838268-180611 X-HE-Meta: U2FsdGVkX1/t8eoTYsy6LoejEm9p3UUgl4bjp9jQOhz2VWoKAZ3tHAdoidePrq3JF7si2iykH25hTg1lQGXYchVBgjiIFMg7IRSZxzh15GlYG3ycp3JB6nj+ViOIHCvOe6jTVYYNsDHuuHi0qlppQpb92m+EngJdTQWY4NXbhh6hU/VpGMEKSI6l5wbZaP12wxmFZ1YNzCBIxJq3t4u7GDVV4iWiw+13NfvmmXDY2xqAlddhKtoZHN/UqLoYjedML4wSmCozc49HvngXkmsyLhWcFxKGe6VnGDjL8A9MIV2DQbT4MRRlMA1t98uVMibBGyDevrWredeEsaJ2UqrvvqxXvZKTe+zhNLDG0dPtUyq2pmzZvIwnR7OU73UdlDJU4O5O/tcd0mvDZ0qMdpNCqssLdFjWR4gPUN73bBNmoeEdqjg2fOoRyw9oVoPwCYUKTl04FuKjggjftRdAOvRKVu06Zk9LBKRTX6j46eqI7USbdBLL6sddM89Q5WEoQcBgyBMihh1SlpvK6CffoApzP6OdaGr9dkHQV3Q8UapgOt9ooltGIOSmT+LDpVhVV4sqIR9+zvH7e2Wxvn066VP61cHDto1IW+a27JcD9g50LtSpS7XDiUwUBR1kL1DtE1WgIU3AlKDixQokAOF6hBxuFpvs+MokRKYe3m6N6GT7bn2LhUiXNfpgWBP8soBXNR0SWhD3difFj9pDWd3bfmc5Lq2d3ZXrBSvrrmcKLKprWzwIy6+3n4kem4t6DdGtE8Tu9DqUKCh1O5RIO4yzclcoVhfuwtMeoiCM943Fbj9rhU6Mo9lkMMrbIM+q7kp6jfYokJvELEvxY9Bu36wsr65/vL51cifdoqABuI2fx+OWFgNGNAYyosBqInoQu2WCWCGnXF2kNdxMmJ65mYnfGClrGjOBN5BQMhNR+MHWZNhy8ayaeDvQaFFxIh0l7xQqY2EhbaHlmSz1Ed1rIXVHuOh JwIF9toq HHbDBkORe9me7ICnNHk1qWCB15/OSyoIoP8e5k11NuCsbE6yWDIPtb7GsyWVuBjaK5XuBqXw/xpC//cITJGf1+LkpVILhCYUR0DLBBKuy/7EtZm9vktdEmYXaYGv2IKI2g9dNRxlXKOIZ2do2VscLnnK0ENlFvRs3xwlXTGKocJlG4XzsSLv1FZBBuxhAZtgGIIkHnp9XHpzvmiqtmCcOHhLRtw0j3CrzIDOqS0TnoXstvWUr1U2GnMJ0uNLiJfeDMNge1dKDujzBiMm+UHWdNSgIHxj5p2X+mTxa5sjirablxfuzB49Hsh846GE97r8E20KARylHX8ouGrkq7TmlYWoAHMuOMPRgqaGgfqTiTUPz/IQc0rDQSAYG1Q== 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 Tue, Apr 18, 2023 at 9:06=E2=80=AFAM Peter Xu wrote: > > On Tue, Apr 18, 2023 at 04:08:58PM +0100, Matthew Wilcox wrote: > > On Tue, Apr 18, 2023 at 07:54:01AM -0700, Suren Baghdasaryan wrote: > > > On Tue, Apr 18, 2023 at 7:25=E2=80=AFAM Matthew Wilcox wrote: > > > > > > > > On Mon, Apr 17, 2023 at 04:17:45PM -0700, Suren Baghdasaryan wrote: > > > > > On Mon, Apr 17, 2023 at 3:52=E2=80=AFPM Matthew Wilcox wrote: > > > > > > > > > > > > On Mon, Apr 17, 2023 at 03:40:33PM -0400, Peter Xu wrote: > > > > > > > > /* > > > > > > > > - * We don't do accounting for some specific faults: > > > > > > > > - * > > > > > > > > - * - Unsuccessful faults (e.g. when the address wasn't = valid). That > > > > > > > > - * includes arch_vma_access_permitted() failing befor= e reaching here. > > > > > > > > - * So this is not a "this many hardware page faults" = counter. We > > > > > > > > - * should use the hw profiling for that. > > > > > > > > - * > > > > > > > > - * - Incomplete faults (VM_FAULT_RETRY). They will onl= y be counted > > > > > > > > - * once they're completed. > > > > > > > > + * Do not account for incomplete faults (VM_FAULT_RETRY= ). They will be > > > > > > > > + * counted upon completion. > > > > > > > > */ > > > > > > > > - if (ret & (VM_FAULT_ERROR | VM_FAULT_RETRY)) > > > > > > > > + if (ret & VM_FAULT_RETRY) > > > > > > > > + return; > > > > > > > > + > > > > > > > > + /* Register both successful and failed faults in PGFAUL= T counters. */ > > > > > > > > + count_vm_event(PGFAULT); > > > > > > > > + count_memcg_event_mm(mm, PGFAULT); > > > > > > > > > > > > > > Is there reason on why vm events accountings need to be expli= citly > > > > > > > different from perf events right below on handling ERROR? > > > > > > > > > > > > I think so. ERROR is quite different from RETRY. If we are, f= or > > > > > > example, handling a SIGSEGV (perhaps a GC language?) that shoul= d be > > > > > > accounted. If we can't handle a page fault right now, and need= to > > > > > > retry within the kernel, that should not be accounted. > > > > > > > > > > IIUC, the question was about the differences in vm vs perf accoun= ting > > > > > for errors, not the difference between ERROR and RETRY cases. Mat= thew, > > > > > are you answering the right question or did I misunderstand your > > > > > answer? > > > > > > > > Maybe I'm misunderstanding what you're proposing. I thought the > > > > proposal was to make neither ERROR nor RETRY increment the counters= , > > > > but if the proposal is to make ERROR increment the perf counters > > > > instead, then that's cool with me. > > > > > > Oh, I think now I understand your answer. You were not highlighting > > > the difference between the who but objecting to the proposal of not > > > counting both ERROR and RETRY. Am I on the same page now? > > > > I think so. Let's see your patch and then we can be sure we're talking > > about the same thing ;-) > > IMHO if there's no explicit reason to differenciate the events, we should > always account them the same way for vm,perf,... either with ERROR > accounted or not. > > I am not sure whether accounting ERROR faults would matter for a mprotect= () > use case, because they shouldn't rely on the counter to work but the SIGB= US > itself to be generated on page accesses with the sighandler doing work. For that example with GC, these are valid page faults IIUC and current PGFAULT counters do register them. Do we want to change that and potentially break assumptions about these counters? > > One trivial benefit of keep accounting ERROR is we only need to modify vm > account ABI so both RETRY & ERROR will be adjusted to match perf,task > counters. OTOH we can also change all to take ERROR into account, but th= en > we're modifying ABI for all counters. So, not accounting them in both vm and perf would be problematic for that GC example and similar cases. Are we left with only two viable options?: 1. skip RETRY for vm and skip ERROR for both vm and perf (this patch) 2. skip RETRY for both vm and perf, account ERROR for both #2 would go against the comment in mm_account_fault() saying that we don't account for unsuccessful faults. I guess there must have been some reason we were not accounting for them (such as access to a faulty address is neither major nor minor fault)? > > -- > Peter Xu > > -- > To unsubscribe from this group and stop receiving emails from it, send an= email to kernel-team+unsubscribe@android.com. >