Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1267627 > unrolled thread

[PATCH v3 00/17] MADFV_FREE support

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 00/17] MADFV_FREE support Minchan Kim <minchan@kernel.org> - 2015-11-12 05:40 +0100
    [PATCH v3 08/17] x86: add pmd_[dirty|mkclean] for THP Minchan Kim <minchan@kernel.org> - 2015-11-12 05:40 +0100
    [PATCH v3 01/17] mm: support madvise(MADV_FREE) Minchan Kim <minchan@kernel.org> - 2015-11-12 05:40 +0100
      Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE) Andy Lutomirski <luto@amacapital.net> - 2015-11-12 06:00 +0100
        Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE) Daniel Micay <danielmicay@gmail.com> - 2015-11-12 06:30 +0100
          Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE) Minchan Kim <minchan@kernel.org> - 2015-11-13 07:20 +0100
            Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE) Daniel Micay <danielmicay@gmail.com> - 2015-11-13 07:20 +0100
              Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE) Minchan Kim <minchan@kernel.org> - 2015-11-13 07:40 +0100
                Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE) Daniel Micay <danielmicay@gmail.com> - 2015-11-13 07:50 +0100
                  Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE) Minchan Kim <minchan@kernel.org> - 2015-11-13 08:10 +0100
                    Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE) Daniel Micay <danielmicay@gmail.com> - 2015-11-13 09:20 +0100
                      Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE) Andy Lutomirski <luto@amacapital.net> - 2015-11-13 20:50 +0100
                        Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE) Minchan Kim <minchan@kernel.org> - 2015-11-16 03:20 +0100
                          Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE) yalin wang <yalin.wang2010@gmail.com> - 2015-11-16 04:20 +0100
      Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE) "Kirill A. Shutemov" <kirill@shutemov.name> - 2015-11-12 12:30 +0100
        Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE) Minchan Kim <minchan@kernel.org> - 2015-11-13 07:20 +0100
    [PATCH v3 11/17] arm: add pmd_mkclean for THP Minchan Kim <minchan@kernel.org> - 2015-11-12 05:40 +0100

#1267627 — [PATCH v3 00/17] MADFV_FREE support

MADV_FREE is on linux-next so long time. The reason was two, I think.

1. MADV_FREE code on reclaim path was really mess.

2. Andrew really want to see voice of userland people who want to use
   the syscall.

A few month ago, Daniel Micay(jemalloc active contributor) requested me
to make progress upstreaming but I was busy at that time so it took
so long time for me to revist the code and finally, I clean it up the
mess recently so it solves the #2 issue.

As well, Daniel and Jason(jemalloc maintainer) requested it to Andrew
again recently and they said it would be great to have even though
it has swap dependency now so Andrew decided he will do that for v4.4.

However, there were some concerns, still.

* hotness

Someone think MADV_FREEed pages are really cold while others are not.
Look at detail in decscription of mm: add knob to tune lazyfreeing.

* swap dependency

In old version, MADV_FREE is equal to MADV_DONTNEED on swapless
system because we don't have aged anonymous LRU list on swapless.
So there are requests for MADV_FREE to support swapless system.

For addressing issues, this version includes new LRU list for
hinted pages and tuning knob. With that, we could support swapless
without zapping hinted pages instantly.

Please, review and comment.

I have been tested it on v4.3-rc7 and couldn't find any problem so far.

git: git://git.kernel.org/pub/scm/linux/kernel/git/minchan/linux.git
branch: mm/madv_free-v4.3-rc7-v3-lazyfreelru

In this stage, I don't think we need to write man page.
It could be done after solid policy and implementation.

 * Change from v2
   * add new LRU list and tuning knob
   * support swapless

 * Change from v1
   * Don't do unnecessary TLB flush - Shaohua
   * Added Acked-by - Hugh, Michal
   * Merge deactivate_page and deactivate_file_page
   * Add pmd_dirty/pmd_mkclean patches for several arches
   * Add lazy THP split patch
   * Drop zhangyanfei@cn.fujitsu.com - Delivery Failure

Chen Gang (1):
  arch: uapi: asm: mman.h: Let MADV_FREE have same value for all
    architectures

