Linux userland API discussions
 help / color / mirror / Atom feed
* Re: CONFIG_ISOLATION=y (was: [PATCH 0/6] support "dataplane" mode for nohz_full)
From: Peter Zijlstra @ 2015-05-12 12:39 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Chris Metcalf, Steven Rostedt, Frederic Weisbecker, Andrew Morton,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Gilad Ben Yossef,
	Rik van Riel, Tejun Heo, Thomas Gleixner, Christoph Lameter,
	Srivatsa S. Bhat, linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150512123440.GA16959-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Tue, May 12, 2015 at 02:34:40PM +0200, Ingo Molnar wrote:
> Yes, that's what I meant: CONFIG_ISOLATION would trigger what is 
> NO_HZ_FULL today - we could possibly even remove CONFIG_NO_HZ_FULL as 
> an individual Kconfig option?

Ah, as a rename of nohz_full, sure that might work.

^ permalink raw reply

* Re: CONFIG_ISOLATION=y (was: [PATCH 0/6] support "dataplane" mode for nohz_full)
From: Ingo Molnar @ 2015-05-12 12:43 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Chris Metcalf, Steven Rostedt, Frederic Weisbecker, Andrew Morton,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Gilad Ben Yossef,
	Rik van Riel, Tejun Heo, Thomas Gleixner, Christoph Lameter,
	Srivatsa S. Bhat, linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150512123912.GO21418-ndre7Fmf5hadTX5a5knrm8zTDFooKrT+cvkQGrU6aU0@public.gmane.org>


* Peter Zijlstra <peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:

> On Tue, May 12, 2015 at 02:34:40PM +0200, Ingo Molnar wrote:
>
> > Yes, that's what I meant: CONFIG_ISOLATION would trigger what is 
> > NO_HZ_FULL today - we could possibly even remove CONFIG_NO_HZ_FULL 
> > as an individual Kconfig option?
> 
> Ah, as a rename of nohz_full, sure that might work.

It could also be named CONFIG_CPU_ISOLATION=y, to make it more 
explicit what it's about.

Thanks,

	Ingo

^ permalink raw reply

* Re: [PATCH 4/6] nohz: support PR_DATAPLANE_QUIESCE
From: Ingo Molnar @ 2015-05-12 12:52 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Chris Metcalf, Gilad Ben Yossef, Steven Rostedt, Ingo Molnar,
	Andrew Morton, Rik van Riel, Tejun Heo, Thomas Gleixner,
	Frederic Weisbecker, Paul E. McKenney, Christoph Lameter,
	Srivatsa S. Bhat, linux-doc, linux-api, linux-kernel
In-Reply-To: <20150512103805.GJ21418@twins.programming.kicks-ass.net>


* Peter Zijlstra <peterz@infradead.org> wrote:

> > So if then a prctl() (or other system call) could be a shortcut 
> > to:
> > 
> >  - move the task to an isolated CPU
> >  - make sure there _is_ such an isolated domain available
> > 
> > I.e. have some programmatic, kernel provided way for an 
> > application to be sure it's running in the right environment. 
> > Relying on random administration flags here and there won't cut 
> > it.
> 
> No, we already have sched_setaffinity() and we should not duplicate 
> its ability to move tasks about.

But sched_setaffinity() does not guarantee isolation - it's just a 
syscall to move a task to a set of CPUs, which might be isolated or 
not.

What I suggested is that it might make sense to offer a system call, 
for example a sched_setparam() variant, that makes such guarantees.

Say if user-space does:

	ret = sched_setscheduler(0, BIND_ISOLATED, &isolation_params);

... then we would get the task moved to an isolated domain and get a 0 
return code if the kernel is able to do all that and if the current 
uid/namespace/etc. has the required permissions and such.

( BIND_ISOLATED will not replace the current p->policy value, so it's
  still possible to use the regular policies as well on top of this. )

I.e. make it programatic instead of relying on a fragile, kernel 
version dependent combination of sysctl, sysfs, kernel config and boot 
parameter details to get us this result.

I.e. provide a central hub to offer this feature in a more structured, 
easier to use fashion.

We might still require the admin (or distro) to separately set up the 
domain of isolated CPUs, and it would still be possible to simply 
'move' tasks there using existing syscalls - but I say that it's not a 
bad idea at all to offer a single central syscall interface for apps 
to request such treatment.

> What this is about is 'clearing' CPU state, its nothing to do with 
> tasks.
> 
> Ideally we'd never have to clear the state because it should be 
> impossible to get into this predicament in the first place.

That I absolutely agree about, that bit is nonsense.

We might offer debugging facilities to debug such bugs, but we won't 
work or hack it around.

Thanks,

	Ingo

^ permalink raw reply

* Re: [PATCH 0/6] support "dataplane" mode for nohz_full
From: Paul E. McKenney @ 2015-05-12 13:18 UTC (permalink / raw)
  To: Chris Metcalf
  Cc: Andy Lutomirski, Ingo Molnar, Andrew Morton, Steven Rostedt,
	Gilad Ben Yossef, Peter Zijlstra, Rik van Riel, Tejun Heo,
	Frederic Weisbecker, Thomas Gleixner, Christoph Lameter,
	Srivatsa S. Bhat, linux-doc@vger.kernel.org, Linux API,
	linux-kernel@vger.kernel.org
In-Reply-To: <55510885.9070101@ezchip.com>

On Mon, May 11, 2015 at 03:52:37PM -0400, Chris Metcalf wrote:
> On 05/09/2015 03:19 AM, Andy Lutomirski wrote:
> >Naming aside, I don't think this should be a per-task flag at all.  We
> >already have way too much overhead per syscall in nohz mode, and it
> >would be nice to get the per-syscall overhead as low as possible.  We
> >should strive, for all tasks, to keep syscall overhead down*and*
> >avoid as many interrupts as possible.
> >
> >That being said, I do see a legitimate use for a way to tell the
> >kernel "I'm going to run in userspace for a long time; stay away".
> >But shouldn't that be a single operation, not an ongoing flag?  IOW, I
> >think that we should have a new syscall quiesce() or something rather
> >than a prctl.
> 
> Yes, if all you are concerned about is quiescing the tick, we could
> probably do it as a new syscall.
> 
> I do note that you'd want to try to actually do the quiesce as late as
> possible - in particular, if you just did it in the usual syscall, you
> might miss out on a timer that is set by softirq, or even something
> that happened when you called schedule() on the syscall exit path.
> Doing it as late as we are doing helps to ensure that that doesn't
> happen.  We could still arrange for this semantics by having a new
> quiesce() syscall set a temporary task bit that was cleared on
> return to userspace, but as you pointed out in a different email,
> that gets tricky if you end up doing multiple user_exit() calls on
> your way back to userspace.
> 
> More to the point, I think it's actually important to know when an
> application believes it's in userspace-only mode as an actual state
> bit, rather than just during its transitional moment.  If an
> application calls the kernel at an unexpected time (third-party code
> is the usual culprit for our customers, whether it's syscalls, page
> faults, or other things) we would prefer to have the "quiesce"
> semantics stay in force and cause the third-party code to be
> visibly very slow, rather than cause a totally unexpected and
> hard-to-diagnose interrupt show up later as we are still going
> around the loop that we thought was safely userspace-only.
> 
> And, for debugging the kernel, it's crazy helpful to have that state
> bit in place: see patch 6/6 in the series for how we can diagnose
> things like "a different core just queued an IPI that will hit a
> dataplane core unexpectedly".  Having that state bit makes this sort
> of thing a trivial check in the kernel and relatively easy to debug.

I agree with this!  It is currently a bit painful to debug problems
that might result in multiple tasks runnable on a given CPU.  If you
suspect a problem, you enable tracing and re-run.  Not paricularly
friendly for chasing down intermittent problems, so some sort of
improvement would be a very good thing.

							Thanx, Paul

> Finally, I proposed a "strict" mode in patch 5/6 where we kill the
> process if it voluntarily enters the kernel by mistake after saying it
> wasn't going to any more.  To do this requires a state bit, so
> carrying another state bit for "quiesce on user entry" seems pretty
> reasonable.
> 
> -- 
> Chris Metcalf, EZChip Semiconductor
> http://www.ezchip.com
> 


^ permalink raw reply

* Re: [PATCH 5/6] nohz: support PR_DATAPLANE_STRICT mode
From: Paul E. McKenney @ 2015-05-12 13:20 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Chris Metcalf, Gilad Ben Yossef, Steven Rostedt, Ingo Molnar,
	Andrew Morton, Rik van Riel, Tejun Heo, Thomas Gleixner,
	Frederic Weisbecker, Christoph Lameter, Srivatsa S. Bhat,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150512093858.GI21418-ndre7Fmf5hadTX5a5knrm8zTDFooKrT+cvkQGrU6aU0@public.gmane.org>

On Tue, May 12, 2015 at 11:38:58AM +0200, Peter Zijlstra wrote:
> On Fri, May 08, 2015 at 01:58:46PM -0400, Chris Metcalf wrote:
> > +++ b/kernel/time/tick-sched.c
> > @@ -436,6 +436,20 @@ static void dataplane_quiesce(void)
> >  			(jiffies - start));
> >  		dump_stack();
> >  	}
> > +
> > +	/*
> > +	 * Kill the process if it violates STRICT mode.  Note that this
> > +	 * code also results in killing the task if a kernel bug causes an
> > +	 * irq to be delivered to this core.
> > +	 */
> > +	if ((task->dataplane_flags & (PR_DATAPLANE_STRICT|PR_DATAPLANE_PRCTL))
> > +	    == PR_DATAPLANE_STRICT) {
> > +		pr_warn("Dataplane STRICT mode violated; process killed.\n");
> > +		dump_stack();
> > +		task->dataplane_flags &= ~PR_DATAPLANE_QUIESCE;
> > +		local_irq_enable();
> > +		do_group_exit(SIGKILL);
> > +	}
> >  }
> 
> So while I'm all for hard fails like this, can we not provide a wee bit
> more information in the siginfo ? And maybe use a slightly less fatal
> signal, such that userspace can actually catch it and dump state in
> debug modes?

Agreed, a bit more debug state would be helpful.

							Thanx, Paul

^ permalink raw reply

* [PATCH v5 0/4] idle memory tracking
From: Vladimir Davydov @ 2015-05-12 13:34 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Johannes Weiner, Michal Hocko, Greg Thelen,
	Michel Lespinasse, David Rientjes, Pavel Emelyanov,
	Cyrill Gorcunov, Jonathan Corbet,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Hi,

This patch set introduces a new user API for tracking user memory pages
that have not been used for a given period of time. The purpose of this
is to provide the userspace with the means of tracking a workload's
working set, i.e. the set of pages that are actively used by the
workload. Knowing the working set size can be useful for partitioning
the system more efficiently, e.g. by tuning memory cgroup limits
appropriately, or for job placement within a compute cluster.

---- USE CASES ----

The unified cgroup hierarchy has memory.low and memory.high knobs, which
are defined as the low and high boundaries for the workload working set
size. However, the working set size of a workload may be unknown or
change in time. With this patch set, one can periodically estimate the
amount of memory unused by each cgroup and tune their memory.low and
memory.high parameters accordingly, therefore optimizing the overall
memory utilization.

Another use case is balancing workloads within a compute cluster.
Knowing how much memory is not really used by a workload unit may help
take a more optimal decision when considering migrating the unit to
another node within the cluster.

