Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [PATCH RFC v4 net-next 03/26] bpf: introduce syscall(BPF, ...) and BPF maps
From: Alexei Starovoitov @ 2014-08-15  6:40 UTC (permalink / raw)
  To: Brendan Gregg
  Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
	Steven Rostedt, Daniel Borkmann, Chema Gonzalez, Eric Dumazet,
	Peter Zijlstra, H. Peter Anvin, Andrew Morton, Kees Cook,
	Linux API, Network Development, LKML
In-Reply-To: <CAE40pdcCqu6zBqDgAXBpKHzX=y7hXtz83yEadYTE2yACiqyT3g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Thu, Aug 14, 2014 at 3:28 PM, Brendan Gregg
<brendan.d.gregg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, Aug 13, 2014 at 12:57 AM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> [...]
>> maps can have different types: hash, bloom filter, radix-tree, etc.
>>
>> The map is defined by:
>>   . type
>>   . max number of elements
>>   . key size in bytes
>>   . value size in bytes
>
> Can values be strings or byte arrays? How would user-level bpf read
> them? The two types of uses I'm thinking are:
>
> A. Constructing a custom string in kernel-context, and using that as
> the value. Eg, a truncated filename, or a dotted quad IP address, or
> the raw contents of a packet.
> B. I have a pointer to an existing buffer or string, eg a filename,
> that will likely be around for some time (>1s). Instead of the value
> storing the string, it could just be a ptr, so long as user-level bpf
> has a way to read it.
>
> Also, can keys be strings? I'd ask about multiple keys, but if they
> can be a string, I can delimit in the key (eg, "PID:filename").

Both map keys and values are opaque byte arrays. eBPF program
can decide to store strings in there. Or concatenate multiple
strings as long as sizes are bounded.
High level scripting languages are dazzling with native strings
support, but I'm trying to stay away from it in the kernel.
Scripting languages should be able to convert string operations
into low level eBPF primitives which are being worked on.
So far I've been able to use ids and pointers and concatenations
of binary things as keys and values, and have user space interpret
them. I agree that having a script that does map[probe_name()]++
is definitely more human readable than storing probe ip into
ebpf map and converting addresses to names in userspace.
I'm hoping that the urge to make cool scripting language will push
somebody to have a dtrace/ktap/stap language compiler into eBPF :)
That will also address your concern of embedded setup where
full llvm is too big, but dtrace_into_ebpf compiler may be just right.
At the same time people who care about last bit of performance
will be using C and llvm or ebpf assembler directly.
Anyway will share string related ebpf helpers soon (not in V5 though)

^ permalink raw reply

* Re: [PATCH v14 6/8] arm: add pmd_mkclean for THP
From: Will Deacon @ 2014-08-15 10:55 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Michael Kerrisk, Linux API, Hugh Dickins, Johannes Weiner,
	Rik van Riel, KOSAKI Motohiro, Mel Gorman, Jason Evans,
	Zhang Yanfei, Kirill A. Shutemov, Catalin Marinas, Russell King,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Steve Capper
In-Reply-To: <1407981212-17818-7-git-send-email-minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On Thu, Aug 14, 2014 at 02:53:30AM +0100, Minchan Kim wrote:
> MADV_FREE needs pmd_dirty and pmd_mkclean for detecting recent
> overwrite of the contents since MADV_FREE syscall is called for
> THP page.
> 
> This patch adds pmd_mkclean for THP page MADV_FREE support.

Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>

^ permalink raw reply

* Re: [PATCH v14 7/8] arm64: add pmd_[dirty|mkclean] for THP
From: Will Deacon @ 2014-08-15 10:55 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Michael Kerrisk, Linux API, Hugh Dickins, Johannes Weiner,
	Rik van Riel, KOSAKI Motohiro, Mel Gorman, Jason Evans,
	Zhang Yanfei, Kirill A. Shutemov, Russell King,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Steve Capper, Catalin Marinas
In-Reply-To: <1407981212-17818-8-git-send-email-minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On Thu, Aug 14, 2014 at 02:53:31AM +0100, Minchan Kim wrote:
> MADV_FREE needs pmd_dirty and pmd_mkclean for detecting recent
> overwrite of the contents since MADV_FREE syscall is called for
> THP page.
> 
> This patch adds pmd_dirty and pmd_mkclean for THP page MADV_FREE
> support.

Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>

^ permalink raw reply

* Re: [PATCH RFC v4 net-next 17/26] tracing: allow eBPF programs to be attached to events
From: Andy Lutomirski @ 2014-08-15 17:20 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Brendan Gregg, David S. Miller, Ingo Molnar, Linus Torvalds,
	Steven Rostedt, Daniel Borkmann, Chema Gonzalez, Eric Dumazet,
	Peter Zijlstra, H. Peter Anvin, Andrew Morton, Kees Cook,
	Linux API, Network Development, LKML
