lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 18 Feb 2014 20:07:30 +0200 (EET)
From:	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
To:	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Mel Gorman <mgorman@...e.de>, Rik van Riel <riel@...hat.com>,
	Andi Kleen <ak@...ux.intel.com>,
	Matthew Wilcox <matthew.r.wilcox@...el.com>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	Dave Chinner <david@...morbit.com>,
	linux-mm <linux-mm@...ck.org>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC, PATCHv2 0/2] mm: map few pages around fault address if they
 are in page cache

Kirill A. Shutemov wrote:
> Linus Torvalds wrote:
> > On Mon, Feb 17, 2014 at 10:38 AM, Kirill A. Shutemov
> > <kirill.shutemov@...ux.intel.com> wrote:
> > >
> > > Now we have ->fault_nonblock() to ask filesystem for a page, if it's
> > > reachable without blocking. We request one page a time. It's not terribly
> > > efficient and I will probably re-think the interface once again to expose
> > > iterator or something...
> > 
> > Hmm. Yeah, clearly this isn't working, since the real workloads all
> > end up looking like
> > 
> > >        115,493,976      minor-faults                                                  ( +-  0.00% ) [100.00%]
> > >       59.686645587 seconds time elapsed                                          ( +-  0.30% )
> >  becomes
> > >         47,428,068      minor-faults                                                  ( +-  0.00% ) [100.00%]
> > >       60.241766430 seconds time elapsed                                          ( +-  0.85% )
> > 
> > and
> > 
> > >        268,039,365      minor-faults                                                 [100.00%]
> > >      132.830612471 seconds time elapsed
> > becomes
> > >        193,550,437      minor-faults                                                 [100.00%]
> > >      132.851823758 seconds time elapsed
> > 
> > and
> > 
> > >          4,967,540      minor-faults                                                  ( +-  0.06% ) [100.00%]
> > >       27.215434226 seconds time elapsed                                          ( +-  0.18% )
> > becomes
> > >          2,285,563      minor-faults                                                  ( +-  0.26% ) [100.00%]
> > >       27.292854546 seconds time elapsed                                          ( +-  0.29% )
> > 
> > ie it shows a clear reduction in faults, but the added costs clearly
> > eat up any wins and it all becomes (just _slightly_) slower.
> 
> I did an experement with setup pte directly in filemap_fault_nonblock() to
> see how much we can get from it. And it helps:
> 
> git:		-1.21s
> clean build:	-2.22s
> rebuild:	-0.63s
> 
> Is it a layering violation to setup pte directly in ->fault_nonblock()?
> 
> perf stat and patch below.
> 
> Git test-suite make -j60 test:
>  1,591,184,058,944      cycles                     ( +-  0.05% ) [100.00%]
>    811,200,260,823      instructions              #    0.51  insns per cycle
>                                                   #    3.24  stalled cycles per insn  ( +-  0.19% ) [100.00%]
>  2,631,511,271,429      stalled-cycles-frontend   #  165.38% frontend cycles idle     ( +-  0.08% )
>         47,305,697      minor-faults                                                  ( +-  0.00% ) [100.00%]
>                  1      major-faults
> 
>       59.028360009 seconds time elapsed                                          ( +-  0.58% )
> 
> Run make -j60 on clean allmodconfig kernel tree:
> 19,163,958,689,310      cycles                    [100.00%]
> 17,446,888,861,177      instructions              #    0.91  insns per cycle
>                                                   #    1.53  stalled cycles per insn [100.00%]
> 26,777,884,033,091      stalled-cycles-frontend   #  139.73% frontend cycles idle
>        193,118,569      minor-faults                                                 [100.00%]
>                  0      major-faults
> 
>      130.631767214 seconds time elapsed
> 
> Run make -j60 on already built allmodconfig kernel tree:
>    282,398,537,719      cycles                     ( +-  0.03% ) [100.00%]
>    385,807,937,931      instructions              #    1.37  insns per cycle
>                                                   #    0.95  stalled cycles per insn  ( +-  0.01% ) [100.00%]
>    365,940,576,310      stalled-cycles-frontend   #  129.58% frontend cycles idle     ( +-  0.07% )
>          2,254,887      minor-faults                                                  ( +-  0.02% ) [100.00%]
>                  0      major-faults
> 
>       26.660708754 seconds time elapsed                                          ( +-  0.29% )

Patch is wrong. Correct one is below.

diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
index a16b0ff497ca..a7f7e41bec37 100644
--- a/fs/9p/vfs_file.c
+++ b/fs/9p/vfs_file.c
@@ -832,6 +832,7 @@ static void v9fs_mmap_vm_close(struct vm_area_struct *vma)
 
 static const struct vm_operations_struct v9fs_file_vm_ops = {
 	.fault = filemap_fault,
+	.fault_nonblock = filemap_fault_nonblock,
 	.page_mkwrite = v9fs_vm_page_mkwrite,
 	.remap_pages = generic_file_remap_pages,
 };
@@ -839,6 +840,7 @@ static const struct vm_operations_struct v9fs_file_vm_ops = {
 static const struct vm_operations_struct v9fs_mmap_file_vm_ops = {
 	.close = v9fs_mmap_vm_close,
 	.fault = filemap_fault,
+	.fault_nonblock = filemap_fault_nonblock,
 	.page_mkwrite = v9fs_vm_page_mkwrite,
 	.remap_pages = generic_file_remap_pages,
 };
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 0165b8672f09..13523a63e1f3 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1993,6 +1993,7 @@ out:
 
 static const struct vm_operations_struct btrfs_file_vm_ops = {
 	.fault		= filemap_fault,
+	.fault_nonblock	= filemap_fault_nonblock,
 	.page_mkwrite	= btrfs_page_mkwrite,
 	.remap_pages	= generic_file_remap_pages,
 };
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 755584684f6c..71aff75e067c 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -3094,6 +3094,7 @@ cifs_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 
 static struct vm_operations_struct cifs_file_vm_ops = {
 	.fault = filemap_fault,
+	.fault_nonblock = filemap_fault_nonblock,
 	.page_mkwrite = cifs_page_mkwrite,
 	.remap_pages = generic_file_remap_pages,
 };
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 1a5073959f32..182ae5543a1d 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -200,6 +200,7 @@ ext4_file_write(struct kiocb *iocb, const struct iovec *iov,
 
 static const struct vm_operations_struct ext4_file_vm_ops = {
 	.fault		= filemap_fault,
+	.fault_nonblock	= filemap_fault_nonblock,
 	.page_mkwrite   = ext4_page_mkwrite,
 	.remap_pages	= generic_file_remap_pages,
 };
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 0dfcef53a6ed..7c48fd2eb99c 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -84,6 +84,7 @@ out:
 
 static const struct vm_operations_struct f2fs_file_vm_ops = {
 	.fault		= filemap_fault,
+	.fault_nonblock	= filemap_fault_nonblock,
 	.page_mkwrite	= f2fs_vm_page_mkwrite,
 	.remap_pages	= generic_file_remap_pages,
 };
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 77bcc303c3ae..e95e52ec7bc2 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1940,6 +1940,7 @@ static int fuse_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 static const struct vm_operations_struct fuse_file_vm_ops = {
 	.close		= fuse_vma_close,
 	.fault		= filemap_fault,
+	.fault_nonblock	= filemap_fault_nonblock,
 	.page_mkwrite	= fuse_page_mkwrite,
 	.remap_pages	= generic_file_remap_pages,
 };
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index efc078f0ee4e..7c4b2f096ac8 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -494,6 +494,7 @@ out:
 
 static const struct vm_operations_struct gfs2_vm_ops = {
 	.fault = filemap_fault,
+	.fault_nonblock = filemap_fault_nonblock,
 	.page_mkwrite = gfs2_page_mkwrite,
 	.remap_pages = generic_file_remap_pages,
 };
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 5bb790a69c71..8fbe80168d1f 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -617,6 +617,7 @@ out:
 
 static const struct vm_operations_struct nfs_file_vm_ops = {
 	.fault = filemap_fault,
+	.fault_nonblock = filemap_fault_nonblock,
 	.page_mkwrite = nfs_vm_page_mkwrite,
 	.remap_pages = generic_file_remap_pages,
 };
diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c
index 08fdb77852ac..adc4aa07d7d8 100644
--- a/fs/nilfs2/file.c
+++ b/fs/nilfs2/file.c
@@ -134,6 +134,7 @@ static int nilfs_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 
 static const struct vm_operations_struct nilfs_file_vm_ops = {
 	.fault		= filemap_fault,
+	.fault_nonblock	= filemap_fault_nonblock,
 	.page_mkwrite	= nilfs_page_mkwrite,
 	.remap_pages	= generic_file_remap_pages,
 };
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 123c79b7261e..f27c4c401a3f 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1538,6 +1538,7 @@ out_unlock:
 
 static const struct vm_operations_struct ubifs_file_vm_ops = {
 	.fault        = filemap_fault,
+	.fault_nonblock = filemap_fault_nonblock,
 	.page_mkwrite = ubifs_vm_page_mkwrite,
 	.remap_pages = generic_file_remap_pages,
 };
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 64b48eade91d..bc619150c960 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1465,6 +1465,7 @@ const struct file_operations xfs_dir_file_operations = {
 
 static const struct vm_operations_struct xfs_file_vm_ops = {
 	.fault		= filemap_fault,
+	.fault_nonblock	= filemap_fault_nonblock,
 	.page_mkwrite	= xfs_vm_page_mkwrite,
 	.remap_pages	= generic_file_remap_pages,
 };
diff --git a/include/linux/mm.h b/include/linux/mm.h
index f28f46eade6a..e671dd5abe27 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -221,6 +221,8 @@ struct vm_operations_struct {
 	void (*open)(struct vm_area_struct * area);
 	void (*close)(struct vm_area_struct * area);
 	int (*fault)(struct vm_area_struct *vma, struct vm_fault *vmf);
+	int (*fault_nonblock)(struct vm_area_struct *vma, struct vm_fault *vmf,
+		pgoff_t max_pgoff, int nr_pages, pte_t *pte);
 
 	/* notification that a previously read-only page is about to become
 	 * writable, if an error is returned it will cause a SIGBUS */
@@ -1810,6 +1812,8 @@ extern void truncate_inode_pages_range(struct address_space *,
 
 /* generic vm_area_ops exported for stackable file systems */
 extern int filemap_fault(struct vm_area_struct *, struct vm_fault *);
+int filemap_fault_nonblock(struct vm_area_struct *vma, struct vm_fault *vmf,
+		pgoff_t max_pgoff, int nr_pages, pte_t *pte);
 extern int filemap_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf);
 
 /* mm/page-writeback.c */
diff --git a/mm/filemap.c b/mm/filemap.c
index 7a13f6ac5421..0a8884efbcd8 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -33,6 +33,7 @@
 #include <linux/hardirq.h> /* for BUG_ON(!in_atomic()) only */
 #include <linux/memcontrol.h>
 #include <linux/cleancache.h>
+#include <linux/rmap.h>
 #include "internal.h"
 
 #define CREATE_TRACE_POINTS
@@ -1726,6 +1730,93 @@ page_not_uptodate:
 }
 EXPORT_SYMBOL(filemap_fault);
 
+void do_set_pte(struct vm_area_struct *vma, unsigned long address,
+		struct page *page, pte_t *pte, bool write, bool anon);
+int filemap_fault_nonblock(struct vm_area_struct *vma, struct vm_fault *vmf,
+		pgoff_t max_pgoff, int nr_pages, pte_t *pte)
+{
+	struct radix_tree_iter iter;
+	void **slot;
+	struct file *file = vma->vm_file;
+	struct address_space *mapping = file->f_mapping;
+	pgoff_t size;
+	struct page *page;
+	unsigned long address = (unsigned long) vmf->virtual_address;
+	unsigned long addr;
+	pte_t *_pte;
+	int ret = 0;
+
+	rcu_read_lock();
+restart:
+	radix_tree_for_each_slot(slot, &mapping->page_tree, &iter, vmf->pgoff) {
+repeat:
+		page = radix_tree_deref_slot(slot);
+
+		if (radix_tree_exception(page)) {
+			if (radix_tree_deref_retry(page)) {
+				/*
+				 * Transient condition which can only trigger
+				 * when entry at index 0 moves out of or back
+				 * to root: none yet gotten, safe to restart.
+				 */
+				WARN_ON(iter.index);
+				goto restart;
+			}
+			/*
+			 * Otherwise, shmem/tmpfs must be storing a swap entry
+			 * here as an exceptional entry: so skip over it -
+			 * we only reach this from invalidate_mapping_pages().
+			 */
+			continue;
+		}
+
+		if (!page_cache_get_speculative(page))
+			goto repeat;
+
+		/* Has the page moved? */
+		if (unlikely(page != *slot)) {
+			page_cache_release(page);
+			goto repeat;
+		}
+
+		if (page->index > max_pgoff) {
+			page_cache_release(page);
+			break;
+		}
+
+		if (PageReadahead(page) || PageHWPoison(page) ||
+				!PageUptodate(page))
+			goto skip;
+		if (!trylock_page(page))
+			goto skip;
+		if (page->mapping != mapping || !PageUptodate(page))
+			goto unlock;
+		size = (i_size_read(mapping->host) + PAGE_CACHE_SIZE - 1)
+			>> PAGE_CACHE_SHIFT;
+		if (page->index >= size)
+			goto unlock;
+		if (file->f_ra.mmap_miss > 0)
+			file->f_ra.mmap_miss--;
+		addr = address + (page->index - vmf->pgoff) * PAGE_SIZE;
+		_pte = pte + page->index - vmf->pgoff;
+		if (!pte_none(*_pte))
+			goto unlock;
+		do_set_pte(vma, addr, page, _pte, false, false);
+
+		unlock_page(page);
+		if (++ret == nr_pages || page->index == max_pgoff)
+			break;
+		continue;
+unlock:
+		unlock_page(page);
+skip:
+		page_cache_release(page);
+	}
+	rcu_read_unlock();
+	return ret;
+}
+EXPORT_SYMBOL(filemap_fault_nonblock);
+
 int filemap_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
 	struct page *page = vmf->page;
@@ -1755,6 +1846,7 @@ EXPORT_SYMBOL(filemap_page_mkwrite);
 
 const struct vm_operations_struct generic_file_vm_ops = {
 	.fault		= filemap_fault,
+	.fault_nonblock	= filemap_fault_nonblock,
 	.page_mkwrite	= filemap_page_mkwrite,
 	.remap_pages	= generic_file_remap_pages,
 };
diff --git a/mm/memory.c b/mm/memory.c
index 7f52c46ef1e1..e79c8d6f6f47 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3318,7 +3318,8 @@ static int __do_fault(struct vm_area_struct *vma, unsigned long address,
 	return ret;
 }
 
-static void do_set_pte(struct vm_area_struct *vma, unsigned long address,
+
+void do_set_pte(struct vm_area_struct *vma, unsigned long address,
 		struct page *page, pte_t *pte, bool write, bool anon)
 {
 	pte_t entry;
@@ -3342,6 +3343,49 @@ static void do_set_pte(struct vm_area_struct *vma, unsigned long address,
 	update_mmu_cache(vma, address, pte);
 }
 
+#define FAULT_AROUND_ORDER 5
+#define FAULT_AROUND_PAGES (1UL << FAULT_AROUND_ORDER)
+#define FAULT_AROUND_MASK ~((1UL << (PAGE_SHIFT + FAULT_AROUND_ORDER)) - 1)
+
+static void do_fault_around(struct vm_area_struct *vma, unsigned long address,
+		pte_t *pte, pgoff_t pgoff, unsigned int flags)
+{
+	unsigned long start_addr;
+	pgoff_t max_pgoff;
+	struct vm_fault vmf;
+	int off, ret;
+
+	/* Do not cross vma or page table border */
+	max_pgoff = min(pgoff - pte_index(address) + PTRS_PER_PTE - 1,
+			vma_pages(vma) + vma->vm_pgoff - 1);
+
+	start_addr = max(address & FAULT_AROUND_MASK, vma->vm_start);
+	if ((start_addr & PMD_MASK) != (address & PMD_MASK))
+		BUG();
+	off = pte_index(start_addr) - pte_index(address);
+	pte += off;
+	pgoff += off;
+
+	/* Check if it makes any sense to call ->fault_nonblock */
+	while (!pte_none(*pte)) {
+		pte++;
+		pgoff++;
+		start_addr += PAGE_SIZE;
+		/* Do not cross vma or page table border */
+		if (!pte_index(start_addr) || start_addr >= vma->vm_end)
+			return;
+		if ((start_addr & PMD_MASK) != (address & PMD_MASK))
+			BUG();
+	}
+
+
+	vmf.virtual_address = (void __user *) start_addr;
+	vmf.pgoff = pgoff;
+	vmf.flags = flags;
+	ret = vma->vm_ops->fault_nonblock(vma, &vmf,
+			max_pgoff, FAULT_AROUND_PAGES, pte);
+}
+
 static int do_read_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 		unsigned long address, pmd_t *pmd,
 		pgoff_t pgoff, unsigned int flags, pte_t orig_pte)
@@ -3363,8 +3407,11 @@ static int do_read_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 		return ret;
 	}
 	do_set_pte(vma, address, fault_page, pte, false, false);
-	pte_unmap_unlock(pte, ptl);
 	unlock_page(fault_page);
+
+	if (vma->vm_ops->fault_nonblock)
+		do_fault_around(vma, address, pte, pgoff, flags);
+	pte_unmap_unlock(pte, ptl);
 	return ret;
 }
 
-- 
 Kirill A. Shutemov
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists