[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJwJo6Zfa0cNubXmwWsyNbUq2Qw1MTH080uacAL7eLb13xH36A@mail.gmail.com>
Date: Fri, 26 Aug 2016 14:16:28 +0300
From: Dmitry Safonov <0x7f454c46@...il.com>
To: "H. Peter Anvin" <hpa@...or.com>
Cc: Dmitry Safonov <dsafonov@...tuozzo.com>,
linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>,
Andy Lutomirski <luto@...capital.net>,
Thomas Gleixner <tglx@...utronix.de>, X86 ML <x86@...nel.org>,
Oleg Nesterov <oleg@...hat.com>,
Steven Rostedt <rostedt@...dmis.org>, viro@...iv.linux.org.uk
Subject: Re: [RFC 0/3] Put vdso in ramfs-like filesystem (vdsofs)
2016-08-26 2:00 GMT+03:00 H. Peter Anvin <hpa@...or.com>:
> On August 25, 2016 3:53:43 PM PDT, Dmitry Safonov <0x7f454c46@...il.com> wrote:
>>2016-08-25 23:49 GMT+03:00 H. Peter Anvin <hpa@...or.com>:
>>> On August 25, 2016 8:21:07 AM PDT, Dmitry Safonov
>><dsafonov@...tuozzo.com> wrote:
>>>>This patches set is cleanly RFC and is not supposed to be applied.
>>>>Also for RFC time it builds only on x86_64.
>>>>
>>>>So, in a mail thread Oleg told that it would be worth to introduce
>>>>vm_file
>>>>for vdso mappings as currently uprobes can not be placed on vDSO VMAs
>>>>[1].
>>>>In this patches set I introduce in-kernel filesystem for vdso files.
>>>>After patches vDSO VMA now has inode and is just a private file
>>>>mapping:
>>>>7ffcc4b2b000-7ffcc4b2d000 r--p 00000000 00:00 0
>>>> [vvar]
>>>>7ffcc4b2d000-7ffcc4b2f000 r-xp 00000000 00:09 18
>>>> [vdso]
>>>>
>>>>Then I introduce interface in uprobe_events to insert uprobes in
>>vdso.
>>>>FWIW:
>>>> [~]# cd kernel/linux
>>>> [linux]# readelf --syms arch/x86/entry/vdso/vdso64.so
>>>>Symbol table '.dynsym' contains 11 entries:
>>>> Num: Value Size Type Bind Vis Ndx Name
>>>> 0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
>>>> 1: 0000000000000470 0 SECTION LOCAL DEFAULT 8
>>>>2: 00000000000008d0 885 FUNC WEAK DEFAULT 12
>>>>clock_gettime@@LINUX_2.6
>>>>3: 0000000000000c50 472 FUNC GLOBAL DEFAULT 12
>>>>__vdso_gettimeofday@@LINUX_2.6
>>>>4: 0000000000000c50 472 FUNC WEAK DEFAULT 12
>>>>gettimeofday@@LINUX_2.6
>>>>5: 0000000000000e30 21 FUNC GLOBAL DEFAULT 12
>>>>__vdso_time@@LINUX_2.6
>>>> 6: 0000000000000e30 21 FUNC WEAK DEFAULT 12
>>time@@LINUX_2.6
>>>>7: 00000000000008d0 885 FUNC GLOBAL DEFAULT 12
>>>>__vdso_clock_gettime@@LINUX_2.6
>>>> 8: 0000000000000000 0 OBJECT GLOBAL DEFAULT ABS LINUX_2.6
>>>>9: 0000000000000e50 41 FUNC GLOBAL DEFAULT 12
>>>>__vdso_getcpu@@LINUX_2.6
>>>>10: 0000000000000e50 41 FUNC WEAK DEFAULT 12
>>>>getcpu@@LINUX_2.6
>>>> [~]# cd /sys/kernel/debug/tracing/
>>>> [tracing]# echo 'p:clock_gettime :vdso:/64:0x8d0' > uprobe_events
>>>> [tracing]# echo 'p:gettimeofday :vdso:/64:0xc50' >> uprobe_events
>>>> [tracing]# echo 'p:time :vdso:/64:0xe30' >> uprobe_events
>>>> [tracing]# echo 1 > events/uprobes/enable
>>>> [tracing]# su test # it has UID=1001
>>>> [tracing]$ date
>>>> Thu Aug 25 17:19:29 MSK 2016
>>>> [tracing]$ exit
>>>> [tracing]# cat trace
>>>> # tracer: nop
>>>> #
>>>> # entries-in-buffer/entries-written: 175/175 #P:4
>>>> #
>>>> # _-----=> irqs-off
>>>> # / _----=> need-resched
>>>> # | / _---=> hardirq/softirq
>>>> # || / _--=> preempt-depth
>>>> # ||| / delay
>>>> # TASK-PID CPU# |||| TIMESTAMP FUNCTION
>>>> # | | | |||| | |
>>>> bash-11560 [001] d... 316.470236: time:
>>(0x7ffcacebae30)
>>>> bash-11560 [001] d... 316.471436: gettimeofday:
>>(0x7ffcacebac50)
>>>> bash-11560 [001] d... 316.477550: time:
>>(0x7ffcacebae30)
>>>> bash-11560 [001] d... 316.477655: time:
>>(0x7ffcacebae30)
>>>> mktemp-11568 [001] d... 316.479589: gettimeofday:
>>(0x7ffc603f0c50)
>>>> date-11571 [001] d... 316.481890: clock_gettime:
>>(0x7ffec9db58d0)
>>>>[...]
>>>>
>>>>If this approach will be decided as fine, I will prepare a better
>>>>version,
>>>>fixing the following things:
>>>>o put vdsofs in generic fs/* dir
>>>>o support other archs and vdso blobs
>>>>o remove BUG_ON()'s and UID==1001 check
>>>>o remove extern's and use headers only
>>>>o refactor code in create_trace_uprobe()
>>>>o add some state to (struct trace_uprobe), so i.e., `cat
>>uprobe_events`
>>>>will
>>>> print those uprobes as vdso-based
>>>>o document this interface in Documentation/trace/uprobetracer.txt
>>>>o prepare nice patches set?
>>>>
>>>>So, opinions? Is it worth to add something like this?
>>>>
>>>>[1]: https://lkml.org/lkml/2016/7/12/346
>>>>
>>>>Dmitry Safonov (3):
>>>> x86/vdso: create vdso file, use it for mapping
>>>> uprobe: drop isdigit() check in create_trace_uprobe
>>>> uprobe: add vdso support
>>>>
>>>>Cc: Oleg Nesterov <oleg@...hat.com>
>>>>Cc: Al Viro <viro@...iv.linux.org.uk>
>>>>Cc: Steven Rostedt <rostedt@...dmis.org>
>>>>Cc: Andy Lutomirski <luto@...capital.net>
>>>>Cc: Thomas Gleixner <tglx@...utronix.de>
>>>>Cc: Ingo Molnar <mingo@...hat.com>
>>>>Cc: "H. Peter Anvin" <hpa@...or.com>
>>>>Cc: x86@...nel.org
>>>>Cc: Dmitry Safonov <0x7f454c46@...il.com>
>>>>
>>>>arch/x86/entry/vdso/vma.c | 148
>>>>++++++++++++++++++++++++++++++++++++++++++--
>>>> kernel/trace/trace_uprobe.c | 50 +++++++++++----
>>>> 2 files changed, 180 insertions(+), 18 deletions(-)
>>>
>>> I think there is a lot to be said for this idea. However, a private
>>mapping is definitely wrong for the vvar data; for the vdso code it
>>could be considered either way I suppose.
>>
>>Thanks on your reply.
>>As you could see, I preserved pure mapping of pfn for vvar:
>>7ffcc4b2b000-7ffcc4b2d000 r--p 00000000 00:00 0
>> [vvar]
>>7ffcc4b2d000-7ffcc4b2f000 r-xp 00000000 00:09 18
>> [vdso]
>>(no inode number).
>>I also think it would be useless to do the same to vvar as it
>>has just data and there is no point in probing it.
>
> Well, it would things like mremap() just work and so on. Let's get rid of special cases if we are.
Well, for RFC it wouldn't move context.vdso pointer on mremap(),
but as RFC is for x86_64 only, it will work on it.
Anyway, I don't think it would be hard to fix and make mremap() work on
other archs on post-RFC.
The only corner-case I see for now is that /proc/self/map_files/<vdso_range>
will point to [vdso] which is broken link. But one could read this file
and dump/read vdso blob.
So, in the other words: if some program assumes that /proc/self/map_files/*
should always point to correct file, it may be confused. Not sure, maybe
it would be confused by orphane-file mappings, so having dangling link
there is just fine.
--
Dmitry
Powered by blists - more mailing lists