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 956A4C7618E for ; Tue, 14 Mar 2023 12:40:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 130CE6B0072; Tue, 14 Mar 2023 08:40:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0E1FA8E0002; Tue, 14 Mar 2023 08:40:20 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EEBA88E0001; Tue, 14 Mar 2023 08:40:19 -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 DCCAF6B0072 for ; Tue, 14 Mar 2023 08:40:19 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id B7A181A0C1E for ; Tue, 14 Mar 2023 12:40:19 +0000 (UTC) X-FDA: 80567461758.24.9D1D604 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf07.hostedemail.com (Postfix) with ESMTP id 76A4E40006 for ; Tue, 14 Mar 2023 12:40:17 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=i1kilWR4; spf=none (imf07.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1678797618; 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=Lbfu5hsFdjJNQ2BcOkd2Ni80XTMP3EUdmJmeyIEDzTQ=; b=kSiMGtxGtgoOKCEiBvRw9Uh+wOlp80EVgACfDDPH5tGTala9N1PqGbkVlzbMXlG+W4Pie9 iWb9mYbe7v0RpIAQK+2pwdBCS6WzSBmGtMIObnG88x/uOjdBngbtGCB8UY506v+7y5LFCf 32ElJLSvmGJ2T48dOZTdqSqFj7Vi0t4= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=i1kilWR4; spf=none (imf07.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1678797618; a=rsa-sha256; cv=none; b=KWOVohdRiUA7L/kqBj9iJXVwOiclRvWGqTAkZXc7EZiFJLj/MEsZFgVWH8koNr4o9ExjA8 71O9Q0MjCnnyuLDvWOs3Vsw6rrvLWyZTKBdE61fJhFNpuZbG2/oa+ZAKPGVcgMuiLozqHb +HAhJeKv7w5P1tlPWbRR+SWxEaf3Qq0= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=Lbfu5hsFdjJNQ2BcOkd2Ni80XTMP3EUdmJmeyIEDzTQ=; b=i1kilWR4cBokymritd2FC6xZ3h E8+hYXkd2phG11jdPl6Q5ALHtqIsEQE8cmyXpIL6PKPSjBq/+UGigifKi97Ix6CnUQoOGUm9Dk6Su RvNARmReK6/Oo1SMeuUlGea35+uvMUWD/Qx0o8twYspe5xA/VYIhwszGBbJvEZyjAYG2x8HynEyzz T+vR8cGtbSZTWSVVHSV8M80kHUvvfkb6uOcyRrvSszoYQ/JOnEZNpo6O8Jt9XIT49qczjn3HWHEHF PL0Jsil5vCjVj7rNnR94vyeULfDiCstL6AKf/uBCMKhA0z7bNMA7DZRwg+C/eYmY4r27wfIdkO3wX j/3Yx/QQ==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1pc3wo-00Cu7b-HS; Tue, 14 Mar 2023 12:40:10 +0000 Date: Tue, 14 Mar 2023 12:40:10 +0000 From: Matthew Wilcox To: Haifeng Xu Cc: David Hildenbrand , akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm: remove redundant check in handle_mm_fault Message-ID: References: <20230306024959.131468-1-haifeng.xu@shopee.com> <354360d5-dce6-a11c-ee61-d41e615bfa05@shopee.com> <6df72872-2829-47ab-552c-7ef8a6470e6f@shopee.com> <562e9cc3-d0aa-23e9-bd19-266b5aef2ae7@redhat.com> <70abf872-99d1-6ff4-3332-d86d320abff2@redhat.com> <1b21ee01-116d-d432-7308-8515510c89f2@shopee.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1b21ee01-116d-d432-7308-8515510c89f2@shopee.com> X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 76A4E40006 X-Stat-Signature: 7pbt6ckgdnw6j755t5ybuhzf3jqbkjz7 X-Rspam-User: X-HE-Tag: 1678797617-591996 X-HE-Meta: U2FsdGVkX1/RnMiXXCIr1TtJzt4gU2rRiijbkVqVKBiBIP93OwuJg4fP//2J9nq8EoF+cQLQ8WBYWLfYBvcHFCjVnF9R2UWxU5DucN10bGck0M+BO2U2hrgIz2GCPRDGkYUuXfRa79f9WX0YtYJqqDkL08CVTEUYuwSXPDXgRfwQuWpq1Rlr33KZ48VQ7ECLPth2oya1srpy+W7sZP5RXBL7EvufeDHvSJR1aHpmMXlUhdy1R3cyn6V7pe2kVOodZ6G+f1HKaTz9tgy9liUPAeSvRogig2gQ9ccus1d/uPEoNOSbCbv6Lt63hWQlC4hjYe3b/hHyAPyyxMQvoF/Fg6trPCUJlRwl2JqB9RMsjLj1duuUGV7j9UwBY8aDWYGZgDuCr6kliNGszjQX+ot7UNM1cc5bUfkFAsb71Unp551MMnmgrTtEy3fGm2fM4ZC+wrR8hxEcBTaG5vODR+KiF+emq804cSTZzTqCfrN3SCDOUiycbUmB9BgB0cXfLxN9TLLf0Q4wzH8SVdiafcgJmDqkvUINnAsHlkTv8yb4m2IRmTFhvY9/E7/E72tgaVdsqJLR/j/7FtFFmZSfTV0AvFQkN101puyYS3yTQCExPxc7/a0aNB7VVGtAFgilxsLb9jW+Qm/7UXOCQzO9pjglaqO4m8umtutt3cbSRassovVuj6OgKl2YKNZRoTnowEW56eO802LXv/OIpOWhKfxhPCqms5mjp/RpCA44x8iR9iwLGye7bCtcpkLBZqb+WYx55Ck04L1sMrypkxWcfQ7GYrSnDNd0mgNfHn1svBhDf6J1RHA/g/PwS4BuK/4fHZsTVFPy/2QxoIBeQwZia411HsLziN5+TKyIg/aL8Bn8FmLWGK8x9X+q3Wx+cWjl16i8kw5ntBxQcmsbqrqqH/ptOQEu3uU5WzTsS97vlZyXvCbDtIJEVED7FIA3qoMvhFCDI5Vo0wJG5xFat5hHhON lFyY7Lb1 LGVXSBSGzzB622KtGzoJFX91CyDbJesQhEQZllMcOfKseG0AS7+6Jbwrgz9S3BQ06deNIkBzSmei5hKzMmqQejwJMEo1LH7Bel7wgob06RVb3slfE/FwekQ9kPRn/sbH0G6QluT7VzEEFm+eToB38DtwePJrkk4Zhx56kFCtiNuJ5B3P83KS9C0gHzeWuBq5zGpwhNpG2bUwBt7I= 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, Mar 14, 2023 at 06:29:24PM +0800, Haifeng Xu wrote: > > > On 2023/3/14 17:09, David Hildenbrand wrote: > > On 14.03.23 09:05, Haifeng Xu wrote: > >> > >> > >> On 2023/3/8 17:13, David Hildenbrand wrote: > >>> On 08.03.23 10:03, Haifeng Xu wrote: > >>>> > >>>> > >>>> On 2023/3/7 10:48, Matthew Wilcox wrote: > >>>>> On Tue, Mar 07, 2023 at 10:36:55AM +0800, Haifeng Xu wrote: > >>>>>> On 2023/3/6 21:49, David Hildenbrand wrote: > >>>>>>> On 06.03.23 03:49, Haifeng Xu wrote: > >>>>>>>> mem_cgroup_oom_synchronize() has checked whether current memcg_in_oom is > >>>>>>>> set or not, so remove the check in handle_mm_fault(). > >>>>>>> > >>>>>>> "mem_cgroup_oom_synchronize() will returned immediately if memcg_in_oom is not set, so remove the check from handle_mm_fault()". > >>>>>>> > >>>>>>> However, that requires now always an indirect function call -- do we care about dropping that optimization? > >>>>>>> > >>>>>>> > >>>>>> > >>>>>> If memcg_in_oom is set, we will check it twice, one is from handle_mm_fault(), the other is from mem_cgroup_oom_synchronize(). That seems a bit redundant. > >>>>>> > >>>>>> if memcg_in_oom is not set, mem_cgroup_oom_synchronize() returns directly. Though it's an indirect function call, but the time spent can be negligible > >>>>>> compare to the whole mm user falut preocess. And that won't cause stack overflow error. > >>>>> > >>>>> I suggest you measure it. > >>>> > >>>> test steps: > >>>> 1) Run command: ./mmap_anon_test(global alloc, so the memcg_in_oom is not set) > >>>> 2) Calculate the quotient of cost time and page-fault counts, run 10 rounds and average the results. > >>>> > >>>> The test result shows that whether using indirect function call or not, the time spent in user fault > >>>> is almost the same, about 2.3ms. > >>> > >>> I guess most of the benchmark time is consumed by allocating fresh pages in your test (also, why exactly do you use MAP_SHARED?). > >>> > >>> Is 2.3ms the total time for writing to that 1GiB of memory or how did you derive that number? Posting both results would be cleaner (with more digits ;) ). > >>> > >> > >> Hi Daivd, the details of test result were posted last week. Do you have any suggestions or more concerns about this change? > > > > No, I guess it really doesn't matter performance wise. > > > > One valid question would be: why perform this change at all? The redundancy doesn't seem to harm performance either. > > > > If the change would obviously improve code readability it would be easy to justify. I'm not convinced, that is the case, but maybe for others. > > Yes, this change doesn't optimize performance, just improve the code readability. > It seems that nobody ack this change, should I change the commit message and resend this patch? I don't see the point of this patch. Just drop it.