Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [RFC PATCH 00/11] Adding FreeBSD's Capsicum security framework (part 1)
From: Paolo Bonzini @ 2014-07-07 12:20 UTC (permalink / raw)
  To: David Drysdale
  Cc: LSM List, linux-kernel@vger.kernel.org, Greg Kroah-Hartman,
	Alexander Viro, Meredydd Luff, Kees Cook, James Morris, Linux API,
	qemu-devel
In-Reply-To: <CAHse=S83APPfnEwO1fQbV_tNd=9zsAsZr42KbYM_He-c3C3P6A@mail.gmail.com>

Il 07/07/2014 12:29, David Drysdale ha scritto:
>> I think that's more easily done by opening the file as O_RDONLY/O_WRONLY
>> /O_RDWR.   You could do it by running the file descriptor's seccomp-bpf
>> program once per iocb with synthesized syscall numbers and argument
>> vectors.
>
> Right, but generating the equivalent seccomp input environment for an
> equivalent single-fd syscall is going to be subtle and complex (which
> are worrying words to mention in a security context).  And how many
> other syscalls are going to need similar special-case processing?
> (poll? select? send[m]msg? ...)

Yeah, the difficult part is getting the right balance between:

1) limitations due to seccomp's impossibility to chase pointers (which 
is not something that can be lifted, as it's required for correctness)

2) subtlety and complexity of the resulting code.

The problem stems when you have a single a single syscall operating on 
multiple file descriptors.  So for example among the cases you mention 
poll and select are problematic; sendm?msg are not.  They would be if 
Capsicum had a capability for SCM_RIGHTS file descriptor passing, but I 
cannot find it.

But then you also have to strike the right balance between a complete 
solution and an overengineered one.

For example, even though poll and select are problematic, I wonder what 
would really the point be in blocking them; poll/select are 
level-triggered, and calling them should be idempotent as far as the 
file descriptor is concerned.  If you want to prevent a process/thread 
from issuing blocking system calls, but you'd do that with a per-process 
filter, not with per-file-descriptor filters or capabilities.

> Capsicum capabilities are associated with the file descriptor (a la
> F_GETFD), not the open file itself -- different FDs with different
> associated rights can map to the same underlying open file.

Good to know, thanks.  I suppose you have testcases that cover this.

Paolo

^ permalink raw reply

* Re: [RFC PATCH 00/11] Adding FreeBSD's Capsicum security framework (part 1)
From: David Drysdale @ 2014-07-07 14:11 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: LSM List, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Greg Kroah-Hartman, Alexander Viro, Meredydd Luff, Kees Cook,
	James Morris, Linux API, qemu-devel
In-Reply-To: <53BA9094.9080401-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Mon, Jul 7, 2014 at 1:20 PM, Paolo Bonzini <pbonzini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Il 07/07/2014 12:29, David Drysdale ha scritto:
>> Capsicum capabilities are associated with the file descriptor (a la
>> F_GETFD), not the open file itself -- different FDs with different
>> associated rights can map to the same underlying open file.
>
>
> Good to know, thanks.  I suppose you have testcases that cover this.
>
> Paolo

Yeah, there's lots of tests at:
  https://github.com/google/capsicum-test
(which is in a separate repo so it's easy to run against
FreeBSD as well as the Linux code); in particular
  https://github.com/google/capsicum-test/blob/dev/capability-fd.cc
has various interactions of capability FDs.

^ permalink raw reply

* Re: [PATCH v10 5/7] s390: add pmd_[dirty|mkclean] for THP
From: Gerald Schaefer @ 2014-07-07 15:31 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Michael Kerrisk, Linux API,
	Hugh Dickins, Johannes Weiner, Rik van Riel, KOSAKI Motohiro,
	Mel Gorman, Jason Evans, Zhang Yanfei, Kirill A. Shutemov,
	Martin Schwidefsky, Heiko Carstens, Dominik Dingel,
	Christian Borntraeger, linux-s390-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1404694438-10272-6-git-send-email-minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On Mon,  7 Jul 2014 09:53:56 +0900
Minchan Kim <minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:

> MADV_FREE needs pmd_dirty and pmd_mkclean for detecting recent
> overwrite of the contents since MADV_FREE syscall is called for
> THP page but for s390 pmds only referenced bit is available
> because there is no free bit left in the pmd entry for the
> software dirty bit so this patch adds dumb pmd_dirty which
> returns always true by suggesting by Martin.
> 
> They finally find a solution in future.
> http://marc.info/?l=linux-api&m=140440328820808&w=2
> 
> Cc: Martin Schwidefsky <schwidefsky-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> Cc: Heiko Carstens <heiko.carstens-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> Cc: Gerald Schaefer <gerald.schaefer-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> Cc: Dominik Dingel <dingel-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> Cc: Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> Cc: linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Minchan Kim <minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
>  arch/s390/include/asm/pgtable.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

Acked-by: Gerald Schaefer <gerald.schaefer-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>

^ permalink raw reply

* (no subject)
From: L polyzoi @ 2014-07-07 17:45 UTC (permalink / raw)
  To: l.polyzoi-cVxd4ZXPr5xnz3GQr/xE2Q, a.salleh-w6+s2I0VQzUQrrorzV6ljw,
	gro.braten-nK3bX95WTZ8, smithm-1go2MUbu68oASWQVBRBxCA,
	ubgfecjwdxtj-76uR8rYFK5asngwezA94EA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	hcxz3inluf8fg7pgjw-OW+95qYkxTcAvxtiuMwx3w,
	scjvlomjmoth7piwct-OW+95qYkxTcAvxtiuMwx3w,
	byvrec-OW+95qYkxTcAvxtiuMwx3w,
	r83jvrrhba1ywnyzao-OW+95qYkxTcAvxtiuMwx3w,
	ecvo8fkluwkdjnc-OW+95qYkxTcAvxtiuMwx3w,
	sjaao9exuog-OW+95qYkxTcAvxtiuMwx3w,
	ky6e8nrsom2iobjjyahvb-OW+95qYkxTcAvxtiuMwx3w,
	sapbctbqcxc8brzn1jra-OW+95qYkxTcAvxtiuMwx3w,
	jkgarmp-OW+95qYkxTcAvxtiuMwx3w, laj-6vVzZc7OGKFaa/9Udqfwiw,
	g.czedik-TVDPYGuBT+ReoWH0uzbU5w, hczovyasghyiruiy-U0Qi+Engdwo,
	jy3efljsazeo-U0Qi+Engdwo, jawkjlygztkdy6g8vgxgc-U0Qi+Engdwo,
	oj0aar4gzfrahhigxw6axba-U0Qi+Engdwo, f6xrtcl9w.c-U0Qi+Engdwo,
	mpt3au8m-U0Qi+Engdwo, zoirkp-U0Qi+Engdwo,
	guanghuazhiye-V1fLS7LMyRxWk0Htik3J/w

Give your loved ones a branded watch today

http://media.skarea2.go.th/components/com_swmenufree/preg_class.php

^ permalink raw reply

* [PATCH v3 0/3] mm: introduce fincore() v3
From: Naoya Horiguchi @ 2014-07-07 18:00 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Konstantin Khlebnikov, Wu Fengguang, Arnaldo Carvalho de Melo,
	Borislav Petkov, Kirill A. Shutemov, Johannes Weiner,
	Rusty Russell, David Miller, Andres Freund, linux-kernel,
	linux-mm, Dave Hansen, Christoph Hellwig, Dave Chinner,
	Michael Kerrisk, Linux API, Naoya Horiguchi

This is the 3rd version of fincore patchset.
I stop exporting PAGECACHE_TAG_* information to userspace via fincore().
Rather than that, no major change since v2.

Any comments/reviews are welcomed.

v2: http://lwn.net/Articles/604380/
v1: http://lwn.net/Articles/601020/

Thanks,
Naoya Horiguchi
---
Tree: git@github.com:Naoya-Horiguchi/linux.git
Branch: v3.16-rc3/fincore.ver3
---
Summary:

Naoya Horiguchi (3):
      mm: introduce fincore()
      selftests/fincore: add test code for fincore()
      man2/fincore.2: document general description about fincore(2)

 arch/x86/syscalls/syscall_64.tbl                   |   1 +
 include/linux/syscalls.h                           |   4 +
 include/uapi/linux/fincore.h                       |  84 +++++
 man2/fincore.2                                     | 348 ++++++++++++++++++++
 mm/Makefile                                        |   2 +-
 mm/fincore.c                                       | 286 ++++++++++++++++
 tools/testing/selftests/Makefile                   |   1 +
 tools/testing/selftests/fincore/Makefile           |  31 ++
 .../selftests/fincore/create_hugetlbfs_file.c      |  49 +++
 tools/testing/selftests/fincore/fincore.c          | 153 +++++++++
 tools/testing/selftests/fincore/run_fincoretests   | 361 +++++++++++++++++++++
 11 files changed, 1319 insertions(+), 1 deletion(-)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* [PATCH v3 1/3] mm: introduce fincore()
From: Naoya Horiguchi @ 2014-07-07 18:00 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Konstantin Khlebnikov, Wu Fengguang, Arnaldo Carvalho de Melo,
	Borislav Petkov, Kirill A. Shutemov, Johannes Weiner,
	Rusty Russell, David Miller, Andres Freund, linux-kernel,
	linux-mm, Dave Hansen, Christoph Hellwig, Dave Chinner,
	Michael Kerrisk, Linux API, Naoya Horiguchi
In-Reply-To: <1404756006-23794-1-git-send-email-n-horiguchi@ah.jp.nec.com>

This patch provides a new system call fincore(), which extracts mincore()-
like information from the kernel, i.e. page residency of a given file.
But unlike mincore(), fincore() has a mode flag which allows us to extract
more detailed information like pfn and page flag. This kind of information
is helpful, for example when applications want to know the file cache status
to control the IO on their own way.
There was some complaint around current mincore() API, where we have only
one byte for each page and mincore() is not extensible without breaking
existing applications, so the mode flag is to avoid the same problem.

The details about the data format being passed to userspace are explained
in inline comment, but generally in long entry format, we can choose which
information is extraced flexibly, so you don't have to waste memory by
extracting unnecessary information. And with FINCORE_PGOFF flag, we can skip
hole pages (not on memory,) which makes us avoid a flood of meaningless
zero entries when calling on extremely large (but only few pages of it
are loaded on memory) file.

Basic testset is added in the next patch on tools/testing/selftests/fincore/.

ChangeLog v3:
- remove pagecache tag things
- rename include/uapi/linux/pagecache.h to include/uapi/linux/fincore.h

ChangeLog v2:
- move definition of FINCORE_* to include/uapi/linux/pagecache.h
- add another parameter fincore_extra to sys_fincore()
- rename FINCORE_SKIP_HOLE to FINCORE_PGOFF and change bit order.
- add valid argument check (start should be inside file address range,
  nr_pages should be positive)
- add end-of-file check (scan to the end of file even if the last page
  is a hole)
- add access_ok(VERIFY_WIRTE) (copied from mincore())
- update inline comments

Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
 arch/x86/syscalls/syscall_64.tbl |   1 +
 include/linux/syscalls.h         |   4 +
 include/uapi/linux/fincore.h     |  84 ++++++++++++
 mm/Makefile                      |   2 +-
 mm/fincore.c                     | 286 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 376 insertions(+), 1 deletion(-)
 create mode 100644 include/uapi/linux/fincore.h
 create mode 100644 mm/fincore.c

diff --git v3.16-rc3.orig/arch/x86/syscalls/syscall_64.tbl v3.16-rc3/arch/x86/syscalls/syscall_64.tbl
index ec255a1646d2..9d291b7081ca 100644
--- v3.16-rc3.orig/arch/x86/syscalls/syscall_64.tbl
+++ v3.16-rc3/arch/x86/syscalls/syscall_64.tbl
@@ -323,6 +323,7 @@
 314	common	sched_setattr		sys_sched_setattr
 315	common	sched_getattr		sys_sched_getattr
 316	common	renameat2		sys_renameat2
+317	common	fincore			sys_fincore
 
 #
 # x32-specific system call numbers start at 512 to avoid cache impact
diff --git v3.16-rc3.orig/include/linux/syscalls.h v3.16-rc3/include/linux/syscalls.h
index b0881a0ed322..60795ee8f9ee 100644
--- v3.16-rc3.orig/include/linux/syscalls.h
+++ v3.16-rc3/include/linux/syscalls.h
@@ -65,6 +65,7 @@ struct old_linux_dirent;
 struct perf_event_attr;
 struct file_handle;
 struct sigaltstack;
+struct fincore_extra;
 
 #include <linux/types.h>
 #include <linux/aio_abi.h>
@@ -866,4 +867,7 @@ asmlinkage long sys_process_vm_writev(pid_t pid,
 asmlinkage long sys_kcmp(pid_t pid1, pid_t pid2, int type,
 			 unsigned long idx1, unsigned long idx2);
 asmlinkage long sys_finit_module(int fd, const char __user *uargs, int flags);
+asmlinkage long sys_fincore(int fd, loff_t start, long nr_pages,
+			int mode, unsigned char __user *vec,
+			struct fincore_extra __user *extra);
 #endif
diff --git v3.16-rc3.orig/include/uapi/linux/fincore.h v3.16-rc3/include/uapi/linux/fincore.h
new file mode 100644
index 000000000000..30797e68a8d4
--- /dev/null
+++ v3.16-rc3/include/uapi/linux/fincore.h
@@ -0,0 +1,84 @@
+#ifndef _UAPI_LINUX_FINCORE_H
+#define _UAPI_LINUX_FINCORE_H
+
+/*
+ * You can control how the buffer in userspace is filled with this mode
+ * parameters:
+ *
+ * - FINCORE_BMAP:
+ *     the page status is returned in a vector of bytes.
+ *     The least significant bit of each byte is 1 if the referenced page
+ *     is in memory, otherwise it is zero.
+ *
+ * - FINCORE_PGOFF:
+ *     if this flag is set, fincore() doesn't store any information about
+ *     holes. Instead each records per page has the entry of page offset,
+ *     using 8 bytes. This mode is useful if we handle a large file and
+ *     only few pages are on memory.
+ *
+ * - FINCORE_PFN:
+ *     stores pfn, using 8 bytes.
+ *
+ * - FINCORE_PAGEFLAGS:
+ *     stores page flags, using 8 bytes. See definition of KPF_* for
+ *     details of each bit.
+ *
+ * FINCORE_BMAP shouldn't be used combined with any other flags, and returnd
+ * data in this mode is like this:
+ *
+ *   page offset  0   1   2   3   4
+ *              +---+---+---+---+---+
+ *              | 1 | 0 | 0 | 1 | 1 | ...
+ *              +---+---+---+---+---+
+ *               <->
+ *              1 byte
+ *
+ * For FINCORE_PFN, page data is formatted like this:
+ *
+ *   page offset    0       1       2       3       4
+ *              +-------+-------+-------+-------+-------+
+ *              |  pfn  |  pfn  |  pfn  |  pfn  |  pfn  | ...
+ *              +-------+-------+-------+-------+-------+
+ *               <----->
+ *               8 byte
+ *
+ * We can use multiple flags among the flags in FINCORE_LONGENTRY_MASK.
+ * For example, when the mode is FINCORE_PFN|FINCORE_PAGEFLAGS, the per-page
+ * information is stored like this:
+ *
+ *    page offset 0    page offset 1   page offset 2   page offset 3
+ *                                        (hole)
+ *   +-------+-------+-------+-------+-------+-------+-------+-------+
+ *   |  pfn  | flags |  pfn  | flags |   0   |   0   |  pfn  | flags | ...
+ *   +-------+-------+-------+-------+-------+-------+-------+-------+
+ *    <-------------> <-------------> <-------------> <------------->
+ *       16 bytes        16 bytes        16 bytes        16 bytes
+ *
+ * When FINCORE_PGOFF is set, we store page offset entry and ignore holes
+ * For example, the data format of mode FINCORE_PGOFF|FINCORE_PFN|
+ * FINCORE_PAGEFLAGS is like follows:
+ *
+ *   +-------+-------+-------+-------+-------+-------+
+ *   | pgoff |  pfn  | flags | pgoff |  pfn  | flags | ...
+ *   +-------+-------+-------+-------+-------+-------+
+ *    <---------------------> <--------------------->
+ *           24 bytes                24 bytes
+ */
+#define FINCORE_BMAP		0x01	/* bytemap mode */
+#define FINCORE_PGOFF		0x02
+#define FINCORE_PFN		0x04
+#define FINCORE_PAGE_FLAGS	0x08
+
+#define FINCORE_MODE_MASK	0x1f
+#define FINCORE_LONGENTRY_MASK	(FINCORE_PGOFF | FINCORE_PFN | \
+				 FINCORE_PAGE_FLAGS)
+
+struct fincore_extra {
+	/*
+	 * (output) the number of entries with valid data, this is useful
+	 * if you set FINCORE_PGOFF and want to know the end of filled data.
+	 */
+	unsigned long nr_entries;
+};
+
+#endif /* _UAPI_LINUX_FINCORE_H */
diff --git v3.16-rc3.orig/mm/Makefile v3.16-rc3/mm/Makefile
index 4064f3ec145e..cc9420221afd 100644
--- v3.16-rc3.orig/mm/Makefile
+++ v3.16-rc3/mm/Makefile
@@ -18,7 +18,7 @@ obj-y			:= filemap.o mempool.o oom_kill.o fadvise.o \
 			   mm_init.o mmu_context.o percpu.o slab_common.o \
 			   compaction.o balloon_compaction.o vmacache.o \
 			   interval_tree.o list_lru.o workingset.o \
-			   iov_iter.o $(mmu-y)
+			   iov_iter.o fincore.o $(mmu-y)
 
 obj-y += init-mm.o
 
