Message-ID: <1435097441.11808.281.camel@misato.fc.hp.com>
Subject: Re: [PATCH] mm: Fix MAP_POPULATE and mlock() for DAX
From: Toshi Kani <toshi.kani@hp.com>
To: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: akpm@linux-foundation.org, kirill.shutemov@linux.intel.com,
        willy@linux.intel.com, linux-mm@kvack.org,
        linux-fsdevel@vger.kernel.org, linux-nvdimm@ml01.01.org,
        linux-kernel@vger.kernel.org
Date: Tue, 23 Jun 2015 16:10:41 -0600
In-Reply-To: <20150623114453.GA8603@node.dhcp.inet.fi>
References: <1434493710-11138-1-git-send-email-toshi.kani@hp.com>
	 <20150620194612.GA5268@node.dhcp.inet.fi>
	 <1435006555.11808.210.camel@misato.fc.hp.com>
	 <20150623114453.GA8603@node.dhcp.inet.fi>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3114
Lines: 72

On Tue, 2015-06-23 at 14:44 +0300, Kirill A. Shutemov wrote:
> On Mon, Jun 22, 2015 at 02:55:55PM -0600, Toshi Kani wrote:
> > On Sat, 2015-06-20 at 22:46 +0300, Kirill A. Shutemov wrote:
> > > On Tue, Jun 16, 2015 at 04:28:30PM -0600, Toshi Kani wrote:
> > > > DAX has the following issues in a shared or read-only private
> > > > mmap'd file.
> > > >  - mmap(MAP_POPULATE) does not pre-fault
> > > >  - mlock() fails with -ENOMEM
> > > > 
> > > > DAX uses VM_MIXEDMAP for mmap'd files, which do not have struct
> > > > page associated with the ranges.  Both MAP_POPULATE and mlock()
> > > > call __mm_populate(), which in turn calls __get_user_pages().
> > > > Because __get_user_pages() requires a valid page returned from
> > > > follow_page_mask(), MAP_POPULATE and mlock(), i.e. FOLL_POPULATE,
> > > > fail in the first page.
> > > > 
> > > > Change __get_user_pages() to proceed FOLL_POPULATE when the
> > > > translation is set but its page does not exist (-EFAULT), and
> > > > @pages is not requested.  With that, MAP_POPULATE and mlock()
> > > > set translations to the requested range and complete successfully.
> > > > 
> > > > MAP_POPULATE still provides a major performance improvement to
> > > > DAX as it will avoid page faults during initial access to the
> > > > pages.
> > > > 
> > > > mlock() continues to set VM_LOCKED to vma and populate the range.
> > > > Since there is no struct page, the range is pinned without marking
> > > > pages mlocked.
> > > > 
> > > > Note, MAP_POPULATE and mlock() already work for a write-able
> > > > private mmap'd file on DAX since populate_vma_page_range() breaks
> > > > COW, which allocates page caches.
> > > 
> > > I don't think that's true in all cases.
> > > 
> > > We would fail to break COW for mlock() if the mapping is populated with
> > > read-only entries by the mlock() time. In this case follow_page_mask()
> > > would fail with -EFAULT and faultin_page() will never executed.
> > 
> > No, mlock() always breaks COW as populate_vma_page_range() sets
> > FOLL_WRITE in case of write-able private mmap.
> > 
> >   /*
> >    * We want to touch writable mappings with a write fault in order
> >    * to break COW, except for shared mappings because these don't COW
> >    * and we would not want to dirty them for nothing.
> >    */
> >   if ((vma->vm_flags & (VM_WRITE | VM_SHARED)) == VM_WRITE)
> >            gup_flags |= FOLL_WRITE;
> 
> Okay, you're right it should work.
> 
> What about doing this in more generic way? The totally untested patch
> below tries to make GUP work on DAX and other pfn maps when struct page
> is not required.
> 
> Any comments?

The changes look good to me.  I have also run my mmap() & mlock() tests
and they passed with the changes.  (They can only exercise
follow_pfn_pte() with FOLL_TOUCH, though.)

Reviewed-by: Toshi Kani <toshi.kani@hp.com>

Thanks,
-Toshi

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/