Also, as noted by Minchan, this would be useful for per-process reclaim
(https://lwn.net/Articles/545668/). With idle tracking, we could reclaim idle
pages only by smart user memory manager.

---- USER API ----

The user API consists of two new proc files:

 * /proc/kpageidle.  This file implements a bitmap where each bit corresponds
   to a page, indexed by PFN. When the bit is set, the corresponding page is
   idle. A page is considered idle if it has not been accessed since it was
   marked idle. To mark a page idle one should set the bit corresponding to the
   page by writing to the file. A value written to the file is OR-ed with the
   current bitmap value. Only user memory pages can be marked idle, for other
   page types input is silently ignored. Writing to this file beyond max PFN
   results in the ENXIO error. Only available when CONFIG_IDLE_PAGE_TRACKING is
   set.

   This file can be used to estimate the amount of pages that are not
   used by a particular workload as follows:

   1. mark all pages of interest idle by setting corresponding bits in the
      /proc/kpageidle bitmap
   2. wait until the workload accesses its working set
   3. read /proc/kpageidle and count the number of bits set

 * /proc/kpagecgroup.  This file contains a 64-bit inode number of the
   memory cgroup each page is charged to, indexed by PFN. Only available when
   CONFIG_MEMCG is set.

   This file can be used to find all pages (including unmapped file
   pages) accounted to a particular cgroup. Using /proc/kpageidle, one
   can then estimate the cgroup working set size.

For an example of using these files for estimating the amount of unused
memory pages per each memory cgroup, please see the script attached
below.

---- REASONING ----

The reason to introduce the new user API instead of using
/proc/PID/{clear_refs,smaps} is that the latter has two serious
drawbacks:

 - it does not count unmapped file pages
 - it affects the reclaimer logic

The new API attempts to overcome them both. For more details on how it
is achieved, please see the comment to patch 3.

---- CHANGE LOG ----

Changes in v5:

 - Fix possible race between kpageidle_clear_pte_refs() and
   __page_set_anon_rmap() by checking that a page is on an LRU list
   under zone->lru_lock (Minchan).
 - Export idle flag via /proc/kpageflags (Minchan).
 - Rebase on top of 4.1-rc3.

Changes in v4:

This iteration primarily addresses Minchan's comments to v3:

 - Implement /proc/kpageidle as a bitmap instead of using u64 per each page,
   because there does not seem to be any future uses for the other 63 bits.
 - Do not double-increase pra->referenced in page_referenced_one() if the page
   was young and referenced recently.
 - Remove the pointless (page_count == 0) check from kpageidle_get_page().
 - Rename kpageidle_clear_refs() to kpageidle_clear_pte_refs().
 - Improve comments to kpageidle-related functions.
 - Rebase on top of 4.1-rc2.

Note it does not address Minchan's concern of possible __page_set_anon_rmap vs
page_referenced race (see https://lkml.org/lkml/2015/5/3/220) since it is still
unclear if this race can really happen (see https://lkml.org/lkml/2015/5/4/160)

Changes in v3:

 - Enable CONFIG_IDLE_PAGE_TRACKING for 32 bit. Since this feature
   requires two extra page flags and there is no space for them on 32
   bit, page ext is used (thanks to Minchan Kim).
 - Minor code cleanups and comments improved.
 - Rebase on top of 4.1-rc1.

Changes in v2:

 - The main difference from v1 is the API change. In v1 the user can
   only set the idle flag for all pages at once, and for clearing the
   Idle flag on pages accessed via page tables /proc/PID/clear_refs
   should be used.
   The main drawback of the v1 approach, as noted by Minchan, is that on
   big machines setting the idle flag for each pages can result in CPU
   bursts, which would be especially frustrating if the user only wanted
   to estimate the amount of idle pages for a particular process or VMA.
   With the new API a more fine-grained approach is possible: one can
   read a process's /proc/PID/pagemap and set/check the Idle flag only
   for those pages of the process's address space he or she is
   interested in.
   Another good point about the v2 API is that it is possible to limit
   /proc/kpage* scanning rate when the user wants to estimate the total
   number of idle pages, which is unachievable with the v1 approach.
 - Make /proc/kpagecgroup return the ino of the closest online ancestor
   in case the cgroup a page is charged to is offline.
 - Fix /proc/PID/clear_refs not clearing Young page flag.
 - Rebase on top of v4.0-rc6-mmotm-2015-04-01-14-54

v4: https://lkml.org/lkml/2015/5/7/580
v3: https://lkml.org/lkml/2015/4/28/224
v2: https://lkml.org/lkml/2015/4/7/260
v1: https://lkml.org/lkml/2015/3/18/794

---- PATCH SET STRUCTURE ----

The patch set is organized as follows:

 - patch 1 adds page_cgroup_ino() helper for the sake of
   /proc/kpagecgroup
 - patch 2 adds /proc/kpagecgroup, which reports cgroup ino each page is
   charged to
 - patch 3 implements the idle page tracking feature, including the
   userspace API, /proc/kpageidle
 - patch 4 exports idle flag via /proc/kpageflags

---- SIMILAR WORKS ----

Originally, the patch for tracking idle memory was proposed back in 2011
by Michel Lespinasse (see http://lwn.net/Articles/459269/). The main
difference between Michel's patch and this one is that Michel
implemented a kernel space daemon for estimating idle memory size per
cgroup while this patch only provides the userspace with the minimal API
for doing the job, leaving the rest up to the userspace. However, they
both share the same idea of Idle/Young page flags to avoid affecting the
reclaimer logic.

---- SCRIPT FOR COUNTING IDLE PAGES PER CGROUP ----
#! /usr/bin/python
#

import os
import stat
import errno
import struct

CGROUP_MOUNT = "/sys/fs/cgroup/memory"
BUFSIZE = 8 * 1024  # must be multiple of 8


def set_idle():
    f = open("/proc/kpageidle", "wb", BUFSIZE)
    while True:
        try:
            f.write(struct.pack("Q", pow(2, 64) - 1))
        except IOError as err:
            if err.errno == errno.ENXIO:
                break
            raise
    f.close()


def count_idle():
    f_flags = open("/proc/kpageflags", "rb", BUFSIZE)
    f_cgroup = open("/proc/kpagecgroup", "rb", BUFSIZE)
    f_idle = open("/proc/kpageidle", "rb", BUFSIZE)

    pfn = 0
    nr_idle = {}
    while True:
        s = f_flags.read(8)
        if not s:
            break

        flags, = struct.unpack('Q', s)
        cgino, = struct.unpack('Q', f_cgroup.read(8))

        bit = pfn % 64
        if not bit:
            idle_bitmap, = struct.unpack('Q', f_idle.read(8))

        idle = idle_bitmap >> bit & 1
        pfn += 1

        unevictable = flags >> 18 & 1
        huge = flags >> 22 & 1

        if idle and not unevictable:
            nr_idle[cgino] = nr_idle.get(cgino, 0) + (512 if huge else 1)

    f_flags.close()
    f_cgroup.close()
    f_idle.close()
    return nr_idle


print "Setting the idle flag for each page..."
set_idle()

raw_input("Wait until the workload accesses its working set, then press Enter")

print "Counting idle pages..."
nr_idle = count_idle()

for dir, subdirs, files in os.walk(CGROUP_MOUNT):
    ino = os.stat(dir)[stat.ST_INO]
    print dir + ": " + str(nr_idle.get(ino, 0) * 4) + " KB"
---- END SCRIPT ----

Comments are more than welcome.

Thanks,

Vladimir Davydov (4):
  memcg: add page_cgroup_ino helper
  proc: add kpagecgroup file
  proc: add kpageidle file
  proc: export idle flag via kpageflags

 Documentation/vm/pagemap.txt           |   22 ++-
 fs/proc/Kconfig                        |    5 +-
 fs/proc/page.c                         |  234 ++++++++++++++++++++++++++++++++
 fs/proc/task_mmu.c                     |    4 +-
 include/linux/memcontrol.h             |    8 +-
 include/linux/mm.h                     |   88 ++++++++++++
 include/linux/page-flags.h             |    9 ++
 include/linux/page_ext.h               |    4 +
 include/uapi/linux/kernel-page-flags.h |    1 +
 mm/Kconfig                             |   12 ++
 mm/debug.c                             |    4 +
 mm/hwpoison-inject.c                   |    5 +-
 mm/memcontrol.c                        |   73 +++++-----
 mm/memory-failure.c                    |   16 +--
 mm/page_ext.c                          |    3 +
 mm/rmap.c                              |    8 ++
 mm/swap.c                              |    2 +
 17 files changed, 434 insertions(+), 64 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* [PATCH v5 1/4] memcg: add page_cgroup_ino helper
From: Vladimir Davydov @ 2015-05-12 13:34 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Johannes Weiner, Michal Hocko, Greg Thelen,
	Michel Lespinasse, David Rientjes, Pavel Emelyanov,
	Cyrill Gorcunov, Jonathan Corbet, linux-api, linux-doc, linux-mm,
	cgroups, linux-kernel
In-Reply-To: <cover.1431437088.git.vdavydov@parallels.com>

Hwpoison allows to filter pages by memory cgroup ino. To ahieve that, it
calls try_get_mem_cgroup_from_page(), then mem_cgroup_css(), and finally
cgroup_ino() on the cgroup returned. This looks bulky. Since in the next
patch I need to get the ino of the memory cgroup a page is charged to
too, in this patch I introduce the page_cgroup_ino() helper.

Note that page_cgroup_ino() only considers those pages that are charged
to mem_cgroup->memory (i.e. page->mem_cgroup != NULL), and for others it
returns 0, while try_get_mem_cgroup_page(), used by hwpoison before, may
extract the cgroup from a swapcache readahead page too. Ignoring
swapcache readahead pages allows to call page_cgroup_ino() on unlocked
pages, which is nice. Hwpoison users will hardly see any difference.

Another difference between try_get_mem_cgroup_page() and
page_cgroup_ino() is that the latter works on pages charged to offline
memory cgroups, returning the inode number of the closest online
ancestor in this case, while the former does not, which is crucial for
the next patch.

Since try_get_mem_cgroup_page() is not used by anyone else, this patch
removes this function. Also, it makes hwpoison memcg filter depend on
CONFIG_MEMCG instead of CONFIG_MEMCG_SWAP (I've no idea why it was made
dependant on CONFIG_MEMCG_SWAP initially).

Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
---
 include/linux/memcontrol.h |    8 ++---
 mm/hwpoison-inject.c       |    5 +--
 mm/memcontrol.c            |   73 ++++++++++++++++++++++----------------------
 mm/memory-failure.c        |   16 ++--------
 4 files changed, 42 insertions(+), 60 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 72dff5fb0d0c..9262a8407af7 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -91,7 +91,6 @@ bool mem_cgroup_is_descendant(struct mem_cgroup *memcg,
 			      struct mem_cgroup *root);
 bool task_in_mem_cgroup(struct task_struct *task, struct mem_cgroup *memcg);
 
-extern struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page);
 extern struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p);
 
 extern struct mem_cgroup *parent_mem_cgroup(struct mem_cgroup *memcg);
@@ -192,6 +191,8 @@ static inline void mem_cgroup_count_vm_event(struct mm_struct *mm,
 void mem_cgroup_split_huge_fixup(struct page *head);
 #endif
 
+unsigned long page_cgroup_ino(struct page *page);
+
 #else /* CONFIG_MEMCG */
 struct mem_cgroup;
 
@@ -252,11 +253,6 @@ static inline struct lruvec *mem_cgroup_page_lruvec(struct page *page,
 	return &zone->lruvec;
 }
 
-static inline struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page)
-{
-	return NULL;
-}
-
 static inline bool mm_match_cgroup(struct mm_struct *mm,
 		struct mem_cgroup *memcg)
 {
diff --git a/mm/hwpoison-inject.c b/mm/hwpoison-inject.c
index 4ca5fe0042e1..d2facac0b01f 100644
--- a/mm/hwpoison-inject.c
+++ b/mm/hwpoison-inject.c
@@ -45,12 +45,9 @@ static int hwpoison_inject(void *data, u64 val)
 	/*
 	 * do a racy check with elevated page count, to make sure PG_hwpoison
 	 * will only be set for the targeted owner (or on a free page).
-	 * We temporarily take page lock for try_get_mem_cgroup_from_page().
 	 * memory_failure() will redo the check reliably inside page lock.
 	 */
-	lock_page(hpage);
 	err = hwpoison_filter(hpage);
-	unlock_page(hpage);
 	if (err)
 		goto put_out;
 
@@ -126,7 +123,7 @@ static int pfn_inject_init(void)
 	if (!dentry)
 		goto fail;
 
-#ifdef CONFIG_MEMCG_SWAP
+#ifdef CONFIG_MEMCG
 	dentry = debugfs_create_u64("corrupt-filter-memcg", 0600,
 				    hwpoison_dir, &hwpoison_filter_memcg);
 	if (!dentry)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 14c2f2017e37..87c7f852d45b 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2349,40 +2349,6 @@ static void cancel_charge(struct mem_cgroup *memcg, unsigned int nr_pages)
 	css_put_many(&memcg->css, nr_pages);
 }
 
-/*
- * try_get_mem_cgroup_from_page - look up page's memcg association
- * @page: the page
- *
- * Look up, get a css reference, and return the memcg that owns @page.
- *
- * The page must be locked to prevent racing with swap-in and page
- * cache charges.  If coming from an unlocked page table, the caller
- * must ensure the page is on the LRU or this can race with charging.
- */
-struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page)
-{
-	struct mem_cgroup *memcg;
-	unsigned short id;
-	swp_entry_t ent;
-
-	VM_BUG_ON_PAGE(!PageLocked(page), page);
-
-	memcg = page->mem_cgroup;
-	if (memcg) {
-		if (!css_tryget_online(&memcg->css))
-			memcg = NULL;
-	} else if (PageSwapCache(page)) {
-		ent.val = page_private(page);
-		id = lookup_swap_cgroup_id(ent);
-		rcu_read_lock();
-		memcg = mem_cgroup_from_id(id);
-		if (memcg && !css_tryget_online(&memcg->css))
-			memcg = NULL;
-		rcu_read_unlock();
-	}
-	return memcg;
-}
-
 static void lock_page_lru(struct page *page, int *isolated)
 {
 	struct zone *zone = page_zone(page);
@@ -2774,6 +2740,31 @@ void mem_cgroup_split_huge_fixup(struct page *head)
 }
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 
+/**
+ * page_cgroup_ino - return inode number of page's memcg
+ * @page: the page
+ *
+ * Look up the closest online ancestor of the memory cgroup @page is charged to
+ * and return its inode number. It is safe to call this function without taking
+ * a reference to the page.
+ */
+unsigned long page_cgroup_ino(struct page *page)
+{
+	struct mem_cgroup *memcg;
+	unsigned long ino = 0;
+
+	rcu_read_lock();
+	memcg = READ_ONCE(page->mem_cgroup);
+	while (memcg && !css_tryget_online(&memcg->css))
+		memcg = parent_mem_cgroup(memcg);
+	rcu_read_unlock();
+	if (memcg) {
+		ino = cgroup_ino(memcg->css.cgroup);
+		css_put(&memcg->css);
+	}
+	return ino;
+}
+
 #ifdef CONFIG_MEMCG_SWAP
 static void mem_cgroup_swap_statistics(struct mem_cgroup *memcg,
 					 bool charge)
@@ -5482,8 +5473,18 @@ int mem_cgroup_try_charge(struct page *page, struct mm_struct *mm,
 		VM_BUG_ON_PAGE(!PageTransHuge(page), page);
 	}
 
-	if (do_swap_account && PageSwapCache(page))
-		memcg = try_get_mem_cgroup_from_page(page);
+	if (do_swap_account && PageSwapCache(page)) {
+		swp_entry_t ent = { .val = page_private(page), };
+		unsigned short id = lookup_swap_cgroup_id(ent);
+
+		VM_BUG_ON_PAGE(!PageLocked(page), page);
+
+		rcu_read_lock();
+		memcg = mem_cgroup_from_id(id);
+		if (memcg && !css_tryget_online(&memcg->css))
+			memcg = NULL;
+		rcu_read_unlock();
+	}
 	if (!memcg)
 		memcg = get_mem_cgroup_from_mm(mm);
 
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 501820c815b3..7166ad81b222 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -128,27 +128,15 @@ static int hwpoison_filter_flags(struct page *p)
  * can only guarantee that the page either belongs to the memcg tasks, or is
  * a freed page.
  */
-#ifdef	CONFIG_MEMCG_SWAP
+#ifdef CONFIG_MEMCG
 u64 hwpoison_filter_memcg;
 EXPORT_SYMBOL_GPL(hwpoison_filter_memcg);
 static int hwpoison_filter_task(struct page *p)
 {
-	struct mem_cgroup *mem;
-	struct cgroup_subsys_state *css;
-	unsigned long ino;
-
 	if (!hwpoison_filter_memcg)
 		return 0;
 
-	mem = try_get_mem_cgroup_from_page(p);
-	if (!mem)
-		return -EINVAL;
-
-	css = mem_cgroup_css(mem);
-	ino = cgroup_ino(css->cgroup);
-	css_put(css);
-
-	if (ino != hwpoison_filter_memcg)
+	if (page_cgroup_ino(p) != hwpoison_filter_memcg)
 		return -EINVAL;
 
 	return 0;
-- 
1.7.10.4

--
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 v5 2/4] proc: add kpagecgroup file
From: Vladimir Davydov @ 2015-05-12 13:34 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Johannes Weiner, Michal Hocko, Greg Thelen,
	Michel Lespinasse, David Rientjes, Pavel Emelyanov,
	Cyrill Gorcunov, Jonathan Corbet, linux-api, linux-doc, linux-mm,
	cgroups, linux-kernel
In-Reply-To: <cover.1431437088.git.vdavydov@parallels.com>

/proc/kpagecgroup contains a 64-bit inode number of the memory cgroup
each page is charged to, indexed by PFN. Having this information is
useful for estimating a cgroup working set size.

The file is present if CONFIG_PROC_PAGE_MONITOR && CONFIG_MEMCG.

Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
---
 Documentation/vm/pagemap.txt |    6 ++++-
 fs/proc/Kconfig              |    5 ++--
 fs/proc/page.c               |   53 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/Documentation/vm/pagemap.txt b/Documentation/vm/pagemap.txt
index 6bfbc172cdb9..a9b7afc8fbc6 100644
--- a/Documentation/vm/pagemap.txt
+++ b/Documentation/vm/pagemap.txt
@@ -5,7 +5,7 @@ pagemap is a new (as of 2.6.25) set of interfaces in the kernel that allow
 userspace programs to examine the page tables and related information by
 reading files in /proc.
 
-There are three components to pagemap:
+There are four components to pagemap:
 
  * /proc/pid/pagemap.  This file lets a userspace process find out which
    physical frame each virtual page is mapped to.  It contains one 64-bit
@@ -65,6 +65,10 @@ There are three components to pagemap:
     23. BALLOON
     24. ZERO_PAGE
 
+ * /proc/kpagecgroup.  This file contains a 64-bit inode number of the
+   memory cgroup each page is charged to, indexed by PFN. Only available when
+   CONFIG_MEMCG is set.
+
 Short descriptions to the page flags:
 
  0. LOCKED
diff --git a/fs/proc/Kconfig b/fs/proc/Kconfig
index 2183fcf41d59..5021a2935bb9 100644
--- a/fs/proc/Kconfig
+++ b/fs/proc/Kconfig
@@ -69,5 +69,6 @@ config PROC_PAGE_MONITOR
  	help
 	  Various /proc files exist to monitor process memory utilization:
 	  /proc/pid/smaps, /proc/pid/clear_refs, /proc/pid/pagemap,
-	  /proc/kpagecount, and /proc/kpageflags. Disabling these
-          interfaces will reduce the size of the kernel by approximately 4kb.
+	  /proc/kpagecount, /proc/kpageflags, and /proc/kpagecgroup.
+	  Disabling these interfaces will reduce the size of the kernel
+	  by approximately 4kb.
diff --git a/fs/proc/page.c b/fs/proc/page.c
index 7eee2d8b97d9..70d23245dd43 100644
--- a/fs/proc/page.c
+++ b/fs/proc/page.c
@@ -9,6 +9,7 @@
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 #include <linux/hugetlb.h>
+#include <linux/memcontrol.h>
 #include <linux/kernel-page-flags.h>
 #include <asm/uaccess.h>
 #include "internal.h"
@@ -225,10 +226,62 @@ static const struct file_operations proc_kpageflags_operations = {
 	.read = kpageflags_read,
 };
 
+#ifdef CONFIG_MEMCG
+static ssize_t kpagecgroup_read(struct file *file, char __user *buf,
+				size_t count, loff_t *ppos)
+{
+	u64 __user *out = (u64 __user *)buf;
+	struct page *ppage;
+	unsigned long src = *ppos;
+	unsigned long pfn;
+	ssize_t ret = 0;
+	u64 ino;
+
+	pfn = src / KPMSIZE;
+	count = min_t(unsigned long, count, (max_pfn * KPMSIZE) - src);
+	if (src & KPMMASK || count & KPMMASK)
+		return -EINVAL;
+
+	while (count > 0) {
+		if (pfn_valid(pfn))
+			ppage = pfn_to_page(pfn);
+		else
+			ppage = NULL;
+
+		if (ppage)
+			ino = page_cgroup_ino(ppage);
+		else
+			ino = 0;
+
+		if (put_user(ino, out)) {
+			ret = -EFAULT;
+			break;
+		}
+
+		pfn++;
+		out++;
+		count -= KPMSIZE;
+	}
+
+	*ppos += (char __user *)out - buf;
+	if (!ret)
+		ret = (char __user *)out - buf;
+	return ret;
+}
+
+static const struct file_operations proc_kpagecgroup_operations = {
+	.llseek = mem_lseek,
+	.read = kpagecgroup_read,
+};
+#endif /* CONFIG_MEMCG */
+
 static int __init proc_page_init(void)
 {
 	proc_create("kpagecount", S_IRUSR, NULL, &proc_kpagecount_operations);
 	proc_create("kpageflags", S_IRUSR, NULL, &proc_kpageflags_operations);
+#ifdef CONFIG_MEMCG
+	proc_create("kpagecgroup", S_IRUSR, NULL, &proc_kpagecgroup_operations);
+#endif
 	return 0;
 }
 fs_initcall(proc_page_init);
-- 
1.7.10.4

--
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 v5 3/4] proc: add kpageidle file
From: Vladimir Davydov @ 2015-05-12 13:34 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Johannes Weiner, Michal Hocko, Greg Thelen,
	Michel Lespinasse, David Rientjes, Pavel Emelyanov,
	Cyrill Gorcunov, Jonathan Corbet, linux-api, linux-doc, linux-mm,
	cgroups, linux-kernel