diff --git v3.16-rc3.orig/mm/fincore.c v3.16-rc3/mm/fincore.c
new file mode 100644
index 000000000000..2f20ac3569df
--- /dev/null
+++ v3.16-rc3/mm/fincore.c
@@ -0,0 +1,286 @@
+/*
+ * fincore(2) system call
+ *
+ * Copyright (C) 2014 NEC Corporation, Naoya Horiguchi
+ */
+
+#include <linux/syscalls.h>
+#include <linux/pagemap.h>
+#include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/mm.h>
+#include <linux/slab.h>
+#include <linux/hugetlb.h>
+#include <uapi/linux/fincore.h>
+
+struct fincore_control {
+	int mode;
+	int width;		/* width of each entry (in bytes) */
+	unsigned char *buffer;
+	long buffer_size;
+	void *cursor;		/* current position on the buffer */
+	pgoff_t pgstart;	/* start point of page cache scan in each run
+				 * of the while loop */
+	long nr_pages;		/* number of pages to be copied to userspace
+				 * (decreasing while scan proceeds) */
+	long scanned_offset;	/* page offset of the lastest scanned page */
+	struct address_space *mapping;
+};
+
+#define store_entry(fc, type, data) ({		\
+	*(type *)fc->cursor = (type)data;	\
+	fc->cursor += sizeof(type);		\
+})
+
+/*
+ * Store page cache data to temporal buffer in the specified format depending
+ * on fincore mode.
+ */
+static void __do_fincore(struct fincore_control *fc, struct page *page,
+			 unsigned long index)
+{
+	VM_BUG_ON(!page);
+	VM_BUG_ON((unsigned long)fc->cursor - (unsigned long)fc->buffer
+		  >= fc->buffer_size);
+	if (fc->mode & FINCORE_BMAP)
+		store_entry(fc, unsigned char, PageUptodate(page));
+	else if (fc->mode & (FINCORE_LONGENTRY_MASK)) {
+		if (fc->mode & FINCORE_PGOFF)
+			store_entry(fc, unsigned long, index);
+		if (fc->mode & FINCORE_PFN)
+			store_entry(fc, unsigned long, page_to_pfn(page));
+		if (fc->mode & FINCORE_PAGE_FLAGS)
+			store_entry(fc, unsigned long, stable_page_flags(page));
+	}
+}
+
+/*
+ * Traverse page cache tree. It's assumed that temporal buffer are zeroed
+ * in advance. Due to this, we don't have to store zero entry explicitly
+ * one-by-one and we just set fc->cursor to the position of the next
+ * on-memory page.
+ *
+ * Return value is the number of pages whose data is stored in fc->buffer.
+ */
+static long do_fincore(struct fincore_control *fc, int nr_pages)
+{
+	pgoff_t pgend = fc->pgstart + nr_pages;
+	struct radix_tree_iter iter;
+	void **slot;
+	long nr = 0;
+
+	fc->cursor = fc->buffer;
+
+	rcu_read_lock();
+restart:
+	radix_tree_for_each_slot(slot, &fc->mapping->page_tree, &iter,
+				 fc->pgstart) {
+		long jump;
+		struct page *page;
+
+		fc->scanned_offset = iter.index;
+		/* Handle holes */
+		jump = iter.index - fc->pgstart - nr;
+		if (jump) {
+			if (!(fc->mode & FINCORE_PGOFF)) {
+				if (iter.index < pgend) {
+					fc->cursor += jump * fc->width;
+					nr = iter.index - fc->pgstart;
+				} else {
+					/*
+					 * Fill remaining buffer as hole. Next
+					 * call should start at offset pgend.
+					 */
+					nr = nr_pages;
+					fc->scanned_offset = pgend - 1;
+					break;
+				}
+			}
+		}
+repeat:
+		page = radix_tree_deref_slot(slot);
+		if (unlikely(!page))
+			/*
+			 * No need to increment nr and fc->cursor, because next
+			 * iteration should detect hole and update them there.
+			 */
+			continue;
+		else 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;
+			}
+			__do_fincore(fc, page, iter.index);
+		} else {
+			if (!page_cache_get_speculative(page))
+				goto repeat;
+
+			/* Has the page moved? */
+			if (unlikely(page != *slot)) {
+				page_cache_release(page);
+				goto repeat;
+			}
+
+			__do_fincore(fc, page, iter.index);
+			page_cache_release(page);
+		}
+
+		if (++nr == nr_pages)
+			break;
+	}
+
+	if (!(fc->mode & FINCORE_PGOFF)) {
+		nr = nr_pages;
+		fc->scanned_offset = pgend - 1;
+	}
+
+	rcu_read_unlock();
+
+	return nr;
+}
+
+static inline bool fincore_validate_mode(int mode)
+{
+	if (mode & ~FINCORE_MODE_MASK)
+		return false;
+	if (!(!!(mode & FINCORE_BMAP) ^ !!(mode & FINCORE_LONGENTRY_MASK)))
+		return false;
+	return true;
+}
+
+#define FINCORE_LOOP_STEP	256L
+
+/*
+ * The fincore(2) system call
+ *
+ *  @fd:        file descriptor of the target file
+ *  @start:     starting address offset of the target file (in byte).
+ *              This should be aligned to page cache size.
+ *  @nr_pages:  the number of pages whose data is passed to userspace.
+ *  @mode       fincore mode flags to determine the entry's format
+ *  @vec        pointer of the userspace buffer. The size must be equal to or
+ *              larger than (@nr_pages * width), where width is the size of
+ *              each entry.
+ *  @extra      used to input/output additional information from/to userspace
+ *
+ * fincore() returns the memory residency status and additional info (like
+ * pfn and page flags) of the given file's pages.
+ *
+ * Depending on the fincore mode, caller can receive the different formatted
+ * information. See the comment on definition of FINCORE_*.
+ *
+ * Because the status of a page can change after fincore() checks it once,
+ * the returned vector may contain stale information.
+ *
+ * return values:
+ *  -EBADF:   @fd isn't a valid open file descriptor
+ *  -EFAULT:  @vec points to an illegal address
+ *  -EINVAL:  @start is unaligned to page cache size or is out of file range.
+ *            Or @nr_pages is non-positive. Or @mode is invalid.
+ *  0:        fincore() is successfully done
+ */
+SYSCALL_DEFINE6(fincore, int, fd, loff_t, start, long, nr_pages,
+		int, mode, unsigned char __user *, vec,
+		struct fincore_extra __user *, extra)
+{
+	long ret = 0;
+	long step;
+	long nr = 0;
+	long pages_to_eof;
+	int pc_shift = PAGE_CACHE_SHIFT;
+	struct fd f;
+
+	struct fincore_control fc = {
+		.mode	= mode,
+		.width	= sizeof(unsigned char),
+	};
+
+	if (start < 0 || nr_pages <= 0)
+		return -EINVAL;
+
+	if (!fincore_validate_mode(mode))
+		return -EINVAL;
+
+	f = fdget(fd);
+
+	if (is_file_hugepages(f.file))
+		pc_shift = huge_page_shift(hstate_file(f.file));
+
+	if (!IS_ALIGNED(start, 1 << pc_shift)) {
+		ret = -EINVAL;
+		goto fput;
+	}
+
+	/*
+	 * TODO: support /dev/mem, /proc/pid/mem for system/process wide
+	 * page survey, which would obsolete /proc/kpageflags, and
+	 * /proc/pid/pagemap.
+	 */
+	if (!S_ISREG(file_inode(f.file)->i_mode)) {
+		ret = -EBADF;
+		goto fput;
+	}
+
+	fc.pgstart = start >> pc_shift;
+	pages_to_eof = DIV_ROUND_UP(i_size_read(file_inode(f.file)),
+				    1UL << pc_shift) - fc.pgstart;
+	/* start is too large */
+	if (pages_to_eof <= 0) {
+		ret = -EINVAL;
+		goto fput;
+	}
+	/* Never go beyond the end of file */
+	fc.nr_pages = min(pages_to_eof, nr_pages);
+	fc.mapping = f.file->f_mapping;
+	if (mode & FINCORE_LONGENTRY_MASK)
+		fc.width = ((mode & FINCORE_PGOFF ? 1 : 0) +
+			    (mode & FINCORE_PFN ? 1 : 0) +
+			    (mode & FINCORE_PAGE_FLAGS ? 1 : 0)
+			) * sizeof(unsigned long);
+
+	if (!access_ok(VERIFY_WRITE, vec, nr_pages * fc.width)) {
+		ret = -EFAULT;
+		goto fput;
+	}
+
+	step = min(fc.nr_pages, FINCORE_LOOP_STEP);
+
+	fc.buffer_size = step * fc.width;
+	fc.buffer = kmalloc(fc.buffer_size, GFP_TEMPORARY);
+	if (!fc.buffer) {
+		ret = -ENOMEM;
+		goto fput;
+	}
+
+	while (fc.nr_pages > 0) {
+		memset(fc.buffer, 0, fc.buffer_size);
+		ret = do_fincore(&fc, min(step, fc.nr_pages));
+		/* Reached the end of the file */
+		if (ret == 0)
+			break;
+		if (ret < 0)
+			break;
+		if (copy_to_user(vec + nr * fc.width,
+				 fc.buffer, ret * fc.width)) {
+			ret = -EFAULT;
+			break;
+		}
+		fc.nr_pages -= ret;
+		fc.pgstart = fc.scanned_offset + 1;
+		nr += ret;
+	}
+
+	kfree(fc.buffer);
+
+	if (extra)
+		__put_user(nr, &extra->nr_entries);
+
+fput:
+	fdput(f);
+	return ret;
+}
-- 
1.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related

* [PATCH v3 2/3] selftests/fincore: add test code for fincore()
From: Naoya Horiguchi @ 2014-07-07 18:00 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Konstantin Khlebnikov, Wu Fengguang, Arnaldo Carvalho de Melo,
	Borislav Petkov, Kirill A. Shutemov, Johannes Weiner,
	Rusty Russell, David Miller, Andres Freund, linux-kernel,
	linux-mm, Dave Hansen, Christoph Hellwig, Dave Chinner,
	Michael Kerrisk, Linux API, Naoya Horiguchi
In-Reply-To: <1404756006-23794-1-git-send-email-n-horiguchi@ah.jp.nec.com>

This patch adds simple test programs for fincore(), which contains the
following testcase:
  - test_smallfile_bytemap
  - test_smallfile_pfn
  - test_smallfile_multientry
  - test_smallfile_pfn_skiphole
  - test_largefile_pfn
  - test_largefile_pfn_offset
  - test_largefile_pfn_overrun
  - test_largefile_pfn_skiphole
  - test_tmpfs_pfn
  - test_hugetlb_pfn
  - test_invalid_start_address
  - test_invalid_len
  - test_invalid_mode
  - test_unaligned_start_address_hugetlb

ChangeLog v3:
- remove pagecache tag stuff

ChangeLog v2:
- include uapi/linux/pagecache.h
- add testcase test_invalid_start_address and test_invalid_len
- other small changes to adjust for the kernel's changes

Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
 tools/testing/selftests/Makefile                   |   1 +
 tools/testing/selftests/fincore/Makefile           |  31 ++
 .../selftests/fincore/create_hugetlbfs_file.c      |  49 +++
 tools/testing/selftests/fincore/fincore.c          | 153 +++++++++
 tools/testing/selftests/fincore/run_fincoretests   | 361 +++++++++++++++++++++
 5 files changed, 595 insertions(+)
 create mode 100644 tools/testing/selftests/fincore/Makefile
 create mode 100644 tools/testing/selftests/fincore/create_hugetlbfs_file.c
 create mode 100644 tools/testing/selftests/fincore/fincore.c
 create mode 100644 tools/testing/selftests/fincore/run_fincoretests

diff --git v3.16-rc3.orig/tools/testing/selftests/Makefile v3.16-rc3/tools/testing/selftests/Makefile
index e66e710cc595..91e817b87a9e 100644
--- v3.16-rc3.orig/tools/testing/selftests/Makefile
+++ v3.16-rc3/tools/testing/selftests/Makefile
@@ -11,6 +11,7 @@ TARGETS += vm
 TARGETS += powerpc
 TARGETS += user
 TARGETS += sysctl
+TARGETS += fincore
 
 all:
 	for TARGET in $(TARGETS); do \
