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 79EF8C6FD1D for ; Fri, 17 Mar 2023 23:48:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 97DA16B0099; Fri, 17 Mar 2023 19:48:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 92D5F6B009A; Fri, 17 Mar 2023 19:48:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7F61B6B009B; Fri, 17 Mar 2023 19:48:23 -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 70A846B0099 for ; Fri, 17 Mar 2023 19:48:23 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 397224070C for ; Fri, 17 Mar 2023 23:48:23 +0000 (UTC) X-FDA: 80580031686.10.DCBFD72 Received: from mail-wm1-f49.google.com (mail-wm1-f49.google.com [209.85.128.49]) by imf14.hostedemail.com (Postfix) with ESMTP id 52068100004 for ; Fri, 17 Mar 2023 23:48:21 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=BLGuVkFj; spf=pass (imf14.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.49 as permitted sender) smtp.mailfrom=lstoakes@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1679096901; 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=LGZ/mr6EHYv8LD66gdjsMBJnOYoWeKXNL/XChhAzOgI=; b=XB4L82Yqyug7s/MIy+Qky58hcrSyGFglE+W8OAFLAyyNgLPKTtB2TN7fDHeXmB44Nv/u2W 6rz8TkY10SQ/Jo8ilT5xW3F1ayo6IsebmRtWzKpPUuIJwrAxmKXcJNKIvssziBfcHcbwVt NcFIwBc/QfViXSgw01d+nbkodeU7SEA= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=BLGuVkFj; spf=pass (imf14.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.49 as permitted sender) smtp.mailfrom=lstoakes@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1679096901; a=rsa-sha256; cv=none; b=pskbba4SU1QV1/FVaTShCPTYVQZhkjCHFvUhuwQR8fqs3XR3QKfJ4YB5lBgyPObNKIhe4d RAnuDrIx2bzGPZ25d08OHL3HUVLBfdpey2Bx7bkAjds0rjdALdDTRSEtl/9ED2rf4te56w 9rHO3s/vzuTTkfZF7Q5am96wHm9Ds9g= Received: by mail-wm1-f49.google.com with SMTP id ip21-20020a05600ca69500b003ed56690948so3920542wmb.1 for ; Fri, 17 Mar 2023 16:48:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679096900; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=LGZ/mr6EHYv8LD66gdjsMBJnOYoWeKXNL/XChhAzOgI=; b=BLGuVkFj8L74wSWvgjbfFO1XE4RUU4mGZxiaCC3fX09Xh+ySzuBnX101ONeQeNJCAx qQdvdWoXlauZjmUgNW5YN5J60dNSsetRC93L9dxL4xf5hNAi6sNi2BjjFXD0ZBhIbRvF 8dMK7T11COsnwNEg0uJX0+otzx/9+k8MIeEg4/HHo2TS4RTBjLpdmZ9e1U54ELLibplz 43XfWmVA2pHmFKOhkoEt8n343iAt0v9osLBtNLz+JOzXroLPEE2Pkx82Ds1RTQZ7eW2Y EpCCbwU+UYCpkEEZfbdJn2Cq8xWgdP718rnrNQrUfFxOMw4fyeW+Cm2qrQx/ixkCaMAq 4lJg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679096900; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=LGZ/mr6EHYv8LD66gdjsMBJnOYoWeKXNL/XChhAzOgI=; b=Q6/qFYOWsc612XdR7YxjS2AMJssW0XGKh5P9Hbcj7MHy+6z1qTqDGDqZt6QxTWBmxi 6xBaYteRDupnhxrivpFIJcHZpQ97h4N40OXLifDIAtGqsuNI4Yy84R+711vYvGJ92BhU 2wI72tQSmqsuIaxrb79y0mln04ob6WoQBpENWcH7/icAlOXTYAA+e4qLkInXNDh8OQY4 VPOG8to+eAT1Kn5y5YOSVmmW+VaUI1vMDYyyGMQSKphmhKfFn7WZXXzpW59l8yoJzP1q 0WZb8rD4ylRGDHvQDUNsFhv1126tauMtKg/2oZvZziCpEDZnzi+9VItP83eTdsO2siyX CKHA== X-Gm-Message-State: AO0yUKXUzqweKNY6XG8gDuKqX52lwRXXU5LV8U18WJtulywL+xOZRmeB 74ndr+NYmpL9CofrZ8+BKkI= X-Google-Smtp-Source: AK7set8JXyByIOhAoDkwpihCiGd4xGld/iiQx1XbkkZH56aBn8iu3ZqZ3F9POrbDUdy4eivRQiBgEQ== X-Received: by 2002:a1c:6a0e:0:b0:3ed:6087:19fe with SMTP id f14-20020a1c6a0e000000b003ed608719femr6473925wmc.21.1679096899691; Fri, 17 Mar 2023 16:48:19 -0700 (PDT) Received: from localhost (host86-146-209-214.range86-146.btcentralplus.com. [86.146.209.214]) by smtp.gmail.com with ESMTPSA id p11-20020a1c740b000000b003e21f959453sm3204847wmc.32.2023.03.17.16.48.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 17 Mar 2023 16:48:18 -0700 (PDT) Date: Fri, 17 Mar 2023 23:48:17 +0000 From: Lorenzo Stoakes To: Andrew Morton Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, David Hildenbrand , Matthew Wilcox , Vlastimil Babka Subject: Re: [PATCH 0/2] Refactor do_fault_around() Message-ID: <98fc8545-6bd3-4c06-9b12-d781a19982ac@lucifer.local> References: <20230317163936.06d9c7d032a5c2296075caa1@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230317163936.06d9c7d032a5c2296075caa1@linux-foundation.org> X-Rspam-User: X-Rspamd-Server: rspam03 X-Stat-Signature: 6hw3f5eihu7ido4q7gh64nwd6xocengp X-Rspamd-Queue-Id: 52068100004 X-HE-Tag: 1679096901-815614 X-HE-Meta: U2FsdGVkX1/bLrrZ9nuJvq0pQm3HnCUVghyVtBVWNQ55wXePHmkcJa6FIP5UEVAVwKLIbOi1D1nV7VP/P3eWkEvV96f6uZ4Sa1EL7rrjDloNQwxrUI6LIXBX+uhMc/d9me1o7jJaJKNWVLd1YeQY24tkR0ZZbax4KQS35mQjOB87IJkRJ9HWiqJVooPq7iAU8voa/KpN7QUSunGLKaKXQrAz0QxefRHpxhxsDCxMWJqaxgRB34A5XJkD0gfLW4enqYhTdBXO0B59/STVDtISnnHaMjvd8DT87nfixSFvQAUpiJoYLcyQ5MlxO64LR72KQoPtB0F84eqnH+iSJn0DNW5Ge6+4zuj8yr5F6XKZyO1ZGz4rPbuVg/5i/4QokGqVQSKCY07tmNNrfDJ4WjKyiDvXnt3ousYmNLmSc9G9wSCC/q5Bg1ApaYCv6Brum1Nlr2BcC2Cq8MUDnlufWh2wT8OagWlUzboZ2kC1gN+EjkGcGv4iuoxjjId0JcSI8KzIshkdqmnvaevQPeU3BhN+T8vDKj78XyL40DkJuB4y+t4eHTrRgmXfRL2cvZaSDNCkIbPIRf1gRrcmmoji+1oEBtL08L68Wz82NumJbfIPoZ/PVZeTtpmJdCNrMUBoKOiMCrhSbYZkV7cH0eDiiEcl2dbeMrtcLKg2+i+HnLtZ9Z6SQQKexKuu4F2/9NQxdIytVzn5llykNixwoLS9fVUVWRK6tvQ0ghbuZoFIvf5Tp/kFsYfKhVbGUOM2BNM5RzQVKzY+DUaQOiwPxgXOb93rS6y0qsMuT2+9LTwAzEgUzdd/WHIuZ89tYkHc95UNCO1zp2iVjdAMo26E8N8KkqmSh21ZzZG0HAOAABoNHMSZ0NTApfuL+2THSALk5NhoEisZHh5jY27sXGy3vp08yuIggXnQpzYepPWOW4q7iuJLFnBPB7Mrxs+eIyKb558O/1g1QOA/2ZJuZnDsKpdAeV1 NDPuBbLw j59cIFEfH7WazyWtU362DMH5rYSH9LS5U2iSjr+6P3gGSOaeJKBiVcRys+o+ntmD0zjnF0Qjq1QusUM6SGP4paH5nbj6icUilsu4nGvjhkWWY8Vx6i3aPQXV5942f/XEG1lh1KzVTY+Sti/AUca7NZZ/TjU5XtishYEM60HXd81OUjNMG5KRy+0lW1isGKzx6L7uFMu+xA5YMlc9TU+hxhtFW4kGp75KJ4kXuSJXquoDPUVFrbKw0Tr1bl0V7MpzGTrpRSVEG1M5TzFpt/B8dWbuv2dK3oVXuPplsLlXKvkqr37EZPzCBKOsAjVmavDlLZbjd3y29WwfA0msRTGVa9JqViO+Il9Q0W6pju2tieZuStQeKnrTIng13S15kpWQgxxqS2Ie6K4735U5O6aTBBw2CpNNwAmlvlSzfQMNe7uDcx72QKJqhbXiKeTlQlvhn5GWMNCwAkcLWfd1exfvifOqQsvs2LZDK1fxL 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, Mar 17, 2023 at 04:39:36PM -0700, Andrew Morton wrote: > On Fri, 17 Mar 2023 21:58:24 +0000 Lorenzo Stoakes wrote: > > > Refactor do_fault_around() to avoid bitwise tricks and arather difficult to > > follow logic. Additionally, prefer fault_around_pages to > > fault_around_bytes as the operations are performed at a base page > > granularity. > > > > I have run this code against a small program I wrote which generates > > significant input data and compares output with the original function to > > ensure that it behaves the same as the old code across varying vmf, vma and > > fault_around_pages inputs. > > Well, what changes were you looking for in that testing? That there was no functional change between this refactoring and the existing logic. > do_fault_around() could become a no-op and most tests wouldn't notice. > What we'd be looking for to test these changes is performance > differences. > > Perhaps one could add a tracepoint to do_fault_around()'s call to > ->map_pages, check that the before-and-after traces are identical. > > > Or, if you're old school and lazy, > > if (!strcmp(current->comm, "name-of-my-test-program")) > printk("name-of-my-test-program: %lu %lu\n", > start_pgoff, end_pgoff) > > then grep-n-diff the dmesg output. I am both old school and lazy, however I went so far as to literally copy/paste the existing code and my speculative change to a userland program, generate a whole host of random sensible input data and compare output data with this and the original logic en masse. This function is actually quite nice for testability as the input and output variables are limited in scope, vmf->address, vmf->pgoff, vmf->vma->vm_start/end + vmf->vma->vm_pgoff and output start_pgoff, end_pgoff. I could attach said program but it's some quite iffy C++ code that might horrify small children and adults alike... I am more than happy to do performance testing to see if there is any impact if you require?