In-Reply-To: <cover.1431437088.git.vdavydov@parallels.com>

Knowing the portion of memory that is not used by a certain application
or memory cgroup (idle memory) can be useful for partitioning the system
efficiently, e.g. by setting memory cgroup limits appropriately.
Currently, the only means to estimate the amount of idle memory provided
by the kernel is /proc/PID/{clear_refs,smaps}: the user can clear the
access bit for all pages mapped to a particular process by writing 1 to
clear_refs, wait for some time, and then count smaps:Referenced.
However, this method has two serious shortcomings:

 - it does not count unmapped file pages
 - it affects the reclaimer logic

To overcome these drawbacks, this patch introduces two new page flags,
Idle and Young, and a new proc file, /proc/kpageidle. A page's Idle flag
can only be set from userspace by setting bit in /proc/kpageidle at the
offset corresponding to the page, and it is cleared whenever the page is
accessed either through page tables (it is cleared in page_referenced()
in this case) or using the read(2) system call (mark_page_accessed()).
Thus by setting the Idle flag for pages of a particular workload, which
can be found e.g. by reading /proc/PID/pagemap, waiting for some time to
let the workload access its working set, and then reading the kpageidle
file, one can estimate the amount of pages that are not used by the
workload.

The Young page flag is used to avoid interference with the memory
reclaimer. A page's Young flag is set whenever the Access bit of a page
table entry pointing to the page is cleared by writing to kpageidle. If
page_referenced() is called on a Young page, it will add 1 to its return
value, therefore concealing the fact that the Access bit was cleared.

Note, since there is no room for extra page flags on 32 bit, this
feature uses extended page flags when compiled on 32 bit.

Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
---
 Documentation/vm/pagemap.txt |   12 ++-
 fs/proc/page.c               |  178 ++++++++++++++++++++++++++++++++++++++++++
 fs/proc/task_mmu.c           |    4 +-
 include/linux/mm.h           |   88 +++++++++++++++++++++
 include/linux/page-flags.h   |    9 +++
 include/linux/page_ext.h     |    4 +
 mm/Kconfig                   |   12 +++
 mm/debug.c                   |    4 +
 mm/page_ext.c                |    3 +
 mm/rmap.c                    |    8 ++
 mm/swap.c                    |    2 +
 11 files changed, 322 insertions(+), 2 deletions(-)

diff --git a/Documentation/vm/pagemap.txt b/Documentation/vm/pagemap.txt
index a9b7afc8fbc6..c9266340852c 100644
--- a/Documentation/vm/pagemap.txt
+++ b/Documentation/vm/pagemap.txt
@@ -5,7 +5,7 @@ pagemap is a new (as of 2.6.25) set of interfaces in the kernel that allow
 userspace programs to examine the page tables and related information by
 reading files in /proc.
 
-There are four components to pagemap:
+There are five components to pagemap:
 
  * /proc/pid/pagemap.  This file lets a userspace process find out which
    physical frame each virtual page is mapped to.  It contains one 64-bit
@@ -69,6 +69,16 @@ There are four components to pagemap:
    memory cgroup each page is charged to, indexed by PFN. Only available when
    CONFIG_MEMCG is set.
 
+ * /proc/kpageidle.  This file implements a bitmap where each bit corresponds
+   to a page, indexed by PFN. When the bit is set, the corresponding page is
+   idle. A page is considered idle if it has not been accessed since it was
+   marked idle. To mark a page idle one should set the bit corresponding to the
+   page by writing to the file. A value written to the file is OR-ed with the
+   current bitmap value. Only user memory pages can be marked idle, for other
+   page types input is silently ignored. Writing to this file beyond max PFN
+   results in the ENXIO error. Only available when CONFIG_IDLE_PAGE_TRACKING is
+   set.
+
 Short descriptions to the page flags:
 
  0. LOCKED
diff --git a/fs/proc/page.c b/fs/proc/page.c
index 70d23245dd43..f42ead08d346 100644
--- a/fs/proc/page.c
+++ b/fs/proc/page.c
@@ -16,6 +16,7 @@
 
 #define KPMSIZE sizeof(u64)
 #define KPMMASK (KPMSIZE - 1)
+#define KPMBITS (KPMSIZE * BITS_PER_BYTE)
 
 /* /proc/kpagecount - an array exposing page counts
  *
@@ -275,6 +276,179 @@ static const struct file_operations proc_kpagecgroup_operations = {
 };
 #endif /* CONFIG_MEMCG */
 