Minchan Kim (16):
  mm: support madvise(MADV_FREE)
  mm: define MADV_FREE for some arches
  mm: free swp_entry in madvise_free
  mm: move lazily freed pages to inactive list
  mm: clear PG_dirty to mark page freeable
  mm: mark stable page dirty in KSM
  x86: add pmd_[dirty|mkclean] for THP
  sparc: add pmd_[dirty|mkclean] for THP
  powerpc: add pmd_[dirty|mkclean] for THP
  arm: add pmd_mkclean for THP
  arm64: add pmd_mkclean for THP
  mm: don't split THP page when syscall is called
  mm: introduce wrappers to add new LRU
  mm: introduce lazyfree LRU list
  mm: support MADV_FREE on swapless system
  mm: add knob to tune lazyfreeing

 Documentation/sysctl/vm.txt               |  13 +++
 arch/alpha/include/uapi/asm/mman.h        |   1 +
 arch/arm/include/asm/pgtable-3level.h     |   1 +
 arch/arm64/include/asm/pgtable.h          |   1 +
 arch/mips/include/uapi/asm/mman.h         |   1 +
 arch/parisc/include/uapi/asm/mman.h       |   1 +
 arch/powerpc/include/asm/pgtable-ppc64.h  |   2 +
 arch/sparc/include/asm/pgtable_64.h       |   9 ++
 arch/x86/include/asm/pgtable.h            |   5 +
 arch/xtensa/include/uapi/asm/mman.h       |   1 +
 drivers/base/node.c                       |   2 +
 drivers/staging/android/lowmemorykiller.c |   3 +-
 fs/proc/meminfo.c                         |   2 +
 include/linux/huge_mm.h                   |   3 +
 include/linux/memcontrol.h                |   1 +
 include/linux/mm_inline.h                 |  83 ++++++++++++++-
 include/linux/mmzone.h                    |  16 ++-
 include/linux/page-flags.h                |   5 +
 include/linux/rmap.h                      |   1 +
 include/linux/swap.h                      |  18 +++-
 include/linux/vm_event_item.h             |   3 +-
 include/trace/events/vmscan.h             |  38 ++++---
 include/uapi/asm-generic/mman-common.h    |   1 +
 kernel/sysctl.c                           |   9 ++
 mm/compaction.c                           |  14 ++-
 mm/huge_memory.c                          |  51 +++++++--
 mm/ksm.c                                  |   6 ++
 mm/madvise.c                              | 171 ++++++++++++++++++++++++++++++
 mm/memcontrol.c                           |  44 +++++++-
 mm/memory-failure.c                       |   7 +-
 mm/memory_hotplug.c                       |   3 +-
 mm/mempolicy.c                            |   3 +-
 mm/migrate.c                              |  28 ++---
 mm/page_alloc.c                           |   3 +
 mm/rmap.c                                 |  14 +++
 mm/swap.c                                 | 128 +++++++++++++++-------
 mm/swap_state.c                           |  11 +-
 mm/truncate.c                             |   2 +-
 mm/vmscan.c                               | 157 ++++++++++++++++++++-------
 mm/vmstat.c                               |   4 +
 40 files changed, 713 insertions(+), 153 deletions(-)

-- 
1.9.1

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

[toc] | [next] | [standalone]

#1267629 — [PATCH v3 08/17] x86: add pmd_[dirty|mkclean] for THP

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.

Signed-off-by: Minchan Kim <minchan@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
 arch/x86/include/asm/pgtable.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 867da5bbb4a3..b964d54300e1 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -267,6 +267,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);
-- 
1.9.1

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

[toc] | [prev] | [next] | [standalone]

#1267630 — [PATCH v3 01/17] mm: support madvise(MADV_FREE)

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).

Jason Evans said:

: Facebook has been using MAP_UNINITIALIZED
: (https://lkml.org/lkml/2012/1/18/308) in some of its applications for
: several years, but there are operational costs to maintaining this
: out-of-tree in our kernel and in jemalloc, and we are anxious to retire it
: in favor of MADV_FREE.  When we first enabled MAP_UNINITIALIZED it
: increased throughput for much of our workload by ~5%, and although the
: benefit has decreased using newer hardware and kernels, there is still
: enough benefit that we cannot reasonably retire it without a replacement.
:
: Aside from Facebook operations, there are numerous broadly used
: applications that would benefit from MADV_FREE.  The ones that immediately
: come to mind are redis, varnish, and MariaDB.  I don't have much insight
: into Android internals and development process, but I would hope to see
: MADV_FREE support eventually end up there as well to benefit applications
: linked with the integrated jemalloc.
:
: jemalloc will use MADV_FREE once it becomes available in the Linux kernel.
: In fact, jemalloc already uses MADV_FREE or equivalent everywhere it's
: available: *BSD, OS X, Windows, and Solaris -- every platform except Linux
: (and AIX, but I'm not sure it even compiles on AIX).  The lack of
: MADV_FREE on Linux forced me down a long series of increasingly
: sophisticated heuristics for madvise() volume reduction, and even so this
: remains a common performance issue for people using jemalloc on Linux.
: Please integrate MADV_FREE; many people will benefit substantially.

How it works:

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):                12
On-line CPU(s) list:   0-11
Thread(s) per core:    1
Core(s) per socket:    1
Socket(s):             12
NUMA node(s):          1
Vendor ID:             GenuineIntel
CPU family:            6
Model:                 2
Stepping:              3
CPU MHz:               3200.185
BogoMIPS:              6400.53
Virtualization:        VT-x
Hypervisor vendor:     KVM
Virtualization type:   full
L1d cache:             32K
L1i cache:             32K
L2 cache:              4096K
NUMA node0 CPU(s):     0-11
ebizzy benchmark(./ebizzy -S 10 -n 512)

Higher avg is better.

 vanilla-jemalloc		MADV_free-jemalloc

1 thread
records: 10			    records: 10
avg:	2961.90			    avg:   12069.70
std:	  71.96(2.43%)		    std:     186.68(1.55%)
max:	3070.00			    max:   12385.00
min:	2796.00			    min:   11746.00

2 thread
records: 10			    records: 10
avg:	5020.00			    avg:   17827.00
std:	 264.87(5.28%)		    std:     358.52(2.01%)
max:	5244.00			    max:   18760.00
min:	4251.00			    min:   17382.00

4 thread
records: 10			    records: 10
avg:	8988.80			    avg:   27930.80
std:	1175.33(13.08%)		    std:    3317.33(11.88%)
max:	9508.00			    max:   30879.00
min:	5477.00			    min:   21024.00

8 thread
records: 10			    records: 10
avg:   13036.50			    avg:   33739.40
std:	 170.67(1.31%)		    std:    5146.22(15.25%)
max:   13371.00			    max:   40572.00
min:   12785.00			    min:   24088.00

16 thread
records: 10			    records: 10
avg:   11092.40			    avg:   31424.20
std:	 710.60(6.41%)		    std:    3763.89(11.98%)
max:   12446.00			    max:   36635.00
min:	9949.00			    min:   25669.00

32 thread
records: 10			    records: 10
avg:   11067.00			    avg:   34495.80
std:	 971.06(8.77%)		    std:    2721.36(7.89%)
max:   12010.00			    max:   38598.00
min:	9002.00			    min:   30636.00

In summary, MADV_FREE is about much faster than MADV_DONTNEED.

Acked-by: Hugh Dickins <hughd@google.com>
Reviewed-by: Michal Hocko <mhocko@suse.cz>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 include/linux/rmap.h                   |   1 +
 include/linux/vm_event_item.h          |   1 +
 include/uapi/asm-generic/mman-common.h |   1 +
 mm/madvise.c                           | 132 +++++++++++++++++++++++++++++++++
 mm/rmap.c                              |   7 ++
 mm/swap_state.c                        |   5 +-
 mm/vmscan.c                            |  10 ++-
 mm/vmstat.c                            |   1 +
 8 files changed, 153 insertions(+), 5 deletions(-)

diff --git a/include/linux/rmap.h b/include/linux/rmap.h
index 29446aeef36e..f4c992826242 100644
--- a/include/linux/rmap.h
+++ b/include/linux/rmap.h
@@ -85,6 +85,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/include/linux/vm_event_item.h b/include/linux/vm_event_item.h
index 9246d32dc973..2b1cef88b827 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 c889fcbb530e..a8813f7b37b3 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -20,6 +20,9 @@
 #include <linux/backing-dev.h>
 #include <linux/swap.h>
 #include <linux/swapops.h>
+#include <linux/mmu_notifier.h>
+
+#include <asm/tlb.h>
 
 /*
  * Any behaviour which results in changes to the vma->vm_flags needs to
@@ -32,6 +35,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 */
@@ -256,6 +260,125 @@ 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 mmu_gather *tlb = walk->private;
+	struct mm_struct *mm = tlb->mm;
+	struct vm_area_struct *vma = walk->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_present(ptent))
+			continue;
+
+		page = vm_normal_page(vma, addr, ptent);
+		if (!page)
+			continue;
+
+		if (PageSwapCache(page)) {
+			if (!trylock_page(page))
+				continue;
+
+			if (!try_to_free_swap(page)) {
+				unlock_page(page);
+				continue;
+			}
+
+			ClearPageDirty(page);
+			unlock_page(page);
+		}
+
+		if (pte_young(ptent) || pte_dirty(ptent)) {
+			/*
+			 * 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 mm_walk free_walk = {
+		.pmd_entry = madvise_free_pte_range,
+		.mm = vma->vm_mm,
+		.private = tlb,
+	};
+
+	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_is_anonymous(vma))
+		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
@@ -379,6 +502,14 @@ 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);
+		/* passthrough */
 	case MADV_DONTNEED:
 		return madvise_dontneed(vma, prev, start, end);
 	default:
@@ -398,6 +529,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 f5b5c1f3dcd7..9449e91839ab 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1374,6 +1374,12 @@ 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 (!PageDirty(page) && (flags & TTU_FREE)) {
+			/* 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.
@@ -1414,6 +1420,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/swap_state.c b/mm/swap_state.c
index d504adb7fa5f..10f63eded7b7 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -185,13 +185,12 @@ int add_to_swap(struct page *page, struct list_head *list)
 	 * deadlock in the swap out path.
 	 */
 	/*
-	 * Add it to the swap cache and mark it dirty
+	 * Add it to the swap cache.
 	 */
 	err = add_to_swap_cache(page, entry,
 			__GFP_HIGH|__GFP_NOMEMALLOC|__GFP_NOWARN);
 
-	if (!err) {	/* Success */
-		SetPageDirty(page);
+	if (!err) {
 		return 1;
 	} else {	/* -ENOMEM radix-tree allocation failure */
 		/*
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 7f63a9381f71..7a415b9fdd34 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -906,6 +906,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();
 
@@ -1049,6 +1050,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,
 				goto keep_locked;
 			if (!add_to_swap(page, page_list))
 				goto activate_locked;
+			freeable = true;
 			may_enter_fs = 1;
 
 			/* Adding to swap updated mapping */
@@ -1060,8 +1062,9 @@ static unsigned long shrink_page_list(struct list_head *page_list,
 		 * processes. Try to unmap it here.
 		 */
 		if (page_mapped(page) && mapping) {
-			switch (try_to_unmap(page,
-					ttu_flags|TTU_BATCH_FLUSH)) {
+			switch (try_to_unmap(page, freeable ?
+				(ttu_flags | TTU_BATCH_FLUSH | TTU_FREE) :
+				(ttu_flags | TTU_BATCH_FLUSH))) {
 			case SWAP_FAIL:
 				goto activate_locked;
 			case SWAP_AGAIN:
@@ -1186,6 +1189,9 @@ static unsigned long shrink_page_list(struct list_head *page_list,
 		 */
 		__clear_page_locked(page);
 free_it:
+		if (freeable && !PageDirty(page))
+			count_vm_event(PGLAZYFREED);
+
 		nr_reclaimed++;
 
 		/*
diff --git a/mm/vmstat.c b/mm/vmstat.c
index fbf14485a049..59d45b22355f 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -759,6 +759,7 @@ const char * const vmstat_text[] = {
 
 	"pgfault",
 	"pgmajfault",
+	"pglazyfreed",
 
 	TEXTS_FOR_ZONES("pgrefill")
 	TEXTS_FOR_ZONES("pgsteal_kswapd")
-- 
1.9.1

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

[toc] | [prev] | [next] | [standalone]

#1267638 — Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE)

On Wed, Nov 11, 2015 at 8:32 PM, Minchan Kim <minchan@kernel.org> 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.


>
> 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.
>

I realize that this lends itself to an efficient implementation, but
it's certainly the case that the kernel *could* use the accessed bit
instead of the dirty bit to give more sensible user semantics, and the
semantics that rely on the dirty bit make me uncomfortable from an ABI
perspective.

I also think that the kernel should commit to either zeroing the page
or leaving it unchanged in response to MADV_FREE (even if the decision
of which to do is made later on).  I think that your patch series does
this, but only after a few of the patches are applied (the swap entry
freeing), and I think that it should be a real guaranteed part of the
semantics and maybe have a test case.

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

[toc] | [prev] | [next] | [standalone]

#1267644 — Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE)

[Multipart message — attachments visible in raw view] — view raw

> I also think that the kernel should commit to either zeroing the page
> or leaving it unchanged in response to MADV_FREE (even if the decision
> of which to do is made later on).  I think that your patch series does
> this, but only after a few of the patches are applied (the swap entry
> freeing), and I think that it should be a real guaranteed part of the
> semantics and maybe have a test case.

This would be a good thing to test because it would be required to add
MADV_FREE_UNDO down the road. It would mean the same semantics as the
MEM_RESET and MEM_RESET_UNDO features on Windows, and there's probably
value in that for the sake of migrating existing software too.

For one example, it could be dropped into Firefox:

https://dxr.mozilla.org/mozilla-central/source/memory/volatile/VolatileBufferWindows.cpp

And in Chromium:

https://code.google.com/p/chromium/codesearch#chromium/src/base/memory/discardable_shared_memory.cc

Worth noting that both also support the API for pinning/unpinning that's
used by Android's ashmem too. Linux really needs a feature like this for
caches. Firefox simply doesn't drop the memory at all on Linux right now:

https://dxr.mozilla.org/mozilla-central/source/memory/volatile/VolatileBufferFallback.cpp

(Lock == pin, Unlock == unpin)

For reference:

https://msdn.microsoft.com/en-us/library/windows/desktop/aa366887(v=vs.85).aspx

[toc] | [prev] | [next] | [standalone]

#1268640 — Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE)

On Thu, Nov 12, 2015 at 12:21:30AM -0500, Daniel Micay wrote:
> > I also think that the kernel should commit to either zeroing the page
> > or leaving it unchanged in response to MADV_FREE (even if the decision
> > of which to do is made later on).  I think that your patch series does
> > this, but only after a few of the patches are applied (the swap entry
> > freeing), and I think that it should be a real guaranteed part of the
> > semantics and maybe have a test case.
> 
> This would be a good thing to test because it would be required to add
> MADV_FREE_UNDO down the road. It would mean the same semantics as the
> MEM_RESET and MEM_RESET_UNDO features on Windows, and there's probably
> value in that for the sake of migrating existing software too.

So, do you mean that we could implement MADV_FREE_UNDO with "read"
opearation("just access bit marking) easily in future?

If so, it would be good reason to change MADV_FREE from dirty bit to
access bit. Okay, I will look at that.

> 
> For one example, it could be dropped into Firefox:
> 
> https://dxr.mozilla.org/mozilla-central/source/memory/volatile/VolatileBufferWindows.cpp
> 
> And in Chromium:
> 
> https://code.google.com/p/chromium/codesearch#chromium/src/base/memory/discardable_shared_memory.cc
> 
> Worth noting that both also support the API for pinning/unpinning that's
> used by Android's ashmem too. Linux really needs a feature like this for
> caches. Firefox simply doesn't drop the memory at all on Linux right now:
> 
> https://dxr.mozilla.org/mozilla-central/source/memory/volatile/VolatileBufferFallback.cpp
> 
> (Lock == pin, Unlock == unpin)
> 
> For reference:
> 
> https://msdn.microsoft.com/en-us/library/windows/desktop/aa366887(v=vs.85).aspx
> 


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

[toc] | [prev] | [next] | [standalone]

#1268642 — Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE)

[Multipart message — attachments visible in raw view] — view raw

On 13/11/15 01:15 AM, Minchan Kim wrote:
> On Thu, Nov 12, 2015 at 12:21:30AM -0500, Daniel Micay wrote:
>>> I also think that the kernel should commit to either zeroing the page
>>> or leaving it unchanged in response to MADV_FREE (even if the decision
>>> of which to do is made later on).  I think that your patch series does
>>> this, but only after a few of the patches are applied (the swap entry
>>> freeing), and I think that it should be a real guaranteed part of the
>>> semantics and maybe have a test case.
>>
>> This would be a good thing to test because it would be required to add
>> MADV_FREE_UNDO down the road. It would mean the same semantics as the
>> MEM_RESET and MEM_RESET_UNDO features on Windows, and there's probably
>> value in that for the sake of migrating existing software too.
> 
> So, do you mean that we could implement MADV_FREE_UNDO with "read"
> opearation("just access bit marking) easily in future?
> 
> If so, it would be good reason to change MADV_FREE from dirty bit to
> access bit. Okay, I will look at that.

I just meant testing that the data is either zero or the old data if
it's read before it's written to. Not having it stay around once there
is a read. Not sure if that's what Andy meant.

[toc] | [prev] | [next] | [standalone]

#1268648 — Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE)

On Fri, Nov 13, 2015 at 01:16:54AM -0500, Daniel Micay wrote:
> On 13/11/15 01:15 AM, Minchan Kim wrote:
> > On Thu, Nov 12, 2015 at 12:21:30AM -0500, Daniel Micay wrote:
> >>> I also think that the kernel should commit to either zeroing the page
> >>> or leaving it unchanged in response to MADV_FREE (even if the decision
> >>> of which to do is made later on).  I think that your patch series does
> >>> this, but only after a few of the patches are applied (the swap entry
> >>> freeing), and I think that it should be a real guaranteed part of the
> >>> semantics and maybe have a test case.
> >>
> >> This would be a good thing to test because it would be required to add
> >> MADV_FREE_UNDO down the road. It would mean the same semantics as the
> >> MEM_RESET and MEM_RESET_UNDO features on Windows, and there's probably
> >> value in that for the sake of migrating existing software too.
> > 
> > So, do you mean that we could implement MADV_FREE_UNDO with "read"
> > opearation("just access bit marking) easily in future?
> > 
> > If so, it would be good reason to change MADV_FREE from dirty bit to
> > access bit. Okay, I will look at that.
> 
> I just meant testing that the data is either zero or the old data if
> it's read before it's written to. Not having it stay around once there
> is a read. Not sure if that's what Andy meant.

Either zero of old data is gauranteed.
Now:

        MADV_FREE(range)
        A = read from the range
        ...
        ...
        B = read from the range


        A and B could have different value. But value should be old or zero.

But Andy want more strict ABI so he suggested access bit instead of dirty bit.

        MADV_FREE(range)
        A = read from the range
        ...
        ...
        B = read from the range

        A and B cannot have different value.

And now I am thinking if we use access bit, we could implment MADV_FREE_UNDO
easily when we need it. Maybe, that's what you want. Right?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1268654 — Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE)

[Multipart message — attachments visible in raw view] — view raw

> And now I am thinking if we use access bit, we could implment MADV_FREE_UNDO
> easily when we need it. Maybe, that's what you want. Right?

Yes, but why the access bit instead of the dirty bit for that? It could
always be made more strict (i.e. access bit) in the future, while going
the other way won't be possible. So I think the dirty bit is really the
more conservative choice since if it turns out to be a mistake it can be
fixed without a backwards incompatible change.

[toc] | [prev] | [next] | [standalone]

#1268659 — Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE)

On Fri, Nov 13, 2015 at 01:45:52AM -0500, Daniel Micay wrote:
> > And now I am thinking if we use access bit, we could implment MADV_FREE_UNDO
> > easily when we need it. Maybe, that's what you want. Right?
> 
> Yes, but why the access bit instead of the dirty bit for that? It could
> always be made more strict (i.e. access bit) in the future, while going
> the other way won't be possible. So I think the dirty bit is really the
> more conservative choice since if it turns out to be a mistake it can be
> fixed without a backwards incompatible change.

Absolutely true. That's why I insist on dirty bit until now although
I didn't tell the reason. But I thought you wanted to change for using
access bit for the future, too. It seems MADV_FREE start to bloat
over and over again before knowing real problems and usecases.
It's almost same situation with volatile ranges so I really want to
stop at proper point which maintainer should decide, I hope.
Without it, we will make the feature a lot heavy by just brain storming
and then causes lots of churn in MM code without real bebenfit
It would be very painful for us.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1268680 — Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE)

[Multipart message — attachments visible in raw view] — view raw

On 13/11/15 02:03 AM, Minchan Kim wrote:
> On Fri, Nov 13, 2015 at 01:45:52AM -0500, Daniel Micay wrote:
>>> And now I am thinking if we use access bit, we could implment MADV_FREE_UNDO
>>> easily when we need it. Maybe, that's what you want. Right?
>>
>> Yes, but why the access bit instead of the dirty bit for that? It could
>> always be made more strict (i.e. access bit) in the future, while going
>> the other way won't be possible. So I think the dirty bit is really the
>> more conservative choice since if it turns out to be a mistake it can be
>> fixed without a backwards incompatible change.
> 
> Absolutely true. That's why I insist on dirty bit until now although
> I didn't tell the reason. But I thought you wanted to change for using
> access bit for the future, too. It seems MADV_FREE start to bloat
> over and over again before knowing real problems and usecases.
> It's almost same situation with volatile ranges so I really want to
> stop at proper point which maintainer should decide, I hope.
> Without it, we will make the feature a lot heavy by just brain storming
> and then causes lots of churn in MM code without real bebenfit
> It would be very painful for us.

Well, I don't think you need more than a good API and an implementation
with no known bugs, kernel security concerns or backwards compatibility
issues. Configuration and API extensions are something for later (i.e.
land a baseline, then submit stuff like sysctl tunables). Just my take
on it though...

[toc] | [prev] | [next] | [standalone]

#1269179 — Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE)

On Fri, Nov 13, 2015 at 12:13 AM, Daniel Micay <danielmicay@gmail.com> wrote:
> On 13/11/15 02:03 AM, Minchan Kim wrote:
>> On Fri, Nov 13, 2015 at 01:45:52AM -0500, Daniel Micay wrote:
>>>> And now I am thinking if we use access bit, we could implment MADV_FREE_UNDO
>>>> easily when we need it. Maybe, that's what you want. Right?
>>>
>>> Yes, but why the access bit instead of the dirty bit for that? It could
>>> always be made more strict (i.e. access bit) in the future, while going
>>> the other way won't be possible. So I think the dirty bit is really the
>>> more conservative choice since if it turns out to be a mistake it can be
>>> fixed without a backwards incompatible change.
>>
>> Absolutely true. That's why I insist on dirty bit until now although
>> I didn't tell the reason. But I thought you wanted to change for using
>> access bit for the future, too. It seems MADV_FREE start to bloat
>> over and over again before knowing real problems and usecases.
>> It's almost same situation with volatile ranges so I really want to
>> stop at proper point which maintainer should decide, I hope.
>> Without it, we will make the feature a lot heavy by just brain storming
>> and then causes lots of churn in MM code without real bebenfit
>> It would be very painful for us.
>
> Well, I don't think you need more than a good API and an implementation
> with no known bugs, kernel security concerns or backwards compatibility
> issues. Configuration and API extensions are something for later (i.e.
> land a baseline, then submit stuff like sysctl tunables). Just my take
> on it though...
>

As long as it's anonymous MAP_PRIVATE only, then the security aspects
should be okay.  MADV_DONTNEED seems to work on pretty much any VMA,
and there's been long history of interesting bugs there.

As for dirty vs accessed, an argument in favor of going straight to
accessed is that it means that users can write code like this without
worrying about whether they have a kernel that uses the dirty bit:

x = mmap(...);
*x = 1;  /* mark it present */

/* i'm done with it */
*x = 1;
madvise(MADV_FREE, x, ...);

wait a while;

/* is it still there? */
if (*x == 1) {
  /* use whatever was cached there */
} else {
 /* reinitialize it */
 *x = 1;
}

With the dirty bit, this will look like it works, but on occasion
users will lose the race where they probe *x to see if the data was
lost and then the data gets lost before the next write comes in.

Sure, that load from *x could be changed to RMW or users could do a
dummy write (e.g. x[1] = 1; if (*x == 1) ...), but people might forget
to do that, and the caching implications are a little bit worse.

Note that switching to RMW is really really dangerous.  Doing:

*x &= 1;
if (*x == 1) ...;

is safe on x86 if the compiler generates:

andl $1, (%[x]);
cmpl $1, (%[x]);

but is unsafe if the compiler generates:

movl (%[x]), %eax;
andl $1, %eax;
movl %eax, (%[x]);
cmpl $1, %eax;

and even worse if the write is omitted when "provably" unnecessary.

OTOH, if switching to the accessed bit is too much of a mess, then
using the dirty bit at first isn't so bad.

--Andy

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

[toc] | [prev] | [next] | [standalone]

#1269810 — Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE)

On Fri, Nov 13, 2015 at 11:46:07AM -0800, Andy Lutomirski wrote:
> On Fri, Nov 13, 2015 at 12:13 AM, Daniel Micay <danielmicay@gmail.com> wrote:
> > On 13/11/15 02:03 AM, Minchan Kim wrote:
> >> On Fri, Nov 13, 2015 at 01:45:52AM -0500, Daniel Micay wrote:
> >>>> And now I am thinking if we use access bit, we could implment MADV_FREE_UNDO
> >>>> easily when we need it. Maybe, that's what you want. Right?
> >>>
> >>> Yes, but why the access bit instead of the dirty bit for that? It could
> >>> always be made more strict (i.e. access bit) in the future, while going
> >>> the other way won't be possible. So I think the dirty bit is really the
> >>> more conservative choice since if it turns out to be a mistake it can be
> >>> fixed without a backwards incompatible change.
> >>
> >> Absolutely true. That's why I insist on dirty bit until now although
> >> I didn't tell the reason. But I thought you wanted to change for using
> >> access bit for the future, too. It seems MADV_FREE start to bloat
> >> over and over again before knowing real problems and usecases.
> >> It's almost same situation with volatile ranges so I really want to
> >> stop at proper point which maintainer should decide, I hope.
> >> Without it, we will make the feature a lot heavy by just brain storming
> >> and then causes lots of churn in MM code without real bebenfit
> >> It would be very painful for us.
> >
> > Well, I don't think you need more than a good API and an implementation
> > with no known bugs, kernel security concerns or backwards compatibility
> > issues. Configuration and API extensions are something for later (i.e.
> > land a baseline, then submit stuff like sysctl tunables). Just my take
> > on it though...
> >
> 
> As long as it's anonymous MAP_PRIVATE only, then the security aspects
> should be okay.  MADV_DONTNEED seems to work on pretty much any VMA,
> and there's been long history of interesting bugs there.
> 
> As for dirty vs accessed, an argument in favor of going straight to
> accessed is that it means that users can write code like this without
> worrying about whether they have a kernel that uses the dirty bit:
> 
> x = mmap(...);
> *x = 1;  /* mark it present */
> 
> /* i'm done with it */
> *x = 1;
> madvise(MADV_FREE, x, ...);
> 
> wait a while;
> 
> /* is it still there? */
> if (*x == 1) {
>   /* use whatever was cached there */
> } else {
>  /* reinitialize it */
>  *x = 1;
> }
> 
> With the dirty bit, this will look like it works, but on occasion
> users will lose the race where they probe *x to see if the data was
> lost and then the data gets lost before the next write comes in.
> 
> Sure, that load from *x could be changed to RMW or users could do a
> dummy write (e.g. x[1] = 1; if (*x == 1) ...), but people might forget
> to do that, and the caching implications are a little bit worse.

I think your example is the case what people abuse MADV_FREE.
What happens if the object(ie, x) spans multiple pages?
User should know object's memory align and investigate all of pages
which span the object. Hmm, I don't think it's good for API.

> 
> Note that switching to RMW is really really dangerous.  Doing:
> 
> *x &= 1;
> if (*x == 1) ...;
> 
> is safe on x86 if the compiler generates:
> 
> andl $1, (%[x]);
> cmpl $1, (%[x]);
> 
> but is unsafe if the compiler generates:
> 
> movl (%[x]), %eax;
> andl $1, %eax;
> movl %eax, (%[x]);
> cmpl $1, %eax;
> 
> and even worse if the write is omitted when "provably" unnecessary.
> 
> OTOH, if switching to the accessed bit is too much of a mess, then
> using the dirty bit at first isn't so bad.

Thanks! I want to use dirty bit first.

About access bit, I don't want to say it to mess but I guess it would
change a lot subtle thing for all architectures. Because we have used
access bit as just *hint* for aging while dirty bit is really
*critical marker* for system integrity. A example in x86, we don't
keep accuracy of access bit for reducing TLB flush IPI. I don't know
what technique other arches have used but they might have.

Thanks.


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

[toc] | [prev] | [next] | [standalone]

#1269822 — Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE)

> On Nov 16, 2015, at 10:13, Minchan Kim <minchan@kernel.org> wrote:
> 
> On Fri, Nov 13, 2015 at 11:46:07AM -0800, Andy Lutomirski wrote:
>> On Fri, Nov 13, 2015 at 12:13 AM, Daniel Micay <danielmicay@gmail.com> wrote:
>>> On 13/11/15 02:03 AM, Minchan Kim wrote:
>>>> On Fri, Nov 13, 2015 at 01:45:52AM -0500, Daniel Micay wrote:
>>>>>> And now I am thinking if we use access bit, we could implment MADV_FREE_UNDO
>>>>>> easily when we need it. Maybe, that's what you want. Right?
>>>>> 
>>>>> Yes, but why the access bit instead of the dirty bit for that? It could
>>>>> always be made more strict (i.e. access bit) in the future, while going
>>>>> the other way won't be possible. So I think the dirty bit is really the
>>>>> more conservative choice since if it turns out to be a mistake it can be
>>>>> fixed without a backwards incompatible change.
>>>> 
>>>> Absolutely true. That's why I insist on dirty bit until now although
>>>> I didn't tell the reason. But I thought you wanted to change for using
>>>> access bit for the future, too. It seems MADV_FREE start to bloat
>>>> over and over again before knowing real problems and usecases.
>>>> It's almost same situation with volatile ranges so I really want to
>>>> stop at proper point which maintainer should decide, I hope.
>>>> Without it, we will make the feature a lot heavy by just brain storming
>>>> and then causes lots of churn in MM code without real bebenfit
>>>> It would be very painful for us.
>>> 
>>> Well, I don't think you need more than a good API and an implementation
>>> with no known bugs, kernel security concerns or backwards compatibility
>>> issues. Configuration and API extensions are something for later (i.e.
>>> land a baseline, then submit stuff like sysctl tunables). Just my take
>>> on it though...
>>> 
>> 
>> As long as it's anonymous MAP_PRIVATE only, then the security aspects
>> should be okay.  MADV_DONTNEED seems to work on pretty much any VMA,
>> and there's been long history of interesting bugs there.
>> 
>> As for dirty vs accessed, an argument in favor of going straight to
>> accessed is that it means that users can write code like this without
>> worrying about whether they have a kernel that uses the dirty bit:
>> 
>> x = mmap(...);
>> *x = 1;  /* mark it present */
>> 
>> /* i'm done with it */
>> *x = 1;
>> madvise(MADV_FREE, x, ...);
>> 
>> wait a while;
>> 
>> /* is it still there? */
>> if (*x == 1) {
>>  /* use whatever was cached there */
>> } else {
>> /* reinitialize it */
>> *x = 1;
>> }
>> 
>> With the dirty bit, this will look like it works, but on occasion
>> users will lose the race where they probe *x to see if the data was
>> lost and then the data gets lost before the next write comes in.
>> 
>> Sure, that load from *x could be changed to RMW or users could do a
>> dummy write (e.g. x[1] = 1; if (*x == 1) ...), but people might forget
>> to do that, and the caching implications are a little bit worse.
> 
> I think your example is the case what people abuse MADV_FREE.
> What happens if the object(ie, x) spans multiple pages?
> User should know object's memory align and investigate all of pages
> which span the object. Hmm, I don't think it's good for API.
> 
>> 
>> Note that switching to RMW is really really dangerous.  Doing:
>> 
>> *x &= 1;
>> if (*x == 1) ...;
>> 
>> is safe on x86 if the compiler generates:
>> 
>> andl $1, (%[x]);
>> cmpl $1, (%[x]);
>> 
>> but is unsafe if the compiler generates:
>> 
>> movl (%[x]), %eax;
>> andl $1, %eax;
>> movl %eax, (%[x]);
>> cmpl $1, %eax;
>> 
>> and even worse if the write is omitted when "provably" unnecessary.
>> 
>> OTOH, if switching to the accessed bit is too much of a mess, then
>> using the dirty bit at first isn't so bad.
> 
> Thanks! I want to use dirty bit first.
> 
> About access bit, I don't want to say it to mess but I guess it would
> change a lot subtle thing for all architectures. Because we have used
> access bit as just *hint* for aging while dirty bit is really
> *critical marker* for system integrity. A example in x86, we don't
> keep accuracy of access bit for reducing TLB flush IPI. I don't know
> what technique other arches have used but they might have.
> 
> Thanks.
> 
i think use access bit is not easy to implement for ANON page in kernel.
we are sure the Anon page is always PageDirty()  if it is !PageSwapCache() ,
unless it is MADV_FREE page ,
but use access bit , how to distinguish Normal ANON page and  MADV_FREE page?
it can be implemented by Access bit , but not easy, need more code change .

Thanks



 




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

[toc] | [prev] | [next] | [standalone]

#1267831 — Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE)

On Thu, Nov 12, 2015 at 01:32:57PM +0900, Minchan Kim wrote:
> @@ -256,6 +260,125 @@ 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 mmu_gather *tlb = walk->private;
> +	struct mm_struct *mm = tlb->mm;
> +	struct vm_area_struct *vma = walk->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_present(ptent))
> +			continue;
> +
> +		page = vm_normal_page(vma, addr, ptent);
> +		if (!page)
> +			continue;
> +
> +		if (PageSwapCache(page)) {

Could you put VM_BUG_ON_PAGE(PageTransCompound(page), page) here?
Just in case.

> +			if (!trylock_page(page))
> +				continue;
> +
> +			if (!try_to_free_swap(page)) {
> +				unlock_page(page);
> +				continue;
> +			}
> +
> +			ClearPageDirty(page);
> +			unlock_page(page);

Hm. Do we handle pages shared over fork() here?
Souldn't we ignore pages with mapcount > 0?

> +		}
> +
> +		if (pte_young(ptent) || pte_dirty(ptent)) {
> +			/*
> +			 * 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;
> +}
> 

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

[toc] | [prev] | [next] | [standalone]

#1268644 — Re: [PATCH v3 01/17] mm: support madvise(MADV_FREE)

On Thu, Nov 12, 2015 at 01:26:20PM +0200, Kirill A. Shutemov wrote:
> On Thu, Nov 12, 2015 at 01:32:57PM +0900, Minchan Kim wrote:
> > @@ -256,6 +260,125 @@ 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 mmu_gather *tlb = walk->private;
> > +	struct mm_struct *mm = tlb->mm;
> > +	struct vm_area_struct *vma = walk->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_present(ptent))
> > +			continue;
> > +
> > +		page = vm_normal_page(vma, addr, ptent);
> > +		if (!page)
> > +			continue;
> > +
> > +		if (PageSwapCache(page)) {
> 
> Could you put VM_BUG_ON_PAGE(PageTransCompound(page), page) here?
> Just in case.

No problem.

> 
> > +			if (!trylock_page(page))
> > +				continue;
> > +
> > +			if (!try_to_free_swap(page)) {
> > +				unlock_page(page);
> > +				continue;
> > +			}
> > +
> > +			ClearPageDirty(page);
> > +			unlock_page(page);
> 
> Hm. Do we handle pages shared over fork() here?
> Souldn't we ignore pages with mapcount > 0?

It was handled later patch by historical reason but it's better
to fold the patch to this.

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

[toc] | [prev] | [next] | [standalone]

#1267631 — [PATCH v3 11/17] arm: add pmd_mkclean for THP

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_mkclean for THP page MADV_FREE support.

Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 arch/arm/include/asm/pgtable-3level.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/include/asm/pgtable-3level.h b/arch/arm/include/asm/pgtable-3level.h
index a745a2a53853..6d6012a320b2 100644
--- a/arch/arm/include/asm/pgtable-3level.h
+++ b/arch/arm/include/asm/pgtable-3level.h
@@ -249,6 +249,7 @@ PMD_BIT_FUNC(mkold,	&= ~PMD_SECT_AF);
 PMD_BIT_FUNC(mksplitting, |= L_PMD_SECT_SPLITTING);
 PMD_BIT_FUNC(mkwrite,   &= ~L_PMD_SECT_RDONLY);
 PMD_BIT_FUNC(mkdirty,   |= L_PMD_SECT_DIRTY);
+PMD_BIT_FUNC(mkclean,   &= ~L_PMD_SECT_DIRTY);
 PMD_BIT_FUNC(mkyoung,   |= PMD_SECT_AF);
 
 #define pmd_mkhuge(pmd)		(__pmd(pmd_val(pmd) & ~PMD_TABLE_BIT))
-- 
1.9.1

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web