diff --git v3.16-rc3.orig/tools/testing/selftests/fincore/Makefile v3.16-rc3/tools/testing/selftests/fincore/Makefile
new file mode 100644
index 000000000000..ab4361c70da5
--- /dev/null
+++ v3.16-rc3/tools/testing/selftests/fincore/Makefile
@@ -0,0 +1,31 @@
+# Makefile for vm selftests
+
+uname_M := $(shell uname -m 2>/dev/null || echo not)
+ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/i386/)
+ifeq ($(ARCH),i386)
+        ARCH := X86
+        CFLAGS := -DCONFIG_X86_32 -D__i386__
+endif
+ifeq ($(ARCH),x86_64)
+        ARCH := X86
+        CFLAGS := -DCONFIG_X86_64 -D__x86_64__
+endif
+
+CC = $(CROSS_COMPILE)gcc
+CFLAGS = -Wall
+CFLAGS += -I../../../../arch/x86/include/generated/
+CFLAGS += -I../../../../include/
+CFLAGS += -I../../../../usr/include/
+CFLAGS += -I../../../../arch/x86/include/
+
+BINARIES = fincore create_hugetlbfs_file
+
+all: $(BINARIES)
+%: %.c
+	$(CC) $(CFLAGS) -o $@ $^
+
+run_tests: all
+	@/bin/sh ./run_fincoretests || (echo "fincoretests: [FAIL]"; exit 1)
+
+clean:
+	$(RM) $(BINARIES)
diff --git v3.16-rc3.orig/tools/testing/selftests/fincore/create_hugetlbfs_file.c v3.16-rc3/tools/testing/selftests/fincore/create_hugetlbfs_file.c
new file mode 100644
index 000000000000..a46ccf0af5f2
--- /dev/null
+++ v3.16-rc3/tools/testing/selftests/fincore/create_hugetlbfs_file.c
@@ -0,0 +1,49 @@
+#define _GNU_SOURCE 1
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdlib.h>
+
+#define err(x) (perror(x), exit(1))
+
+unsigned long default_hugepage_size(void)
+{
+	unsigned long hps = 0;
+	char *line = NULL;
+	size_t linelen = 0;
+	FILE *f = fopen("/proc/meminfo", "r");
+	if (!f)
+		err("open /proc/meminfo");
+	while (getline(&line, &linelen, f) > 0) {
+		if (sscanf(line, "Hugepagesize:	%lu kB", &hps) == 1) {
+			hps <<= 10;
+			break;
+		}
+	}
+	free(line);
+	return hps;
+}
+
+int main(int argc, char **argv)
+{
+	int ret;
+	int fd;
+	char *p;
+	unsigned long hpsize = default_hugepage_size();
+	fd = open(argv[1], O_RDWR|O_CREAT);
+	if (fd == -1)
+		err("open");
+	p = mmap(NULL, 10 * hpsize, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
+	if (p == (void *)-1)
+		err("mmap");
+	memset(p, 'a', 3 * hpsize);
+	memset(p + 7 * hpsize, 'a', 3 * hpsize - 1);
+	ret = close(fd);
+	if (ret == -1)
+		err("close");
+	return 0;
+}
diff --git v3.16-rc3.orig/tools/testing/selftests/fincore/fincore.c v3.16-rc3/tools/testing/selftests/fincore/fincore.c
new file mode 100644
index 000000000000..786a985ba5bd
--- /dev/null
+++ v3.16-rc3/tools/testing/selftests/fincore/fincore.c
@@ -0,0 +1,153 @@
+/*
+ * fincore(2) test program
+ */
+
+#define _GNU_SOURCE 1
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <getopt.h>
+#include <assert.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <uapi/linux/fincore.h>
+
+#define err(x) (perror(x), exit(1))
+
+void usage(char *str)
+{
+	printf(
+		"Usage: %s [-s start] [-l len] [-m mode] [-p pagesize] file\n"
+		"  -s: start offset (in bytes)\n"
+		"  -l: length to scan (in bytes)\n"
+		"  -m: fincore mode\n"
+		"  -p: set page size (for hugepage)\n"
+		"  -h: show this message\n"
+		, str);
+	exit(EXIT_SUCCESS);
+}
+
+static void show_fincore_buffer(long start, long nr_pages, int records_per_page,
+				int mode, unsigned char *buf)
+{
+	int i, j;
+	unsigned char *curuc = (unsigned char *)buf;
+	unsigned long *curul = (unsigned long *)buf;
+
+	for (i = 0; i < nr_pages; i++) {
+		j = 0;
+		if (mode & FINCORE_BMAP)
+			printf("buffer: 0x%lx\t%d", start + i, curuc[i + j]);
+		else if (mode & (FINCORE_LONGENTRY_MASK)) {
+			if (mode & FINCORE_PGOFF)
+				printf("buffer: 0x%lx",
+				       curul[i * records_per_page + (j++)]);
+			else
+				printf("buffer: 0x%lx", start + i);
+			if (mode & FINCORE_PFN)
+				printf("\t0x%lx",
+				       curul[i * records_per_page + (j++)]);
+			if (mode & FINCORE_PAGE_FLAGS)
+				printf("\t0x%lx",
+				       curul[i * records_per_page + (j++)]);
+		}
+		printf("\n");
+	}
+}
+
+int main(int argc, char *argv[])
+{
+	char c;
+	int fd;
+	int ret;
+	int mode = FINCORE_PFN;
+	int width = sizeof(unsigned char);
+	int records_per_page = 1;
+	long pagesize = sysconf(_SC_PAGESIZE);
+	long nr_pages;
+	unsigned long start = 0;
+	int len_not_given = 1;
+	long len = 0;
+	long buffer_size = 0;
+	unsigned char *buf;
+	struct stat stat;
+	int extra = 0;
+	struct fincore_extra fe = {};
+
+	while ((c = getopt(argc, argv, "s:l:m:p:et:b:h")) != -1) {
+		switch (c) {
+		case 's':
+			start = strtoul(optarg, NULL, 0);
+			break;
+		case 'l':
+			len_not_given = 0;
+			len = strtol(optarg, NULL, 0);
+			break;
+		case 'm':
+			mode = strtoul(optarg, NULL, 0);
+			break;
+		case 'p':
+			pagesize = strtoul(optarg, NULL, 0);
+			break;
+		case 'e':
+			extra = 1;
+			break;
+		case 'b':
+			buffer_size = strtoul(optarg, NULL, 0);
+			break;
+		case 'h':
+		default:
+			usage(argv[0]);
+		}
+	}
+
+	fd = open(argv[optind], O_RDWR);
+	if (fd == -1)
+		err("open failed.");
+
+	/* scan to the end of file by default */
+	if (len_not_given) {
+		ret = fstat(fd, &stat);
+		if (ret == -1)
+			err("fstat failed.");
+		len = stat.st_size - start;
+	}
+
+	if (mode & FINCORE_LONGENTRY_MASK) {
+		records_per_page = ((mode & FINCORE_PGOFF ? 1 : 0) +
+				    (mode & FINCORE_PFN ? 1 : 0) +
+				    (mode & FINCORE_PAGE_FLAGS ? 1 : 0)
+			);
+		width = records_per_page * sizeof(unsigned long);
+	}
+
+	nr_pages = ((len + pagesize - 1) & (~(pagesize - 1))) / pagesize;
+	printf("start:0x%lx, len:%ld, mode:%d, pagesize:0x%lx,\n"
+	       "buffer_size:0x%lx, nr_pages:0x%lx, width:%d\n",
+	       start, len, mode, pagesize, buffer_size, nr_pages, width);
+	buf = malloc(buffer_size > 0 ? buffer_size : nr_pages * width);
+	if (!buf)
+		err("malloc");
+
+	ret = syscall(__NR_fincore, fd, start, nr_pages, mode, buf,
+		      extra ? &fe : NULL);
+	if (ret < 0)
+		err("fincore");
+	/*
+	 * print buffer to stdout, and parse it later for validation check.
+	 * fincore() returns the number of entries written to the buffer.
+	 */
+	show_fincore_buffer(start / pagesize, nr_pages, records_per_page,
+			    mode, buf);
+
+	if (extra)
+		printf("fincore_extra->nr_entries: %ld\n", fe.nr_entries);
+
+	ret = close(fd);
+	if (ret < 0)
+		err("close");
+	return 0;
+}
diff --git v3.16-rc3.orig/tools/testing/selftests/fincore/run_fincoretests v3.16-rc3/tools/testing/selftests/fincore/run_fincoretests
new file mode 100644
index 000000000000..881a1ce1fbee
--- /dev/null
+++ v3.16-rc3/tools/testing/selftests/fincore/run_fincoretests
@@ -0,0 +1,361 @@
+#!/bin/bash
+
+WDIR=./fincore_work
+mkdir $WDIR 2> /dev/null
+TMPF=`mktemp --tmpdir=$WDIR -d`
+export LANG=C
+
+sysctl -q vm.nr_hugepages=50
+
+#
+# common routines
+#
+abort() {
+    echo "Test abort"
+    exit 1
+}
+
+create_small_file() {
+    dd if=/dev/urandom of=$WDIR/smallfile bs=4096 count=4 > /dev/null 2>&1
+    dd if=/dev/urandom of=$WDIR/smallfile bs=4096 count=4 seek=8> /dev/null 2>&1
+    date >> $WDIR/smallfile
+    sync
+}
+
+create_large_file() {
+    dd if=/dev/urandom of=$WDIR/largefile bs=4096 count=384 > /dev/null 2>&1
+    dd if=/dev/urandom of=$WDIR/largefile bs=4096 count=384 seek=640> /dev/null 2>&1
+    sync
+}
+
+create_tmpfs_file() {
+    dd if=/dev/urandom of=/tmp/tmpfile bs=4096 count=4 > /dev/null 2>&1
+    dd if=/dev/urandom of=/tmp/tmpfile bs=4096 count=4 seek=8> /dev/null 2>&1
+    date >> /tmp/tmpfile
+    sync
+}
+
+create_hugetlb_file() {
+    if mount | grep $WDIR/hugepages > /dev/null ; then
+        echo "$WDIR/hugepages already mounted"
+    else
+        mkdir -p $WDIR/hugepages 2> /dev/null
+        mount -t hugetlbfs none $WDIR/hugepages 2> /dev/null
+        if [ $? -ne 0 ] ; then
+            echo "Failed to mount hugetlbfs" >&2
+            return 1
+        fi
+    fi
+    local hptotal=$(grep HugePages_Total: /proc/meminfo | tr -s ' ' | cut -f2 -d' ')
+    if [ "$hptotal" -lt 10 ] ; then
+        echo "Hugepage pool size need to be >= 10" >&2
+        return 1
+    fi
+    ./create_hugetlbfs_file $WDIR/hugepages/file
+    if [ $? -ne 0 ] ; then
+        echo "Failed to create hugetlb file" >&2
+        return 1
+    fi
+    return 0;
+}
+
+get_buffer() {
+    cat "$1" | grep '^buffer:' | cut -f 2- -d ' '
+}
+
+get_fincore_extra_nr_entries() {
+    cat "$1" | grep '^fincore_extra->nr_entries' | cut -f 2 -d ' '
+}
+
+nr_of_exist_should_be() {
+    if [ "$1" -ne "$2" ] ; then
+        echo "[FAIL] $3: Number of on-memory pages should be $1, but got $2"
+        return 1
+    fi
+    return 0
+}
+
+nr_of_nonexist_should_be() {
+    if [ "$1" -ne "$2" ] ; then
+        echo "[FAIL] $3: Number of hole entries should be $1, but got $2"
+        return 1
+    fi
+    return 0
+}
+
+nr_of_valid_entries_should_be() {
+    if [ "$1" -ne "$2" ] ; then
+        echo "[FAIL] $3: Number of valid entries should be $1, but got $2"
+        return 1
+    fi
+    return 0
+}
+
+check_einval() {
+    grep "fincore: Invalid argument" "$1" > /dev/null
+}
+
+#
+# Testcases
+#
+test_smallfile_bytemap() {
+    local exist
+    local nonexist
+    create_small_file
+
+    ./fincore -m 0x1 $WDIR/smallfile > $TMPF/$FUNCNAME 2>&1
+    exist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep 1 | wc -l)
+    nonexist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep 0 | wc -l)
+    nr_of_exist_should_be 9 "$exist" "$FUNCNAME" || return 1
+    nr_of_nonexist_should_be 4 "$nonexist" "$FUNCNAME" || return 1
+    echo "[PASS] $FUNCNAME"
+}
+
+test_smallfile_pfn() {
+    local exist
+    local nonexist
+    create_small_file
+
+    ./fincore -m 0x4 $WDIR/smallfile > $TMPF/$FUNCNAME 2>&1
+    exist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep -v 0x0 | wc -l)
+    nonexist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep 0x0 | wc -l)
+    nr_of_exist_should_be 9 "$exist" "$FUNCNAME" || return 1
+    nr_of_nonexist_should_be 4 "$nonexist" "$FUNCNAME" || return 1
+    echo "[PASS] $FUNCNAME"
+}
+
+test_smallfile_multientry() {
+    local exist
+    local nonexist
+    create_small_file
+
+    ./fincore -m 0xc -e $WDIR/smallfile > $TMPF/$FUNCNAME 2>&1
+    exist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2,3 | grep -vP "0x0\t0x0" | wc -l)
+    nonexist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2,3 | grep -P "0x0\t0x0" | wc -l)
+    nr_of_exist_should_be 9 "$exist" "$FUNCNAME" || return 1
+    nr_of_nonexist_should_be 4 "$nonexist" "$FUNCNAME" || return 1
+    echo "[PASS] $FUNCNAME"
+}
+
+test_smallfile_pfn_skiphole() {
+    local exist
+    local nonexist
+    local nr_entries
+    create_small_file
+
+    ./fincore -m 0x6 -e $WDIR/smallfile > $TMPF/$FUNCNAME 2>&1
+    exist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep -v 0x0 | wc -l)
+    nonexist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep 0x0 | wc -l)
+    nr_entries=$(get_fincore_extra_nr_entries $TMPF/$FUNCNAME)
+    nr_of_exist_should_be 9 "$exist" "$FUNCNAME" || return 1
+    nr_of_nonexist_should_be 4 "$nonexist" "$FUNCNAME" || return 1
+    nr_of_valid_entries_should_be 9 "$nr_entries" "$FUNCNAME" || return 1
+    echo "[PASS] $FUNCNAME"
+}
+
+# in-kernel function sys_fincore() repeat copy_to_user() per 256 entries,
+# so testing for large file is meaningful testcase.
+test_largefile_pfn() {
+    local exist
+    local nonexist
+    create_large_file
+
+    ./fincore -m 0x4 -e $WDIR/largefile > $TMPF/$FUNCNAME 2>&1
+    exist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep -v 0x0 | wc -l)
+    nonexist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep 0x0 | wc -l)
+    nr_of_exist_should_be 768 "$exist" "$FUNCNAME" || return 1
+    nr_of_nonexist_should_be 256 "$nonexist" "$FUNCNAME" || return 1
+    echo "[PASS] $FUNCNAME"
+}
+
+test_largefile_pfn_offset() {
+    local exist
+    local nonexist
+    create_large_file
+
+    ./fincore -m 0x4 -s 0x80000 $WDIR/largefile > $TMPF/$FUNCNAME 2>&1
+    exist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep -v 0x0 | wc -l)
+    nonexist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep 0x0 | wc -l)
+    nr_of_exist_should_be 640 "$exist" "$FUNCNAME" || return 1
+    nr_of_nonexist_should_be 256 "$nonexist" "$FUNCNAME" || return 1
+    echo "[PASS] $FUNCNAME"
+}
+
+test_largefile_pfn_overrun() {
+    local exist
+    local nonexist
+    local nr_entries
+    create_large_file
+
+    ./fincore -m 0x4 -s 0x80000 -l 0x400000 -e $WDIR/largefile > $TMPF/$FUNCNAME 2>&1
+    exist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep -v 0x0 | wc -l)
+    nonexist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep 0x0 | wc -l)
+    nr_entries=$(get_fincore_extra_nr_entries $TMPF/$FUNCNAME)
+    nr_of_exist_should_be 640 "$exist" "$FUNCNAME" || return 1
+    nr_of_nonexist_should_be 384 "$nonexist" "$FUNCNAME" || return 1
+    nr_of_valid_entries_should_be 896 "$nr_entries" "$FUNCNAME" || return 1
+    echo "[PASS] $FUNCNAME"
+}
+
+test_largefile_pfn_skiphole() {
+    local exist
+    local nonexist
+    create_large_file
+
+    ./fincore -m 0x6 -s 0x100000 -l 0x102000 -e $WDIR/largefile > $TMPF/$FUNCNAME 2>&1
+    exist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep -v 0x0 | wc -l)
+    nonexist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep 0x0 | wc -l)
+    nr_entries=$(get_fincore_extra_nr_entries $TMPF/$FUNCNAME)
+    nr_of_exist_should_be 258 "$exist" "$FUNCNAME" || return 1
+    nr_of_nonexist_should_be 0 "$nonexist" "$FUNCNAME" || return 1
+    nr_of_valid_entries_should_be 258 "$nr_entries" "$FUNCNAME" || return 1
+    echo "[PASS] $FUNCNAME"
+}
+
+test_tmpfs_pfn() {
+    local exist
+    local nonexist
+    create_tmpfs_file
+
+    ./fincore -m 0x4 /tmp/tmpfile > $TMPF/$FUNCNAME 2>&1
+    exist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep -v 0x0 | wc -l)
+    nonexist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep 0x0 | wc -l)
+    nr_of_exist_should_be 9 "$exist" "$FUNCNAME" || return 1
+    nr_of_nonexist_should_be 4 "$nonexist" "$FUNCNAME" || return 1
+    echo "[PASS] $FUNCNAME"
+}
+
+test_hugetlb_pfn() {
+    local exist
+    local nonexist
+    local exitcode=0
+    create_hugetlb_file
+    if [ $? -ne 0 ] ; then
+        echo "[FAIL] $FUNCNAME: fail to create a file on hugetlbfs"
+        return 1
+    fi
+    local hugepagesize=$[$(cat /proc/meminfo  | grep Hugepagesize: | tr -s ' ' | cut -f2 -d' ') * 1024]
+    ./fincore -p $hugepagesize -m 0x4 -e $WDIR/hugepages/file > $TMPF/$FUNCNAME 2>&1
+    exist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep -v 0x0 | wc -l)
+    nonexist=$(get_buffer $TMPF/$FUNCNAME | cut -f 2 | grep 0x0 | wc -l)
+    nr_entries=$(get_fincore_extra_nr_entries $TMPF/$FUNCNAME)
+    nr_of_exist_should_be 6 "$exist" "$FUNCNAME" || return 1
+    nr_of_nonexist_should_be 4 "$nonexist" "$FUNCNAME" || return 1
+    nr_of_valid_entries_should_be 10 "$nr_entries" "$FUNCNAME" || return 1
+    rm -rf $WDIR/hugepages/file
+    echo "[PASS] $FUNCNAME"
+}
+
+test_invalid_start_address() {
+    create_small_file
+    ./fincore -m 0x4 -s -0x4000 -l 1 -e $WDIR/smallfile > $TMPF/$FUNCNAME 2>&1
+    if [ $? -eq 0 ] || ! check_einval $TMPF/$FUNCNAME ; then
+        echo "[FAIL] $FUNCNAME: negative start is invalid"
+        return 1
+    fi
+    ./fincore -m 0x4 -s 0x100000 -l 1 -e $WDIR/smallfile > $TMPF/$FUNCNAME 2>&1
+    if [ $? -eq 0 ] || ! check_einval $TMPF/$FUNCNAME ; then
+        echo "[FAIL] $FUNCNAME: too large start is invalid"
+        return 1
+    fi
+    ./fincore -m 0x4 -s 0x30 $WDIR/smallfile > $TMPF/$FUNCNAME 2>&1
+    if [ $? -eq 0 ] || ! check_einval $TMPF/$FUNCNAME ; then
+        echo "[FAIL] $FUNCNAME: fincore should fail for unaligned start address"
+        return 1
+    fi
+    echo "[PASS] $FUNCNAME"
+}
+
+test_invalid_len() {
+    create_small_file
+    ./fincore -m 0x4 -l 0 $WDIR/smallfile > $TMPF/$FUNCNAME 2>&1
+    if [ $? -eq 0 ] || ! check_einval $TMPF/$FUNCNAME ; then
+        echo "[FAIL] $FUNCNAME: zero len is invalid"
+        return 1
+    fi
+    ./fincore -m 0x4 -l -10 $WDIR/smallfile > $TMPF/$FUNCNAME 2>&1
+    if [ $? -eq 0 ] || ! check_einval $TMPF/$FUNCNAME ; then
+        echo "[FAIL] $FUNCNAME: negative len is invalid"
+        return 1
+    fi
+    echo "[PASS] $FUNCNAME"
+}
+
+test_invalid_mode() {
+    create_small_file
+    ./fincore -m 0x0 $WDIR/smallfile > $TMPF/$FUNCNAME 2>&1
+    if [ $? -eq 0 ] || ! check_einval $TMPF/$FUNCNAME ; then
+        echo "[FAIL] $FUNCNAME: mode == NULL is invalid mode"
+        return 1
+    fi
+    ./fincore -m 0x5 $WDIR/smallfile > $TMPF/$FUNCNAME 2>&1
+    if [ $? -eq 0 ] || ! check_einval $TMPF/$FUNCNAME ; then
+        echo "[FAIL] $FUNCNAME: mode == (FINCORE_BMAP|FINCORE_PFN) is invalid mode"
+        return 1
+    fi
+    ./fincore -m 0x3 $WDIR/smallfile > $TMPF/$FUNCNAME 2>&1
+    if [ $? -eq 0 ] || ! check_einval $TMPF/$FUNCNAME ; then
+        echo "[FAIL] $FUNCNAME: mode == (FINCORE_BMAP|FINCORE_PGOFF) is invalid mode"
+        return 1
+    fi
+    ./fincore -m 0x6 $WDIR/smallfile > $TMPF/$FUNCNAME 2>&1
+    if [ $? -ne 0 ] ; then
+        echo "[FAIL] $FUNCNAME: mode == (FINCORE_PGOFF|FINCORE_PFN) is valid mode"
+        return 1
+    fi
+    ./fincore -m 0x2 $WDIR/smallfile > $TMPF/$FUNCNAME 2>&1
+    if [ $? -ne 0 ] ; then
+        echo "[FAIL] $FUNCNAME: mode == (FINCORE_PGOFF) is valid mode"
+        return 1
+    fi
+    ./fincore -m 0x1004 $WDIR/smallfile > $TMPF/$FUNCNAME 2>&1
+    if [ $? -eq 0 ] || ! check_einval $TMPF/$FUNCNAME ; then
+        echo "[FAIL] $FUNCNAME: mode == (Unknown|FINCORE_PFN) is invalid mode"
+        return 1
+    fi
+    echo "[PASS] $FUNCNAME"
+}
+
+test_unaligned_start_address_hugetlb() {
+    local exist
+    local nonexist
+    local exitcode=0
+    create_hugetlb_file
+    if [ $? -ne 0 ] ; then
+        echo "[FAIL] $FUNCNAME: fail to create a file on hugetlbfs"
+        return 1
+    fi
+    local hugepagesize=$[$(cat /proc/meminfo  | grep Hugepagesize: | tr -s ' ' | cut -f2 -d' ') * 1024]
+    ./fincore -p $hugepagesize -m 0x4 -s 0x1000 $WDIR/hugepages/file > $TMPF/$FUNCNAME 2>&1
+    if [ $? -eq 0 ] || ! check_einval $TMPF/$FUNCNAME ; then
+        echo "[FAIL] $FUNCNAME: fincore should fail for page-unaligned start address"
+        return 1
+    fi
+    ./fincore -p $hugepagesize -m 0x4 -s $hugepagesize $WDIR/hugepages/file > $TMPF/$FUNCNAME 2>&1
+    if [ $? -ne 0 ] ; then
+        echo "[FAIL] $FUNCNAME: fincore should pass for hugepage-aligned start address"
+        return 1
+    fi
+    echo "[PASS] $FUNCNAME"
+}
+
+test_smallfile_bytemap                 || abort
+test_smallfile_pfn                     || abort
+test_smallfile_multientry              || abort
+test_smallfile_pfn_skiphole            || abort
+test_largefile_pfn                     || abort
+test_largefile_pfn_offset              || abort
+test_largefile_pfn_overrun             || abort
+test_largefile_pfn_skiphole            || abort
+test_tmpfs_pfn                         || abort
+test_hugetlb_pfn                       || abort
+test_invalid_start_address             || abort
+test_invalid_len                       || abort
+test_invalid_mode                      || abort
+test_unaligned_start_address_hugetlb   || abort
+
+# cleanup
+rm -rf $WDIR/hugepages/file
+umount $WDIR/hugepages > /dev/null 2>&1
+
+exit 0
-- 
1.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related

* [PATCH v3 3/3] man2/fincore.2: document general description about fincore(2)
From: Naoya Horiguchi @ 2014-07-07 18:00 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Konstantin Khlebnikov, Wu Fengguang, Arnaldo Carvalho de Melo,
	Borislav Petkov, Kirill A. Shutemov, Johannes Weiner,
	Rusty Russell, David Miller, Andres Freund, linux-kernel,
	linux-mm, Dave Hansen, Christoph Hellwig, Dave Chinner,
	Michael Kerrisk, Linux API, Naoya Horiguchi
In-Reply-To: <1404756006-23794-1-git-send-email-n-horiguchi@ah.jp.nec.com>

This patch adds the man page for the new system call fincore(2).

ChangeLog v3:
- remove page cache tag stuff

Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
 man2/fincore.2 | 348 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 348 insertions(+)
 create mode 100644 man2/fincore.2

