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=-1.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no 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 52FE6C433DF for ; Fri, 26 Jun 2020 21:53:55 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 143C12089D for ; Fri, 26 Jun 2020 21:53:55 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="LnosIQ6Z" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 143C12089D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 8CE186B0003; Fri, 26 Jun 2020 17:53:54 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8579A6B0005; Fri, 26 Jun 2020 17:53:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 71EED6B0006; Fri, 26 Jun 2020 17:53:54 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0101.hostedemail.com [216.40.44.101]) by kanga.kvack.org (Postfix) with ESMTP id 56E7B6B0003 for ; Fri, 26 Jun 2020 17:53:54 -0400 (EDT) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id BDD29824556B for ; Fri, 26 Jun 2020 21:53:53 +0000 (UTC) X-FDA: 76972715946.21.sand43_3517eae26e58 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin21.hostedemail.com (Postfix) with ESMTP id 9BA27180442C3 for ; Fri, 26 Jun 2020 21:53:53 +0000 (UTC) X-HE-Tag: sand43_3517eae26e58 X-Filterd-Recvd-Size: 7463 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [205.139.110.61]) by imf26.hostedemail.com (Postfix) with ESMTP for ; Fri, 26 Jun 2020 21:53:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1593208432; 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=FV8Bsjn54kOoJliJlKrbcRPo+jc1JY+WRfUoqlGGQhU=; b=LnosIQ6ZSA/O6f2itGGEN5Ien24a0SHb4iSatUJB4Ct4MBY4QOtJzIHAWyMyragbpKojuz vGYMuIwxJijAOpipsFyd+o3DZyuHIs5WoZy7QW/tCp4oLGE6B+hFrI1gmhUyJjJcFtTu/G aVbnqajyBjEzT6Hn/Q/azRxw7+aldFM= Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-160-1RwiqXYMPQCJz53cwcMcEQ-1; Fri, 26 Jun 2020 17:53:50 -0400 X-MC-Unique: 1RwiqXYMPQCJz53cwcMcEQ-1 Received: by mail-qv1-f69.google.com with SMTP id q5so7280992qvp.23 for ; Fri, 26 Jun 2020 14:53:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=FV8Bsjn54kOoJliJlKrbcRPo+jc1JY+WRfUoqlGGQhU=; b=BXW15ytBNYc3VwtXu91gSv8txXpjFAhWWm3zv9tpm+kZziiQBtOZ5IrNXzbP8YACt+ Rm5er7nv6fN/Jfuv5yDhGRJZIFB8qj2UmY6tG7J+vO0pzsWSx1Dgl6mATyNNT0rZbXK6 8SEsUqZluIS2jAVqjjad3h4G4JNVq7oK4Fri0CQkxbNOpnSMVy3yu3ZPmt/rdfoHc6Eq lztLc+kRia02J166vaznYU9dUA3sqd0edB3omEONlECkx4FRRC8TPuM09TNYfh5BRcQe n6qvp9cBJPHRGdeHqxHC3l5tbhmLpLSzIH5XvsM5fRR8a8rat+s94cKgMSMbfb2nNnYA PYHg== X-Gm-Message-State: AOAM533hNANC7KQsgpDaiqUp8sPV/6r3XiSbbDcCCnjOdV95/chqbvY5 Nwhmpfn89GuYnyLOhwZPrQ6dCQ/zUkrnEs+XDkKbcGroH4N9jwlsZT1TtXBuSjSw/3x2XpMsV+T b8awRylOd3DI= X-Received: by 2002:a05:620a:2050:: with SMTP id d16mr5094858qka.215.1593208429869; Fri, 26 Jun 2020 14:53:49 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzkOQYx4uI8uZYLEy+pRDdcczh+I4pfBIpILOimX9az7o45fXQBXY+UDqXHB+0u+UMO7CpWOA== X-Received: by 2002:a05:620a:2050:: with SMTP id d16mr5094832qka.215.1593208429427; Fri, 26 Jun 2020 14:53:49 -0700 (PDT) Received: from xz-x1 ([2607:9880:19c0:32::2]) by smtp.gmail.com with ESMTPSA id g16sm11470330qko.5.2020.06.26.14.53.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 26 Jun 2020 14:53:48 -0700 (PDT) Date: Fri, 26 Jun 2020 17:53:46 -0400 From: Peter Xu To: Gerald Schaefer Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Andrew Morton , Andrea Arcangeli , Will Deacon , Michael Ellerman , Linus Torvalds Subject: Re: [PATCH 01/26] mm: Do page fault accounting in handle_mm_fault Message-ID: <20200626215346.GE175520@xz-x1> References: <20200619160538.8641-1-peterx@redhat.com> <20200619160538.8641-2-peterx@redhat.com> <20200624204903.097a5a58@thinkpad> <20200624203412.GB64004@xz-x1> <20200626215424.581d6077@thinkpad> MIME-Version: 1.0 In-Reply-To: <20200626215424.581d6077@thinkpad> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-Rspamd-Queue-Id: 9BA27180442C3 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam05 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 Fri, Jun 26, 2020 at 09:54:24PM +0200, Gerald Schaefer wrote: > On Wed, 24 Jun 2020 16:34:12 -0400 > Peter Xu wrote: > > > On Wed, Jun 24, 2020 at 08:49:03PM +0200, Gerald Schaefer wrote: > > > On Fri, 19 Jun 2020 12:05:13 -0400 > > > Peter Xu wrote: > > > > > > [...] > > > > > > > @@ -4393,6 +4425,38 @@ vm_fault_t handle_mm_fault(struct vm_area_struct *vma, unsigned long address, > > > > mem_cgroup_oom_synchronize(false); > > > > } > > > > > > > > + if (ret & VM_FAULT_RETRY) > > > > + return ret; > > > > > > I'm wondering if this also needs a check and exit for VM_FAULT_ERROR. > > > In arch code (s390 and all others I briefly checked), the accounting > > > was skipped for VM_FAULT_ERROR case. > > > > Yes. I didn't explicitly add the check because I thought it's still OK to count > > the error cases, especially after we've discussed about > > PERF_COUNT_SW_PAGE_FAULTS in v1. So far, the major reason (iiuc) to have > > PERF_COUNT_SW_PAGE_FAULTS still in per-arch handlers is to also cover these > > corner cases like VM_FAULT_ERROR. So to me it makes sense too to also count > > them in here. But I agree it changes the old counting on most archs. > > Having PERF_COUNT_SW_PAGE_FAULTS count everything including VM_FAULT_ERROR > is OK. Just major/minor accounting should be only about successes, IIRC from > v1 discussion. > > The "new rules" also say > > + * - faults that never even got here (because the address > + * wasn't valid). That includes arch_vma_access_permitted() > + * failing above. > > VM_FAULT_ERROR, and also the arch-specific VM_FAULT_BADxxx, qualify > as "address wasn't valid" I think, so they should not be counted as > major/minor. > > IIRC from v1, and we want to only count success as major/minor, maybe > the rule could also be made more clear about that, e.g. like > > + * - unsuccessful faults (because the address wasn't valid) > + * do not count. That includes arch_vma_access_permitted() > + * failing above. Sure. > > > > > Again, I don't have strong opinion either on this, just like the same to > > PERF_COUNT_SW_PAGE_FAULTS... But if no one disagree, I will change this to: > > > > if (ret & (VM_FAULT_RETRY | VM_FAULT_ERROR)) > > return ret; > > > > So we try our best to follow the past. > > Sounds good to me, and VM_FAULT_BADxxx should never show up here. > > > > > Btw, note that there will still be some even more special corner cases. E.g., > > for ARM64 it's also not accounted for some ARM64 specific fault errors > > (VM_FAULT_BADMAP, VM_FAULT_BADACCESS). So even if we don't count > > VM_FAULT_ERROR, we might still count these for ARM64. We can try to redefine > > VM_FAULT_ERROR in ARM64 to cover all the arch-specific errors, however that > > seems an overkill to me sololy for fault accountings, so hopefully I can ignore > > that difference. > > Hmm, arm64 already does not count the VM_FAULT_BADxxx, but also does not > call handle_mm_fault() for those, so no change with this patch. arm (and > also unicore32) do count those, but also not call handle_mm_fault(), so > there would be the change that they lose accounting, IIUC. Oh you are right... I just noticed that VM_FAULT_BADMAP and VM_FAULT_BADACCESS can never returned in handle_mm_fault() itself. > > I agree that this probably can be ignored. The code in arm64 also looks > more recent, so it's probably just a left-over in arm/unicore32 code. Anyway, glad to know that we've reached consensus so that we can accept these differences. Since this patch seems to be the only one that needs a new post so far, I'll repost this patch only by replying to itself with v2.1. Hopefully that can avoid some unecessary mail bombs. Thanks for the very detailed review! -- Peter Xu