+#ifdef CONFIG_IDLE_PAGE_TRACKING
+/*
+ * Idle page tracking only considers user memory pages, for other types of
+ * pages the idle flag is always unset and an attempt to set it is silently
+ * ignored.
+ *
+ * We treat a page as a user memory page if it is on an LRU list, because it is
+ * always safe to pass such a page to page_referenced(), which is essential for
+ * idle page tracking. With such an indicator of user pages we can skip
+ * isolated pages, but since there are not usually many of them, it will hardly
+ * affect the overall result.
+ *
+ * This function tries to get a user memory page by pfn as described above.
+ */
+static struct page *kpageidle_get_page(unsigned long pfn)
+{
+	struct page *page;
+	struct zone *zone;
+
+	if (!pfn_valid(pfn))
+		return NULL;
+
+	page = pfn_to_page(pfn);
+	if (!page || !PageLRU(page))
+		return NULL;
+	if (!get_page_unless_zero(page))
+		return NULL;
+
+	zone = page_zone(page);
+	spin_lock_irq(&zone->lru_lock);
+	if (unlikely(!PageLRU(page))) {
+		put_page(page);
+		page = NULL;
+	}
+	spin_unlock_irq(&zone->lru_lock);
+	return page;
+}
+
+/*
+ * This function calls page_referenced() to clear the referenced bit for all
+ * mappings to a page. Since the latter also clears the page idle flag if the
+ * page was referenced, it can be used to update the idle flag of a page.
+ */
+static void kpageidle_clear_pte_refs(struct page *page)
+{
+	unsigned long dummy;
+
+	if (page_referenced(page, 0, NULL, &dummy))
+		/*
+		 * We cleared the referenced bit in a mapping to this page. To
+		 * avoid interference with the reclaimer, mark it young so that
+		 * the next call to page_referenced() will also return > 0 (see
+		 * page_referenced_one())
+		 */
+		set_page_young(page);
+}
+
+static ssize_t kpageidle_read(struct file *file, char __user *buf,
+			      size_t count, loff_t *ppos)
+{
+	u64 __user *out = (u64 __user *)buf;
+	struct page *page;
+	unsigned long pfn, end_pfn;
+	ssize_t ret = 0;
+	u64 idle_bitmap = 0;
+	int bit;
+
+	if (*ppos & KPMMASK || count & KPMMASK)
+		return -EINVAL;
+
+	pfn = *ppos * BITS_PER_BYTE;
+	if (pfn >= max_pfn)
+		return 0;
+
+	end_pfn = pfn + count * BITS_PER_BYTE;
+	if (end_pfn > max_pfn)
+		end_pfn = ALIGN(max_pfn, KPMBITS);
+
+	for (; pfn < end_pfn; pfn++) {
+		bit = pfn % KPMBITS;
+		page = kpageidle_get_page(pfn);
+		if (page) {
+			if (page_is_idle(page)) {
+				/*
+				 * The page might have been referenced via a
+				 * pte, in which case it is not idle. Clear
+				 * refs and recheck.
+				 */
+				kpageidle_clear_pte_refs(page);
+				if (page_is_idle(page))
+					idle_bitmap |= 1ULL << bit;
+			}
+			put_page(page);
+		}
+		if (bit == KPMBITS - 1) {
+			if (put_user(idle_bitmap, out)) {
+				ret = -EFAULT;
+				break;
+			}
+			idle_bitmap = 0;
+			out++;
+		}
+	}
+
+	*ppos += (char __user *)out - buf;
+	if (!ret)
+		ret = (char __user *)out - buf;
+	return ret;
+}
+
+static ssize_t kpageidle_write(struct file *file, const char __user *buf,
+			       size_t count, loff_t *ppos)
+{
+	const u64 __user *in = (const u64 __user *)buf;
+	struct page *page;
+	unsigned long pfn, end_pfn;
+	ssize_t ret = 0;
+	u64 idle_bitmap = 0;
+	int bit;
+
+	if (*ppos & KPMMASK || count & KPMMASK)
+		return -EINVAL;
+
+	pfn = *ppos * BITS_PER_BYTE;
+	if (pfn >= max_pfn)
+		return -ENXIO;
+
+	end_pfn = pfn + count * BITS_PER_BYTE;
+	if (end_pfn > max_pfn)
+		end_pfn = ALIGN(max_pfn, KPMBITS);
+
+	for (; pfn < end_pfn; pfn++) {
+		bit = pfn % KPMBITS;
+		if (bit == 0) {
+			if (get_user(idle_bitmap, in)) {
+				ret = -EFAULT;
+				break;
+			}
+			in++;
+		}
+		if (idle_bitmap >> bit & 1) {
+			page = kpageidle_get_page(pfn);
+			if (page) {
+				kpageidle_clear_pte_refs(page);
+				set_page_idle(page);
+				put_page(page);
+			}
+		}
+	}
+
+	*ppos += (const char __user *)in - buf;
+	if (!ret)
+		ret = (const char __user *)in - buf;
+	return ret;
+}
+
+static const struct file_operations proc_kpageidle_operations = {
+	.llseek = mem_lseek,
+	.read = kpageidle_read,
+	.write = kpageidle_write,
+};
+
+#ifndef CONFIG_64BIT
+static bool need_page_idle(void)
+{
+	return true;
+}
+struct page_ext_operations page_idle_ops = {
+	.need = need_page_idle,
+};
+#endif
+#endif /* CONFIG_IDLE_PAGE_TRACKING */
+
 static int __init proc_page_init(void)
 {
 	proc_create("kpagecount", S_IRUSR, NULL, &proc_kpagecount_operations);
@@ -282,6 +456,10 @@ static int __init proc_page_init(void)
 #ifdef CONFIG_MEMCG
 	proc_create("kpagecgroup", S_IRUSR, NULL, &proc_kpagecgroup_operations);
 #endif
+#ifdef CONFIG_IDLE_PAGE_TRACKING
+	proc_create("kpageidle", S_IRUSR | S_IWUSR, NULL,
+		    &proc_kpageidle_operations);
+#endif
 	return 0;
 }
 fs_initcall(proc_page_init);
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 6dee68d013ff..ab04846f7dd5 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -458,7 +458,7 @@ static void smaps_account(struct mem_size_stats *mss, struct page *page,
 
 	mss->resident += size;
 	/* Accumulate the size in pages that have been accessed. */
-	if (young || PageReferenced(page))
+	if (young || page_is_young(page) || PageReferenced(page))
 		mss->referenced += size;
 	mapcount = page_mapcount(page);
 	if (mapcount >= 2) {
@@ -808,6 +808,7 @@ static int clear_refs_pte_range(pmd_t *pmd, unsigned long addr,
 
 		/* Clear accessed and referenced bits. */
 		pmdp_test_and_clear_young(vma, addr, pmd);
+		clear_page_young(page);
 		ClearPageReferenced(page);
 out:
 		spin_unlock(ptl);
@@ -835,6 +836,7 @@ out:
 
 		/* Clear accessed and referenced bits. */
 		ptep_test_and_clear_young(vma, addr, pte);
+		clear_page_young(page);
 		ClearPageReferenced(page);
 	}
 	pte_unmap_unlock(pte - 1, ptl);
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0755b9fd03a7..794d29aa2317 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2200,5 +2200,93 @@ void __init setup_nr_node_ids(void);
 static inline void setup_nr_node_ids(void) {}
 #endif
 
+#ifdef CONFIG_IDLE_PAGE_TRACKING
+#ifdef CONFIG_64BIT
+static inline bool page_is_young(struct page *page)
+{
+	return PageYoung(page);
+}
+
+static inline void set_page_young(struct page *page)
+{
+	SetPageYoung(page);
+}
+
+static inline void clear_page_young(struct page *page)
+{
+	ClearPageYoung(page);
+}
+
+static inline bool page_is_idle(struct page *page)
+{
+	return PageIdle(page);
+}
+
+static inline void set_page_idle(struct page *page)
+{
+	SetPageIdle(page);
+}
+
+static inline void clear_page_idle(struct page *page)
+{
+	ClearPageIdle(page);
+}
+#else /* !CONFIG_64BIT */
+/*
+ * If there is not enough space to store Idle and Young bits in page flags, use
+ * page ext flags instead.
+ */
+extern struct page_ext_operations page_idle_ops;
+
+static inline bool page_is_young(struct page *page)
+{
+	return test_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags);
+}
+
+static inline void set_page_young(struct page *page)
+{
+	set_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags);
+}
+
+static inline void clear_page_young(struct page *page)
+{
+	clear_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags);
+}
+
+static inline bool page_is_idle(struct page *page)
+{
+	return test_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags);
+}
+
+static inline void set_page_idle(struct page *page)
+{
+	set_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags);
+}
+
+static inline void clear_page_idle(struct page *page)
+{
+	clear_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags);
+}
+#endif /* CONFIG_64BIT */
+#else /* !CONFIG_IDLE_PAGE_TRACKING */
+static inline bool page_is_young(struct page *page)
+{
+	return false;
+}
+
+static inline void clear_page_young(struct page *page)
+{
+}
+
+static inline bool page_is_idle(struct page *page)
+{
+	return false;
+}
+
+static inline void clear_page_idle(struct page *page)
+{
+}
+#endif /* CONFIG_IDLE_PAGE_TRACKING */
+
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MM_H */
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index f34e040b34e9..5e7c4f50a644 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -109,6 +109,10 @@ enum pageflags {
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 	PG_compound_lock,
 #endif
+#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT)
+	PG_young,
+	PG_idle,
+#endif
 	__NR_PAGEFLAGS,
 
 	/* Filesystems */
@@ -289,6 +293,11 @@ PAGEFLAG_FALSE(HWPoison)
 #define __PG_HWPOISON 0
 #endif
 
+#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT)
+PAGEFLAG(Young, young)
+PAGEFLAG(Idle, idle)
+#endif
+
 /*
  * On an anonymous page mapped into a user virtual memory area,
  * page->mapping points to its anon_vma, not to a struct address_space;
diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
index c42981cd99aa..17f118a82854 100644
--- a/include/linux/page_ext.h
+++ b/include/linux/page_ext.h
@@ -26,6 +26,10 @@ enum page_ext_flags {
 	PAGE_EXT_DEBUG_POISON,		/* Page is poisoned */
 	PAGE_EXT_DEBUG_GUARD,
 	PAGE_EXT_OWNER,
+#if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
+	PAGE_EXT_YOUNG,
+	PAGE_EXT_IDLE,
+#endif
 };
 
 /*
diff --git a/mm/Kconfig b/mm/Kconfig
index 390214da4546..3600eace4774 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -635,3 +635,15 @@ config MAX_STACK_SIZE_MB
 	  changed to a smaller value in which case that is used.
 
 	  A sane initial value is 80 MB.
+
+config IDLE_PAGE_TRACKING
+	bool "Enable idle page tracking"
+	select PROC_PAGE_MONITOR
+	select PAGE_EXTENSION if !64BIT
+	help
+	  This feature allows to estimate the amount of user pages that have
+	  not been touched during a given period of time. This information can
+	  be useful to tune memory cgroup limits and/or for job placement
+	  within a compute cluster.
+
+	  See Documentation/vm/pagemap.txt for more details.
diff --git a/mm/debug.c b/mm/debug.c
index 3eb3ac2fcee7..bb66f9ccec03 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -48,6 +48,10 @@ static const struct trace_print_flags pageflag_names[] = {
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 	{1UL << PG_compound_lock,	"compound_lock"	},
 #endif
+#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT)
+	{1UL << PG_young,		"young"		},
+	{1UL << PG_idle,		"idle"		},
+#endif
 };
 
 static void dump_flags(unsigned long flags,
diff --git a/mm/page_ext.c b/mm/page_ext.c
index d86fd2f5353f..e4b3af054bf2 100644
--- a/mm/page_ext.c
+++ b/mm/page_ext.c
@@ -59,6 +59,9 @@ static struct page_ext_operations *page_ext_ops[] = {
 #ifdef CONFIG_PAGE_OWNER
 	&page_owner_ops,
 #endif
+#if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
+	&page_idle_ops,
+#endif
 };
 
 static unsigned long total_usage;
diff --git a/mm/rmap.c b/mm/rmap.c
index 8b18fd4227d1..3650793eaeab 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -781,6 +781,14 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
 		pte_unmap_unlock(pte, ptl);
 	}
 
+	if (referenced && page_is_idle(page))
+		clear_page_idle(page);
+
+	if (page_is_young(page)) {
+		clear_page_young(page);
+		referenced++;
+	}
+
 	if (referenced) {
 		pra->referenced++;
 		pra->vm_flags |= vma->vm_flags;
diff --git a/mm/swap.c b/mm/swap.c
index a7251a8ed532..6bf6f293a9ea 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -623,6 +623,8 @@ void mark_page_accessed(struct page *page)
 	} else if (!PageReferenced(page)) {
 		SetPageReferenced(page);
 	}
+	if (page_is_idle(page))
+		clear_page_idle(page);
 }
 EXPORT_SYMBOL(mark_page_accessed);
 
-- 
1.7.10.4

--
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 v5 4/4] proc: export idle flag via kpageflags
From: Vladimir Davydov @ 2015-05-12 13:34 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Johannes Weiner, Michal Hocko, Greg Thelen,
	Michel Lespinasse, David Rientjes, Pavel Emelyanov,
	Cyrill Gorcunov, Jonathan Corbet,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <cover.1431437088.git.vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>

As noted by Minchan, a benefit of reading idle flag from
/proc/kpageflags is that one can easily filter dirty and/or unevictable
pages while estimating the size of unused memory.

Note that idle flag read from /proc/kpageflags may be stale in case the
page was accessed via a PTE, because it would be too costly to iterate
over all page mappings on each /proc/kpageflags read to provide an
up-to-date value. To make sure the flag is up-to-date one has to read
/proc/kpageidle first.

Signed-off-by: Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
---
 Documentation/vm/pagemap.txt           |    6 ++++++
 fs/proc/page.c                         |    3 +++
 include/uapi/linux/kernel-page-flags.h |    1 +
 3 files changed, 10 insertions(+)

diff --git a/Documentation/vm/pagemap.txt b/Documentation/vm/pagemap.txt
index c9266340852c..5896b7d7fd74 100644
--- a/Documentation/vm/pagemap.txt
+++ b/Documentation/vm/pagemap.txt
@@ -64,6 +64,7 @@ There are five components to pagemap:
     22. THP
     23. BALLOON
     24. ZERO_PAGE
+    25. IDLE
 
  * /proc/kpagecgroup.  This file contains a 64-bit inode number of the
    memory cgroup each page is charged to, indexed by PFN. Only available when
@@ -124,6 +125,11 @@ Short descriptions to the page flags:
 24. ZERO_PAGE
     zero page for pfn_zero or huge_zero page
 
+25. IDLE
+    page has not been accessed since it was marked idle (see /proc/kpageidle)
+    Note that this flag may be stale in case the page was accessed via a PTE.
+    To make sure the flag is up-to-date one has to read /proc/kpageidle first.
+
     [IO related page flags]
  1. ERROR     IO error occurred
  3. UPTODATE  page has up-to-date data
diff --git a/fs/proc/page.c b/fs/proc/page.c
index f42ead08d346..24748be3dd65 100644
--- a/fs/proc/page.c
+++ b/fs/proc/page.c
@@ -148,6 +148,9 @@ u64 stable_page_flags(struct page *page)
 	if (PageBalloon(page))
 		u |= 1 << KPF_BALLOON;
 
+	if (page_is_idle(page))
+		u |= 1 << KPF_IDLE;
+
 	u |= kpf_copy_bit(k, KPF_LOCKED,	PG_locked);
 
 	u |= kpf_copy_bit(k, KPF_SLAB,		PG_slab);
diff --git a/include/uapi/linux/kernel-page-flags.h b/include/uapi/linux/kernel-page-flags.h
index a6c4962e5d46..5da5f8751ce7 100644
--- a/include/uapi/linux/kernel-page-flags.h
+++ b/include/uapi/linux/kernel-page-flags.h
@@ -33,6 +33,7 @@
 #define KPF_THP			22
 #define KPF_BALLOON		23
 #define KPF_ZERO_PAGE		24
+#define KPF_IDLE		25
 
 
 #endif /* _UAPILINUX_KERNEL_PAGE_FLAGS_H */
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH RFC] vfs: add a O_NOMTIME flag
From: John Stoffel @ 2015-05-12 13:41 UTC (permalink / raw)
  To: Sage Weil
  Cc: Trond Myklebust, Dave Chinner, Zach Brown, Alexander Viro,
	Linux FS-devel Mailing List, Linux Kernel Mailing List,
	Linux API Mailing List
In-Reply-To: <alpine.DEB.2.00.1505111020120.28239-vIokxiIdD2AQNTJnQDzGJqxOck334EZe@public.gmane.org>

>>>>> "Sage" == Sage Weil <sage-BnTBU8nroG7k1uMJSBkQmQ@public.gmane.org> writes:

Sage> On Mon, 11 May 2015, Trond Myklebust wrote:
>> On Mon, May 11, 2015 at 12:39 PM, Sage Weil <sage-BnTBU8nroG7k1uMJSBkQmQ@public.gmane.org> wrote:
>> > On Mon, 11 May 2015, Dave Chinner wrote:
>> >> On Sun, May 10, 2015 at 07:13:24PM -0400, Trond Myklebust wrote:
>> >> > On Fri, May 8, 2015 at 6:24 PM, Sage Weil <sage-BnTBU8nroG7k1uMJSBkQmQ@public.gmane.org> wrote:
>> >> > > I'm sure you realize what we're try to achieve is the same "invisible IO"
>> >> > > that the XFS open by handle ioctls do by default.  Would you be more
>> >> > > comfortable if this option where only available to the generic
>> >> > > open_by_handle syscall, and not to open(2)?
>> >> >
>> >> > It should be an ioctl(). It has no business being part of
>> >> > open_by_handle either, since that is another generic interface.
>> >
>> > Our use-case doesn't make sense on network file systems, but it does on
>> > any reasonably featureful local filesystem, and the goal is to be generic
>> > there.  If mtime is critical to a network file system's consistency it
>> > seems pretty reasonable to disallow/ignore it for just that file system
>> > (e.g., by masking off the flag at open time), as others won't have that
>> > same problem (cephfs doesn't, for example).
>> >
>> > Perhaps making each fs opt-in instead of handling it in a generic path
>> > would alleviate this concern?
>> 
>> The issue isn't whether or not you have a network file system, it's
>> whether or not you want users to be able to manage data. mtime isn't
>> useful for the application (which knows whether or not it has changed
>> the file) or for the filesystem (ditto). It exists, rather, in order
>> to enable data management by users and other applications, letting
>> them know whether or not the data contents of the file have changed,
>> and when that change occurred.

Sage> Agreed.
 
>> If you are able to guarantee that your users don't care about that,
>> then fine, but that would be a very special case that doesn't fit the
>> way that most data centres are run. Backups are one case where mtime
>> matters, tiering and archiving is another.

Sage> This is true, although I argue it is becoming increasingly
Sage> common for the data management (including backups and so forth)
Sage> to be layered not on top of the POSIX file system but on
Sage> something higher up in the stack. This is true of pretty much
Sage> any distributed system (ceph, cassandra, mongo, etc., and I
Sage> assume commercial databases like Oracle, too) where backups,
Sage> replication, and any other DR strategies need to be orchestrated
Sage> across nodes to be consistent--simply copying files out from
Sage> underneath them is already insufficient and a recipe for
Sage> disaster.

you're smoking crack here.  Backups are not layered at higher layers
unless absolutely necessary, such as for databases.  Now Mongo, Hadoop
and others might also fit this model, but for day to day backup of
data, it's mtime all the way.  

I don't see why you insist that this is a good idea to implement for a
very special corner case.  

Sage> There is a growing category of applications that can benefit
Sage> from this capability...

There is a perceived growing category of super special niche
applications which might think they want this capability.  

Why are you even using a filesystem in the first place if you're so
worried about writing out inodes being a performance problem?  Just
use raw partitions and do all the work yourself.  Oracle and other DBs
can do this when they want.  

>> Neither of these examples
>> cases are under the control of the application that calls
>> open(O_NOMTIME).

Sage> Wouldn't a mount option (e.g., allow_nomtime) address this
Sage> concern?  Only nodes provisioned explicitly to run these systems
Sage> would be enable this option.

Why do you keep coming back to a mount option?  What's wrong with a
per-file ioctl option?  Making this a mount option means that you
default to a fail hard setup.  If someone screws up and mounts user
home directories with this option thinking that it's like the noatime
option, then suddenly all their backups will silently break unless
they're aware of disk space churn numbers and notice that they are
only backing up tiny bits.

With an ioctl, it's upto the damn application to *request* this
change, and then the VFS/filesystem and *maybe* support this, but the
application shouldn't actually know or care what the result is, it's
just a performance hint/request.  

We should default to sane semantics and not give out such a big
foot-gun if at all possible.  

I'm a sysadm by day (and night, evening, early morning... :-) and I
know my user's don't think about thinks like this. They don't even
think about backups until they want to restore something.  User's only
care about restores, not backups.

John

^ permalink raw reply

* Re: [PATCH RFC] vfs: add a O_NOMTIME flag
From: John Stoffel @ 2015-05-12 13:54 UTC (permalink / raw)
  To: Austin S Hemmelgarn
  Cc: Kevin Easton, Theodore Ts'o, Sage Weil, Trond Myklebust,
	Dave Chinner, Zach Brown, Alexander Viro,
	Linux FS-devel Mailing List, Linux Kernel Mailing List,
	Linux API Mailing List
In-Reply-To: <5551E7EB.8040301-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

>>>>> "Austin" == Austin S Hemmelgarn <ahferroin7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:

Austin> On 2015-05-12 01:08, Kevin Easton wrote:
>> On Mon, May 11, 2015 at 07:10:21PM -0400, Theodore Ts'o wrote:
>>> On Mon, May 11, 2015 at 09:24:09AM -0700, Sage Weil wrote:
>>>>> Let me re-ask the question that I asked last week (and was apparently
>>>>> ignored).  Why not trying to use the lazytime feature instead of
>>>>> pointing a head straight at the application's --- and system
>>>>> administrators' --- heads?
>>>> 
>>>> Sorry Ted, I thought I responded already.
>>>> 
>>>> The goal is to avoid inode writeout entirely when we can, and
>>>> as I understand it lazytime will still force writeout before the inode
>>>> is dropped from the cache.  In systems like Ceph in particular, the
>>>> IOs can be spread across lots of files, so simply deferring writeout
>>>> doesn't always help.
>>> 
>>> Sure, but it would reduce the writeout by orders of magnitude.  I can
>>> understand if you want to reduce it further, but it might be good
>>> enough for your purposes.
>>> 
>>> I considered doing the equivalent of O_NOMTIME for our purposes at
>>> $WORK, and our use case is actually not that different from Ceph's
>>> (i.e., using a local disk file system to support a cluster file
>>> system), and lazytime was (a) something I figured was something I
>>> could upstream in good conscience, and (b) was more than good enough
>>> for us.
>> 
>> A safer alternative might be a chattr file attribute that if set, the
>> mtime is not updated on writes, and stat() on the file always shows the
>> mtime as "right now".  At least that way, the file won't accidentally
>> get left out of backups that rely on the mtime.
>> 
>> (If the file attribute is unset, you immediately update the mtime then
>> too, and from then on the file is back to normal).
>> 

Austin> I like this even better than the flag suggestion, it provides
Austin> better control, means that you don't need to update
Austin> applications to get the benefits, and prevents backup software
Austin> from breaking (although backups would be bigger).

Me too, it fails in a safer mode, where you do more work on backups
than strictly needed.  I'm still against this as a mount option
though, way way way too many bullets in the foot gun.  And as someone
else said, once you mount with O_NOMTIME, then unmount, then mount
again without O_NOMTIME, you've lost information.  Not good.  

^ permalink raw reply

* Re: [PATCH RFC] vfs: add a O_NOMTIME flag
From: J. Bruce Fields @ 2015-05-12 14:36 UTC (permalink / raw)
  To: John Stoffel
  Cc: Austin S Hemmelgarn, Kevin Easton, Theodore Ts'o, Sage Weil,
	Trond Myklebust, Dave Chinner, Zach Brown, Alexander Viro,
	Linux FS-devel Mailing List, Linux Kernel Mailing List,
	Linux API Mailing List
In-Reply-To: <21842.1555.38099.868100@quad.stoffel.home>

On Tue, May 12, 2015 at 09:54:27AM -0400, John Stoffel wrote:
> >>>>> "Austin" == Austin S Hemmelgarn <ahferroin7@gmail.com> writes:
> 
> Austin> On 2015-05-12 01:08, Kevin Easton wrote:
> >> On Mon, May 11, 2015 at 07:10:21PM -0400, Theodore Ts'o wrote:
> >>> On Mon, May 11, 2015 at 09:24:09AM -0700, Sage Weil wrote:
> >>>>> Let me re-ask the question that I asked last week (and was apparently
> >>>>> ignored).  Why not trying to use the lazytime feature instead of
> >>>>> pointing a head straight at the application's --- and system
> >>>>> administrators' --- heads?
> >>>> 
> >>>> Sorry Ted, I thought I responded already.
> >>>> 
> >>>> The goal is to avoid inode writeout entirely when we can, and
> >>>> as I understand it lazytime will still force writeout before the inode
> >>>> is dropped from the cache.  In systems like Ceph in particular, the
> >>>> IOs can be spread across lots of files, so simply deferring writeout
> >>>> doesn't always help.
> >>> 
> >>> Sure, but it would reduce the writeout by orders of magnitude.  I can
> >>> understand if you want to reduce it further, but it might be good
> >>> enough for your purposes.
> >>> 
> >>> I considered doing the equivalent of O_NOMTIME for our purposes at
> >>> $WORK, and our use case is actually not that different from Ceph's
> >>> (i.e., using a local disk file system to support a cluster file
> >>> system), and lazytime was (a) something I figured was something I
> >>> could upstream in good conscience, and (b) was more than good enough
> >>> for us.
> >> 
> >> A safer alternative might be a chattr file attribute that if set, the
> >> mtime is not updated on writes, and stat() on the file always shows the
> >> mtime as "right now".  At least that way, the file won't accidentally
> >> get left out of backups that rely on the mtime.
> >> 
> >> (If the file attribute is unset, you immediately update the mtime then
> >> too, and from then on the file is back to normal).
> >> 
> 
> Austin> I like this even better than the flag suggestion, it provides
> Austin> better control, means that you don't need to update
> Austin> applications to get the benefits, and prevents backup software
> Austin> from breaking (although backups would be bigger).
> 
> Me too, it fails in a safer mode, where you do more work on backups
> than strictly needed.  I'm still against this as a mount option
> though, way way way too many bullets in the foot gun.  And as someone
> else said, once you mount with O_NOMTIME, then unmount, then mount
> again without O_NOMTIME, you've lost information.  Not good.  

That was me.  Zach also pointed out to me that'd mean figuring out where
to store that information on-disk for every filesystem you care about.
I like the idea of something persistent, but maybe it's more trouble
than it's worth--I honestly don't know.

--b.

^ permalink raw reply

* Re: [PATCH RFC] vfs: add a O_NOMTIME flag
From: Austin S Hemmelgarn @ 2015-05-12 14:53 UTC (permalink / raw)
  To: J. Bruce Fields, John Stoffel
  Cc: Kevin Easton, Theodore Ts'o, Sage Weil, Trond Myklebust,
	Dave Chinner, Zach Brown, Alexander Viro,
	Linux FS-devel Mailing List, Linux Kernel Mailing List,
	Linux API Mailing List
In-Reply-To: <20150512143637.GA6370-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>

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

On 2015-05-12 10:36, J. Bruce Fields wrote:
> On Tue, May 12, 2015 at 09:54:27AM -0400, John Stoffel wrote:
>>>>>>> "Austin" == Austin S Hemmelgarn <ahferroin7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>>
>> Austin> On 2015-05-12 01:08, Kevin Easton wrote:
>>>> On Mon, May 11, 2015 at 07:10:21PM -0400, Theodore Ts'o wrote:
>>>>> On Mon, May 11, 2015 at 09:24:09AM -0700, Sage Weil wrote:
>>>>>>> Let me re-ask the question that I asked last week (and was apparently
>>>>>>> ignored).  Why not trying to use the lazytime feature instead of
>>>>>>> pointing a head straight at the application's --- and system
>>>>>>> administrators' --- heads?
>>>>>>
>>>>>> Sorry Ted, I thought I responded already.
>>>>>>
>>>>>> The goal is to avoid inode writeout entirely when we can, and
>>>>>> as I understand it lazytime will still force writeout before the inode
>>>>>> is dropped from the cache.  In systems like Ceph in particular, the
>>>>>> IOs can be spread across lots of files, so simply deferring writeout
>>>>>> doesn't always help.
>>>>>
>>>>> Sure, but it would reduce the writeout by orders of magnitude.  I can
>>>>> understand if you want to reduce it further, but it might be good
>>>>> enough for your purposes.
>>>>>
>>>>> I considered doing the equivalent of O_NOMTIME for our purposes at
>>>>> $WORK, and our use case is actually not that different from Ceph's
>>>>> (i.e., using a local disk file system to support a cluster file
>>>>> system), and lazytime was (a) something I figured was something I
>>>>> could upstream in good conscience, and (b) was more than good enough
>>>>> for us.
>>>>
>>>> A safer alternative might be a chattr file attribute that if set, the
>>>> mtime is not updated on writes, and stat() on the file always shows the
>>>> mtime as "right now".  At least that way, the file won't accidentally
>>>> get left out of backups that rely on the mtime.
>>>>
>>>> (If the file attribute is unset, you immediately update the mtime then
>>>> too, and from then on the file is back to normal).
>>>>
>>
>> Austin> I like this even better than the flag suggestion, it provides
>> Austin> better control, means that you don't need to update
>> Austin> applications to get the benefits, and prevents backup software
>> Austin> from breaking (although backups would be bigger).
>>
>> Me too, it fails in a safer mode, where you do more work on backups
>> than strictly needed.  I'm still against this as a mount option
>> though, way way way too many bullets in the foot gun.  And as someone
>> else said, once you mount with O_NOMTIME, then unmount, then mount
>> again without O_NOMTIME, you've lost information.  Not good.
>
> That was me.  Zach also pointed out to me that'd mean figuring out where
> to store that information on-disk for every filesystem you care about.
> I like the idea of something persistent, but maybe it's more trouble
> than it's worth--I honestly don't know.
>
But if we do it as a flag controlled by the API used by chattr, it 
becomes the responsibility of the filesystems to deal with where to 
store the information, assuming they choose to support it; personally, I 
would be really surprised if XFS and BTRFS didn't add support for this 
relatively soon after the API getting merged upstream, and ext4 would 
likely follow soon afterwards.

As far as support goes, I really think this will be easier to _safely_ 
implement (mount options are just too easy to arbitrarily change without 
knowing the consequences), although I think that reporting mtime as the 
current wall time for files under this effect is important regardless of 
what methodology get's implemented.



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 2967 bytes --]

^ permalink raw reply

* Re: [PATCH v2 2/3] pagemap: hide physical addresses from non-privileged users
From: Linus Torvalds @ 2015-05-12 15:06 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: linux-mm, Naoya Horiguchi, Linux Kernel Mailing List,
	Andrew Morton, Mark Williamson, Pavel Emelyanov, Linux API,
	Andy Lutomirski, Vlastimil Babka, Pavel Machek, Mark Seaborn,
	Kirill A. Shutemov, Daniel James, Finn Grimwood
In-Reply-To: <20150512094305.24768.51807.stgit@buzz>

