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 66A5EC77B76 for ; Tue, 18 Apr 2023 21:49:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F1409900002; Tue, 18 Apr 2023 17:49:13 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E9D208E0001; Tue, 18 Apr 2023 17:49:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D16F5900002; Tue, 18 Apr 2023 17:49:13 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id BCFE28E0001 for ; Tue, 18 Apr 2023 17:49:13 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 8C87A4040A for ; Tue, 18 Apr 2023 21:49:13 +0000 (UTC) X-FDA: 80695852986.13.C0357FC Received: from mail-yw1-f181.google.com (mail-yw1-f181.google.com [209.85.128.181]) by imf30.hostedemail.com (Postfix) with ESMTP id D211C8000D for ; Tue, 18 Apr 2023 21:49:10 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=lLRKUZZZ; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf30.hostedemail.com: domain of surenb@google.com designates 209.85.128.181 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=1681854550; 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=Y0caMjMTNLRVb5B71eXz6SyruGYjt/PXLO1p8S+PAzs=; b=s27i5ymT2+Re2zjQ/NVLJkEsudkabJp+wgUA+xp1DpCR3MuebpopbTqGfOHd9Hf2hLkZsP 6dORCUA+tSdDi0jRUeHKtgen1h8PSpteVKAke5LdLyIqdxtihPBRYVBssSHmJV4t9cH8C3 TpKLd6rCLXjifjVYvn82bL787G7Boc8= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=lLRKUZZZ; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf30.hostedemail.com: domain of surenb@google.com designates 209.85.128.181 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1681854550; a=rsa-sha256; cv=none; b=gcEFPnABWEBh+Ke+I7WBtwfWG+pZqm3f+cCAVzrtU2zv6aQJ2JmOWvp5kSjlZKTxj/Y+3A Q1a6nIfvTnPprgEwPAfrqABYmPCOE38Geb9BA7o6lRf6Xd3KuoM03MAzqS4eh7WsdEJBVD fqW14+ib39hZZzhVDIe1H5hspaCnsDM= Received: by mail-yw1-f181.google.com with SMTP id 00721157ae682-54c12009c30so583476907b3.9 for ; Tue, 18 Apr 2023 14:49:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1681854550; x=1684446550; 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=Y0caMjMTNLRVb5B71eXz6SyruGYjt/PXLO1p8S+PAzs=; b=lLRKUZZZhn34mOqNMLyVuFoJZ6lFIAcY+3oo1CyOrqxeXEcmFwKLCG1F4PljZO+SLH V7YeO3yghhuvAeYqGOBVUuRo6UGaFVC9XmCBJVbUNKtN5YOodZDrvhrsLZVo9Wg2iAqF NXzYskI9jVVCWy4Xo6b0JMwvMSGmRsVJn1QPcyG+utXSpiYBdFv2W8YOzGFFzeh/Ve1O w/mWhL8LqpFPkO13297rcnoB6sl2saU7Jb8kFDx+c1e3OeFNhZV6L5iA3H/tdMMsovJc OsvQBa9joGiBsCAXzpByOrJtC7suudMpbmyLnP7dOa++H3xh7UY+LI1tU7/3vfmfmgsJ cyeA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681854550; x=1684446550; 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=Y0caMjMTNLRVb5B71eXz6SyruGYjt/PXLO1p8S+PAzs=; b=fAAaEIYZQzsKdPanyAMH4PCQmoBd5FzlceXJPSp7YviQXK6goXrMaTGCLZivGC5/qz TYFW3GW+z6Y/sNkWGf5WeC0iLEugz4vbACrBGJoU0uF+jU8GVhJQ9dLA6NDwkRI/9qPt 2H0kEpdB8M4uFHJ4n24tl8r3LHD6BZ96OL19us+qEjbUHWkNivnp+1dez3W+BCU0oAul uPU99qYwBZH5jEqYa8cTTX182KLGxucD+fwk3AooEd4PMyvBWJJY60Y86AjfQhqsG+Ta naniGuzBJCj0hP0oT9b80nMng1pnDIa37jPfCZO0ZHbZQDpNzsVhp/o54Ksw3pywC33U rCyA== X-Gm-Message-State: AAQBX9d6a7ZcC/pE7k69aS/lgSUIn0fWg74Iqb238rngvrmGdT+gbhFn 0CRvNLaZrnhGrCjY/8CUXuO3dp0Q7gjtkB5zkyuBzg== X-Google-Smtp-Source: AKy350Ybh3tGHfTHDnFslZANDLtloOBdFnmuSlARp0u119p4ENg3icAyy36YW5AFQ5BgcwSthM4EIFaHI/0IhQJylfI= X-Received: by 2002:a81:650a:0:b0:533:8eac:77c8 with SMTP id z10-20020a81650a000000b005338eac77c8mr1512446ywb.2.1681854549772; Tue, 18 Apr 2023 14:49:09 -0700 (PDT) MIME-Version: 1.0 References: <20230415000818.1955007-1-surenb@google.com> In-Reply-To: From: Suren Baghdasaryan Date: Tue, 18 Apr 2023 14:48:58 -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: D211C8000D X-Rspamd-Server: rspam09 X-Rspam-User: X-Stat-Signature: q9id9t4ai9f5gorpx9jxe7ayj7izqp7h X-HE-Tag: 1681854550-405399 X-HE-Meta: U2FsdGVkX1/PecpPurbqfAPYa6ABL+lfndi5LxnOnYoV06YCqBD057ygQvFhVDEKmUuZ0uNJ0Gdq/rrL3WXQoSUjl7fY572wkdiuoKsba42ajCYJMRLDSzci5qJYMSMU0OnaBJTeEzNpY52dZSlbcrxzzsxDaVYLHIvuU5uc/K86SveUDIkSTcc6+gyMsKXk1z4tZ77KYwM5/k+EBZpKNaHJI60R77opy32LxFbNeGP18+2kqFCAKduLfs2bS6/Amv0SnxDEer9yd0jqPp5EHyd8dOc8gCu2IF3B2anoHy4I6V7uWg1nicF3ZOba88mUSyaxixbageks9XNgJBLuwbCIu9rCFQAmFo2ap/P3OpwCADviMAp7OlJY9jlGf2dA0qiHWs41naF60iC+eirVEtfYVbXJNbmAcKkGFSKU5xt91Ku18JdmAKgUDy3EdLEK3GQtAeamnP1jAxr9YPmufxyMvfJQyQIp0jcE2Jy2pZm/Je1ZywVrVdM8yDxxbHxV+K8L9A1JK1idVZcYsbcWj6YWkQMbo3TeS1gj3SiXUeWEZb+87aBv0QrgrCp/ke2rvyn5m3yDb0f7eKbn8LMasKPOfruIXNamiRfDADgAvP8ktGqnoyMLevOgCn57IKEFAot8Uqv0InrnMKksR8EuOtyW8edSoU1c1LmAG+2X/sutP6NcHKhAp0a9ahO2zzk8zVer6FnJ8CUFZX3ug5TyvniiDhxKs5kW8264Q9oCPB+jLofH1Oa7JuubCQDcXwZSxQX9pLbfUX4jSIgud4SQy7mO5s/Q5skmFt7r5l177j9jQHd6y8ziJEz00CnxA6Ncs/A6rL5KxuejCLSP3QwqAB5k3SliB3t3hAu2IwJgKw8UUIANVERrzhWtzWGHwyxmZ8IydIEj19msW2atCHN14/s3F9rYBW+ZUzo6yarFVAdyYs9wSFpI4NEXkq/hWzNipTZIZVBwugG3gvcaREF fzAX6yBT A+qKoYW0InedhBw5+9XwZOS+fU948Bf5nrS6y3rPa7sPyrs7wmHQEM5SALL+WbjLjD1s4X4tb13GPBLyOyvX2A2jNJMBtPEd8irx2wFMmtwiylNHeCpeD8rReoTXWO9uj8OajnSjvsAuJLAQkhUqHSmI+RfuOIlxjMHbX8A51M77oIGZ9p+Du680LiVqev/ZgW6Tappy8M9LnlbcwguRbTfW4ru4kQB8jcDMI/bOj/kpJDNPJ79IGUq5kbmo6e5vu8MvCITckDdGZJN5VeKDRjXI1C2JST3Xaz4YrnDju8IUoSnyNFliSBi77uACh6s/a4JZKbJ2PgN7Xmy+n5HSb7xHJ3+kKgTeHZh2MnlBvtaPNEZQmvIsFswU3bv0eqQulpsfyMo8iBx6g3+vrDsAtT8rj8+R0habD4ZEHB4ZuXXmxlrxKYMUPTvHDS/DQ7haQLqADJinxfZau57g= 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 10:17=E2=80=AFAM Suren Baghdasaryan wrote: > > On Tue, Apr 18, 2023 at 9:06=E2=80=AFAM Peter Xu wrot= e: > > > > 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 wrot= e: > > > > > > 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 bef= ore 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 o= nly be counted > > > > > > > > > - * once they're completed. > > > > > > > > > + * Do not account for incomplete faults (VM_FAULT_RET= RY). 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 PGFA= ULT counters. */ > > > > > > > > > + count_vm_event(PGFAULT); > > > > > > > > > + count_memcg_event_mm(mm, PGFAULT); > > > > > > > > > > > > > > > > Is there reason on why vm events accountings need to be exp= licitly > > > > > > > > different from perf events right below on handling ERROR? > > > > > > > > > > > > > > I think so. ERROR is quite different from RETRY. If we are,= for > > > > > > > example, handling a SIGSEGV (perhaps a GC language?) that sho= uld be > > > > > > > accounted. If we can't handle a page fault right now, and ne= ed to > > > > > > > retry within the kernel, that should not be accounted. > > > > > > > > > > > > IIUC, the question was about the differences in vm vs perf acco= unting > > > > > > for errors, not the difference between ERROR and RETRY cases. M= atthew, > > > > > > are you answering the right question or did I misunderstand you= r > > > > > > answer? > > > > > > > > > > Maybe I'm misunderstanding what you're proposing. I thought the > > > > > proposal was to make neither ERROR nor RETRY increment the counte= rs, > > > > > 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 talki= ng > > > about the same thing ;-) > > > > IMHO if there's no explicit reason to differenciate the events, we shou= ld > > 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 mprote= ct() > > use case, because they shouldn't rely on the counter to work but the SI= GBUS > > 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 = then > > 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)? I did some digging in the history and looks like the check for ERROR was added after this discussion: https://lore.kernel.org/all/20200624203412.GB64004@xz-x1/ and IIUC the reason was that previous code also skipped VM_FAULT_ERROR. Peter, is that correct? It seems this discussion is becoming longer than it should be. How about we keep the behavior of all counters as they are to avoid breaking any possible usecases and just fix the double-counting issue for RETRY cases? > > > > > -- > > Peter Xu > > > > -- > > To unsubscribe from this group and stop receiving emails from it, send = an email to kernel-team+unsubscribe@android.com. > >