diff --git v3.16-rc3.orig/man2/fincore.2 v3.16-rc3/man2/fincore.2
new file mode 100644
index 000000000000..b4893084cd36
--- /dev/null
+++ v3.16-rc3/man2/fincore.2
@@ -0,0 +1,348 @@
+.\" Copyright (C) 2014 Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
+.\"
+.\" %%%LICENSE_START(VERBATIM)
+.\" Permission is granted to make and distribute verbatim copies of this
+.\" manual provided the copyright notice and this permission notice are
+.\" preserved on all copies.
+.\"
+.\" Permission is granted to copy and distribute modified versions of this
+.\" manual under the conditions for verbatim copying, provided that the
+.\" entire resulting derived work is distributed under the terms of a
+.\" permission notice identical to this one.
+.\"
+.\" Since the Linux kernel and libraries are constantly changing, this
+.\" manual page may be incorrect or out-of-date.  The author(s) assume no
+.\" responsibility for errors or omissions, or for damages resulting from
+.\" the use of the information contained herein.  The author(s) may not
+.\" have taken the same level of care in the production of this manual,
+.\" which is licensed free of charge, as they might when working
+.\" professionally.
+.\"
+.\" Formatted or processed versions of this manual, if unaccompanied by
+.\" the source, must acknowledge the copyright and authors of this work.
+.\" %%%LICENSE_END
+.\"
+.TH FINCORE 2 2014-07-07 "Linux" "Linux Programmer's Manual"
+.SH NAME
+fincore \- get page cache information
+.SH SYNOPSIS
+.nf
+.B #include <sys/fincore.h>
+.B #include <sys/kernel-page-flags.h>
+.sp
+.BI "int fincore(int " fd ", loff_t " start ", long " nr_pages ", int " mode ,
+.BI "            unsigned char *" vec ", struct fincore_extra *" extra );
+.fi
+.SH DESCRIPTION
+.BR fincore ()
+extracts information of in-core data of (i.e., page caches for)
+the file referred to by the file descriptor
+.IR fd .
+The kernel scans over the page cache tree,
+starting at the in-file offset
+.I start
+(in bytes) until
+.I nr_pages
+entries in the userspace buffer pointed to by
+.IR vec
+are filled with the page cache's data,
+or until the scan reached the end of the file.
+The format of each entry stored in
+.I vec
+depends on the
+.IR mode .
+The extra argument
+.I extra
+is used to pass the additional data between the kernel and the userspace.
+This is optional, so you may set
+.I extra
+to NULL if unnecessary.
+The structure
+.I fincore_extra
+is defined like:
+.in +4n
+.nf
+
+struct fincore_extra {
+        unsigned long nr_entries;
+};
+
+.fi
+.in
+The field
+.I nr_entries
+is an output parameter, set to the number of valid entries stored in
+.IR vec
+by the kernel on return.
+
+The
+.I start
+argument must be aligned to the page cache size boundary.
+In most cases, it's the page size boundary,
+but if called for a hugetlbfs file,
+the page cache size is the size of the hugepage associated with the file,
+so
+.I start
+must be aligned to the hugepage size boundary.
+
+The
+.I mode
+argument determines the data format of each entry in the user buffer
+.IR vec :
+.TP
+.B FINCORE_BMAP (0)
+In this mode,
+1 byte vector is stored in
+.I vec
+on return.
+The least significant bit of each byte is set if the corresponding page
+is currently resident in memory, and is cleared otherwise.
+(The other bits in each byte are undefined and reserved for future use.)
+.LP
+Any of the following flags are to be set to add an 8 byte field in each entry.
+You can set any of these flags at the same time, although you can't set
+FINCORE_BMAP combined with these 8 byte field flags.
+.TP
+.B FINCORE_PGOFF (1)
+This flag indicates that each entry contains a page offset field.
+With this information, you don't have to get data for hole range,
+so they are not stored in
+.I vec
+any longer.
+Note that if you call with this flag, you can't predict how many valid
+entries are stored in the buffer on return. So the
+.I nr_entries
+field in
+.I struct fincore_extra
+is useful if you want it.
+.TP
+.B FINCORE_PFN (2)
+This flag indicates that each entry contains a page frame number
+(i.e., physical address in page size unit) field.
+.TP
+.B FINCORE_PAGE_FLAGS (3)
+This flag indicates that each entry contains a page flags field.
+See KERNEL PAGE FLAGS section for more detail about each bit.
+.LP
+The size of the buffer
+.I vec
+must be at least
+.I nr_pages
+bytes if FINCORE_BMAP is set,
+and
+.I (8*n*nr_pages)
+bytes if some of the 8 byte field flags are set,
+where
+.I n
+means the number of 8 byte field flags being set.
+When multiple 8 byte field flags are set, the order of data in each
+entry is the same as one in the bit definition order (shown above
+as the numbers in parentheses.)
+For example, when you set FINCORE_PGOFF (bit 1) and FINCORE_PAGE_FLAGS (bit 3,)
+the first 8 bytes in an entry is the page offset,
+and the second 8 bytes is the page flags.
+
+Note that the information returned by the kernel is just a snapshot:
+pages which are not locked in memory can be freed at any moment, and
+the contents of
+.I vec
+may already be stale by the time the caller refers to the data.
+.SH KERNEL PAGE FLAGS
+.TP
+.B KPF_LOCKED (0)
+The lock on the page is held, suggesting that the kernel may be
+doing some page-related sensitive operation.
+.TP
+.B KPF_ERROR (1)
+The page was affected by IO error or memory error, so the data on the page
+might be lost.
+.TP
+.B KPF_REFERENCED (2)
+This page flag is used to control the page reclaim, combined with KPF_ACTIVE.
+.TP
+.B KPF_UPTODATE (3)
+The page has valid contents.
+.TP
+.B KPF_DIRTY (4)
+The data of the page is not synchronized with one on the backing storage.
+.TP
+.B KPF_LRU (5)
+The page is linked to one of the LRU (Least Recently Update) lists.
+.TP
+.B KPF_ACTIVE (6)
+The page is linked to one of the active LRU lists.
+.TP
+.B KPF_SLAB (7)
+The page is used to construct slabs, which is managed by the kernel
+to allocate various types of kernel objects.
+.TP
+.B KPF_WRITEBACK (8)
+The page is under the writeback operation.
+.TP
+.B KPF_RECLAIM (9)
+The page is under the page reclaim operation.
+.TP
+.B KPF_BUDDY (10)
+The page is under the buddy allocator as a free page. Note that this flag
+is only set to the first page of the "buddy" (i.e., the chunk of free pages.)
+.TP
+.B KPF_MMAP (11)
+The page is mapped to the virtual address space of some processes.
+.TP
+.B KPF_ANON (12)
+The page is anonymous page.
+.TP
+.B KPF_SWAPCACHE (13)
+The page has its own copy of the data on the swap device.
+.TP
+.B KPF_SWAPBACKED (14)
+The page can be swapped out. This flag is set on anonymous pages,
+tmpfs pages, or shmem page.
+.TP
+.B KPF_COMPOUND_HEAD (15)
+The page belongs to a high-order page, and is its first page.
+.TP
+.B KPF_COMPOUND_TAIL (16)
+The page belongs to a high-order page, and is not its first page.
+.TP
+.B KPF_HUGE (17)
+The page is used to construct a hugepage.
+.TP
+.B KPF_UNEVICTABLE (18)
+The page is prevented from being freed.
+This is caused by
+.BR mlock (2)
+or shared memory with
+.BR SHM_LOCK .
+.TP
+.B KPF_HWPOISON (19)
+The page is affected by a hardware error on the memory.
+.TP
+.B KPF_NOPAGE (20)
+This is a pseudo page flag which indicates that the given address
+has no struct page backed.
+.TP
+.B KPF_KSM (21)
+The page is a shared page governed by KSM (Kernel Shared Merging.)
+.TP
+.B KPF_THP (22)
+The page is used to construct a transparent hugepage.
+.LP
+.SH RETURN VALUE
+On success,
+.BR fincore ()
+returns 0.
+On error, \-1 is returned, and
+.I errno
+is set appropriately.
+.SH ERRORS
+.TP
+.B EBADF
+.I fd
+is not a valid file descriptor.
+.TP
+.B EFAULT
+.I vec
+points to an invalid address.
+.TP
+.B EINVAL
+.I start
+is unaligned to page cache size or is out-of-range
+(negative or larger than the file size.)
+Or
+.I nr_pages
+is not a positive value.
+Or
+.I mode
+contained a undefined flag, or contained no flag,
+or contained both of FINCORE_BMAP and one of the "8 byte field" flags.
+.SH VERSIONS
+TBD
+.SH CONFORMING TO
+TBD
+
+.SH EXAMPLE
+.PP
+The following program is an example that shows the page cache information
+of the file specified in its first command-line argument to the standard
+output.
+
+.nf
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/fincore.h>
+
+#define err(msg) do { perror(msg); exit(1); } while (0)
+
+int main(int argc, char *argv[])
+{
+    int i, j;
+    int fd;
+    int ret;
+    long ps = sysconf(_SC_PAGESIZE);
+    long nr_pages;
+    unsigned char *buf;
+    struct stat stat;
+    struct fincore_extra fe = {};
+
+    fd = open(argv[1], O_RDWR);
+    if (fd == \-1)
+        err("open");
+
+    ret = fstat(fd, &stat);
+    if (ret == \-1)
+        err("fstat");
+    nr_pages = ((stat.st_size + ps \- 1) & (~(ps \- 1))) / ps;
+
+    buf = malloc(nr_pages * 24);
+    if (!buf)
+        err("malloc");
+
+    /* byte map */
+    ret = fincore(fd, 0, nr_pages, FINCORE_BMAP, buf, NULL);
+    if (ret < 0)
+        err("fincore");
+    printf("Page residency:");
+    for (i = 0; i < nr_pages; i++)
+        printf("%d", buf[i]);
+    printf("\\n\\n");
+
+    /* 8 byte entry */
+    ret = fincore(fd, 0, nr_pages,
+                  FINCORE_PFN|FINCORE_PAGE_FLAGS, buf, &fe);
+    if (ret < 0)
+        err("fincore");
+    printf("pfn\\tflags %lx\\n", fe.nr_entries);
+    for (i = 0; i < fe.nr_entries; i++) {
+        for (j = 0; j < 2; j++)
+            printf("0x%lx\\t", *(unsigned long *)(buf + (i*2+j)*8));
+        printf("\\n");
+    }
+    printf("\\n");
+
+    /* 8 byte entry with page offset (no hole scanned) */
+    ret = fincore(fd, 0, nr_pages,
+              FINCORE_PGOFF|FINCORE_PFN|FINCORE_PAGE_FLAGS, buf, &fe);
+    if (ret < 0)
+        err("fincore");
+    printf("pgoff\\tpfn\\tflags %lx\\n", fe.nr_entries);
+    for (i = 0; i < fe.nr_entries; i++) {
+        for (j = 0; j < 3; j++)
+            printf("0x%lx\\t", *(unsigned long *)(buf + (i*3+j)*8));
+        printf("\\n");
+    }
+
+    free(buf);
+
+    ret = close(fd);
+    if (ret < 0)
+        err("close");
+    return 0;
+}
+.fi
+.SH SEE ALSO
+.BR mincore (2),
+.BR fsync (2)
-- 
1.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related

* Re: [PATCH v3 1/3] mm: introduce fincore()
From: Dave Hansen @ 2014-07-07 19:01 UTC (permalink / raw)
  To: Naoya Horiguchi, Andrew Morton
  Cc: Konstantin Khlebnikov, Wu Fengguang, Arnaldo Carvalho de Melo,
	Borislav Petkov, Kirill A. Shutemov, Johannes Weiner,
	Rusty Russell, David Miller, Andres Freund, linux-kernel,
	linux-mm, Christoph Hellwig, Dave Chinner, Michael Kerrisk,
	Linux API, Naoya Horiguchi, Kees Cook
In-Reply-To: <1404756006-23794-2-git-send-email-n-horiguchi@ah.jp.nec.com>

> +/*
> + * You can control how the buffer in userspace is filled with this mode
> + * parameters:

I agree that we don't have any good mechanisms for looking at the page
cache from userspace.  I've hacked some things up using mincore() and
they weren't pretty, so I welcome _something_ like this.

But, is this trying to do too many things at once?  Do we have solid use
cases spelled out for each of these modes?  Have we thought out how they
will be used in practice?

The biggest question for me, though, is whether we want to start
designing these per-page interfaces to consider different page sizes, or
whether we're going to just continue to pretend that the entire world is
4k pages.  Using FINCORE_BMAP on 1GB hugetlbfs files would be a bit
silly, for instance.

> + * - FINCORE_BMAP:
> + *     the page status is returned in a vector of bytes.
> + *     The least significant bit of each byte is 1 if the referenced page
> + *     is in memory, otherwise it is zero.

I know this is consistent with mincore(), but it did always bother me
that mincore() was so sparse.  Seems like it is wasting 7/8 of its bits.

> + * - FINCORE_PGOFF:
> + *     if this flag is set, fincore() doesn't store any information about
> + *     holes. Instead each records per page has the entry of page offset,
> + *     using 8 bytes. This mode is useful if we handle a large file and
> + *     only few pages are on memory.

This bothers me a bit.  How would someone know how sparse file was
before calling this?  If it's not sparse, and they use this, they'll end
up using 8x the memory they would have using FINCORE_BMAP.  If it *is*
sparse, and they use FINCORE_BMAP, they will either waste tons of memory
on buffers, or have to make a ton of calls.

I guess this could also be used to do *searches*, which would let you
search out holes.  Let's say you have a 2TB file.  You could call this
with a buffer size of 1 entry and do searches, say 0->1TB.  If you get
your one entry back, you know it's not completely sparse.

But, that wouldn't work with it as-designed.  The length of the buffer
and the range of the file being checked are coupled together, so you
can't say:

	vec = malloc(sizeof(long));
	fincore(fd, 0, 1TB, FINCORE_PGOFF, vec, extra);

without overflowing vec.

Is it really right to say this is going to be 8 bytes?  Would we want it
to share types with something else, like be an loff_t?

> + * - FINCORE_PFN:
> + *     stores pfn, using 8 bytes.

These are all an unprivileged operations from what I can tell.  I know
we're going to a lot of trouble to hide kernel addresses from being seen
in userspace.  This seems like it would be undesirable for the folks
that care about not leaking kernel addresses, especially for
unprivileged users.

This would essentially tell userspace where in the kernel's address
space some user-controlled data will be.

> + * We can use multiple flags among the flags in FINCORE_LONGENTRY_MASK.
> + * For example, when the mode is FINCORE_PFN|FINCORE_PAGEFLAGS, the per-page
> + * information is stored like this:

Instead of specifying the ordering in the manpages alone, would it be
smarter to just say that the ordering of the items is dependent on the
ordering of the flags?  In other words if FINCORE_PFN <
FINCORE_PAGEFLAGS, then its field comes first?

^ permalink raw reply

* Re: [PATCH v3 3/3] man2/fincore.2: document general description about fincore(2)
From: Dave Hansen @ 2014-07-07 19:08 UTC (permalink / raw)
  To: Naoya Horiguchi, Andrew Morton
  Cc: Konstantin Khlebnikov, Wu Fengguang, Arnaldo Carvalho de Melo,
	Borislav Petkov, Kirill A. Shutemov, Johannes Weiner,
	Rusty Russell, David Miller, Andres Freund,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Christoph Hellwig, Dave Chinner,
	Michael Kerrisk, Linux API, Naoya Horiguchi
In-Reply-To: <1404756006-23794-4-git-send-email-n-horiguchi-PaJj6Psr51x8UrSeD/g0lQ@public.gmane.org>

On 07/07/2014 11:00 AM, Naoya Horiguchi wrote:
> +.SH RETURN VALUE
> +On success,
> +.BR fincore ()
> +returns 0.
> +On error, \-1 is returned, and
> +.I errno
> +is set appropriately.

Is this accurate?  From reading the syscall itself, it looked like it
did this:

> + * Return value is the number of pages whose data is stored in fc->buffer.
> + */
> +static long do_fincore(struct fincore_control *fc, int nr_pages)

and:

> +SYSCALL_DEFINE6(fincore, int, fd, loff_t, start, long, nr_pages,
...
> +	while (fc.nr_pages > 0) {
> +		memset(fc.buffer, 0, fc.buffer_size);
> +		ret = do_fincore(&fc, min(step, fc.nr_pages));
> +		/* Reached the end of the file */
> +		if (ret == 0)
> +			break;
> +		if (ret < 0)
> +			break;
...
> +	}
...
> +	return ret;
> +}

Which seems that for a given loop of do_fincore(), you might end up
returning the result of that *single* iteration of do_fincore() instead
of the aggregate of the entire syscall.

So, it can return <0 on failure, 0 on success, or also an essentially
random >0 number on success too.

Why not just use the return value for something useful instead of
hacking in the extras->nr_entries stuff?  Oh, and what if that

> +	if (extra)
> +		__put_user(nr, &extra->nr_entries);

fails?  It seems like we might silently forget to tell userspace how
many entries we filled.

^ permalink raw reply

* Re: [PATCH v3 1/3] mm: introduce fincore()
From: Naoya Horiguchi @ 2014-07-07 20:21 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Andrew Morton, Konstantin Khlebnikov, Wu Fengguang,
	Arnaldo Carvalho de Melo, Borislav Petkov, Kirill A. Shutemov,
	Johannes Weiner, Rusty Russell, David Miller, Andres Freund,
	linux-kernel, linux-mm, Christoph Hellwig, Dave Chinner,
	Michael Kerrisk, Linux API, Naoya Horiguchi, Kees Cook
In-Reply-To: <53BAEE95.50807@intel.com>

Hi Dave,

Thank you for the comments.