In-Reply-To: <CAMEtUuymPDhYBe42i4DJNXsdgZRaq9LuEU_nGSsqrY1FcFHqhQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Thu, Aug 14, 2014 at 11:08 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> On Thu, Aug 14, 2014 at 2:20 PM, Brendan Gregg
> <brendan.d.gregg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Wed, Aug 13, 2014 at 12:57 AM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>> [...]
>>> +/* For tracing filters save first six arguments of tracepoint events.
>>> + * On 64-bit architectures argN fields will match one to one to arguments passed
>>> + * to tracepoint events.
>>> + * On 32-bit architectures u64 arguments to events will be seen into two
>>> + * consecutive argN, argN+1 fields. Pointers, u32, u16, u8, bool types will
>>> + * match one to one
>>> + */
>>> +struct bpf_context {
>>> +       unsigned long arg1;
>>> +       unsigned long arg2;
>>> +       unsigned long arg3;
>>> +       unsigned long arg4;
>>> +       unsigned long arg5;
>>> +       unsigned long arg6;
>>> +       unsigned long ret;
>>> +};
>>
>> While this works, the argN+1 shift for 32-bit is a gotcha to learn.
>> Lets say arg1 was 64-bit, and my program only examined arg2. I'd need
>> two programs, one for 64-bit (using arg2) and 32-bit (arg3). If there
>
> correct.
> I've picked 'long' type for these tracepoint 'arguments' to match
> what is going on at assembler level.
> 32-bit archs are passing 64-bit values in two consecutive registers
> or two stack slots. So it's partially exposing architectural details.
> I've tried to use u64 here, but it complicated tracepoint+ebpf patch
> a lot, since I need per-architecture support for moving C arguments
> into u64 variables and hacking tracepoint event definitions in a nasty
> ways. This 'long' type approach is the least intrusive I could find.
> Also out of 1842 total tracepoint fields, only 144 fields are 64-bit,
> so rarely one would need to deal with u64. Most of the tracepoint
> arguments are either longs, ints or pointers, which fits this approach
> the best.
> In general the eBPF design approach is to keep kernel bits as simple
> as possible and move complexity to user space.
> In this case some higher language than C for writing scripts can
> hide this oddity.

The downside of this approach is that compat support might be
difficult or impossible.

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC

^ permalink raw reply

* Re: [PATCH RFC v4 net-next 17/26] tracing: allow eBPF programs to be attached to events
From: Andy Lutomirski @ 2014-08-15 17:25 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
	Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
	H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
	Network Development, linux-kernel@vger.kernel.org
In-Reply-To: <1407916658-8731-18-git-send-email-ast@plumgrid.com>

On Wed, Aug 13, 2014 at 12:57 AM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> User interface:
> fd = open("/sys/kernel/debug/tracing/__event__/filter")
>
> write(fd, "bpf_123")

I didn't follow all the code flow leading to parsing the "bpf_123"
string, but if it works the way I imagine it does, it's a security
problem.  In general, write(2) should never do anything that involves
any security-relevant context of the caller.

Ideally, you would look up fd 123 in the file table of whomever called
open.  If that's difficult to implement efficiently, then it would be
nice to have some check that the callers of write(2) and open(2) are
the same task and that exec wasn't called in between.

This isn't a very severe security issue because you need privilege to
open the thing in the first place, but it would still be nice to
address.

--Andy

^ permalink raw reply

* Re: [PATCH RFC v4 net-next 17/26] tracing: allow eBPF programs to be attached to events
From: Alexei Starovoitov @ 2014-08-15 17:36 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Brendan Gregg, David S. Miller, Ingo Molnar, Linus Torvalds,
	Steven Rostedt, Daniel Borkmann, Chema Gonzalez, Eric Dumazet,
	Peter Zijlstra, H. Peter Anvin, Andrew Morton, Kees Cook,
	Linux API, Network Development, LKML
In-Reply-To: <CALCETrVH8KXr8uSHAVy5eBsqmi1LjB5QZpboAGcjYswXvW1opA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Aug 15, 2014 at 10:20 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> The downside of this approach is that compat support might be
> difficult or impossible.

Would do you mean by compat? 32-bit programs on 64-bit kernels?
There is no such concept for eBPF. All eBPF programs are always
operating on 64-bit registers.

^ permalink raw reply

* Re: [PATCH RFC v4 net-next 17/26] tracing: allow eBPF programs to be attached to events
From: Alexei Starovoitov @ 2014-08-15 17:51 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
	Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
	H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
	Network Development,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CALCETrV7vO6r--G2ns+A6qmDQYSzNXeemT=x41EF+XWmayM95g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Aug 15, 2014 at 10:25 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> On Wed, Aug 13, 2014 at 12:57 AM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>> User interface:
>> fd = open("/sys/kernel/debug/tracing/__event__/filter")
>>
>> write(fd, "bpf_123")
>
> I didn't follow all the code flow leading to parsing the "bpf_123"
> string, but if it works the way I imagine it does, it's a security
> problem.  In general, write(2) should never do anything that involves
> any security-relevant context of the caller.
>
> Ideally, you would look up fd 123 in the file table of whomever called
> open.  If that's difficult to implement efficiently, then it would be
> nice to have some check that the callers of write(2) and open(2) are
> the same task and that exec wasn't called in between.
>
> This isn't a very severe security issue because you need privilege to
> open the thing in the first place, but it would still be nice to
> address.

hmm. you need to be root to open the events anyway.
pretty much the whole tracing for root only, since any kernel data
structures can be printed, stored into maps and so on.
So I don't quite follow your security concern here.

Even say root opens a tracepoint and does exec() of another
app that uploads ebpf program, gets program_fd and does
write into tracepoint fd. The root app that did this open() is
doing exec() on purpose. It's not like it's exec-ing something
it doesn't know about.

Remember, FDs was your idea in the first place ;)
I had global ids and everything root initially.

^ permalink raw reply

