Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 5 Dec 2018 09:58:10 -0500
From:   Josef Bacik <josef@toxicpanda.com>
To:     Andrew Morton <akpm@linux-foundation.org>
Cc:     Josef Bacik <josef@toxicpanda.com>, kernel-team@fb.com,
        hannes@cmpxchg.org, linux-kernel@vger.kernel.org, tj@kernel.org,
        david@fromorbit.com, linux-fsdevel@vger.kernel.org,
        linux-mm@kvack.org, riel@redhat.com, jack@suse.cz
Subject: Re: [PATCH 4/4] mm: use the cached page for filemap_fault
Message-ID: <20181205145808.kzsro4a7vqaxx3cu@MacBook-Pro-91.local>
References: <20181130195812.19536-1-josef@toxicpanda.com>
 <20181130195812.19536-5-josef@toxicpanda.com>
 <20181204145034.4b69bdea36506be45946f8c9@linux-foundation.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20181204145034.4b69bdea36506be45946f8c9@linux-foundation.org>
User-Agent: NeoMutt/20180716
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Dec 04, 2018 at 02:50:34PM -0800, Andrew Morton wrote:
> On Fri, 30 Nov 2018 14:58:12 -0500 Josef Bacik <josef@toxicpanda.com> wrote:
> 
> > If we drop the mmap_sem we have to redo the vma lookup which requires
> > redoing the fault handler.  Chances are we will just come back to the
> > same page, so save this page in our vmf->cached_page and reuse it in the
> > next loop through the fault handler.
> > 
> 
> Is this really worthwhile?  Rerunning the fault handler is rare (we
> hope) and a single pagecache lookup is fast.
> 
> Some performance testing results would be helpful here.  It's
> practically obligatory when claiming a performance improvement.
> 
> 

Honestly the big thing is just not doing IO under the mmap_sem.  I had this
infrastructure originally for the mkwrite portion of these patches that I
dropped, because I was worried about the page being messed with after we did all
the mkwrite work.  However since I'm not doing that anymore there's less of a
need for it.  I have no performance numbers for this, just seemed like a good
idea since we are likely to just have the page again, and this keeps us from
evicting the page right away and causing more thrashing.

I'll try and set something up to see if there's a difference.  If there's no
difference do you want me to drop this?  Thanks,

Josef