On Mon, Jul 07, 2014 at 12:01:41PM -0700, Dave Hansen wrote:
> > +/*
> > + * You can control how the buffer in userspace is filled with this mode
> > + * parameters:
> 
> I agree that we don't have any good mechanisms for looking at the page
> cache from userspace.  I've hacked some things up using mincore() and
> they weren't pretty, so I welcome _something_ like this.
> 
> But, is this trying to do too many things at once?  Do we have solid use
> cases spelled out for each of these modes?  Have we thought out how they
> will be used in practice?

tools/vm/page-types.c will be an in-kernel user after this base code is
accepted. The idea of doing fincore() thing comes up during the discussion
with Konstantin over file cache mode of this tool.
pfn and page flag are needed there, so I think it's one clear usecase.

> The biggest question for me, though, is whether we want to start
> designing these per-page interfaces to consider different page sizes, or
> whether we're going to just continue to pretend that the entire world is
> 4k pages.  Using FINCORE_BMAP on 1GB hugetlbfs files would be a bit
> silly, for instance.
> 
> > + * - FINCORE_BMAP:
> > + *     the page status is returned in a vector of bytes.
> > + *     The least significant bit of each byte is 1 if the referenced page
> > + *     is in memory, otherwise it is zero.
> 
> I know this is consistent with mincore(), but it did always bother me
> that mincore() was so sparse.  Seems like it is wasting 7/8 of its bits.

Yes, I got the same comment in previous round. So, OK, not a few people
seem to think that space efficiency is more important than the consistency,
so I'm OK to do it.

We have an idea of making fincore() cover the whole mincore()'s feature
by letting fincore() handle /proc/pid/mem. So mincore() will be obsolete,
and no one has to care about consistency beteen mincore and fincore.
That might be another reason justifying the idea above.

> > + * - FINCORE_PGOFF:
> > + *     if this flag is set, fincore() doesn't store any information about
> > + *     holes. Instead each records per page has the entry of page offset,
> > + *     using 8 bytes. This mode is useful if we handle a large file and
> > + *     only few pages are on memory.
> 
> This bothers me a bit.  How would someone know how sparse file was
> before calling this?  If it's not sparse, and they use this, they'll end
> up using 8x the memory they would have using FINCORE_BMAP.  If it *is*
> sparse, and they use FINCORE_BMAP, they will either waste tons of memory
> on buffers, or have to make a ton of calls.

Yes, that's the hard point.
Some new mode (FINCORE_SUM for example) to get how many pages of a file
is in memory might be helpful to choose which mode, although we need 2 calls.

> I guess this could also be used to do *searches*, which would let you
> search out holes.  Let's say you have a 2TB file.  You could call this
> with a buffer size of 1 entry and do searches, say 0->1TB.  If you get
> your one entry back, you know it's not completely sparse.
> 
> But, that wouldn't work with it as-designed.  The length of the buffer
> and the range of the file being checked are coupled together,

This is only correct for !FINCORE_PGOFF.

> so you
> can't say:
> 
> 	vec = malloc(sizeof(long));
> 	fincore(fd, 0, 1TB, FINCORE_PGOFF, vec, extra);
> 
> without overflowing vec.

The 3rd parameter is the number of pages whose data is passed to userspace,
so we expect userspace to set it according to the buffer size.

But yes, I still have a problem. In FINCORE_PGOFF mode we only scan until
the buffer becomes full, but userspace doesn't know at which point the
scan stopped. It can guess the end point from the pgoff of the last buffer,
but it might not be straightforward or well-designed.
And I should describe this behavior more.

> Is it really right to say this is going to be 8 bytes?  Would we want it
> to share types with something else, like be an loff_t?

Could you elaborate it more?

> > + * - FINCORE_PFN:
> > + *     stores pfn, using 8 bytes.
> 
> These are all an unprivileged operations from what I can tell.  I know
> we're going to a lot of trouble to hide kernel addresses from being seen
> in userspace.  This seems like it would be undesirable for the folks
> that care about not leaking kernel addresses, especially for
> unprivileged users.
> 
> This would essentially tell userspace where in the kernel's address
> space some user-controlled data will be.

OK, so this and FINCORE_PAGEFLAGS will be limited for privileged users.

> > + * We can use multiple flags among the flags in FINCORE_LONGENTRY_MASK.
> > + * For example, when the mode is FINCORE_PFN|FINCORE_PAGEFLAGS, the per-page
> > + * information is stored like this:
> 
> Instead of specifying the ordering in the manpages alone, would it be
> smarter to just say that the ordering of the items is dependent on the
> ordering of the flags?  In other words if FINCORE_PFN <
> FINCORE_PAGEFLAGS, then its field comes first?

Ah, right. I should've referred to the ordering here also.

Thanks,
Naoya Horiguchi

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v3 1/3] mm: introduce fincore()
From: Dave Hansen @ 2014-07-07 20:43 UTC (permalink / raw)
  To: Naoya Horiguchi
  Cc: Andrew Morton, Konstantin Khlebnikov, Wu Fengguang,
	Arnaldo Carvalho de Melo, Borislav Petkov, Kirill A. Shutemov,
	Johannes Weiner, Rusty Russell, David Miller, Andres Freund,
	linux-kernel, linux-mm, Christoph Hellwig, Dave Chinner,
	Michael Kerrisk, Linux API, Naoya Horiguchi, Kees Cook
In-Reply-To: <20140707202108.GA5031@nhori.bos.redhat.com>

On 07/07/2014 01:21 PM, Naoya Horiguchi wrote:
> On Mon, Jul 07, 2014 at 12:01:41PM -0700, Dave Hansen wrote:
>> But, is this trying to do too many things at once?  Do we have solid use
>> cases spelled out for each of these modes?  Have we thought out how they
>> will be used in practice?
> 
> tools/vm/page-types.c will be an in-kernel user after this base code is
> accepted. The idea of doing fincore() thing comes up during the discussion
> with Konstantin over file cache mode of this tool.
> pfn and page flag are needed there, so I think it's one clear usecase.

I'm going to take that as a no. :)

The whole FINCORE_PGOFF vs. FINCORE_BMAP issue is something that will
come up in practice.  We just don't have the interfaces for an end user
to pick which one they want to use.
>> Is it really right to say this is going to be 8 bytes?  Would we want it
>> to share types with something else, like be an loff_t?
> 
> Could you elaborate it more?

We specify file offsets in other system calls, like the lseek family.  I
was just thinking that this type should match up with those calls since
they are expressing the same data type with the same ranges and limitations.

>>> + * - FINCORE_PFN:
>>> + *     stores pfn, using 8 bytes.
>>
>> These are all an unprivileged operations from what I can tell.  I know
>> we're going to a lot of trouble to hide kernel addresses from being seen
>> in userspace.  This seems like it would be undesirable for the folks
>> that care about not leaking kernel addresses, especially for
>> unprivileged users.
>>
>> This would essentially tell userspace where in the kernel's address
>> space some user-controlled data will be.
> 
> OK, so this and FINCORE_PAGEFLAGS will be limited for privileged users.

Then I'd just question their usefulness outside of a debugging
environment, especially when you can get at them in other (more
roundabout) ways in a debugging environment.

This is really looking to me like two system calls.  The bitmap-based
one, and another more extensible one.  I don't think there's any harm in
having two system calls, especially when they're trying to glue together
two disparate interfaces.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v3 3/3] man2/fincore.2: document general description about fincore(2)
From: Naoya Horiguchi @ 2014-07-07 20:59 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Andrew Morton, Konstantin Khlebnikov, Wu Fengguang,
	Arnaldo Carvalho de Melo, Borislav Petkov, Kirill A. Shutemov,
	Johannes Weiner, Rusty Russell, David Miller, Andres Freund,
	linux-kernel, linux-mm, Christoph Hellwig, Dave Chinner,
	Michael Kerrisk, Linux API, Naoya Horiguchi
In-Reply-To: <53BAF01C.8010700@intel.com>

On Mon, Jul 07, 2014 at 12:08:12PM -0700, Dave Hansen wrote:
> On 07/07/2014 11:00 AM, Naoya Horiguchi wrote:
> > +.SH RETURN VALUE
> > +On success,
> > +.BR fincore ()
> > +returns 0.
> > +On error, \-1 is returned, and
> > +.I errno
> > +is set appropriately.
> 
> Is this accurate?  From reading the syscall itself, it looked like it
> did this:
> 
> > + * Return value is the number of pages whose data is stored in fc->buffer.
> > + */
> > +static long do_fincore(struct fincore_control *fc, int nr_pages)
> 
> and:
> 
> > +SYSCALL_DEFINE6(fincore, int, fd, loff_t, start, long, nr_pages,
> ...
> > +	while (fc.nr_pages > 0) {
> > +		memset(fc.buffer, 0, fc.buffer_size);
> > +		ret = do_fincore(&fc, min(step, fc.nr_pages));
> > +		/* Reached the end of the file */
> > +		if (ret == 0)
> > +			break;
> > +		if (ret < 0)
> > +			break;
> ...
> > +	}
> ...
> > +	return ret;
> > +}
> 
> Which seems that for a given loop of do_fincore(), you might end up
> returning the result of that *single* iteration of do_fincore() instead
> of the aggregate of the entire syscall.
> 
> So, it can return <0 on failure, 0 on success, or also an essentially
> random >0 number on success too.

We don't break this while loop if do_fincore() returned a positive value
unless copy_to_user() fails. And in that case ret is set to -EFAULT.
So I think sys_fincore() never returns a positive value.

BTW, we don't have to check "if (ret == 0)" and "if (ret < 0)" separately,
I'll fix it.

> Why not just use the return value for something useful instead of
> hacking in the extras->nr_entries stuff?

Hmm, I got the opposite complaint previously, where we shouldn't
interpret the return value differently depending on the flag.
And I'd like to keep the extra argument for future extensibility.
For example, if we want to collect pages only with a specific
set of page flags, this extra argument will be necessary.

>  Oh, and what if that
> 
> > +	if (extra)
> > +		__put_user(nr, &extra->nr_entries);
> 
> fails?  It seems like we might silently forget to tell userspace how
> many entries we filled.

Oh, I forget to check it.

Thanks,
Naoya Horiguchi

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v3 1/3] mm: introduce fincore()
From: Naoya Horiguchi @ 2014-07-07 21:48 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Andrew Morton, Konstantin Khlebnikov, Wu Fengguang,
	Arnaldo Carvalho de Melo, Borislav Petkov, Kirill A. Shutemov,
	Johannes Weiner, Rusty Russell, David Miller, Andres Freund,
	linux-kernel, linux-mm, Christoph Hellwig, Dave Chinner,
	Michael Kerrisk, Linux API, Naoya Horiguchi, Kees Cook
In-Reply-To: <53BB0673.8020604@intel.com>

On Mon, Jul 07, 2014 at 01:43:31PM -0700, Dave Hansen wrote:
> On 07/07/2014 01:21 PM, Naoya Horiguchi wrote:
> > On Mon, Jul 07, 2014 at 12:01:41PM -0700, Dave Hansen wrote:
> >> But, is this trying to do too many things at once?  Do we have solid use
> >> cases spelled out for each of these modes?  Have we thought out how they
> >> will be used in practice?
> > 
> > tools/vm/page-types.c will be an in-kernel user after this base code is
> > accepted. The idea of doing fincore() thing comes up during the discussion
> > with Konstantin over file cache mode of this tool.
> > pfn and page flag are needed there, so I think it's one clear usecase.
> 
> I'm going to take that as a no. :)

As for other usecases, database developers should have some demand for
physical addresses (especially numa node?) or page flags (especially
page reclaim or writeback related ones).
But I'm not a database expert so can't say how, sorry.

> The whole FINCORE_PGOFF vs. FINCORE_BMAP issue is something that will
> come up in practice.  We just don't have the interfaces for an end user
> to pick which one they want to use.
> 
> >> Is it really right to say this is going to be 8 bytes?  Would we want it
> >> to share types with something else, like be an loff_t?
> > 
> > Could you elaborate it more?
> 
> We specify file offsets in other system calls, like the lseek family.  I
> was just thinking that this type should match up with those calls since
> they are expressing the same data type with the same ranges and limitations.

The 2nd parameter is loff_t, do we already do this?

> >>> + * - FINCORE_PFN:
> >>> + *     stores pfn, using 8 bytes.
> >>
> >> These are all an unprivileged operations from what I can tell.  I know
> >> we're going to a lot of trouble to hide kernel addresses from being seen
> >> in userspace.  This seems like it would be undesirable for the folks
> >> that care about not leaking kernel addresses, especially for
> >> unprivileged users.
> >>
> >> This would essentially tell userspace where in the kernel's address
> >> space some user-controlled data will be.
> > 
> > OK, so this and FINCORE_PAGEFLAGS will be limited for privileged users.

Sorry, this statement of mine might a bit short-sighted, and I'd like
to revoke it.
I think that some page flags and/or numa info should be useful outside
the debugging environment, and safe to expose to userspace. So limiting
to bitmap-one for unprivileged users is too strict.

> Then I'd just question their usefulness outside of a debugging
> environment, especially when you can get at them in other (more
> roundabout) ways in a debugging environment.
> 
> This is really looking to me like two system calls.  The bitmap-based
> one, and another more extensible one.  I don't think there's any harm in
> having two system calls, especially when they're trying to glue together
> two disparate interfaces.

I think that if separating syscall into two, one for privileged users
and one for unprivileged users migth be fine (rather than bitmap-based
one and extensible one.)

Thanks,
Naoya Horiguchi

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [RFC PATCH 00/11] Adding FreeBSD's Capsicum security framework (part 1)
From: Alexei Starovoitov @ 2014-07-07 22:33 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: David Drysdale, LSM List,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Greg Kroah-Hartman, Alexander Viro, Meredydd Luff, Kees Cook,
	James Morris, Linux API, qemu-devel
In-Reply-To: <53BA9094.9080401-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Mon, Jul 7, 2014 at 5:20 AM, Paolo Bonzini <pbonzini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Il 07/07/2014 12:29, David Drysdale ha scritto:
>
>>> I think that's more easily done by opening the file as O_RDONLY/O_WRONLY
>>> /O_RDWR.   You could do it by running the file descriptor's seccomp-bpf
>>> program once per iocb with synthesized syscall numbers and argument
>>> vectors.
>>
>>
>> Right, but generating the equivalent seccomp input environment for an
>> equivalent single-fd syscall is going to be subtle and complex (which
>> are worrying words to mention in a security context).  And how many
>> other syscalls are going to need similar special-case processing?
>> (poll? select? send[m]msg? ...)
>
>
> Yeah, the difficult part is getting the right balance between:
>
> 1) limitations due to seccomp's impossibility to chase pointers (which is
> not something that can be lifted, as it's required for correctness)

btw once seccomp moves to eBPF it will be able to 'chase pointers',
since pointer walking will be possible via bpf_load_pointer() function call,
which is a wrapper of:
  probe_kernel_read(&ptr, unsafe_ptr, sizeof(void *));
  return ptr;
Not sure whether it helps this case or not. Just fyi.

> 2) subtlety and complexity of the resulting code.
>
> The problem stems when you have a single a single syscall operating on
> multiple file descriptors.  So for example among the cases you mention poll
> and select are problematic; sendm?msg are not.  They would be if Capsicum
> had a capability for SCM_RIGHTS file descriptor passing, but I cannot find
> it.
>
> But then you also have to strike the right balance between a complete
> solution and an overengineered one.
>
> For example, even though poll and select are problematic, I wonder what
> would really the point be in blocking them; poll/select are level-triggered,
> and calling them should be idempotent as far as the file descriptor is
> concerned.  If you want to prevent a process/thread from issuing blocking
> system calls, but you'd do that with a per-process filter, not with
> per-file-descriptor filters or capabilities.
>
>
>> Capsicum capabilities are associated with the file descriptor (a la
>> F_GETFD), not the open file itself -- different FDs with different
>> associated rights can map to the same underlying open file.
>
>
> Good to know, thanks.  I suppose you have testcases that cover this.
>
> Paolo
> --
>
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: [PATCH v3 3/3] man2/fincore.2: document general description about fincore(2)
From: Dave Hansen @ 2014-07-07 22:34 UTC (permalink / raw)
  To: Naoya Horiguchi
  Cc: Andrew Morton, Konstantin Khlebnikov, Wu Fengguang,
	Arnaldo Carvalho de Melo, Borislav Petkov, Kirill A. Shutemov,
	Johannes Weiner, Rusty Russell, David Miller, Andres Freund,
	linux-kernel, linux-mm, Christoph Hellwig, Dave Chinner,
	Michael Kerrisk, Linux API, Naoya Horiguchi
In-Reply-To: <20140707205956.GB5031@nhori.bos.redhat.com>

On 07/07/2014 01:59 PM, Naoya Horiguchi wrote:
> On Mon, Jul 07, 2014 at 12:08:12PM -0700, Dave Hansen wrote:
>> On 07/07/2014 11:00 AM, Naoya Horiguchi wrote:
>>> +.SH RETURN VALUE
>>> +On success,
>>> +.BR fincore ()
>>> +returns 0.
>>> +On error, \-1 is returned, and
>>> +.I errno
>>> +is set appropriately.
>>
>> Is this accurate?  From reading the syscall itself, it looked like it
>> did this:
>>
>>> + * Return value is the number of pages whose data is stored in fc->buffer.
>>> + */
>>> +static long do_fincore(struct fincore_control *fc, int nr_pages)
>>
>> and:
>>
>>> +SYSCALL_DEFINE6(fincore, int, fd, loff_t, start, long, nr_pages,
>> ...
>>> +	while (fc.nr_pages > 0) {
>>> +		memset(fc.buffer, 0, fc.buffer_size);
>>> +		ret = do_fincore(&fc, min(step, fc.nr_pages));
>>> +		/* Reached the end of the file */
>>> +		if (ret == 0)
>>> +			break;
>>> +		if (ret < 0)
>>> +			break;
>> ...
>>> +	}
>> ...
>>> +	return ret;
>>> +}
>>
>> Which seems that for a given loop of do_fincore(), you might end up
>> returning the result of that *single* iteration of do_fincore() instead
>> of the aggregate of the entire syscall.
>>
>> So, it can return <0 on failure, 0 on success, or also an essentially
>> random >0 number on success too.
> 
> We don't break this while loop if do_fincore() returned a positive value
> unless copy_to_user() fails. And in that case ret is set to -EFAULT.
> So I think sys_fincore() never returns a positive value.

OK, that makes sense as I'm reading it again.

>> Why not just use the return value for something useful instead of
>> hacking in the extras->nr_entries stuff?
> 
> Hmm, I got the opposite complaint previously, where we shouldn't
> interpret the return value differently depending on the flag.
> And I'd like to keep the extra argument for future extensibility.
> For example, if we want to collect pages only with a specific
> set of page flags, this extra argument will be necessary.

Couldn't it simply be the number of elements that it wrote in to the
buffer, or even the number of bytes?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v3 1/3] mm: introduce fincore()
From: Dave Hansen @ 2014-07-07 22:44 UTC (permalink / raw)
  To: Naoya Horiguchi
  Cc: Andrew Morton, Konstantin Khlebnikov, Wu Fengguang,
	Arnaldo Carvalho de Melo, Borislav Petkov, Kirill A. Shutemov,
	Johannes Weiner, Rusty Russell, David Miller, Andres Freund,
	linux-kernel, linux-mm, Christoph Hellwig, Dave Chinner,
	Michael Kerrisk, Linux API, Naoya Horiguchi, Kees Cook
In-Reply-To: <20140707214820.GA13596@nhori.bos.redhat.com>

On 07/07/2014 02:48 PM, Naoya Horiguchi wrote:
> On Mon, Jul 07, 2014 at 01:43:31PM -0700, Dave Hansen wrote:
>> The whole FINCORE_PGOFF vs. FINCORE_BMAP issue is something that will
>> come up in practice.  We just don't have the interfaces for an end user
>> to pick which one they want to use.
>>
>>>> Is it really right to say this is going to be 8 bytes?  Would we want it
>>>> to share types with something else, like be an loff_t?
>>>
>>> Could you elaborate it more?
>>
>> We specify file offsets in other system calls, like the lseek family.  I
>> was just thinking that this type should match up with those calls since
>> they are expressing the same data type with the same ranges and limitations.
> 
> The 2nd parameter is loff_t, do we already do this?

I mean the fields in the buffer, like:

> +Any of the following flags are to be set to add an 8 byte field in each entry.
> +You can set any of these flags at the same time, although you can't set
> +FINCORE_BMAP combined with these 8 byte field flags.


>>>> This would essentially tell userspace where in the kernel's address
>>>> space some user-controlled data will be.
>>>
>>> OK, so this and FINCORE_PAGEFLAGS will be limited for privileged users.
> 
> Sorry, this statement of mine might a bit short-sighted, and I'd like
> to revoke it.
> I think that some page flags and/or numa info should be useful outside
> the debugging environment, and safe to expose to userspace. So limiting
> to bitmap-one for unprivileged users is too strict.

The PFN is not the same as NUMA information, and the PFN is insufficient
to describe the NUMA node on all systems that Linux supports.