* Re: [PATCH RFC v4 net-next 17/26] tracing: allow eBPF programs to be attached to events
From: Andy Lutomirski @ 2014-08-15 18:50 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Daniel Borkmann, H. Peter Anvin, Andrew Morton, Linux API,
	Chema Gonzalez, Eric Dumazet, David S. Miller, Brendan Gregg,
	Linus Torvalds, Steven Rostedt, LKML, Peter Zijlstra, Kees Cook,
	Network Development, Ingo Molnar
In-Reply-To: <CAMEtUuzey7PanznrAguOpvPLxyhgJB++ovE8RNys7srs=EY1qg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Aug 15, 2014 10:36 AM, "Alexei Starovoitov" <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>
> On Fri, Aug 15, 2014 at 10:20 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> > The downside of this approach is that compat support might be
> > difficult or impossible.
>
> Would do you mean by compat? 32-bit programs on 64-bit kernels?
> There is no such concept for eBPF. All eBPF programs are always
> operating on 64-bit registers.

Doesn't the eBPF program need to know sizeof(long) to read these
fields correctly?  Or am I misunderstanding what the code does?

--Andy

^ permalink raw reply

* Re: [PATCH RFC v4 net-next 17/26] tracing: allow eBPF programs to be attached to events
From: Andy Lutomirski @ 2014-08-15 18:53 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
	Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
	H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
	Network Development,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAMEtUuzCyxdOo+yYYZfDPRAu2yeQOw8TbUABwU-HD0+78PnV7A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Aug 15, 2014 at 10:51 AM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> On Fri, Aug 15, 2014 at 10:25 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>> On Wed, Aug 13, 2014 at 12:57 AM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>>> User interface:
>>> fd = open("/sys/kernel/debug/tracing/__event__/filter")
>>>
>>> write(fd, "bpf_123")
>>
>> I didn't follow all the code flow leading to parsing the "bpf_123"
>> string, but if it works the way I imagine it does, it's a security
>> problem.  In general, write(2) should never do anything that involves
>> any security-relevant context of the caller.
>>
>> Ideally, you would look up fd 123 in the file table of whomever called
>> open.  If that's difficult to implement efficiently, then it would be
>> nice to have some check that the callers of write(2) and open(2) are
>> the same task and that exec wasn't called in between.
>>
>> This isn't a very severe security issue because you need privilege to
>> open the thing in the first place, but it would still be nice to
>> address.
>
> hmm. you need to be root to open the events anyway.
> pretty much the whole tracing for root only, since any kernel data
> structures can be printed, stored into maps and so on.
> So I don't quite follow your security concern here.
>
> Even say root opens a tracepoint and does exec() of another
> app that uploads ebpf program, gets program_fd and does
> write into tracepoint fd. The root app that did this open() is
> doing exec() on purpose. It's not like it's exec-ing something
> it doesn't know about.

As long as everyone who can debugfs/tracing/whatever has all
privileges, then this is fine.

If not, then it's a minor capability or MAC bypass.  Suppose you only
have one capability or, more realistically, limited MAC permissions.
You can still open the tracing file, pass it to an unwitting program
with elevated permission (e.g. using selinux's entrypoint mechanism),
and trick that program into writing bpf_123.

Admittedly, it's unlikely that fd 123 will be an *eBPF* fd, but the
attack is possible.

I don't think that fixing this should be a prerequisite for merging,
since the risk is so small.  Nonetheless, it would be nice.  (This
family of attacks has lead to several root vulnerabilities in the
past.)

--Andy

>
> Remember, FDs was your idea in the first place ;)
> I had global ids and everything root initially.



-- 
Andy Lutomirski
AMA Capital Management, LLC

^ permalink raw reply

* Re: [PATCH RFC v4 net-next 17/26] tracing: allow eBPF programs to be attached to events
From: Alexei Starovoitov @ 2014-08-15 18:56 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Daniel Borkmann, H. Peter Anvin, Andrew Morton, Linux API,
	Chema Gonzalez, Eric Dumazet, David S. Miller, Brendan Gregg,
	Linus Torvalds, Steven Rostedt, LKML, Peter Zijlstra, Kees Cook,
	Network Development, Ingo Molnar
In-Reply-To: <CALCETrVhjO5c7ob1vntx031c5RmxRHimkRt1F2EsmzdKB53_NA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Aug 15, 2014 at 11:50 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> On Aug 15, 2014 10:36 AM, "Alexei Starovoitov" <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>>
>> On Fri, Aug 15, 2014 at 10:20 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>> > The downside of this approach is that compat support might be
>> > difficult or impossible.
>>
>> Would do you mean by compat? 32-bit programs on 64-bit kernels?
>> There is no such concept for eBPF. All eBPF programs are always
>> operating on 64-bit registers.
>
> Doesn't the eBPF program need to know sizeof(long) to read these
> fields correctly?  Or am I misunderstanding what the code does?

correct. eBPF program would be using 8-byte read on 64-bit kernel
and 4-byte read on 32-bit kernel. Same with access to ptrace fields
and pretty much all other fields in the kernel. The program will be
different on different kernels.
Say, this bpf_context struct doesn't exist at all. The programs would
still need to be different to walk in-kernel data structures...

^ permalink raw reply