On Tue, May 12, 2015 at 2:43 AM, Konstantin Khlebnikov
<khlebnikov@yandex-team.ru> wrote:
> @@ -1260,6 +1269,8 @@ static ssize_t pagemap_read(struct file *file, char __user *buf,
>         if (!count)
>                 goto out_task;
>
> +       /* do not disclose physical addresses: attack vector */
> +       pm.show_pfn = capable(CAP_SYS_ADMIN);
>         pm.v2 = soft_dirty_cleared;
>         pm.len = (PAGEMAP_WALK_SIZE >> PAGE_SHIFT);
>         pm.buffer = kmalloc(pm.len * PM_ENTRY_BYTES, GFP_TEMPORARY);

NO! Dammit, no, no, no!

How many times must people do this major security faux-pas before we learn?

WE DO NOT CHECK CURRENT CAPABILITIES AT READ/WRITE TIME!

It's a bug. It's a security issue. It's not how Unix capabilities work!

Capabilities are checked at open time.:

> @@ -1335,9 +1346,6 @@ out:
>
>  static int pagemap_open(struct inode *inode, struct file *file)
>  {
> -       /* do not disclose physical addresses: attack vector */
> -       if (!capable(CAP_SYS_ADMIN))
> -               return -EPERM;

THIS  is where you are supposed to check for capabilities. The place
where you removed it!

The reason we check capabilities at open time, and open time ONLY is
because that is really very integral to the whole Unix security model.
Otherwise, you get into this situation:

 - unprivileged process opens file

 - unprivileged process tricks suid process to do the actual access for it

where the traditional model is to just force a "write()" by opening
the file as stderr, and then executing a suid process (traditionally
"sudo") that writes an error message to it.

So *don't* do permission checks using read/write time credentials.
They are wrong.

Now, if there is some reason that you really can't do it when opening
the file, and you actually need to use capability information at
read/write time, you use the "file->f_cred" field, which is the
open-time capabilities. So you _can_ do permission checks at
read/write time, but you have to use the credentials of the opener,
not "current".

So in this case, I guess you could use

        pm.show_pfn = file_ns_capable(file, &init_user_ns, CAP_SYS_ADMIN);

if you really need to do this at read time, and cannot fill in that
"show_pfn" at open-time.

                        Linus

--
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 1/2] clone: Support passing tls argument via C rather than pt_regs magic
From: Josh Triplett @ 2015-05-12 15:15 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andrew Morton, Thomas Gleixner, Andy Lutomirski, Ingo Molnar,
	H. Peter Anvin, Peter Zijlstra, Linus Torvalds,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, x86-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <20150512081727.GA6058-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Tue, May 12, 2015 at 10:17:28AM +0200, Ingo Molnar wrote:
> * Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org> wrote:
> > > Looks good to me, but I have not looked very deeply ...
> > 
> > I sent out a v2 with the co-author information moved from the 
> > signoffs to the commit message.  If it looks reasonable to you, can 
> > you take it through the tip tree please?
> 
> So since this is multi-arch, and changes kernel/fork.c, I'd say -mm is 
> a more appropriate home for it? (Assuming Linus does not object.)

Works for me.

- Josh Triplett

^ permalink raw reply

* Re: CONFIG_ISOLATION=y (was: [PATCH 0/6] support "dataplane" mode for nohz_full)
From: Frederic Weisbecker @ 2015-05-12 15:36 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Peter Zijlstra, Chris Metcalf, Steven Rostedt, Andrew Morton,
	paulmck, Gilad Ben Yossef, Rik van Riel, Tejun Heo,
	Thomas Gleixner, Christoph Lameter, Srivatsa S. Bhat, linux-doc,
	linux-api, linux-kernel
In-Reply-To: <20150512123440.GA16959@gmail.com>

On Tue, May 12, 2015 at 02:34:40PM +0200, Ingo Molnar wrote:
> 
> * Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Tue, May 12, 2015 at 11:10:32AM +0200, Ingo Molnar wrote:
> > > 
> > > So I'd vote for Frederic's CONFIG_ISOLATION=y, mostly because this 
> > > is a high level kernel feature, so it won't conflict with 
> > > isolation concepts in lower level subsystems such as IOMMU 
> > > isolation - and other higher level features like scheduler 
> > > isolation are basically another partial implementation we want to 
> > > merge with all this...
> > 
> > But why do we need a CONFIG flag for something that has no content?
> > 
> > That is, I do not see anything much; except the 'I want to stay in 
> > userspace and kill me otherwise' flag, and I'm not sure that 
> > warrants a CONFIG flag like this.
> > 
> > Other than that, its all a combination of NOHZ_FULL and 
> > cpusets/isolcpus and whatnot.
> 
> Yes, that's what I meant: CONFIG_ISOLATION would trigger what is 
> NO_HZ_FULL today - we could possibly even remove CONFIG_NO_HZ_FULL as 
> an individual Kconfig option?

Right, we could return to what we had previously: CONFIG_NO_HZ. A config
that enables dynticks-idle by default and allows full dynticks if nohz_full=
boot option is passed (or something driven by higher level isolation interface).

Because eventually, distros enable NO_HZ_FULL so that their 0.0001% users
can use it. Well at least Red Hat does.

> 
> CONFIG_ISOLATION=y would express the guarantee from the kernel that 
> it's possible for user-space to configure itself to run undisturbed - 
> instead of the current inconsistent set of options and facilities.
> 
> A bit like CONFIG_PREEMPT_RT is more than just preemptable spinlocks, 
> it also tries to offer various facilities and tune the defaults to 
> turn the kernel hard-rt.
> 
> Does that make sense to you?

Right although distros tend to want features to be enabled dynamically
so that they have a single kernel to maintain. Things like PREEMPT_RT
really need to be a different kernel because fundamental primitives like
spinlocks must be implemented statically.

But isolation can be a boot-enabled, or even runtime-enabled, as it's only
about timer,irq,task affinity. Full Nohz is more complicated but it can
be runtime toggled in the future.

So we can bring CONFIG_CPU_ISOLATION, at least for distros that are really
not interested in that so they can disable it. CONFIG_CPU_ISOLATION=y would
bring an ability which is default-disabled and driven dynamically through whatever
interface.

^ permalink raw reply

* Re: [PATCH v2 2/3] pagemap: hide physical addresses from non-privileged users
From: Konstantin Khlebnikov @ 2015-05-12 15:41 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-mm, Naoya Horiguchi, Linux Kernel Mailing List,
	Andrew Morton, Mark Williamson, Pavel Emelyanov, Linux API,
	Andy Lutomirski, Vlastimil Babka, Pavel Machek, Mark Seaborn,
	Kirill A. Shutemov, Daniel James, Finn Grimwood
In-Reply-To: <CA+55aFyKpWrt_Ajzh1rzp_GcwZ4=6Y=kOv8hBz172CFJp6L8Tg@mail.gmail.com>

On 12.05.2015 18:06, Linus Torvalds wrote:
> On Tue, May 12, 2015 at 2:43 AM, Konstantin Khlebnikov
> <khlebnikov@yandex-team.ru> wrote:
>> @@ -1260,6 +1269,8 @@ static ssize_t pagemap_read(struct file *file, char __user *buf,
>>          if (!count)
>>                  goto out_task;
>>
>> +       /* do not disclose physical addresses: attack vector */
>> +       pm.show_pfn = capable(CAP_SYS_ADMIN);
>>          pm.v2 = soft_dirty_cleared;
>>          pm.len = (PAGEMAP_WALK_SIZE >> PAGE_SHIFT);
>>          pm.buffer = kmalloc(pm.len * PM_ENTRY_BYTES, GFP_TEMPORARY);
>
> NO! Dammit, no, no, no!
>
> How many times must people do this major security faux-pas before we learn?

Oops. Sorry. I guess everybody must do that mistake at least once.
That's my first time. =)


So, in this case existing call of mm_access() from pagemap_read()
is a bug too because it checks CAP_SYS_PTRACE for current task.

I'll rework it in the same way as /proc/*/[s]maps.

>
> WE DO NOT CHECK CURRENT CAPABILITIES AT READ/WRITE TIME!
>
> It's a bug. It's a security issue. It's not how Unix capabilities work!
>
> Capabilities are checked at open time.:
>
>> @@ -1335,9 +1346,6 @@ out:
>>
>>   static int pagemap_open(struct inode *inode, struct file *file)
>>   {
>> -       /* do not disclose physical addresses: attack vector */
>> -       if (!capable(CAP_SYS_ADMIN))
>> -               return -EPERM;
>
> THIS  is where you are supposed to check for capabilities. The place
> where you removed it!
>
> The reason we check capabilities at open time, and open time ONLY is
> because that is really very integral to the whole Unix security model.
> Otherwise, you get into this situation:
>
>   - unprivileged process opens file
>
>   - unprivileged process tricks suid process to do the actual access for it
>
> where the traditional model is to just force a "write()" by opening
> the file as stderr, and then executing a suid process (traditionally
> "sudo") that writes an error message to it.
>
> So *don't* do permission checks using read/write time credentials.
> They are wrong.
>
> Now, if there is some reason that you really can't do it when opening
> the file, and you actually need to use capability information at
> read/write time, you use the "file->f_cred" field, which is the
> open-time capabilities. So you _can_ do permission checks at
> read/write time, but you have to use the credentials of the opener,
> not "current".
>
> So in this case, I guess you could use
>
>          pm.show_pfn = file_ns_capable(file, &init_user_ns, CAP_SYS_ADMIN);
>
> if you really need to do this at read time, and cannot fill in that
> "show_pfn" at open-time.
>
>                          Linus
>


-- 
Konstantin

--
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 1/5] selftests: Add futex functional tests
From: Darren Hart @ 2015-05-12 16:10 UTC (permalink / raw)
  To: Cyril Hrubis
  Cc: Shuah Khan, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Linux Kernel Mailing List, John Stultz, Ingo Molnar,
	Peter Zijlstra, Thomas Gleixner, Davidlohr Bueso, KOSAKI Motohiro
In-Reply-To: <20150512100256.GA5122-J5syqNJeCN4b1SvskN2V4Q@public.gmane.org>

On 5/12/15, 3:02 AM, "Cyril Hrubis" <chrubis-AlSwsSmVLrQ@public.gmane.org> wrote:

>Hi!
>> I'm happy to do that, but I would like to make sure I'm doing the right
>> thing.
>
>The right thing here is to add -pthread to CFLAGS which sets both flags
>for preprocessor and linker (see man gcc).

Hi Cyril,

Thanks. I read that, and mentioned it, but my concern with -pthread in the
CFLAGS and LDFLAGS is that it is a non-standard compiler flag. I
understand we have a number of gcc-isms in our build - but do we want to
add more?

I'm also struggling to find any kind of prescribed documentation on this
beyond the short blurb in the gcc man page which describes what this
option does, but not when to use it. I'll need something concrete to
justify changes to testcase Makefiles to Shuah.

Unfortunately, all these options are functional for me, while some are not
for Shuah, so I'm currently having to rely on documentation which seems
pretty sparse.

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply

* Re: [PATCH 1/5] selftests: Add futex functional tests
From: Shuah Khan @ 2015-05-12 16:47 UTC (permalink / raw)
  To: Darren Hart, Cyril Hrubis
  Cc: linux-api-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
	John Stultz, Ingo Molnar, Peter Zijlstra, Thomas Gleixner,
	Davidlohr Bueso, KOSAKI Motohiro, Shuah Khan
In-Reply-To: <D1777231.CC5D0%dvhart-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

On 05/12/2015 10:10 AM, Darren Hart wrote:
> On 5/12/15, 3:02 AM, "Cyril Hrubis" <chrubis-AlSwsSmVLrQ@public.gmane.org> wrote:
> 
>> Hi!
>>> I'm happy to do that, but I would like to make sure I'm doing the right
>>> thing.
>>
>> The right thing here is to add -pthread to CFLAGS which sets both flags
>> for preprocessor and linker (see man gcc).
> 
> Hi Cyril,
> 
> Thanks. I read that, and mentioned it, but my concern with -pthread in the
> CFLAGS and LDFLAGS is that it is a non-standard compiler flag. I
> understand we have a number of gcc-isms in our build - but do we want to
> add more?
> 
> I'm also struggling to find any kind of prescribed documentation on this
> beyond the short blurb in the gcc man page which describes what this
> option does, but not when to use it. I'll need something concrete to
> justify changes to testcase Makefiles to Shuah.
> 
> Unfortunately, all these options are functional for me, while some are not
> for Shuah, so I'm currently having to rely on documentation which seems
> pretty sparse.
> 

Hi Daren,

My goals are really simple. make kselftest doesn't break. All tests
compile and run and install or fail gracefully when dependencies aren't
met. As long as the above are met, I don't really worry about what
individual test Makefiles do.

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978

^ permalink raw reply

* Re: [PATCH 1/5] selftests: Add futex functional tests
From: Darren Hart @ 2015-05-12 17:05 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Darren Hart, Cyril Hrubis, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Linux Kernel Mailing List, John Stultz, Ingo Molnar,
	Peter Zijlstra, Thomas Gleixner, Davidlohr Bueso, KOSAKI Motohiro
In-Reply-To: <55522E92.8090609-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

On Tue, May 12, 2015 at 10:47:14AM -0600, Shuah Khan wrote:
> On 05/12/2015 10:10 AM, Darren Hart wrote:
> > On 5/12/15, 3:02 AM, "Cyril Hrubis" <chrubis-AlSwsSmVLrQ@public.gmane.org> wrote:
> > 
> >> Hi!
> >>> I'm happy to do that, but I would like to make sure I'm doing the right
> >>> thing.
> >>
> >> The right thing here is to add -pthread to CFLAGS which sets both flags
> >> for preprocessor and linker (see man gcc).
> > 
> > Hi Cyril,
> > 
> > Thanks. I read that, and mentioned it, but my concern with -pthread in the
> > CFLAGS and LDFLAGS is that it is a non-standard compiler flag. I
> > understand we have a number of gcc-isms in our build - but do we want to
> > add more?
> > 
> > I'm also struggling to find any kind of prescribed documentation on this
> > beyond the short blurb in the gcc man page which describes what this
> > option does, but not when to use it. I'll need something concrete to
> > justify changes to testcase Makefiles to Shuah.
> > 
> > Unfortunately, all these options are functional for me, while some are not
> > for Shuah, so I'm currently having to rely on documentation which seems
> > pretty sparse.
> > 
> 
> Hi Daren,
> 
> My goals are really simple. make kselftest doesn't break. All tests
> compile and run and install or fail gracefully when dependencies aren't
> met. As long as the above are met, I don't really worry about what
> individual test Makefiles do.

Understood. Given the variability across distros/compiler versions, the
"doesn't break" part isn't as straight forward as it should be. I could fix
this so it builds for you and me, but I'm trying to build confidence that I'm
doing this "correctly" so it doesn't break in other scenarios - since I'm not
seeing any breakage on my systems.

Shuah, which distro and compiler version are you seeing the build failure on?
I'd like to at least reproduce that so I can be confident my next version will
at least build in your environment.

Currently, the only direction I have is gcc man page and Cyril's advocacy for
-pthread. Does the following work for you (it still does for me)?


>From 0b7284551d6419520d5ce8833e177ba5b8be2286 Mon Sep 17 00:00:00 2001
Message-Id: <0b7284551d6419520d5ce8833e177ba5b8be2286.1431449782.git.dvhart-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
From: Darren Hart <dvhart-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Date: Tue, 12 May 2015 09:54:39 -0700
Subject: [PATCH] selftests/futex: Add -pthread to CFLAGS and LDFLAGS

Certain environments fail to build with the "-lpthread -lrt" LDFLAGS.
Using "-pthread" instead of "-lpthread" resolves this and is the
documented mechanism in the gcc man page.

Signed-off-by: Darren Hart <dvhart-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 tools/testing/selftests/futex/functional/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/futex/functional/Makefile b/tools/testing/selftests/futex/functional/Makefile
index e64d43b..9d6b75e 100644
--- a/tools/testing/selftests/futex/functional/Makefile
+++ b/tools/testing/selftests/futex/functional/Makefile
@@ -1,6 +1,6 @@
 INCLUDES := -I../include -I../../
-CFLAGS := $(CFLAGS) -g -O2 -Wall -D_GNU_SOURCE $(INCLUDES)
-LDFLAGS := $(LDFLAGS) -lpthread -lrt
+CFLAGS := $(CFLAGS) -g -O2 -Wall -D_GNU_SOURCE -pthread $(INCLUDES)
+LDFLAGS := $(LDFLAGS) -pthread -lrt
 
 HEADERS := ../include/futextest.h
 TARGETS := \
-- 
2.1.4


-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply related

* Re: [PATCH 1/5] selftests: Add futex functional tests
From: Shuah Khan @ 2015-05-12 17:14 UTC (permalink / raw)
  To: Darren Hart
  Cc: Darren Hart, Cyril Hrubis, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Linux Kernel Mailing List, John Stultz, Ingo Molnar,
	Peter Zijlstra, Thomas Gleixner, Davidlohr Bueso, KOSAKI Motohiro,
	Shuah Khan
In-Reply-To: <20150512170544.GA30184-JIrPihikzLEQaXB9iyTzyw@public.gmane.org>

On 05/12/2015 11:05 AM, Darren Hart wrote:
> On Tue, May 12, 2015 at 10:47:14AM -0600, Shuah Khan wrote:
>> On 05/12/2015 10:10 AM, Darren Hart wrote:
>>> On 5/12/15, 3:02 AM, "Cyril Hrubis" <chrubis-AlSwsSmVLrQ@public.gmane.org> wrote:
>>>
>>>> Hi!
>>>>> I'm happy to do that, but I would like to make sure I'm doing the right
>>>>> thing.
>>>>
>>>> The right thing here is to add -pthread to CFLAGS which sets both flags
>>>> for preprocessor and linker (see man gcc).
>>>
>>> Hi Cyril,
>>>
>>> Thanks. I read that, and mentioned it, but my concern with -pthread in the
>>> CFLAGS and LDFLAGS is that it is a non-standard compiler flag. I
>>> understand we have a number of gcc-isms in our build - but do we want to
>>> add more?
>>>
>>> I'm also struggling to find any kind of prescribed documentation on this
>>> beyond the short blurb in the gcc man page which describes what this
>>> option does, but not when to use it. I'll need something concrete to
>>> justify changes to testcase Makefiles to Shuah.
>>>
>>> Unfortunately, all these options are functional for me, while some are not
>>> for Shuah, so I'm currently having to rely on documentation which seems
>>> pretty sparse.
>>>
>>
>> Hi Daren,
>>
>> My goals are really simple. make kselftest doesn't break. All tests
>> compile and run and install or fail gracefully when dependencies aren't
>> met. As long as the above are met, I don't really worry about what
>> individual test Makefiles do.
> 
> Understood. Given the variability across distros/compiler versions, the
> "doesn't break" part isn't as straight forward as it should be. I could fix
> this so it builds for you and me, but I'm trying to build confidence that I'm
> doing this "correctly" so it doesn't break in other scenarios - since I'm not
> seeing any breakage on my systems.
> 
> Shuah, which distro and compiler version are you seeing the build failure on?
> I'd like to at least reproduce that so I can be confident my next version will
> at least build in your environment.
> 
> Currently, the only direction I have is gcc man page and Cyril's advocacy for
> -pthread. Does the following work for you (it still does for me)?
> 
> 
>>From 0b7284551d6419520d5ce8833e177ba5b8be2286 Mon Sep 17 00:00:00 2001
> Message-Id: <0b7284551d6419520d5ce8833e177ba5b8be2286.1431449782.git.dvhart-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> From: Darren Hart <dvhart-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Date: Tue, 12 May 2015 09:54:39 -0700
> Subject: [PATCH] selftests/futex: Add -pthread to CFLAGS and LDFLAGS
> 
> Certain environments fail to build with the "-lpthread -lrt" LDFLAGS.
> Using "-pthread" instead of "-lpthread" resolves this and is the
> documented mechanism in the gcc man page.
> 
> Signed-off-by: Darren Hart <dvhart-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
>  tools/testing/selftests/futex/functional/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/futex/functional/Makefile b/tools/testing/selftests/futex/functional/Makefile
> index e64d43b..9d6b75e 100644
> --- a/tools/testing/selftests/futex/functional/Makefile
> +++ b/tools/testing/selftests/futex/functional/Makefile
> @@ -1,6 +1,6 @@
>  INCLUDES := -I../include -I../../
> -CFLAGS := $(CFLAGS) -g -O2 -Wall -D_GNU_SOURCE $(INCLUDES)
> -LDFLAGS := $(LDFLAGS) -lpthread -lrt
> +CFLAGS := $(CFLAGS) -g -O2 -Wall -D_GNU_SOURCE -pthread $(INCLUDES)
> +LDFLAGS := $(LDFLAGS) -pthread -lrt
>  
>  HEADERS := ../include/futextest.h
>  TARGETS := \
> 


Daren,

The above worked on my system. I have gcc version 4.9.2 (Ubuntu
4.9.2-10ubuntu13) on mine just in case you need that info.

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978

^ permalink raw reply

* Re: [PATCH V6 05/10] audit: log creation and deletion of namespace instances
From: Richard Guy Briggs @ 2015-05-12 19:57 UTC (permalink / raw)
  To: Steve Grubb
  Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-audit-H+wXaHxf7aLQT0dZR+AlfA, eparis-FjpueFixGhCM4zKIHC2jIg,
	pmoore-H+wXaHxf7aLQT0dZR+AlfA, arozansk-H+wXaHxf7aLQT0dZR+AlfA,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w, serge-A9i7LUbDfNHQT0dZR+AlfA,
	zohar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <2487286.y6vyJ9A3er@x2>

On 15/05/05, Steve Grubb wrote:
> Hello,
> 
> I think there needs to be some more discussion around this. It seems like this 
> is not exactly recording things that are useful for audit.

It seems to me that either audit has to assemble that information, or
the kernel has to do so.  The kernel doesn't know about containers
(yet?).

> On Friday, April 17, 2015 03:35:52 AM Richard Guy Briggs wrote:
> > Log the creation and deletion of namespace instances in all 6 types of
> > namespaces.
> > 
> > Twelve new audit message types have been introduced:
> > AUDIT_NS_INIT_MNT       1330    /* Record mount namespace instance creation
> > */ AUDIT_NS_INIT_UTS       1331    /* Record UTS namespace instance
> > creation */ AUDIT_NS_INIT_IPC       1332    /* Record IPC namespace
> > instance creation */ AUDIT_NS_INIT_USER      1333    /* Record USER
> > namespace instance creation */ AUDIT_NS_INIT_PID       1334    /* Record
> > PID namespace instance creation */ AUDIT_NS_INIT_NET       1335    /*
> > Record NET namespace instance creation */ AUDIT_NS_DEL_MNT        1336   
> > /* Record mount namespace instance deletion */ AUDIT_NS_DEL_UTS        1337
> >    /* Record UTS namespace instance deletion */ AUDIT_NS_DEL_IPC       
> > 1338    /* Record IPC namespace instance deletion */ AUDIT_NS_DEL_USER     
> >  1339    /* Record USER namespace instance deletion */ AUDIT_NS_DEL_PID    
> >    1340    /* Record PID namespace instance deletion */ AUDIT_NS_DEL_NET   
> >     1341    /* Record NET namespace instance deletion */
> 
> The requirements for auditing of containers should be derived from VPP. In it, 
> it asks for selectable auditing, selective audit, and selective audit review. 
> What this means is that we need the container and all its children to have one 
> identifier that is inserted into all the events that are associated with the 
> container.

Is that requirement for the records that are sent from the kernel, or
for the records stored by auditd, or by another facility that delivers
those records to a final consumer?

> With this, its possible to do a search for all events related to a container. 
> Its possible to exclude events from a container. Its possible to not get any 
> events.
> 
> The requirements also call out for the identification of the subject. This 
> means that the event should be bound to a syscall such as clone, setns, or 
> unshare.

Is it useful to have a reference of the init namespace set from which
all others are spawned?

If it isn't bound, I assume the subject should be added to the message
format?  I'm thinking of messages without an audit_context such as audit
user messages (such as AUDIT_NS_INFO and AUDIT_VIRT_CONTROL).

For now, we should not need to log namespaces with AUDIT_FEATURE_CHANGE
or AUDIT_CONFIG_CHANGE messages since only initial user namespace with
initial pid namespace has permission to do so.  This will need to be
addressed by having non-init config changes be limited to that container
or set of namespaces and possibly its children.  The other possibility
is to add the subject to the stand-alone message.

> Also, any user space events originating inside the container needs to have the 
> container ID added to the user space event - just like auid and session id.

This sounds like every task needs to record a container ID since that
information is otherwise unknown by the kernel except by what might be
provided by an audit user message such as AUDIT_VIRT_CONTROL or possibly
the new AUDIT_NS_INFO request.  It could be stored in struct task_struct
or in struct audit_context.  I don't have a suggestion on how to get
that information securely into the kernel.

> Recording each instance of a name space is giving me something that I cannot 
> use to do queries required by the security target. Given these events, how do 
> I locate a web server event where it accesses a watched file? That 
> authentication failed? That an update within the container failed?
> 
> The requirements are that we have to log the creation, suspension, migration, 
> and termination of a container. The requirements are not on the individual 
> name space.

Ok.  Do we have a robust definition of a container?  Where is that
definition managed?  If it is a userspace concept, then I think either
userspace should be assembling this information, or providing that
information to the entity that will be expected to know about and
provide it.

> Maybe I'm missing how these events give me that. But I'd like to hear how I 
> would be able to meet requirements with these 12 events.

Adding the infrastructure to give each of those 12 events an audit
context to be able to give meaningful subject fields in audit records
appears to require adding a struct task_struct argument to calls to
copy_mnt_ns(), copy_utsname(), copy_ipcs(), copy_pid_ns(),
copy_net_ns(), create_user_ns() unless I use current.  I think we must
use current since the userns is created before the spawned process is
mature or has an audit context in the case of clone.

Either that, or I have mis-understood and I should be stashing this
namespace ID information in an audit_aux_data structure or a more
permanent part of struct audit_context to be printed when required on
syscall exit.  I'm trying to think through if it is needed in any
non-syscall audit messages.

Another RFC patch set coming...

> -Steve
>  
> > As suggested by Eric Paris, there are 12 message types, one for each of
> > creation and deletion, one for each type of namespace so that text searches
> > are easier in conjunction with the AUDIT_NS_INFO message type, being able
> > to search for all records such as "netns=4 " and to avoid fields
> > disappearing per message type to make ausearch more efficient.
> > 
> > A typical startup would look roughly like:
> > 
> > 	type=AUDIT_NS_INIT_UTS msg=audit(1408577534.868:5): pid=1 uid=0
> > auid=4294967295 ses=4294967295 subj=kernel dev=00:03 old_utsns=(none)
> > utsns=-2 res=1 type=AUDIT_NS_INIT_USER msg=audit(1408577534.868:6): pid=1
> > uid=0 auid=4294967295 ses=4294967295 subj=kernel dev=00:03
> > old_userns=(none) userns=-3 res=1 type=AUDIT_NS_INIT_PID
> > msg=audit(1408577534.868:7): pid=1 uid=0 auid=4294967295 ses=4294967295
> > subj=kernel dev=00:03 old_pidns=(none) pidns=-4 res=1
> > type=AUDIT_NS_INIT_MNT msg=audit(1408577534.868:8): pid=1 uid=0
> > auid=4294967295 ses=4294967295 subj=kernel dev=00:03 old_mntns=(none)
> > mntns=0 res=1 type=AUDIT_NS_INIT_IPC msg=audit(1408577534.868:9): pid=1
> > uid=0 auid=4294967295 ses=4294967295 subj=kernel dev=00:03 old_ipcns=(none)
> > ipcns=-1 res=1 type=AUDIT_NS_INIT_NET msg=audit(1408577533.500:10): pid=1
> > uid=0 auid=4294967295 ses=4294967295 subj=kernel dev=00:03 old_netns=(none)
> > netns=2 res=1
> > 
> > And a CLONE action would result in:
> > 	type=type=AUDIT_NS_INIT_NET msg=audit(1408577535.306:81): pid=481 uid=0
> > auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0 dev=00:03
> > old_netns=2 netns=3 res=1
> > 
> > While deleting a namespace would result in:
> > 	type=type=AUDIT_NS_DEL_MNT msg=audit(1408577552.221:85): pid=481 uid=0
> > auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0 dev=00:03
> > mntns=4 res=1
> > 
> > If not "(none)", old_XXXns lists the namespace from which it was cloned.
> > 
> > Signed-off-by: Richard Guy Briggs <rgb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> >  fs/namespace.c             |   13 +++++++++
> >  include/linux/audit.h      |    8 +++++
> >  include/uapi/linux/audit.h |   12 ++++++++
> >  ipc/namespace.c            |   12 ++++++++
> >  kernel/audit.c             |   64
> > ++++++++++++++++++++++++++++++++++++++++++++ kernel/pid_namespace.c     |  
> > 13 +++++++++
> >  kernel/user_namespace.c    |   13 +++++++++
> >  kernel/utsname.c           |   12 ++++++++
> >  net/core/net_namespace.c   |   12 ++++++++
> >  9 files changed, 159 insertions(+), 0 deletions(-)
> > 
> > diff --git a/fs/namespace.c b/fs/namespace.c
> > index 182bc41..7b62543 100644
> > --- a/fs/namespace.c
> > +++ b/fs/namespace.c
> > @@ -24,6 +24,7 @@
> >  #include <linux/proc_ns.h>
> >  #include <linux/magic.h>
> >  #include <linux/bootmem.h>
> > +#include <linux/audit.h>
> >  #include "pnode.h"
> >  #include "internal.h"
> > 
> > @@ -2459,6 +2460,7 @@ dput_out:
> > 
> >  static void free_mnt_ns(struct mnt_namespace *ns)
> >  {
> > +	audit_log_ns_del(AUDIT_NS_DEL_MNT, ns->proc_inum);
> >  	proc_free_inum(ns->proc_inum);
> >  	put_user_ns(ns->user_ns);
> >  	kfree(ns);
> > @@ -2518,6 +2520,7 @@ struct mnt_namespace *copy_mnt_ns(unsigned long flags,
> > struct mnt_namespace *ns, new_ns = alloc_mnt_ns(user_ns);
> >  	if (IS_ERR(new_ns))
> >  		return new_ns;
> > +	audit_log_ns_init(AUDIT_NS_INIT_MNT, ns->proc_inum, new_ns->proc_inum);
> > 
> >  	namespace_lock();
> >  	/* First pass: copy the tree topology */
> > @@ -2830,6 +2833,16 @@ static void __init init_mount_tree(void)
> >  	set_fs_root(current->fs, &root);
> >  }
> > 
> > +/* log the ID of init mnt namespace after audit service starts */
> > +static int __init mnt_ns_init_log(void)
> > +{
> > +	struct mnt_namespace *init_mnt_ns = init_task.nsproxy->mnt_ns;
> > +
> > +	audit_log_ns_init(AUDIT_NS_INIT_MNT, 0, init_mnt_ns->proc_inum);
> > +	return 0;
> > +}
> > +late_initcall(mnt_ns_init_log);
> > +
> >  void __init mnt_init(void)
> >  {
> >  	unsigned u;
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index 71698ec..b28dfb0 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -484,6 +484,9 @@ extern void		    audit_log_ns_info(struct 
> task_struct
> > *tsk); static inline void	    audit_log_ns_info(struct task_struct *tsk) {
> > }
> >  #endif
> > +extern void		    audit_log_ns_init(int type, unsigned int old_inum,
> > +					      unsigned int inum);
> > +extern void		    audit_log_ns_del(int type, unsigned int inum);
> > 
> >  extern int		    audit_update_lsm_rules(void);
> > 
> > @@ -542,6 +545,11 @@ static inline void audit_log_task_info(struct
> > audit_buffer *ab, { }
> >  static inline void audit_log_ns_info(struct task_struct *tsk)
> >  { }
> > +static inline int audit_log_ns_init(int type, unsigned int old_inum,
> > +				    unsigned int inum)
> > +{ }
> > +static inline int audit_log_ns_del(int type, unsigned int inum)
> > +{ }
> >  #define audit_enabled 0
> >  #endif /* CONFIG_AUDIT */
> >  static inline void audit_log_string(struct audit_buffer *ab, const char
> > *buf) diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 1ffb151..487cad6 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -111,6 +111,18 @@
> >  #define AUDIT_PROCTITLE		1327	/* Proctitle emit event */
> >  #define AUDIT_FEATURE_CHANGE	1328	/* audit log listing feature changes 
> */
> >  #define AUDIT_NS_INFO		1329	/* Record process namespace IDs */
> > +#define AUDIT_NS_INIT_MNT	1330	/* Record mount namespace instance 
> creation
> > */ +#define AUDIT_NS_INIT_UTS	1331	/* Record UTS namespace instance
> > creation */ +#define AUDIT_NS_INIT_IPC	1332	/* Record IPC namespace
> > instance creation */ +#define AUDIT_NS_INIT_USER	1333	/* Record USER
> > namespace instance creation */ +#define AUDIT_NS_INIT_PID	1334	/* Record
> > PID namespace instance creation */ +#define AUDIT_NS_INIT_NET	1335	/*
> > Record NET namespace instance creation */ +#define AUDIT_NS_DEL_MNT	1336	
> /*
> > Record mount namespace instance deletion */ +#define
> > AUDIT_NS_DEL_UTS	1337	/* Record UTS namespace instance deletion */ 
> +#define
> > AUDIT_NS_DEL_IPC	1338	/* Record IPC namespace instance deletion */ 
> +#define
> > AUDIT_NS_DEL_USER	1339	/* Record USER namespace instance deletion */
> > +#define AUDIT_NS_DEL_PID	1340	/* Record PID namespace instance 
> deletion */
> > +#define AUDIT_NS_DEL_NET	1341	/* Record NET namespace instance deletion 
> */
> > 
> >  #define AUDIT_AVC		1400	/* SE Linux avc denial or grant */
> >  #define AUDIT_SELINUX_ERR	1401	/* Internal SE Linux Errors */
> > diff --git a/ipc/namespace.c b/ipc/namespace.c
> > index 59451c1..73727ce 100644
> > --- a/ipc/namespace.c
> > +++ b/ipc/namespace.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/mount.h>
> >  #include <linux/user_namespace.h>
> >  #include <linux/proc_ns.h>
> > +#include <linux/audit.h>
> > 
> >  #include "util.h"
> > 
> > @@ -41,6 +42,8 @@ static struct ipc_namespace *create_ipc_ns(struct
> > user_namespace *user_ns, }
> >  	atomic_inc(&nr_ipc_ns);
> > 
> > +	audit_log_ns_init(AUDIT_NS_INIT_IPC, old_ns->proc_inum, ns->proc_inum);
> > +
> >  	sem_init_ns(ns);
> >  	msg_init_ns(ns);
> >  	shm_init_ns(ns);
> > @@ -119,6 +122,7 @@ static void free_ipc_ns(struct ipc_namespace *ns)
> >  	 */
> >  	ipcns_notify(IPCNS_REMOVED);
> >  	put_user_ns(ns->user_ns);
> > +	audit_log_ns_del(AUDIT_NS_DEL_IPC, ns->proc_inum);
> >  	proc_free_inum(ns->proc_inum);
> >  	kfree(ns);
> >  }
> > @@ -197,3 +201,11 @@ const struct proc_ns_operations ipcns_operations = {
> >  	.install	= ipcns_install,
> >  	.inum		= ipcns_inum,
> >  };
> > +
> > +/* log the ID of init IPC namespace after audit service starts */
> > +static int __init ipc_namespaces_init(void)
> > +{
> > +	audit_log_ns_init(AUDIT_NS_INIT_IPC, 0, init_ipc_ns.proc_inum);
> > +	return 0;
> > +}
> > +late_initcall(ipc_namespaces_init);
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index 63f32f4..e6230c4 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -1978,6 +1978,70 @@ out:
> >  	kfree(name);
> >  }
> > 
> > +#ifdef CONFIG_NAMESPACES
> > +static char *ns_name[] = {
> > +	"mnt",
> > +	"uts",
> > +	"ipc",
> > +	"user",
> > +	"pid",
> > +	"net",
> > +};
> > +
> > +/**
> > + * audit_log_ns_init - report a namespace instance creation
> > + * @type: type of audit namespace instance created message
> > + * @old_inum: the ID number of the cloned namespace instance
> > + * @inum: the ID number of the new namespace instance
> > + */
> > +void  audit_log_ns_init(int type, unsigned int old_inum, unsigned int inum)
> > +{
> > +	struct audit_buffer *ab;
> > +	char *audit_ns_name = ns_name[type - AUDIT_NS_INIT_MNT];
> > +	struct vfsmount *mnt = task_active_pid_ns(current)->proc_mnt;
> > +	struct super_block *sb = mnt->mnt_sb;
> > +	char old_ns[16];
> > +
> > +	if (type < AUDIT_NS_INIT_MNT || type > AUDIT_NS_INIT_NET) {
> > +		WARN(1, "audit_log_ns_init: type:%d out of range", type);
> > +		return;
> > +	}
> > +	if (!old_inum)
> > +		sprintf(old_ns, "(none)");
> > +	else
> > +		sprintf(old_ns, "%d", old_inum - PROC_DYNAMIC_FIRST);
> > +	audit_log_common_recv_msg(&ab, type);
> > +	audit_log_format(ab, " dev=%02x:%02x old_%sns=%s %sns=%d res=1",
> > +			 MAJOR(sb->s_dev), MINOR(sb->s_dev),
> > +			 audit_ns_name, old_ns,
> > +			 audit_ns_name, inum - PROC_DYNAMIC_FIRST);
> > +	audit_log_end(ab);
> > +}
> > +
> > +/**
> > + * audit_log_ns_del - report a namespace instance deleted
> > + * @type: type of audit namespace instance deleted message
> > + * @inum: the ID number of the namespace instance
> > + */
> > +void audit_log_ns_del(int type, unsigned int inum)
> > +{
> > +	struct audit_buffer *ab;
> > +	char *audit_ns_name = ns_name[type - AUDIT_NS_DEL_MNT];
> > +	struct vfsmount *mnt = task_active_pid_ns(current)->proc_mnt;
> > +	struct super_block *sb = mnt->mnt_sb;
> > +
> > +	if (type < AUDIT_NS_DEL_MNT || type > AUDIT_NS_DEL_NET) {
> > +		WARN(1, "audit_log_ns_del: type:%d out of range", type);
> > +		return;
> > +	}
> > +	audit_log_common_recv_msg(&ab, type);
> > +	audit_log_format(ab, " dev=%02x:%02x %sns=%d res=1",
> > +			 MAJOR(sb->s_dev), MINOR(sb->s_dev), audit_ns_name,
> > +			 inum - PROC_DYNAMIC_FIRST);
> > +	audit_log_end(ab);
> > +}
> > +#endif /* CONFIG_NAMESPACES */
> > +
> >  /**
> >   * audit_log_end - end one audit record
> >   * @ab: the audit_buffer
> > diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
> > index db95d8e..d28fd14 100644
> > --- a/kernel/pid_namespace.c
> > +++ b/kernel/pid_namespace.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/proc_ns.h>
> >  #include <linux/reboot.h>
> >  #include <linux/export.h>
> > +#include <linux/audit.h>
> > 
> >  struct pid_cache {
> >  	int nr_ids;
> > @@ -109,6 +110,9 @@ static struct pid_namespace *create_pid_namespace(struct
> > user_namespace *user_ns if (err)
> >  		goto out_free_map;
> > 
> > +	audit_log_ns_init(AUDIT_NS_INIT_PID, parent_pid_ns->proc_inum,
> > +			  ns->proc_inum);
> > +
> >  	kref_init(&ns->kref);
> >  	ns->level = level;
> >  	ns->parent = get_pid_ns(parent_pid_ns);
> > @@ -142,6 +146,7 @@ static void destroy_pid_namespace(struct pid_namespace
> > *ns) {
> >  	int i;
> > 
> > +	audit_log_ns_del(AUDIT_NS_DEL_PID, ns->proc_inum);
> >  	proc_free_inum(ns->proc_inum);
> >  	for (i = 0; i < PIDMAP_ENTRIES; i++)
> >  		kfree(ns->pidmap[i].page);
> > @@ -388,3 +393,11 @@ static __init int pid_namespaces_init(void)
> >  }
> > 
> >  __initcall(pid_namespaces_init);
> > +
> > +/* log the ID of init PID namespace after audit service starts */
> > +static __init int pid_namespaces_late_init(void)
> > +{
> > +	audit_log_ns_init(AUDIT_NS_INIT_PID, 0, init_pid_ns.proc_inum);
> > +	return 0;
> > +}
> > +late_initcall(pid_namespaces_late_init);
> > diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> > index fcc0256..89c2517 100644
> > --- a/kernel/user_namespace.c
> > +++ b/kernel/user_namespace.c
> > @@ -22,6 +22,7 @@
> >  #include <linux/ctype.h>
> >  #include <linux/projid.h>
> >  #include <linux/fs_struct.h>
> > +#include <linux/audit.h>
> > 
> >  static struct kmem_cache *user_ns_cachep __read_mostly;
> > 
> > @@ -92,6 +93,9 @@ int create_user_ns(struct cred *new)
> >  		return ret;
> >  	}
> > 
> > +	audit_log_ns_init(AUDIT_NS_INIT_USER, parent_ns->proc_inum,
> > +			  ns->proc_inum);
> > +
> >  	atomic_set(&ns->count, 1);
> >  	/* Leave the new->user_ns reference with the new user namespace. */
> >  	ns->parent = parent_ns;
> > @@ -136,6 +140,7 @@ void free_user_ns(struct user_namespace *ns)
> >  #ifdef CONFIG_PERSISTENT_KEYRINGS
> >  		key_put(ns->persistent_keyring_register);
> >  #endif
> > +		audit_log_ns_del(AUDIT_NS_DEL_USER, ns->proc_inum);
> >  		proc_free_inum(ns->proc_inum);
> >  		kmem_cache_free(user_ns_cachep, ns);
> >  		ns = parent;
> > @@ -909,3 +914,11 @@ static __init int user_namespaces_init(void)
> >  	return 0;
> >  }
> >  subsys_initcall(user_namespaces_init);
> > +
> > +/* log the ID of init user namespace after audit service starts */
> > +static __init int user_namespaces_late_init(void)
> > +{
> > +	audit_log_ns_init(AUDIT_NS_INIT_USER, 0, init_user_ns.proc_inum);
> > +	return 0;
> > +}
> > +late_initcall(user_namespaces_late_init);
> > diff --git a/kernel/utsname.c b/kernel/utsname.c
> > index fd39312..fa21e8d 100644
> > --- a/kernel/utsname.c
> > +++ b/kernel/utsname.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/slab.h>
> >  #include <linux/user_namespace.h>
> >  #include <linux/proc_ns.h>
> > +#include <linux/audit.h>
> > 
> >  static struct uts_namespace *create_uts_ns(void)
> >  {
> > @@ -48,6 +49,8 @@ static struct uts_namespace *clone_uts_ns(struct
> > user_namespace *user_ns, return ERR_PTR(err);
> >  	}
> > 
> > +	audit_log_ns_init(AUDIT_NS_INIT_UTS, old_ns->proc_inum, ns->proc_inum);
> > +
> >  	down_read(&uts_sem);
> >  	memcpy(&ns->name, &old_ns->name, sizeof(ns->name));
> >  	ns->user_ns = get_user_ns(user_ns);
> > @@ -84,6 +87,7 @@ void free_uts_ns(struct kref *kref)
> > 
> >  	ns = container_of(kref, struct uts_namespace, kref);
> >  	put_user_ns(ns->user_ns);
> > +	audit_log_ns_del(AUDIT_NS_DEL_UTS, ns->proc_inum);
> >  	proc_free_inum(ns->proc_inum);
> >  	kfree(ns);
> >  }
> > @@ -138,3 +142,11 @@ const struct proc_ns_operations utsns_operations = {
> >  	.install	= utsns_install,
> >  	.inum		= utsns_inum,
> >  };
> > +
> > +/* log the ID of init UTS namespace after audit service starts */
> > +static int __init uts_namespaces_init(void)
> > +{
> > +	audit_log_ns_init(AUDIT_NS_INIT_UTS, 0, init_uts_ns.proc_inum);
> > +	return 0;
> > +}
> > +late_initcall(uts_namespaces_init);
> > diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> > index 85b6269..562eb85 100644
> > --- a/net/core/net_namespace.c
> > +++ b/net/core/net_namespace.c
> > @@ -17,6 +17,7 @@
> >  #include <linux/user_namespace.h>
> >  #include <net/net_namespace.h>
> >  #include <net/netns/generic.h>
> > +#include <linux/audit.h>
> > 
> >  /*
> >   *	Our network namespace constructor/destructor lists
> > @@ -253,6 +254,8 @@ struct net *copy_net_ns(unsigned long flags,
> >  	mutex_lock(&net_mutex);
> >  	rv = setup_net(net, user_ns);
> >  	if (rv == 0) {
> > +		audit_log_ns_init(AUDIT_NS_INIT_NET, old_net->proc_inum,
> > +				  net->proc_inum);
> >  		rtnl_lock();
> >  		list_add_tail_rcu(&net->list, &net_namespace_list);
> >  		rtnl_unlock();
> > @@ -389,6 +392,7 @@ static __net_init int net_ns_net_init(struct net *net)
> > 
> >  static __net_exit void net_ns_net_exit(struct net *net)
> >  {
> > +	audit_log_ns_del(AUDIT_NS_DEL_NET, net->proc_inum);
> >  	proc_free_inum(net->proc_inum);
> >  }
> > 
> > @@ -435,6 +439,14 @@ static int __init net_ns_init(void)
> > 
> >  pure_initcall(net_ns_init);
> > 
> > +/* log the ID of init_net namespace after audit service starts */
> > +static int __init net_ns_init_log(void)
> > +{
> > +	audit_log_ns_init(AUDIT_NS_INIT_NET, 0, init_net.proc_inum);
> > +	return 0;
> > +}
> > +late_initcall(net_ns_init_log);
> > +
> >  #ifdef CONFIG_NET_NS
> >  static int __register_pernet_operations(struct list_head *list,
> >  					struct pernet_operations *ops)
> 

- RGB

--
Richard Guy Briggs <rbriggs-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

^ permalink raw reply

* Re: [PATCH 1/5] selftests: Add futex functional tests
From: Cyril Hrubis @ 2015-05-12 20:05 UTC (permalink / raw)
  To: Darren Hart
  Cc: Shuah Khan, linux-api, Linux Kernel Mailing List, John Stultz,
	Ingo Molnar, Peter Zijlstra, Thomas Gleixner, Davidlohr Bueso,
	KOSAKI Motohiro
In-Reply-To: <D1777231.CC5D0%dvhart@linux.intel.com>

Hi!
> >> I'm happy to do that, but I would like to make sure I'm doing the right
> >> thing.
> >
> >The right thing here is to add -pthread to CFLAGS which sets both flags
> >for preprocessor and linker (see man gcc).
> 
> Hi Cyril,
> 
> Thanks. I read that, and mentioned it, but my concern with -pthread in the
> CFLAGS and LDFLAGS is that it is a non-standard compiler flag. I
> understand we have a number of gcc-isms in our build - but do we want to
> add more?
>
> I'm also struggling to find any kind of prescribed documentation on this
> beyond the short blurb in the gcc man page which describes what this
> option does, but not when to use it. I'll need something concrete to
> justify changes to testcase Makefiles to Shuah.

Sorry to mislead you with the pointing at gcc man page.

It is a Linux standard. Have a look at pthreads manual page:
http://man7.org/linux/man-pages/man7/pthreads.7.html

"On Linux, programs that use the Pthreads API should be compiled using
 cc -pthread."

Or any pthread_foo() manual page that starts with:

"Compile and link with -pthread."

The portable way i.e. POSIX would be getting compiler flags with getconf
but as this is a Linux kernel testsuite I would not bother with that.
Hmm, and it looks like this is not implemented on Linux anyway.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply

* Re: [PATCH 1/5] selftests: Add futex functional tests
From: Darren Hart @ 2015-05-12 20:15 UTC (permalink / raw)
  To: Cyril Hrubis
  Cc: Shuah Khan, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Linux Kernel Mailing List, John Stultz, Ingo Molnar,
	Peter Zijlstra, Thomas Gleixner, Davidlohr Bueso, KOSAKI Motohiro
In-Reply-To: <20150512200528.GA1336@rei>

On 5/12/15, 1:05 PM, "Cyril Hrubis" <chrubis-AlSwsSmVLrQ@public.gmane.org> wrote:

>Hi!
>> >> I'm happy to do that, but I would like to make sure I'm doing the
>>right
>> >> thing.
>> >
>> >The right thing here is to add -pthread to CFLAGS which sets both flags
>> >for preprocessor and linker (see man gcc).
>> 
>> Hi Cyril,
>> 
>> Thanks. I read that, and mentioned it, but my concern with -pthread in
>>the
>> CFLAGS and LDFLAGS is that it is a non-standard compiler flag. I
>> understand we have a number of gcc-isms in our build - but do we want to
>> add more?
>>
>> I'm also struggling to find any kind of prescribed documentation on this
>> beyond the short blurb in the gcc man page which describes what this
>> option does, but not when to use it. I'll need something concrete to
>> justify changes to testcase Makefiles to Shuah.
>
>Sorry to mislead you with the pointing at gcc man page.
>
>It is a Linux standard. Have a look at pthreads manual page:
>http://man7.org/linux/man-pages/man7/pthreads.7.html
>
>"On Linux, programs that use the Pthreads API should be compiled using
> cc -pthread."
>
>Or any pthread_foo() manual page that starts with:
>
>"Compile and link with -pthread."
>
>The portable way i.e. POSIX would be getting compiler flags with getconf
>but as this is a Linux kernel testsuite I would not bother with that.
>Hmm, and it looks like this is not implemented on Linux anyway.

Thanks Cyril, that's perfect.

I'll roll my latest example patch adding -pthread to LDFLAGS and CFLAGS
into the initial patch and resubmit the patch series as v4.

Thanks all.

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply


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