Trying to get NUMA information back out is a good goal, but doing it
with PFNs is a bad idea since they have so many consequences.

I'm also bummed exporting NUMA information was a design goal of these
patches, but they weren't mentioned in any of the patch descriptions.

>> Then I'd just question their usefulness outside of a debugging
>> environment, especially when you can get at them in other (more
>> roundabout) ways in a debugging environment.
>>
>> This is really looking to me like two system calls.  The bitmap-based
>> one, and another more extensible one.  I don't think there's any harm in
>> having two system calls, especially when they're trying to glue together
>> two disparate interfaces.
> 
> I think that if separating syscall into two, one for privileged users
> and one for unprivileged users migth be fine (rather than bitmap-based
> one and extensible one.)

The problem as I see it is shoehorning two interfaces in to the same
syscall.  If there are privileged and unprivileged operations that use
the same _interfaces_ I think they should share a syscall.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v10 1/7] mm: support madvise(MADV_FREE)
From: Minchan Kim @ 2014-07-08  0:36 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Andrew Morton, linux-kernel, linux-mm, Michael Kerrisk, Linux API,
	Hugh Dickins, Johannes Weiner, Rik van Riel, KOSAKI Motohiro,
	Mel Gorman, Jason Evans, Zhang Yanfei
In-Reply-To: <20140707104137.GA23150@node.dhcp.inet.fi>