* Re: [PATCH RFC v4 net-next 17/26] tracing: allow eBPF programs to be attached to events
From: Andy Lutomirski @ 2014-08-15 19:02 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Daniel Borkmann, H. Peter Anvin, Andrew Morton, Linux API,
	Chema Gonzalez, Eric Dumazet, David S. Miller, Brendan Gregg,
	Linus Torvalds, Steven Rostedt, LKML, Peter Zijlstra, Kees Cook,
	Network Development, Ingo Molnar
In-Reply-To: <CAMEtUuzT53jeH-L+saW-RopSR2EERO5UKVHyeORTGHVMCHbYag-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Aug 15, 2014 at 11:56 AM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> On Fri, Aug 15, 2014 at 11:50 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>> On Aug 15, 2014 10:36 AM, "Alexei Starovoitov" <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>>>
>>> On Fri, Aug 15, 2014 at 10:20 AM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>>> > The downside of this approach is that compat support might be
>>> > difficult or impossible.
>>>
>>> Would do you mean by compat? 32-bit programs on 64-bit kernels?
>>> There is no such concept for eBPF. All eBPF programs are always
>>> operating on 64-bit registers.
>>
>> Doesn't the eBPF program need to know sizeof(long) to read these
>> fields correctly?  Or am I misunderstanding what the code does?
>
> correct. eBPF program would be using 8-byte read on 64-bit kernel
> and 4-byte read on 32-bit kernel. Same with access to ptrace fields
> and pretty much all other fields in the kernel. The program will be
> different on different kernels.
> Say, this bpf_context struct doesn't exist at all. The programs would
> still need to be different to walk in-kernel data structures...

Hmm.  I guess this isn't so bad.

What's the actual difficulty with using u64?  ISTM that, if the clang
front-end can't deal with u64, there's a bigger problem.  Or is it
something else I don't understand.

--Andy

^ permalink raw reply

* Re: [PATCH RFC v4 net-next 17/26] tracing: allow eBPF programs to be attached to events
From: Alexei Starovoitov @ 2014-08-15 19:07 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
	Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
	H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
	Network Development, linux-kernel@vger.kernel.org
In-Reply-To: <CALCETrUQN=vt3EUO=jNFC+sDHAiJdNyZWg4rwiEF7WKjPLi8Bg@mail.gmail.com>

On Fri, Aug 15, 2014 at 11:53 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Fri, Aug 15, 2014 at 10:51 AM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>> On Fri, Aug 15, 2014 at 10:25 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> On Wed, Aug 13, 2014 at 12:57 AM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>>>> User interface:
>>>> fd = open("/sys/kernel/debug/tracing/__event__/filter")
>>>>
>>>> write(fd, "bpf_123")
>>>
>>> I didn't follow all the code flow leading to parsing the "bpf_123"
>>> string, but if it works the way I imagine it does, it's a security
>>> problem.  In general, write(2) should never do anything that involves
>>> any security-relevant context of the caller.
>>>
>>> Ideally, you would look up fd 123 in the file table of whomever called
>>> open.  If that's difficult to implement efficiently, then it would be
>>> nice to have some check that the callers of write(2) and open(2) are
>>> the same task and that exec wasn't called in between.
>>>
>>> This isn't a very severe security issue because you need privilege to
>>> open the thing in the first place, but it would still be nice to
>>> address.
>>
>> hmm. you need to be root to open the events anyway.
>> pretty much the whole tracing for root only, since any kernel data
>> structures can be printed, stored into maps and so on.
>> So I don't quite follow your security concern here.
>>
>> Even say root opens a tracepoint and does exec() of another
>> app that uploads ebpf program, gets program_fd and does
>> write into tracepoint fd. The root app that did this open() is
>> doing exec() on purpose. It's not like it's exec-ing something
>> it doesn't know about.
>
> As long as everyone who can debugfs/tracing/whatever has all
> privileges, then this is fine.
>
> If not, then it's a minor capability or MAC bypass.  Suppose you only
> have one capability or, more realistically, limited MAC permissions.

Hard to think of MAC abbreviation other than in networking way... ;)
MAC bypass... kinda sounds like L3 networking without L2... ;)

> You can still open the tracing file, pass it to an unwitting program
> with elevated permission (e.g. using selinux's entrypoint mechanism),
> and trick that program into writing bpf_123.

hmm, but to open tracing file you'd need to be root already...
otherwise yeah, if non-root could open it and pass it, then it
would be nasty.

> Admittedly, it's unlikely that fd 123 will be an *eBPF* fd, but the
> attack is possible.
>
> I don't think that fixing this should be a prerequisite for merging,
> since the risk is so small.  Nonetheless, it would be nice.  (This
> family of attacks has lead to several root vulnerabilities in the
> past.)

Ok. I think keeping a track of pid between open and write is kinda
ugly. Should we add some new CAP flag and check it for all file
ops? Another option is to conditionally make open() of tracing
files as cloexec...

^ permalink raw reply

* Re: [PATCH RFC v4 net-next 17/26] tracing: allow eBPF programs to be attached to events
From: Alexei Starovoitov @ 2014-08-15 19:16 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Daniel Borkmann, H. Peter Anvin, Andrew Morton, Linux API,
	Chema Gonzalez, Eric Dumazet, David S. Miller, Brendan Gregg,
	Linus Torvalds, Steven Rostedt, LKML, Peter Zijlstra, Kees Cook,
	Network Development, Ingo Molnar
In-Reply-To: <CALCETrUNvg88_VrwE+9RKnSgts=3zVcXbX6tuNLgUxJU436CrQ@mail.gmail.com>

On Fri, Aug 15, 2014 at 12:02 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>
>> correct. eBPF program would be using 8-byte read on 64-bit kernel
>> and 4-byte read on 32-bit kernel. Same with access to ptrace fields
>> and pretty much all other fields in the kernel. The program will be
>> different on different kernels.
>> Say, this bpf_context struct doesn't exist at all. The programs would
>> still need to be different to walk in-kernel data structures...
>
> Hmm.  I guess this isn't so bad.
>
> What's the actual difficulty with using u64?  ISTM that, if the clang
> front-end can't deal with u64, there's a bigger problem.  Or is it
> something else I don't understand.

clang/llvm has no problem with u64 :)
This bpf_context struct for tracing is trying to answer the question:
 'what's the most convenient way to access tracepoint arguments