On Mon, Jul 07, 2014 at 01:41:37PM +0300, Kirill A. Shutemov wrote:
> On Mon, Jul 07, 2014 at 09:53:52AM +0900, Minchan Kim wrote:
> > Linux doesn't have an ability to free pages lazy while other OS
> > already have been supported that named by madvise(MADV_FREE).
> > 
> > The gain is clear that kernel can discard freed pages rather than
> > swapping out or OOM if memory pressure happens.
> > 
> > Without memory pressure, freed pages would be reused by userspace
> > without another additional overhead(ex, page fault + allocation
> > + zeroing).
> > 
> > How to work is following as.
> > 
> > When madvise syscall is called, VM clears dirty bit of ptes of
> > the range. If memory pressure happens, VM checks dirty bit of
> > page table and if it found still "clean", it means it's a
> > "lazyfree pages" so VM could discard the page instead of swapping out.
> > Once there was store operation for the page before VM peek a page
> > to reclaim, dirty bit is set so VM can swap out the page instead of
> > discarding.
> > 
> > Firstly, heavy users would be general allocators(ex, jemalloc,
> > tcmalloc and hope glibc supports it) and jemalloc/tcmalloc already
> > have supported the feature for other OS(ex, FreeBSD)
> > 
> > barrios@blaptop:~/benchmark/ebizzy$ lscpu
> > Architecture:          x86_64
> > CPU op-mode(s):        32-bit, 64-bit
> > Byte Order:            Little Endian
> > CPU(s):                4
> > On-line CPU(s) list:   0-3
> > Thread(s) per core:    2
> > Core(s) per socket:    2
> > Socket(s):             1
> > NUMA node(s):          1
> > Vendor ID:             GenuineIntel
> > CPU family:            6
> > Model:                 42
> > Stepping:              7
> > CPU MHz:               2801.000
> > BogoMIPS:              5581.64
> > Virtualization:        VT-x
> > L1d cache:             32K
> > L1i cache:             32K
> > L2 cache:              256K
> > L3 cache:              4096K
> > NUMA node0 CPU(s):     0-3
> > 
> > ebizzy benchmark(./ebizzy -S 10 -n 512)
> > 
> >  vanilla-jemalloc		MADV_free-jemalloc
> > 
> > 1 thread
> > records:  10              records:  10
> > avg:      7682.10         avg:      15306.10
> > std:      62.35(0.81%)    std:      347.99(2.27%)
> > max:      7770.00         max:      15622.00
> > min:      7598.00         min:      14772.00
> > 
> > 2 thread
> > records:  10              records:  10
> > avg:      12747.50        avg:      24171.00
> > std:      792.06(6.21%)   std:      895.18(3.70%)
> > max:      13337.00        max:      26023.00
> > min:      10535.00        min:      23152.00
> > 
> > 4 thread
> > records:  10              records:  10
> > avg:      16474.60        avg:      33717.90
> > std:      1496.45(9.08%)  std:      2008.97(5.96%)
> > max:      17877.00        max:      35958.00
> > min:      12224.00        min:      29565.00
> > 
> > 8 thread
> > records:  10              records:  10
> > avg:      16778.50        avg:      33308.10
> > std:      825.53(4.92%)   std:      1668.30(5.01%)
> > max:      17543.00        max:      36010.00
> > min:      14576.00        min:      29577.00
> > 
> > 16 thread
> > records:  10              records:  10
> > avg:      20614.40        avg:      35516.30
> > std:      602.95(2.92%)   std:      1283.65(3.61%)
> > max:      21753.00        max:      37178.00
> > min:      19605.00        min:      33217.00
> > 
> > 32 thread
> > records:  10              records:  10
> > avg:      22771.70        avg:      36018.50
> > std:      598.94(2.63%)   std:      1046.76(2.91%)
> > max:      24035.00        max:      37266.00
> > min:      22108.00        min:      34149.00
> > 
> > In summary, MADV_FREE is about 2 time faster than MADV_DONTNEED.
> > 
> > Cc: Michael Kerrisk <mtk.manpages@gmail.com>
> > Cc: Linux API <linux-api@vger.kernel.org>
> > Cc: Hugh Dickins <hughd@google.com>
> > Cc: Johannes Weiner <hannes@cmpxchg.org>
> > Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> > Cc: Mel Gorman <mgorman@suse.de>
> > Cc: Jason Evans <je@fb.com>
> > Cc: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
> > Acked-by: Rik van Riel <riel@redhat.com>
> > Signed-off-by: Minchan Kim <minchan@kernel.org>
> > ---
> 
> ...
> 
> > +static void madvise_free_page_range(struct mmu_gather *tlb,
> > +			     struct vm_area_struct *vma,
> > +			     unsigned long addr, unsigned long end)
> > +{
> > +	pgd_t *pgd;
> > +	unsigned long next;
> > +
> > +	BUG_ON(addr >= end);
> > +	tlb_start_vma(tlb, vma);
> > +	pgd = pgd_offset(vma->vm_mm, addr);
> > +	do {
> > +		next = pgd_addr_end(addr, end);
> > +		if (pgd_none_or_clear_bad(pgd))
> > +			continue;
> > +		next = madvise_free_pud_range(tlb, vma, pgd, addr, next);
> > +	} while (pgd++, addr = next, addr != end);
> > +	tlb_end_vma(tlb, vma);
> 
> Any particular reason why pagewalker can't be used here?

Nothing special. I just copied from MADV_DONTNEED.
Will try it.

> 
> > +}
> 
> ...
> 
> > @@ -381,6 +547,13 @@ madvise_vma(struct vm_area_struct *vma, struct vm_area_struct **prev,
> >  		return madvise_remove(vma, prev, start, end);
> >  	case MADV_WILLNEED:
> >  		return madvise_willneed(vma, prev, start, end);
> > +	case MADV_FREE:
> > +		/*
> > +		 * XXX: In this implementation, MADV_FREE works like
> > +		 * MADV_DONTNEED on swapless system or full swap.
> > +		 */
> > +		if (get_nr_swap_pages() > 0)
> > +			return madvise_free(vma, prev, start, end);
> 
> Looks racy wrt to full swap. What will happen if we will do madvise_free()
> on full swap?

Now, we don't age anonymous LRU list if swap is full so that VM would lose
the chance to discard freed page via shrink_page_list in this implementation
if that race hppanes.

But it would be not severe because MADV_FREE semantic doesn't say VM must
discard them but it is just hint from userside that specified range is
no longer important so that VM can gave a freedom to free and I think
it's not a common case.

In addition, I have a plan to support MADV_FREE on swapless system, too.

> 
> >  	case MADV_DONTNEED:
> >  		return madvise_dontneed(vma, prev, start, end);
> >  	default:
> 
> ...
> 
> > @@ -1204,6 +1223,16 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
> >  			}
> >  			dec_mm_counter(mm, MM_ANONPAGES);
> >  			inc_mm_counter(mm, MM_SWAPENTS);
> > +		} else if (flags & TTU_UNMAP) {
> > +			if (dirty || PageDirty(page)) {
> > +				set_pte_at(mm, address, pte, pteval);
> > +				ret = SWAP_FAIL;
> > +				goto out_unmap;
> 
> I don't get this part.
> Looks like it will fail to unmap the page if it's dirty and not backed by
> swapcache. Current code doesn't have such limitation.
> Do we really need this?

Good point. Code is rather ugly even, it has side-effect with hwpoisend
page unmapping.

How about this? I didn't test it but if there is no objection,
I will go this with stress testing.

---
 include/linux/rmap.h |  1 +
 mm/rmap.c            | 22 ++++++++++++----------
 mm/vmscan.c          |  5 +++--
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/include/linux/rmap.h b/include/linux/rmap.h
index dea05914f167..0ba377b97a38 100644
--- a/include/linux/rmap.h
+++ b/include/linux/rmap.h
@@ -75,6 +75,7 @@ enum ttu_flags {
 	TTU_UNMAP = 1,			/* unmap mode */
 	TTU_MIGRATION = 2,		/* migration mode */
 	TTU_MUNLOCK = 4,		/* munlock mode */
+	TTU_FREE = 8,			/* free mode */
 
 	TTU_IGNORE_MLOCK = (1 << 8),	/* ignore mlock */
 	TTU_IGNORE_ACCESS = (1 << 9),	/* don't age */
diff --git a/mm/rmap.c b/mm/rmap.c
index 3c415eb8b6f0..010d51ea26c4 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1209,6 +1209,18 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 		swp_entry_t entry = { .val = page_private(page) };
 		pte_t swp_pte;
 
+		if (flags & TTU_FREE) {
+			if (dirty || PageDirty(page)) {
+				set_pte_at(mm, address, pte, pteval);
+				ret = SWAP_FAIL;
+				goto out_unmap;
+			} else {
+				/* It's a freeable page by MADV_FREE */
+				dec_mm_counter(mm, MM_ANONPAGES);
+				goto discard;
+			}
+		}
+
 		if (PageSwapCache(page)) {
 			/*
 			 * Store the swap location in the pte.
@@ -1227,16 +1239,6 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 			}
 			dec_mm_counter(mm, MM_ANONPAGES);
 			inc_mm_counter(mm, MM_SWAPENTS);
-		} else if (flags & TTU_UNMAP) {
-			if (dirty || PageDirty(page)) {
-				set_pte_at(mm, address, pte, pteval);
-				ret = SWAP_FAIL;
-				goto out_unmap;
-			} else {
-				/* It's a freeable page by madvise_free */
-				dec_mm_counter(mm, MM_ANONPAGES);
-				goto discard;
-			}
 		} else if (IS_ENABLED(CONFIG_MIGRATION)) {
 			/*
 			 * Store the pfn of the page in a special migration
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 4e15babf4414..a7dbce703208 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1549,8 +1549,9 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
 	if (nr_taken == 0)
 		return 0;
 
-	nr_reclaimed = shrink_page_list(&page_list, zone, sc, TTU_UNMAP,
-				&nr_dirty, &nr_unqueued_dirty, &nr_congested,
+	nr_reclaimed = shrink_page_list(&page_list, zone, sc,
+				TTU_UNMAP|TTU_FREE, &nr_dirty,
+				&nr_unqueued_dirty, &nr_congested,
 				&nr_writeback, &nr_immediate,
 				false);
 
-- 
2.0.0


> 
> > +			} else {
> > +				/* It's a freeable page by madvise_free */
> > +				dec_mm_counter(mm, MM_ANONPAGES);
> > +				goto discard;
> > +			}
> >  		} else if (IS_ENABLED(CONFIG_MIGRATION)) {
> >  			/*
> >  			 * Store the pfn of the page in a special migration
> 
> -- 
>  Kirill A. Shutemov
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related

* Re: [PATCH v10 6/7] ARM: add pmd_[dirty|mkclean] for THP
From: Minchan Kim @ 2014-07-08  1:09 UTC (permalink / raw)
  To: Steve Capper
  Cc: Andrew Morton, linux-kernel, linux-mm, Michael Kerrisk, Linux API,
	Hugh Dickins, Johannes Weiner, Rik van Riel, KOSAKI Motohiro,
	Mel Gorman, Jason Evans, Zhang Yanfei, Kirill A. Shutemov,
	Catalin Marinas, Will Deacon, Russell King, linux-arm-kernel
In-Reply-To: <20140707092247.GA15168@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 3346 bytes --]

On Mon, Jul 07, 2014 at 10:22:48AM +0100, Steve Capper wrote:
> On Mon, Jul 07, 2014 at 09:53:57AM +0900, Minchan Kim wrote:
> > MADV_FREE needs pmd_dirty and pmd_mkclean for detecting recent
> > overwrite of the contents since MADV_FREE syscall is called for
> > THP page.
> > 
> > This patch adds pmd_dirty and pmd_mkclean for THP page MADV_FREE
> > support.
> > 
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Steve Capper <steve.capper@linaro.org>
> > Cc: Russell King <linux@arm.linux.org.uk>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Signed-off-by: Minchan Kim <minchan@kernel.org>
> > ---
> >  arch/arm64/include/asm/pgtable.h | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> > index 579702086488..f3ec01cef04f 100644
> > --- a/arch/arm64/include/asm/pgtable.h
> > +++ b/arch/arm64/include/asm/pgtable.h
> > @@ -240,10 +240,12 @@ static inline pmd_t pte_pmd(pte_t pte)
> >  #endif
> >  
> >  #define pmd_young(pmd)		pte_young(pmd_pte(pmd))
> > +#define pmd_dirty(pmd)		pte_dirty(pmd_pte(pmd))
> >  #define pmd_wrprotect(pmd)	pte_pmd(pte_wrprotect(pmd_pte(pmd)))
> >  #define pmd_mksplitting(pmd)	pte_pmd(pte_mkspecial(pmd_pte(pmd)))
> >  #define pmd_mkold(pmd)		pte_pmd(pte_mkold(pmd_pte(pmd)))
> >  #define pmd_mkwrite(pmd)	pte_pmd(pte_mkwrite(pmd_pte(pmd)))
> > +#define pmd_mkclean(pmd)	pte_pmd(pte_mkclean(pmd_pte(pmd)))
> >  #define pmd_mkdirty(pmd)	pte_pmd(pte_mkdirty(pmd_pte(pmd)))
> >  #define pmd_mkyoung(pmd)	pte_pmd(pte_mkyoung(pmd_pte(pmd)))
> >  #define pmd_mknotpresent(pmd)	(__pmd(pmd_val(pmd) & ~PMD_TYPE_MASK))
> > -- 
> > 2.0.0
> >
> 
> Hi Minchan,

Hello Steve and Will,

> 
> This looks good to me too.
> As Will said this applies to arm64, we will also need a version for:
> arch/arm/include/asm/pgtable-3level.h.

Does it work?

diff --git a/arch/arm/include/asm/pgtable-3level.h b/arch/arm/include/asm/pgtable-3level.h
index 85c60adc8b60..3a7bb8dc7d05 100644
--- a/arch/arm/include/asm/pgtable-3level.h
+++ b/arch/arm/include/asm/pgtable-3level.h
@@ -220,6 +220,8 @@ static inline pmd_t *pmd_offset(pud_t *pud, unsigned long addr)
 #define pmd_trans_splitting(pmd) (pmd_val(pmd) & PMD_SECT_SPLITTING)
 #endif
 
+#define pmd_dirty	(pmd_val(pmd) & PMD_SECT_DIRTY)
+
 #define PMD_BIT_FUNC(fn,op) \
 static inline pmd_t pmd_##fn(pmd_t pmd) { pmd_val(pmd) op; return pmd; }
 
@@ -228,6 +230,7 @@ PMD_BIT_FUNC(mkold,	&= ~PMD_SECT_AF);
 PMD_BIT_FUNC(mksplitting, |= PMD_SECT_SPLITTING);
 PMD_BIT_FUNC(mkwrite,   &= ~PMD_SECT_RDONLY);
 PMD_BIT_FUNC(mkdirty,   |= PMD_SECT_DIRTY);
+PMD_BIT_FUNC(mkclean,   &= ~PMD_SECT_DIRTY);
 PMD_BIT_FUNC(mkyoung,   |= PMD_SECT_AF);
 
 #define pmd_mkhuge(pmd)		(__pmd(pmd_val(pmd) & ~PMD_TABLE_BIT))
-- 
2.0.0

> 
> Is there a testcase we can run to check that this patch set is working
> well for arm/arm64?

I just run several instance of attached simple stress with heavy kernel
build in parallel on 1G RAM machine.

Thanks for the review!

> 
> Cheers,
> -- 
> Steve 
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Kind regards,
Minchan Kim

[-- Attachment #2: thp_alloc.c --]
[-- Type: text/x-csrc, Size: 1328 bytes --]

#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <pthread.h>

#define CHUNK_SIZE (20<<20)
#define SLEEP_TIME_SEC 	2
#define NUM_THREAD	12
#define QUIT		1000

int quit;

void alloc_thp()
{
	int i;
	int ret;
	char *ptr;

	/* should be aligned with 2M which is THP page size */
	ret = posix_memalign((void**)&ptr, 2<<20, CHUNK_SIZE);
	if (ret) {
		fprintf(stderr, "fail to allocate\n");
		return;
	}
		
	memset(ptr, 'a', CHUNK_SIZE);
	ret = madvise(ptr, CHUNK_SIZE, 5);
	if (ret) {
		perror("fail to madvise");
		return;
	}

	sleep(SLEEP_TIME_SEC);

	memset(ptr, 'b', CHUNK_SIZE);
	sleep(SLEEP_TIME_SEC);

	for (i = 0; i < CHUNK_SIZE; i++) {
		if (ptr[i] != 'b') {
			fprintf(stderr, "fail to verify\n");
			fprintf(stderr, "Something wrong\n");
			return;
		}
	}

	free(ptr);
}

void *thread_alloc(void *priv)
{
	int n = 0;

	while(!quit) {
		alloc_thp();
		if (!(n++ % 5))
			printf("I'm working\n");
		if (n == QUIT)
			return;
	}
}

int main()
{
	int i, ret;
	pthread_t thread[NUM_THREAD];

	for (i = 0; i < NUM_THREAD; i++) {
		ret = pthread_create(&thread[i], NULL, thread_alloc, NULL);
		if (ret) {
			perror("fail to create thread\n");
			return 1;
		}
	}

	scanf("%d", &quit);
	for (i = 0; i < NUM_THREAD; i++)
		pthread_join(thread[i], NULL);

	printf("Test Done\n");	
	return 0;
}

^ permalink raw reply related

* Re: [PATCH v10 7/7] mm: Don't split THP page when syscall is called
From: Minchan Kim @ 2014-07-08  1:30 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Andrew Morton, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Michael Kerrisk, Linux API,
	Hugh Dickins, Johannes Weiner, Rik van Riel, KOSAKI Motohiro,
	Mel Gorman, Jason Evans, Zhang Yanfei
In-Reply-To: <20140707111303.GC23150-nhfs4B5ZimeFUdmeq17FyvUpdFzICT1y@public.gmane.org>

On Mon, Jul 07, 2014 at 02:13:03PM +0300, Kirill A. Shutemov wrote:
> On Mon, Jul 07, 2014 at 09:53:58AM +0900, Minchan Kim wrote:
> > We don't need to split THP page when MADV_FREE syscall is
> > called. It could be done when VM decide really frees it so
> > we could reduce the number of THP split.
> > 
> > Signed-off-by: Minchan Kim <minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > ---
> >  include/linux/huge_mm.h |  3 +++
> >  mm/huge_memory.c        | 25 +++++++++++++++++++++++++
> >  mm/madvise.c            | 19 +++++++++++++++++--
> >  mm/rmap.c               |  4 ++++
> >  mm/vmscan.c             | 24 ++++++++++++++++--------
> >  5 files changed, 65 insertions(+), 10 deletions(-)
> > 
> > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> > index 63579cb8d3dc..f0d37238cf8f 100644
> > --- a/include/linux/huge_mm.h
> > +++ b/include/linux/huge_mm.h
> > @@ -19,6 +19,9 @@ extern struct page *follow_trans_huge_pmd(struct vm_area_struct *vma,
> >  					  unsigned long addr,
> >  					  pmd_t *pmd,
> >  					  unsigned int flags);
> > +extern int madvise_free_pmd(struct mmu_gather *tlb,
> > +			struct vm_area_struct *vma,
> > +			pmd_t *pmd, unsigned long addr);
> >  extern int zap_huge_pmd(struct mmu_gather *tlb,
> >  			struct vm_area_struct *vma,
> >  			pmd_t *pmd, unsigned long addr);
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 5d562a9fe931..2a70069dcfc0 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -1384,6 +1384,31 @@ out:
> >  	return 0;
> >  }
> >  
> > +int madvise_free_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
> > +		 pmd_t *pmd, unsigned long addr)
> > +{
> > +	spinlock_t *ptl;
> > +	int ret = 0;
> > +
> > +	if (__pmd_trans_huge_lock(pmd, vma, &ptl) == 1) {
> > +		pmd_t orig_pmd;
> > +		struct mm_struct *mm = vma->vm_mm;
> > +
> > +		/* No hugepage in swapcache */
> > +		VM_BUG_ON(PageSwapCache(pmd_page(orig_pmd)));
> 
> VM_BUG_ON_PAGE() ?

NP.

> 
> > +
> > +		orig_pmd = pmdp_get_and_clear(tlb->mm, addr, pmd);
> > +		orig_pmd = pmd_mkold(orig_pmd);
> > +		orig_pmd = pmd_mkclean(orig_pmd);
> > +
> > +		set_pmd_at(mm, addr, pmd, orig_pmd);
> > +		tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
> > +		spin_unlock(ptl);
> > +		ret = 1;
> > +	}
> > +	return ret;
> > +}
> > +
> >  int zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
> >  		 pmd_t *pmd, unsigned long addr)
> >  {
> > diff --git a/mm/madvise.c b/mm/madvise.c
> > index 372a25a8ea82..3c99919ee094 100644
> > --- a/mm/madvise.c
> > +++ b/mm/madvise.c
> > @@ -320,8 +320,23 @@ static inline unsigned long madvise_free_pmd_range(struct mmu_gather *tlb,
> >  		 * if the range covers.
> >  		 */
> >  		next = pmd_addr_end(addr, end);
> > -		if (pmd_trans_huge(*pmd))
> > -			split_huge_page_pmd(vma, addr, pmd);
> > +		if (pmd_trans_huge(*pmd)) {
> > +			if (next - addr != HPAGE_PMD_SIZE) {
> > +#ifdef CONFIG_DEBUG_VM
> > +				if (!rwsem_is_locked(&tlb->mm->mmap_sem)) {
> > +					pr_err("%s: mmap_sem is unlocked! addr=0x%lx end=0x%lx vma->vm_start=0x%lx vma->vm_end=0x%lx\n",
> > +						__func__, addr, end,
> > +						vma->vm_start,
> > +						vma->vm_end);
> > +					BUG();
> > +				}
> > +#endif
> > +				split_huge_page_pmd(vma, addr, pmd);
> > +			} else if (madvise_free_pmd(tlb, vma, pmd, addr))
> > +				goto next;
> > +			/* fall through */
> > +		}
> > +
> >  		/*
> >  		 * Here there can be other concurrent MADV_DONTNEED or
> >  		 * trans huge page faults running, and if the pmd is
> > diff --git a/mm/rmap.c b/mm/rmap.c
> > index ee495d84c8b3..3c415eb8b6f0 100644
> > --- a/mm/rmap.c
> > +++ b/mm/rmap.c
> > @@ -702,6 +702,10 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
> >  		/* go ahead even if the pmd is pmd_trans_splitting() */
> >  		if (pmdp_clear_flush_young_notify(vma, address, pmd))
> >  			referenced++;
> > +
> > +		if (pmd_dirty(*pmd))
> > +			dirty++;
> > +
> >  		spin_unlock(ptl);
> >  	} else {
> >  		pte_t *pte;
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index f7a45600846f..4e15babf4414 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -971,15 +971,23 @@ static unsigned long shrink_page_list(struct list_head *page_list,
> >  		 * Anonymous process memory has backing store?
> >  		 * Try to allocate it some swap space here.
> >  		 */
> > -		if (PageAnon(page) && !PageSwapCache(page) && !freeable) {
> > -			if (!(sc->gfp_mask & __GFP_IO))
> > -				goto keep_locked;
> > -			if (!add_to_swap(page, page_list))
> > -				goto activate_locked;
> > -			may_enter_fs = 1;
> > +		if (PageAnon(page) && !PageSwapCache(page)) {
> > +			if (!freeable) {
> > +				if (!(sc->gfp_mask & __GFP_IO))
> > +					goto keep_locked;
> > +				if (!add_to_swap(page, page_list))
> > +					goto activate_locked;
> > +				may_enter_fs = 1;
> >  
> > -			/* Adding to swap updated mapping */
> > -			mapping = page_mapping(page);
> > +				/* Adding to swap updated mapping */
> > +				mapping = page_mapping(page);
> > +			} else {
> > +				if (unlikely(PageTransHuge(page))) {
> > +					if (unlikely(split_huge_page_to_list(
> > +						page, page_list)))
> > +						goto keep_locked;
> 
> Hm. It would be better to free the huge page without splitting. 
> It shouldn't be a big deal: walk over rmap and zap all pmds.
> Or I miss something?

Actually, I did but found no problem except CONFIG_DEBUG_VM but rollback
after peeking [1].
When I read the description in detail by your review, I think we can remove
BUG_ON(PageTransHuge(page)) in try_to_unmap and go with no split for lazyfree
page because they are not in swapcache any more so the assumption of [1] is
not valid. Will do it in next revision.

Thanks for the review, Kirill!

[1] thp: split_huge_page paging, 3f04f62f9


> 
> > +				}
> > +			}
> >  		}
> >  
> >  		/*
> > -- 
> > 2.0.0
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-api" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> -- 
>  Kirill A. Shutemov
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo-Bw31MaZKKs0EbZ0PF+XxCw@public.gmane.org  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org"> email-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org </a>

-- 
Kind regards,
Minchan Kim

^ permalink raw reply

* Re: [PATCH v10 1/7] mm: support madvise(MADV_FREE)
From: Zhang Yanfei @ 2014-07-08  3:54 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Michael Kerrisk, Linux API,
	Hugh Dickins, Johannes Weiner, Rik van Riel, KOSAKI Motohiro,
	Mel Gorman, Jason Evans, Kirill A. Shutemov
In-Reply-To: <1404694438-10272-2-git-send-email-minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Hi Minchan,

On 07/07/2014 08:53 AM, Minchan Kim wrote:
> Linux doesn't have an ability to free pages lazy while other OS
> already have been supported that named by madvise(MADV_FREE).
> 
> The gain is clear that kernel can discard freed pages rather than
> swapping out or OOM if memory pressure happens.
> 
> Without memory pressure, freed pages would be reused by userspace
> without another additional overhead(ex, page fault + allocation
> + zeroing).
> 
> How to work is following as.
> 
> When madvise syscall is called, VM clears dirty bit of ptes of
> the range. 

This should be updated because the implementation has been changed.
It also remove the page from the swapcache if it is.

Thank you for your effort!

-- 
Thanks.
Zhang Yanfei

^ permalink raw reply

* Re: [PATCH v10 1/7] mm: support madvise(MADV_FREE)
From: Minchan Kim @ 2014-07-08  4:45 UTC (permalink / raw)
  To: Zhang Yanfei
  Cc: Andrew Morton, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Michael Kerrisk, Linux API,
	Hugh Dickins, Johannes Weiner, Rik van Riel, KOSAKI Motohiro,
	Mel Gorman, Jason Evans, Kirill A. Shutemov
In-Reply-To: <53BB6B64.1080807-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>

Hello Zhang,

On Tue, Jul 08, 2014 at 11:54:12AM +0800, Zhang Yanfei wrote:
> Hi Minchan,
> 
> On 07/07/2014 08:53 AM, Minchan Kim wrote:
> > Linux doesn't have an ability to free pages lazy while other OS
> > already have been supported that named by madvise(MADV_FREE).
> > 
> > The gain is clear that kernel can discard freed pages rather than
> > swapping out or OOM if memory pressure happens.
> > 
> > Without memory pressure, freed pages would be reused by userspace
> > without another additional overhead(ex, page fault + allocation
> > + zeroing).
> > 
> > How to work is following as.
> > 
> > When madvise syscall is called, VM clears dirty bit of ptes of
> > the range. 
> 
> This should be updated because the implementation has been changed.
> It also remove the page from the swapcache if it is.

You're right in current implementation but it's rather one of
implementation technique(ie, it could be changed later) but still
main main logic from MADV_FREE is tightly coupled with pte dirty bit
so I don't feel I added it in description but it would be better to
add it as comment.

Thanks for the review!

> 
> Thank you for your effort!
> 
> -- 
> Thanks.
> Zhang Yanfei
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo-Bw31MaZKKs0EbZ0PF+XxCw@public.gmane.org  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org"> email-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org </a>

-- 
Kind regards,
Minchan Kim

^ permalink raw reply

* [PATCH v11 0/7] MADV_FREE support
From: Minchan Kim @ 2014-07-08  6:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, linux-mm, Michael Kerrisk, Linux API, Hugh Dickins,
	Johannes Weiner, Rik van Riel, KOSAKI Motohiro, Mel Gorman,
	Jason Evans, Zhang Yanfei, Kirill A. Shutemov, Minchan Kim

This patch enable MADV_FREE hint for madvise syscall, which have
been supported by other OSes. [PATCH 1] includes the details.

[1] support MADVISE_FREE for !THP page so if VM encounter
THP page in syscall context, it splits THP page.
[2-6] is to preparing to call madvise syscall without THP plitting
[7] enable THP page support for MADV_FREE.

* From v10
 * Add Acked-by from arch stuff(x86, s390)
 * Pagewalker based pagetable working - Kirill
 * Fix try_to_unmap_one broken with hwpoison - Kirill
 * Use VM_BUG_ON_PAGE in madvise_free_pmd - Kirill
 * Fix pgtable-3level.h for arm - Steve

* From v9
 * Add Acked-by - Rik
 * Add THP page support - Kirill
 * Rebased-on v3.16-rc3-mmotm-2014-07-02-15-07

* From v8
 * Rebased-on v3.16-rc2-mmotm-2014-06-25-16-44

* From v7
 * Rebased-on next-20140613
 
* From v6
 * Remove page from swapcache in syscal time
 * Move utility functions from memory.c to madvise.c - Johannes
 * Rename untilify functtions - Johannes
 * Remove unnecessary checks from vmscan.c - Johannes
 * Rebased-on v3.15-rc5-mmotm-2014-05-16-16-56
 * Drop Reviewe-by because there was some changes since then.

* From v5
 * Fix PPC problem which don't flush TLB - Rik
 * Remove unnecessary lazyfree_range stub function - Rik
 * Rebased on v3.15-rc5

* From v4
 * Add Reviewed-by: Zhang Yanfei
 * Rebase on v3.15-rc1-mmotm-2014-04-15-16-14

* From v3
 * Add "how to work part" in description - Zhang
 * Add page_discardable utility function - Zhang
 * Clean up

* From v2
 * Remove forceful dirty marking of swap-readed page - Johannes
 * Remove deactivation logic of lazyfreed page
 * Rebased on 3.14
 * Remove RFC tag

* From v1
 * Use custom page table walker for madvise_free - Johannes
 * Remove PG_lazypage flag - Johannes
 * Do madvise_dontneed instead of madvise_freein swapless system

Minchan Kim (7):
  [1] mm: support madvise(MADV_FREE)
  [2] x86: add pmd_[dirty|mkclean] for THP
  [3] sparc: add pmd_[dirty|mkclean] for THP
  [4] powerpc: add pmd_[dirty|mkclean] for THP
  [5] s390: add pmd_[dirty|mkclean] for THP
  [6] ARM: add pmd_[dirty|mkclean] for THP
  [7] mm: Don't split THP page when syscall is called

 arch/arm/include/asm/pgtable-3level.h    |   3 +
 arch/arm64/include/asm/pgtable.h         |   2 +
 arch/powerpc/include/asm/pgtable-ppc64.h |   2 +
 arch/s390/include/asm/pgtable.h          |  12 +++
 arch/sparc/include/asm/pgtable_64.h      |  16 ++++
 arch/x86/include/asm/pgtable.h           |  10 ++
 include/linux/rmap.h                     |   9 +-
 include/linux/vm_event_item.h            |   1 +
 include/uapi/asm-generic/mman-common.h   |   1 +
 mm/madvise.c                             | 153 +++++++++++++++++++++++++++++++
 mm/rmap.c                                |  41 ++++++++-
 mm/vmscan.c                              |  64 +++++++++----
 mm/vmstat.c                              |   1 +
 13 files changed, 295 insertions(+), 20 deletions(-)

-- 
2.0.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* [PATCH v11 1/7] mm: support madvise(MADV_FREE)
From: Minchan Kim @ 2014-07-08  6:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, linux-mm, Michael Kerrisk, Linux API, Hugh Dickins,
	Johannes Weiner, Rik van Riel, KOSAKI Motohiro, Mel Gorman,
	Jason Evans, Zhang Yanfei, Kirill A. Shutemov, Minchan Kim
In-Reply-To: <1404799424-1120-1-git-send-email-minchan@kernel.org>

Linux doesn't have an ability to free pages lazy while other OS
already have been supported that named by madvise(MADV_FREE).

The gain is clear that kernel can discard freed pages rather than
swapping out or OOM if memory pressure happens.

Without memory pressure, freed pages would be reused by userspace
without another additional overhead(ex, page fault + allocation
+ zeroing).

How to work is following as.

When madvise syscall is called, VM clears dirty bit of ptes of
the range. If memory pressure happens, VM checks dirty bit of
page table and if it found still "clean", it means it's a
"lazyfree pages" so VM could discard the page instead of swapping out.
Once there was store operation for the page before VM peek a page
to reclaim, dirty bit is set so VM can swap out the page instead of
discarding.

Firstly, heavy users would be general allocators(ex, jemalloc,
tcmalloc and hope glibc supports it) and jemalloc/tcmalloc already
have supported the feature for other OS(ex, FreeBSD)

barrios@blaptop:~/benchmark/ebizzy$ lscpu
Architecture:          x86_64
CPU op-mode(s):        32-bit, 64-bit
Byte Order:            Little Endian
CPU(s):                4
On-line CPU(s) list:   0-3
Thread(s) per core:    2
Core(s) per socket:    2
Socket(s):             1
NUMA node(s):          1
Vendor ID:             GenuineIntel
CPU family:            6
Model:                 42
Stepping:              7
CPU MHz:               2801.000
BogoMIPS:              5581.64
Virtualization:        VT-x
L1d cache:             32K
L1i cache:             32K
L2 cache:              256K
L3 cache:              4096K
NUMA node0 CPU(s):     0-3

ebizzy benchmark(./ebizzy -S 10 -n 512)

 vanilla-jemalloc		MADV_free-jemalloc

1 thread
records:  10              records:  10
avg:      7682.10         avg:      15306.10
std:      62.35(0.81%)    std:      347.99(2.27%)
max:      7770.00         max:      15622.00
min:      7598.00         min:      14772.00

2 thread
records:  10              records:  10
avg:      12747.50        avg:      24171.00
std:      792.06(6.21%)   std:      895.18(3.70%)
max:      13337.00        max:      26023.00
min:      10535.00        min:      23152.00

4 thread
records:  10              records:  10
avg:      16474.60        avg:      33717.90
std:      1496.45(9.08%)  std:      2008.97(5.96%)
max:      17877.00        max:      35958.00
min:      12224.00        min:      29565.00

8 thread
records:  10              records:  10
avg:      16778.50        avg:      33308.10
std:      825.53(4.92%)   std:      1668.30(5.01%)
max:      17543.00        max:      36010.00
min:      14576.00        min:      29577.00

16 thread
records:  10              records:  10
avg:      20614.40        avg:      35516.30
std:      602.95(2.92%)   std:      1283.65(3.61%)
max:      21753.00        max:      37178.00
min:      19605.00        min:      33217.00

32 thread
records:  10              records:  10
avg:      22771.70        avg:      36018.50
std:      598.94(2.63%)   std:      1046.76(2.91%)
max:      24035.00        max:      37266.00
min:      22108.00        min:      34149.00

In summary, MADV_FREE is about 2 time faster than MADV_DONTNEED.

Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Linux API <linux-api@vger.kernel.org>
Cc: Hugh Dickins <hughd@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Jason Evans <je@fb.com>
Cc: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
Acked-by: Rik van Riel <riel@redhat.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 include/linux/rmap.h                   |   9 ++-
 include/linux/vm_event_item.h          |   1 +
 include/uapi/asm-generic/mman-common.h |   1 +
 mm/madvise.c                           | 135 +++++++++++++++++++++++++++++++++
 mm/rmap.c                              |  42 +++++++++-
 mm/vmscan.c                            |  40 ++++++++--
 mm/vmstat.c                            |   1 +
 7 files changed, 217 insertions(+), 12 deletions(-)

diff --git a/include/linux/rmap.h b/include/linux/rmap.h
index be574506e6a9..0ba377b97a38 100644
--- a/include/linux/rmap.h
+++ b/include/linux/rmap.h
@@ -75,6 +75,7 @@ enum ttu_flags {
 	TTU_UNMAP = 1,			/* unmap mode */
 	TTU_MIGRATION = 2,		/* migration mode */
 	TTU_MUNLOCK = 4,		/* munlock mode */
+	TTU_FREE = 8,			/* free mode */
 
 	TTU_IGNORE_MLOCK = (1 << 8),	/* ignore mlock */
 	TTU_IGNORE_ACCESS = (1 << 9),	/* don't age */
@@ -181,7 +182,8 @@ static inline void page_dup_rmap(struct page *page)
  * Called from mm/vmscan.c to handle paging out
  */
 int page_referenced(struct page *, int is_locked,
-			struct mem_cgroup *memcg, unsigned long *vm_flags);
+			struct mem_cgroup *memcg, unsigned long *vm_flags,
+			int *is_dirty);
 
 #define TTU_ACTION(x) ((x) & TTU_ACTION_MASK)
 
@@ -260,9 +262,12 @@ int rmap_walk(struct page *page, struct rmap_walk_control *rwc);
 
 static inline int page_referenced(struct page *page, int is_locked,
 				  struct mem_cgroup *memcg,
-				  unsigned long *vm_flags)
+				  unsigned long *vm_flags,
+				  int *is_pte_dirty)
 {
 	*vm_flags = 0;
+	if (is_pte_dirty)
+		*is_pte_dirty = 0;
 	return 0;
 }
 
diff --git a/include/linux/vm_event_item.h b/include/linux/vm_event_item.h
index ced92345c963..e2d3fb1e9814 100644
--- a/include/linux/vm_event_item.h
+++ b/include/linux/vm_event_item.h
@@ -25,6 +25,7 @@ enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT,
 		FOR_ALL_ZONES(PGALLOC),
 		PGFREE, PGACTIVATE, PGDEACTIVATE,
 		PGFAULT, PGMAJFAULT,
+		PGLAZYFREED,
 		FOR_ALL_ZONES(PGREFILL),
 		FOR_ALL_ZONES(PGSTEAL_KSWAPD),
 		FOR_ALL_ZONES(PGSTEAL_DIRECT),
diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h
index ddc3b36f1046..7a94102b7a02 100644
--- a/include/uapi/asm-generic/mman-common.h
+++ b/include/uapi/asm-generic/mman-common.h
@@ -34,6 +34,7 @@
 #define MADV_SEQUENTIAL	2		/* expect sequential page references */
 #define MADV_WILLNEED	3		/* will need these pages */
 #define MADV_DONTNEED	4		/* don't need these pages */
+#define MADV_FREE	5		/* free pages only if memory pressure */
 
 /* common parameters: try to keep these consistent across architectures */
 #define MADV_REMOVE	9		/* remove these pages & resources */
diff --git a/mm/madvise.c b/mm/madvise.c
index 0938b30da4ab..a6aa7d4c4e02 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -19,6 +19,14 @@
 #include <linux/blkdev.h>
 #include <linux/swap.h>
 #include <linux/swapops.h>
+#include <linux/mmu_notifier.h>
+
+#include <asm/tlb.h>
+
+struct madvise_free_private {
+	struct vm_area_struct *vma;
+	struct mmu_gather *tlb;
+};
 
 /*
  * Any behaviour which results in changes to the vma->vm_flags needs to
@@ -31,6 +39,7 @@ static int madvise_need_mmap_write(int behavior)
 	case MADV_REMOVE:
 	case MADV_WILLNEED:
 	case MADV_DONTNEED:
+	case MADV_FREE:
 		return 0;
 	default:
 		/* be safe, default to 1. list exceptions explicitly */
@@ -251,6 +260,124 @@ static long madvise_willneed(struct vm_area_struct *vma,
 	return 0;
 }
 
+static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
+				unsigned long end, struct mm_walk *walk)
+
+{
+	struct madvise_free_private *fp = walk->private;
+	struct mmu_gather *tlb = fp->tlb;
+	struct mm_struct *mm = tlb->mm;
+	struct vm_area_struct *vma = fp->vma;
+	spinlock_t *ptl;
+	pte_t *pte, ptent;
+	struct page *page;
+
+	split_huge_page_pmd(vma, addr, pmd);
+	if (pmd_trans_unstable(pmd))
+		return 0;
+
+	pte = pte_offset_map_lock(mm, pmd, addr, &ptl);
+	arch_enter_lazy_mmu_mode();
+	for (; addr != end; pte++, addr += PAGE_SIZE) {
+		ptent = *pte;
+
+		if (pte_none(ptent))
+			continue;
+
+		if (!pte_present(ptent))
+			continue;
+
+		page = vm_normal_page(vma, addr, ptent);
+		if (page && PageSwapCache(page)) {
+			if (trylock_page(page)) {
+				if (try_to_free_swap(page))
+					ClearPageDirty(page);
+				unlock_page(page);
+			} else
+				continue;
+		}
+
+		/*
+		 * Some of architecture(ex, PPC) don't update TLB
+		 * with set_pte_at and tlb_remove_tlb_entry so for
+		 * the portability, remap the pte with old|clean
+		 * after pte clearing.
+		 */
+		ptent = ptep_get_and_clear_full(mm, addr, pte,
+						tlb->fullmm);
+		ptent = pte_mkold(ptent);
+		ptent = pte_mkclean(ptent);
+		set_pte_at(mm, addr, pte, ptent);
+		tlb_remove_tlb_entry(tlb, pte, addr);
+	}
+	arch_leave_lazy_mmu_mode();
+	pte_unmap_unlock(pte - 1, ptl);
+	cond_resched();
+	return 0;
+}
+
+static void madvise_free_page_range(struct mmu_gather *tlb,
+			     struct vm_area_struct *vma,
+			     unsigned long addr, unsigned long end)
+{
+	struct madvise_free_private fp = {
+		.vma = vma,
+		.tlb = tlb,
+	};
+
+	struct mm_walk free_walk = {
+		.pmd_entry = madvise_free_pte_range,
+		.mm = vma->vm_mm,
+		.private = &fp,
+	};
+
+	BUG_ON(addr >= end);
+	tlb_start_vma(tlb, vma);
+	walk_page_range(addr, end, &free_walk);
+	tlb_end_vma(tlb, vma);
+}
+
+static int madvise_free_single_vma(struct vm_area_struct *vma,
+			unsigned long start_addr, unsigned long end_addr)
+{
+	unsigned long start, end;
+	struct mm_struct *mm = vma->vm_mm;
+	struct mmu_gather tlb;
+
+	if (vma->vm_flags & (VM_LOCKED|VM_HUGETLB|VM_PFNMAP))
+		return -EINVAL;
+
+	/* MADV_FREE works for only anon vma at the moment */
+	if (vma->vm_file)
+		return -EINVAL;
+
+	start = max(vma->vm_start, start_addr);
+	if (start >= vma->vm_end)
+		return -EINVAL;
+	end = min(vma->vm_end, end_addr);
+	if (end <= vma->vm_start)
+		return -EINVAL;
+
+	lru_add_drain();
+	tlb_gather_mmu(&tlb, mm, start, end);
+	update_hiwater_rss(mm);
+
+	mmu_notifier_invalidate_range_start(mm, start, end);
+	madvise_free_page_range(&tlb, vma, start, end);
+	mmu_notifier_invalidate_range_end(mm, start, end);
+	tlb_finish_mmu(&tlb, start, end);
+
+	return 0;
+}
+
+static long madvise_free(struct vm_area_struct *vma,
+			     struct vm_area_struct **prev,
+			     unsigned long start, unsigned long end)
+{
+	*prev = vma;
+	return madvise_free_single_vma(vma, start, end);
+}
+
 /*
  * Application no longer needs these pages.  If the pages are dirty,
  * it's OK to just throw them away.  The app will be more careful about
@@ -381,6 +508,13 @@ madvise_vma(struct vm_area_struct *vma, struct vm_area_struct **prev,
 		return madvise_remove(vma, prev, start, end);
 	case MADV_WILLNEED:
 		return madvise_willneed(vma, prev, start, end);
+	case MADV_FREE:
+		/*
+		 * XXX: In this implementation, MADV_FREE works like
+		 * MADV_DONTNEED on swapless system or full swap.
+		 */
+		if (get_nr_swap_pages() > 0)
+			return madvise_free(vma, prev, start, end);
 	case MADV_DONTNEED:
 		return madvise_dontneed(vma, prev, start, end);
 	default:
@@ -400,6 +534,7 @@ madvise_behavior_valid(int behavior)
 	case MADV_REMOVE:
 	case MADV_WILLNEED:
 	case MADV_DONTNEED:
+	case MADV_FREE:
 #ifdef CONFIG_KSM
 	case MADV_MERGEABLE:
 	case MADV_UNMERGEABLE:
diff --git a/mm/rmap.c b/mm/rmap.c
index 7928ddd91b6e..a8e34596dc97 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -663,6 +663,7 @@ int page_mapped_in_vma(struct page *page, struct vm_area_struct *vma)
 }
 
 struct page_referenced_arg {
+	int dirtied;
 	int mapcount;
 	int referenced;
 	unsigned long vm_flags;
@@ -677,6 +678,7 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
 	struct mm_struct *mm = vma->vm_mm;
 	spinlock_t *ptl;
 	int referenced = 0;
+	int dirty = 0;
 	struct page_referenced_arg *pra = arg;
 
 	if (unlikely(PageTransHuge(page))) {
@@ -700,6 +702,11 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
 		/* go ahead even if the pmd is pmd_trans_splitting() */
 		if (pmdp_clear_flush_young_notify(vma, address, pmd))
 			referenced++;
+
+		/*
+		 * In this implmentation, MADV_FREE doesn't support THP free
+		 */
+		dirty++;
 		spin_unlock(ptl);
 	} else {
 		pte_t *pte;
@@ -729,6 +736,10 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
 			if (likely(!(vma->vm_flags & VM_SEQ_READ)))
 				referenced++;
 		}
+
+		if (pte_dirty(*pte))
+			dirty++;
+
 		pte_unmap_unlock(pte, ptl);
 	}
 