from a script'.
When kernel code has something like:
 trace_kfree_skb(skb, net_tx_action);
the script needs to be able to access this 'skb' and 'net_tx_action'
values through _single_ data structure.
In this proposal they are ctx->arg1 and ctx->arg2.
I've considered having different bpf_context's for every event, but
the complexity explodes. I need to hack all event definitions and so on.
imo it's better to move complexity to userspace, so program author
or high level language abstracts these details.

^ permalink raw reply

* Re: [PATCH RFC v4 net-next 17/26] tracing: allow eBPF programs to be attached to events
From: Andy Lutomirski @ 2014-08-15 19:18 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Daniel Borkmann, H. Peter Anvin, Andrew Morton, Linux API,
	Chema Gonzalez, Eric Dumazet, David S. Miller, Brendan Gregg,
	Linus Torvalds, Steven Rostedt, LKML, Peter Zijlstra, Kees Cook,
	Network Development, Ingo Molnar
In-Reply-To: <CAMEtUuwF2_+qzkaW6rkw9cyYJ2eb01B_ZyBcwrJ7nd+GqN5-mQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Aug 15, 2014 at 12:16 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> On Fri, Aug 15, 2014 at 12:02 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>>>
>>> correct. eBPF program would be using 8-byte read on 64-bit kernel
>>> and 4-byte read on 32-bit kernel. Same with access to ptrace fields
>>> and pretty much all other fields in the kernel. The program will be
>>> different on different kernels.
>>> Say, this bpf_context struct doesn't exist at all. The programs would
>>> still need to be different to walk in-kernel data structures...
>>
>> Hmm.  I guess this isn't so bad.
>>
>> What's the actual difficulty with using u64?  ISTM that, if the clang
>> front-end can't deal with u64, there's a bigger problem.  Or is it
>> something else I don't understand.
>
> clang/llvm has no problem with u64 :)
> This bpf_context struct for tracing is trying to answer the question:
>  'what's the most convenient way to access tracepoint arguments
> from a script'.
> When kernel code has something like:
>  trace_kfree_skb(skb, net_tx_action);
> the script needs to be able to access this 'skb' and 'net_tx_action'
> values through _single_ data structure.
> In this proposal they are ctx->arg1 and ctx->arg2.
> I've considered having different bpf_context's for every event, but
> the complexity explodes. I need to hack all event definitions and so on.
> imo it's better to move complexity to userspace, so program author
> or high level language abstracts these details.

I still don't understand why making them long instead of u64 is
helpful, though.  I feel like I'm missing obvious here.

-- 
Andy Lutomirski
AMA Capital Management, LLC

^ permalink raw reply

* Re: [PATCH RFC v4 net-next 17/26] tracing: allow eBPF programs to be attached to events
From: Andy Lutomirski @ 2014-08-15 19:20 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
	Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
	H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
	Network Development, linux-kernel@vger.kernel.org
In-Reply-To: <CAMEtUuyDJeEJjzsZemFLT_y_QJ+vnJjze=KOxV+7g58Tsf4cTw@mail.gmail.com>

On Fri, Aug 15, 2014 at 12:07 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> On Fri, Aug 15, 2014 at 11:53 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Fri, Aug 15, 2014 at 10:51 AM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>>> On Fri, Aug 15, 2014 at 10:25 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>> On Wed, Aug 13, 2014 at 12:57 AM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>>>>> User interface:
>>>>> fd = open("/sys/kernel/debug/tracing/__event__/filter")
>>>>>
>>>>> write(fd, "bpf_123")
>>>>
>>>> I didn't follow all the code flow leading to parsing the "bpf_123"
>>>> string, but if it works the way I imagine it does, it's a security
>>>> problem.  In general, write(2) should never do anything that involves
>>>> any security-relevant context of the caller.
>>>>
>>>> Ideally, you would look up fd 123 in the file table of whomever called
>>>> open.  If that's difficult to implement efficiently, then it would be
>>>> nice to have some check that the callers of write(2) and open(2) are
>>>> the same task and that exec wasn't called in between.
>>>>
>>>> This isn't a very severe security issue because you need privilege to
>>>> open the thing in the first place, but it would still be nice to
>>>> address.
>>>
>>> hmm. you need to be root to open the events anyway.
>>> pretty much the whole tracing for root only, since any kernel data
>>> structures can be printed, stored into maps and so on.
>>> So I don't quite follow your security concern here.
>>>
>>> Even say root opens a tracepoint and does exec() of another
>>> app that uploads ebpf program, gets program_fd and does
>>> write into tracepoint fd. The root app that did this open() is
>>> doing exec() on purpose. It's not like it's exec-ing something
>>> it doesn't know about.
>>
>> As long as everyone who can debugfs/tracing/whatever has all
>> privileges, then this is fine.
>>
>> If not, then it's a minor capability or MAC bypass.  Suppose you only
>> have one capability or, more realistically, limited MAC permissions.
>
> Hard to think of MAC abbreviation other than in networking way... ;)
> MAC bypass... kinda sounds like L3 networking without L2... ;)
>
>> You can still open the tracing file, pass it to an unwitting program
>> with elevated permission (e.g. using selinux's entrypoint mechanism),
>> and trick that program into writing bpf_123.
>
> hmm, but to open tracing file you'd need to be root already...
> otherwise yeah, if non-root could open it and pass it, then it
> would be nasty.
>
>> Admittedly, it's unlikely that fd 123 will be an *eBPF* fd, but the
>> attack is possible.
>>
>> I don't think that fixing this should be a prerequisite for merging,
>> since the risk is so small.  Nonetheless, it would be nice.  (This
>> family of attacks has lead to several root vulnerabilities in the
>> past.)
>
> Ok. I think keeping a track of pid between open and write is kinda
> ugly.

Agreed.

TBH, I would just add a comment to the open implementation saying
that, if unprivileged or less privileged open is allowed, then this
needs to be fixed.

> Should we add some new CAP flag and check it for all file
> ops? Another option is to conditionally make open() of tracing
> files as cloexec...

That won't help.  The same attack can be done with SCM_RIGHTS, and
cloexec can be cleared.

-- 
Andy Lutomirski
AMA Capital Management, LLC

^ permalink raw reply

* Re: [PATCH RFC v4 net-next 17/26] tracing: allow eBPF programs to be attached to events
From: Alexei Starovoitov @ 2014-08-15 19:29 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
	Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
	H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
	Network Development,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CALCETrW4Yscrte9=_ks_1BhSE9FTe-KZTv_a=g5wrwKhKkiuow-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Aug 15, 2014 at 12:20 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>>>
>>> I don't think that fixing this should be a prerequisite for merging,
>>> since the risk is so small.  Nonetheless, it would be nice.  (This
>>> family of attacks has lead to several root vulnerabilities in the
>>> past.)
>>
>> Ok. I think keeping a track of pid between open and write is kinda
>> ugly.
>
> Agreed.
>
> TBH, I would just add a comment to the open implementation saying
> that, if unprivileged or less privileged open is allowed, then this
> needs to be fixed.

ok. will do.

>> Should we add some new CAP flag and check it for all file
>> ops? Another option is to conditionally make open() of tracing
>> files as cloexec...
>
> That won't help.  The same attack can be done with SCM_RIGHTS, and
> cloexec can be cleared.

ouch, can we then make ebpf FDs and may be debugfs FDs
not passable at all? Otherwise it feels that generality and
flexibility of FDs is becoming a burden.

^ permalink raw reply

* Re: [PATCH RFC v4 net-next 17/26] tracing: allow eBPF programs to be attached to events
From: Andy Lutomirski @ 2014-08-15 19:32 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Steven Rostedt,
	Daniel Borkmann, Chema Gonzalez, Eric Dumazet, Peter Zijlstra,
	H. Peter Anvin, Andrew Morton, Kees Cook, Linux API,
	Network Development,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAMEtUuzDxzPHsch24U_NjX23r6BvmK9b723HHJeNwQOJeA8r1A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Aug 15, 2014 at 12:29 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> On Fri, Aug 15, 2014 at 12:20 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>>>>
>>>> I don't think that fixing this should be a prerequisite for merging,
>>>> since the risk is so small.  Nonetheless, it would be nice.  (This
>>>> family of attacks has lead to several root vulnerabilities in the
>>>> past.)
>>>
>>> Ok. I think keeping a track of pid between open and write is kinda
>>> ugly.
>>
>> Agreed.
>>
>> TBH, I would just add a comment to the open implementation saying
>> that, if unprivileged or less privileged open is allowed, then this
>> needs to be fixed.
>
> ok. will do.
>
>>> Should we add some new CAP flag and check it for all file
>>> ops? Another option is to conditionally make open() of tracing
>>> files as cloexec...
>>
>> That won't help.  The same attack can be done with SCM_RIGHTS, and
>> cloexec can be cleared.
>
> ouch, can we then make ebpf FDs and may be debugfs FDs
> not passable at all? Otherwise it feels that generality and
> flexibility of FDs is becoming a burden.

I'm not sure there's much of a general problem.  The issue is when
there's an fd for which write(2) (or other
assumed-to-not-check-permissions calls like read, pread, pwrite, etc)
depend on context.  This is historically an issue for netlink and
various /proc files.

--Andy

^ permalink raw reply

* Re: [PATCH RFC v4 net-next 17/26] tracing: allow eBPF programs to be attached to events
From: Alexei Starovoitov @ 2014-08-15 19:35 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Daniel Borkmann, H. Peter Anvin, Andrew Morton, Linux API,
	Chema Gonzalez, Eric Dumazet, David S. Miller, Brendan Gregg,
	Linus Torvalds, Steven Rostedt, LKML, Peter Zijlstra, Kees Cook,
	Network Development, Ingo Molnar
In-Reply-To: <CALCETrUqop+UB-BhyX4Y41kELO+6kcFdS1F7ZyN0CzRwg4UGhA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Aug 15, 2014 at 12:18 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>>
>> clang/llvm has no problem with u64 :)
>> This bpf_context struct for tracing is trying to answer the question:
>>  'what's the most convenient way to access tracepoint arguments
>> from a script'.
>> When kernel code has something like:
>>  trace_kfree_skb(skb, net_tx_action);
>> the script needs to be able to access this 'skb' and 'net_tx_action'
>> values through _single_ data structure.
>> In this proposal they are ctx->arg1 and ctx->arg2.
>> I've considered having different bpf_context's for every event, but
>> the complexity explodes. I need to hack all event definitions and so on.
>> imo it's better to move complexity to userspace, so program author
>> or high level language abstracts these details.
>
> I still don't understand why making them long instead of u64 is
> helpful, though.  I feel like I'm missing obvious here.

I promise to come back to this... Have to go off grid...
will think of it in the mean time... Appreciate this discussion!!

^ permalink raw reply

* Re: [RFC PATCH 00/11] Adding FreeBSD's Capsicum security framework (part 1)
From: Pavel Machek @ 2014-08-16 15:41 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Paolo Bonzini, David Drysdale, LSM List,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Greg Kroah-Hartman, Alexander Viro, Meredydd Luff, Kees Cook,
	James Morris, Linux API, qemu-devel
In-Reply-To: <CAADnVQ+c2E6eG_juEDh-GyheveqScxQ=98jqO1ZOjp1PgfVBGQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi!

> >>> I think that's more easily done by opening the file as O_RDONLY/O_WRONLY
> >>> /O_RDWR.   You could do it by running the file descriptor's seccomp-bpf
> >>> program once per iocb with synthesized syscall numbers and argument
> >>> vectors.
> >>
> >>
> >> Right, but generating the equivalent seccomp input environment for an
> >> equivalent single-fd syscall is going to be subtle and complex (which
> >> are worrying words to mention in a security context).  And how many
> >> other syscalls are going to need similar special-case processing?
> >> (poll? select? send[m]msg? ...)
> >
> >
> > Yeah, the difficult part is getting the right balance between:
> >
> > 1) limitations due to seccomp's impossibility to chase pointers (which is
> > not something that can be lifted, as it's required for correctness)
> 
> btw once seccomp moves to eBPF it will be able to 'chase pointers',
> since pointer walking will be possible via bpf_load_pointer() function call,
> which is a wrapper of:

Even if you could make capscium work with eBPF... please don't.

Capscium is kind of obvious, elegant solution. BPF is quite
complex. And security semantics should not be pushed to userspace...

						Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: Does anyone know when FUTEX_WAIT can fail with EAGAIN?
From: Eric Wong @ 2014-08-16 21:51 UTC (permalink / raw)
  To: Steven Stewart-Gallus; +Cc: linux-kernel, linux-api, mtk
In-Reply-To: <fb82d1d46d8f.53d8186c@langara.bc.ca>

Steven Stewart-Gallus <sstewartgallus00@mylangara.bc.ca> wrote:
> I'm trying to debug a hangup where my program loops with FUTEX_WAIT (actually
> FUTEX_WAIT_PRIVATE but same thing)  endlessly erring out with EAGAIN. I would
> like to know if anyone on the mailing list knows when FUTEX_WAIT can fail with
> EAGAIN.

FUTEX_WAIT fails with EAGAIN if the value of *uaddr no longer matches
the expected val.
---
#include <linux/futex.h>
#include <sys/syscall.h>
#include <unistd.h>
#include <errno.h>
int main(void) {
	int op = FUTEX_WAIT;
	int exp = 1;
	int *addr = &exp;
	int val = 1; /* XXX change this to anything else to get EAGAIN */
	int rc = syscall(SYS_futex, addr, op, val, 0, 0, 0);

	if (rc < 0 && errno == EAGAIN)
		write(1, "EAGAIN\n", 7);

	return 0;
}
---
I just encountered this myself yesterday while implementing futex-based
locks/condvars for Ruby:
https://bugs.ruby-lang.org/issues/10009#change-48372

^ permalink raw reply

* Re: Does anyone know when FUTEX_WAIT can fail with EAGAIN?
From: Davidlohr Bueso @ 2014-08-17 15:48 UTC (permalink / raw)
  To: Steven Stewart-Gallus
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, mtk-ASgREoAs3yw,
	davidlohr-VXdhtT5mjnY
In-Reply-To: <fb82d1d46d8f.53d8186c-BTv7Ps/Sm75C8prJL3GQQw@public.gmane.org>

On Tue, 2014-07-29 at 21:55 +0000, Steven Stewart-Gallus wrote:
> Hello,
> 
> I'm trying to debug a hangup where my program loops with FUTEX_WAIT (actually
> FUTEX_WAIT_PRIVATE but same thing)  endlessly erring out with EAGAIN. I would
> like to know if anyone on the mailing list knows when FUTEX_WAIT can fail with
> EAGAIN.

You're probably running into the EWOULDBLOCK return -- which on Linux is
just another name for EAGAIN. So if you didn't know this, and were
reading the code related to FUTEX_WAIT, you'd get mislead by this.

The check is there to avoid userspace misusing futexes and screwing up
applications. When the futex_wait() call is issued, a lot can happen in
the kernel before the task is blocked (key hashing, reference counting,
acquire internal spinlock, etc.). At any point while preparing to block,
another userspace thread could have changed the futex value, and thus
the condition for which futex_wait() call was issued in the first place
might no longer be true.

Thanks,
Davidlohr

^ permalink raw reply

* Re: [PATCH v14 5/8] s390: add pmd_[dirty|mkclean] for THP
From: Minchan Kim @ 2014-08-18  0:06 UTC (permalink / raw)
  To: Martin Schwidefsky
  Cc: Andrew Morton, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Michael Kerrisk, Linux API,
	Hugh Dickins, Johannes Weiner, Rik van Riel, KOSAKI Motohiro,
	Mel Gorman, Jason Evans, Zhang Yanfei, Kirill A. Shutemov,
	Heiko Carstens, Dominik Dingel, Christian Borntraeger,
	linux-s390-u79uwXL29TY76Z2rM5mHXA, Gerald Schaefer
In-Reply-To: <20140814091614.4a0d5178@mschwide>

Hello,

On Thu, Aug 14, 2014 at 09:16:14AM +0200, Martin Schwidefsky wrote:
> On Thu, 14 Aug 2014 10:53:29 +0900
> Minchan Kim <minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> 
> > MADV_FREE needs pmd_dirty and pmd_mkclean for detecting recent
> > overwrite of the contents since MADV_FREE syscall is called for
> > THP page but for s390 pmds only referenced bit is available
> > because there is no free bit left in the pmd entry for the
> > software dirty bit so this patch adds dumb pmd_dirty which
> > returns always true by suggesting by Martin.
> > 
> > They finally find a solution in future.
> > http://marc.info/?l=linux-api&m=140440328820808&w=2
> 
> The solution is already there, see git commit 152125b7a882df36.
> You can drop this patch.

Thanks for the heads up. I will drop it in next spin.
> 
> -- 
> blue skies,
>    Martin.
> 
> "Reality continues to ruin my life." - Calvin.
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo-Bw31MaZKKs0EbZ0PF+XxCw@public.gmane.org  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org"> email-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org </a>

-- 
Kind regards,
Minchan Kim

^ permalink raw reply

* [PATCH v15 0/7] MADV_FREE support
From: Minchan Kim @ 2014-08-18  0:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Michael Kerrisk,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Hugh Dickins, Johannes Weiner,
	Rik van Riel, KOSAKI Motohiro, Mel Gorman, Jason Evans,
	zhangyanfei-BthXqXjhjHXQFUHtdCDX3A, Kirill A. Shutemov,
	Minchan Kim

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

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

* from v14
 * Add more Ackedy-by from arch people(sparc, arm64 and arm)
 * Drop s390 since pmd_dirty/clean was merged

* from v13
 * Add more Ackedy-by from arch people(arm, arm64 and ppc)
 * Rebased on mmotm 2014-08-13-14-29

* from v12
 * Fix - skip to mark free pte on try_to_free_swap failed page - Kirill
 * Add more Acked-by from arch maintainers and Kirill

* From v11
 * Fix arm build - Steve
 * Separate patch for arm and arm64 - Steve
 * Remove unnecessary check - Kirill
 * Skip non-vm_normal page - Kirill
 * Add Acked-by - Zhang
 * Sparc64 build fix
 * Pagetable walker THP handling fix

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

* From v9
 * Add Acked-by - Rik
 * Add THP page support - Kirill

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

* From v7
 * Rebased-on next-20140613

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

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

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

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

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

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

Minchan Kim (7):
  mm: support madvise(MADV_FREE)
  x86: add pmd_[dirty|mkclean] for THP
  sparc: add pmd_[dirty|mkclean] for THP
  powerpc: add pmd_[dirty|mkclean] for THP
  arm: add pmd_mkclean for THP
  arm64: add pmd_[dirty|mkclean] for THP
  mm: Don't split THP page when syscall is called

 arch/arm/include/asm/pgtable-3level.h    |   1 +
 arch/arm64/include/asm/pgtable.h         |   2 +
 arch/powerpc/include/asm/pgtable-ppc64.h |   2 +
 arch/sparc/include/asm/pgtable_64.h      |  16 ++++
 arch/x86/include/asm/pgtable.h           |  10 ++
 include/linux/huge_mm.h                  |   4 +
 include/linux/rmap.h                     |   9 +-
 include/linux/vm_event_item.h            |   1 +
 include/uapi/asm-generic/mman-common.h   |   1 +
 mm/huge_memory.c                         |  35 +++++++
 mm/madvise.c                             | 159 +++++++++++++++++++++++++++++++
 mm/rmap.c                                |  46 ++++++++-
 mm/vmscan.c                              |  64 +++++++++----
 mm/vmstat.c                              |   1 +
 14 files changed, 331 insertions(+), 20 deletions(-)

-- 
2.0.0

^ permalink raw reply

* [PATCH v15 1/7] mm: support madvise(MADV_FREE)
From: Minchan Kim @ 2014-08-18  0:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, linux-mm, Michael Kerrisk, linux-api, Hugh Dickins,
	Johannes Weiner, Rik van Riel, KOSAKI Motohiro, Mel Gorman,
	Jason Evans, zhangyanfei, Kirill A. Shutemov, Minchan Kim,
	Kirill A. Shutemov
In-Reply-To: <1408321076-2231-1-git-send-email-minchan@kernel.org>

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

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

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

How to work is following as.

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

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

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

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

 vanilla-jemalloc		MADV_free-jemalloc

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

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

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

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

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

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

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

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

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

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

^ permalink raw reply related

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

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

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

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

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

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

^ permalink raw reply related


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