@@ -737,6 +748,9 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
 		pra->vm_flags |= vma->vm_flags;
 	}
 
+	if (dirty)
+		pra->dirtied++;
+
 	pra->mapcount--;
 	if (!pra->mapcount)
 		return SWAP_SUCCESS; /* To break the loop */
@@ -761,6 +775,7 @@ static bool invalid_page_referenced_vma(struct vm_area_struct *vma, void *arg)
  * @is_locked: caller holds lock on the page
  * @memcg: target memory cgroup
  * @vm_flags: collect encountered vma->vm_flags who actually referenced the page
+ * @is_pte_dirty: ptes which have marked dirty bit - used for lazyfree page
  *
  * Quick test_and_clear_referenced for all mappings to a page,
  * returns the number of ptes which referenced the page.
@@ -768,7 +783,8 @@ static bool invalid_page_referenced_vma(struct vm_area_struct *vma, void *arg)
 int page_referenced(struct page *page,
 		    int is_locked,
 		    struct mem_cgroup *memcg,
-		    unsigned long *vm_flags)
+		    unsigned long *vm_flags,
+		    int *is_pte_dirty)
 {
 	int ret;
 	int we_locked = 0;
@@ -783,6 +799,9 @@ int page_referenced(struct page *page,
 	};
 
 	*vm_flags = 0;
+	if (is_pte_dirty)
+		*is_pte_dirty = 0;
+
 	if (!page_mapped(page))
 		return 0;
 
@@ -810,6 +829,9 @@ int page_referenced(struct page *page,
 	if (we_locked)
 		unlock_page(page);
 
+	if (is_pte_dirty)
+		*is_pte_dirty = pra.dirtied;
+
 	return pra.referenced;
 }
 
@@ -1128,6 +1150,7 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 	spinlock_t *ptl;
 	int ret = SWAP_AGAIN;
 	enum ttu_flags flags = (enum ttu_flags)arg;
+	int dirty = 0;
 
 	pte = page_check_address(page, mm, address, &ptl, 0);
 	if (!pte)
@@ -1157,7 +1180,8 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 	pteval = ptep_clear_flush(vma, address, pte);
 
 	/* Move the dirty bit to the physical page now the pte is gone. */
-	if (pte_dirty(pteval))
+	dirty = pte_dirty(pteval);
+	if (dirty)
 		set_page_dirty(page);
 
 	/* Update high watermark before we lower rss */
@@ -1186,6 +1210,19 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 		swp_entry_t entry = { .val = page_private(page) };
 		pte_t swp_pte;
 
+		if (flags & TTU_FREE) {
+			VM_BUG_ON_PAGE(PageSwapCache(page), page);
+			if (dirty || PageDirty(page)) {
+				set_pte_at(mm, address, pte, pteval);
+				ret = SWAP_FAIL;
+				goto out_unmap;
+			} else {
+				/* It's a freeable page by MADV_FREE */
+				dec_mm_counter(mm, MM_ANONPAGES);
+				goto discard;
+			}
+		}
+
 		if (PageSwapCache(page)) {
 			/*
 			 * Store the swap location in the pte.
@@ -1227,6 +1264,7 @@ static int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 	} else
 		dec_mm_counter(mm, MM_FILEPAGES);
 
+discard:
 	page_remove_rmap(page);
 	page_cache_release(page);
 
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 6d24fd63b209..d88413ccadcc 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -707,13 +707,17 @@ enum page_references {
 };
 
 static enum page_references page_check_references(struct page *page,
-						  struct scan_control *sc)
+						  struct scan_control *sc,
+						  bool *freeable)
 {
 	int referenced_ptes, referenced_page;
 	unsigned long vm_flags;
+	int pte_dirty;
+
+	VM_BUG_ON_PAGE(!PageLocked(page), page);
 
 	referenced_ptes = page_referenced(page, 1, sc->target_mem_cgroup,
-					  &vm_flags);
+					  &vm_flags, &pte_dirty);
 	referenced_page = TestClearPageReferenced(page);
 
 	/*
@@ -754,6 +758,10 @@ static enum page_references page_check_references(struct page *page,
 		return PAGEREF_KEEP;
 	}
 
+	if (PageAnon(page) && !pte_dirty && !PageSwapCache(page) &&
+			!PageDirty(page))
+		*freeable = true;
+
 	/* Reclaim if clean, defer dirty pages to writeback */
 	if (referenced_page && !PageSwapBacked(page))
 		return PAGEREF_RECLAIM_CLEAN;
@@ -823,6 +831,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,
 		int may_enter_fs;
 		enum page_references references = PAGEREF_RECLAIM_CLEAN;
 		bool dirty, writeback;
+		bool freeable = false;
 
 		cond_resched();
 
@@ -945,7 +954,8 @@ static unsigned long shrink_page_list(struct list_head *page_list,
 		}
 
 		if (!force_reclaim)
-			references = page_check_references(page, sc);
+			references = page_check_references(page, sc,
+							&freeable);
 
 		switch (references) {
 		case PAGEREF_ACTIVATE:
@@ -961,7 +971,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,
 		 * Anonymous process memory has backing store?
 		 * Try to allocate it some swap space here.
 		 */
-		if (PageAnon(page) && !PageSwapCache(page)) {
+		if (PageAnon(page) && !PageSwapCache(page) && !freeable) {
 			if (!(sc->gfp_mask & __GFP_IO))
 				goto keep_locked;
 			if (!add_to_swap(page, page_list))
@@ -976,8 +986,9 @@ static unsigned long shrink_page_list(struct list_head *page_list,
 		 * The page is mapped into the page tables of one or more
 		 * processes. Try to unmap it here.
 		 */
-		if (page_mapped(page) && mapping) {
-			switch (try_to_unmap(page, ttu_flags)) {
+		if (page_mapped(page) && (mapping || freeable)) {
+			switch (try_to_unmap(page, ttu_flags |
+					(freeable ? TTU_FREE : 0))) {
 			case SWAP_FAIL:
 				goto activate_locked;
 			case SWAP_AGAIN:
@@ -985,7 +996,20 @@ static unsigned long shrink_page_list(struct list_head *page_list,
 			case SWAP_MLOCK:
 				goto cull_mlocked;
 			case SWAP_SUCCESS:
-				; /* try to free the page below */
+				/* try to free the page below */
+				if (!freeable)
+					break;
+				/*
+				 * Freeable anon page doesn't have mapping
+				 * due to skipping of swapcache so we free
+				 * page in here rather than __remove_mapping.
+				 */
+				VM_BUG_ON_PAGE(PageSwapCache(page), page);
+				if (!page_freeze_refs(page, 1))
+					goto keep_locked;
+				__clear_page_locked(page);
+				count_vm_event(PGLAZYFREED);
+				goto free_it;
 			}
 		}
 
@@ -1727,7 +1751,7 @@ static void shrink_active_list(unsigned long nr_to_scan,
 		}
 
 		if (page_referenced(page, 0, sc->target_mem_cgroup,
-				    &vm_flags)) {
+				    &vm_flags, NULL)) {
 			nr_rotated += hpage_nr_pages(page);
 			/*
 			 * Identify referenced, file-backed active pages and
diff --git a/mm/vmstat.c b/mm/vmstat.c
index eef6321c8470..da18337c6c66 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -794,6 +794,7 @@ const char * const vmstat_text[] = {
 
 	"pgfault",
 	"pgmajfault",
+	"pglazyfreed",
 
 	TEXTS_FOR_ZONES("pgrefill")
 	TEXTS_FOR_ZONES("pgsteal_kswapd")
-- 
2.0.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related

* [PATCH v11 2/7] x86: add pmd_[dirty|mkclean] for THP
From: Minchan Kim @ 2014-07-08  6:03 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, linux-mm, Michael Kerrisk, Linux API, Hugh Dickins,
	Johannes Weiner, Rik van Riel, KOSAKI Motohiro, Mel Gorman,
	Jason Evans, Zhang Yanfei, Kirill A. Shutemov, Minchan Kim,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86
In-Reply-To: <1404799424-1120-1-git-send-email-minchan@kernel.org>

MADV_FREE needs pmd_dirty and pmd_mkclean for detecting recent
overwrite of the contents since MADV_FREE syscall is called for
THP page.

This patch adds pmd_dirty and pmd_mkclean for THP page MADV_FREE
support.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 arch/x86/include/asm/pgtable.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 0ec056012618..329865799653 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -104,6 +104,11 @@ static inline int pmd_young(pmd_t pmd)
 	return pmd_flags(pmd) & _PAGE_ACCESSED;
 }
 
+static inline int pmd_dirty(pmd_t pmd)
+{
+	return pmd_flags(pmd) & _PAGE_DIRTY;
+}
+
 static inline int pte_write(pte_t pte)
 {
 	return pte_flags(pte) & _PAGE_RW;
@@ -267,6 +272,11 @@ static inline pmd_t pmd_mkold(pmd_t pmd)
 	return pmd_clear_flags(pmd, _PAGE_ACCESSED);
 }
 
+static inline pmd_t pmd_mkclean(pmd_t pmd)
+{
+	return pmd_clear_flags(pmd, _PAGE_DIRTY);
+}
+
 static inline pmd_t pmd_wrprotect(pmd_t pmd)
 {
 	return pmd_clear_flags(pmd, _PAGE_RW);
-- 
2.0.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox