Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [PATCH 13/18] io_uring: add file set registration
From: Al Viro @ 2019-02-07  4:00 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Jann Horn, linux-aio, linux-block, Linux API, hch, jmoyer, avi,
	linux-fsdevel
In-Reply-To: <8f124de2-d6da-d656-25e4-b4d9e58f880e@kernel.dk>

On Wed, Feb 06, 2019 at 06:41:00AM -0700, Jens Axboe wrote:
> On 2/5/19 5:56 PM, Al Viro wrote:
> > On Tue, Feb 05, 2019 at 12:08:25PM -0700, Jens Axboe wrote:
> >> Proof is in the pudding, here's the main commit introducing io_uring
> >> and now wiring it up to the AF_UNIX garbage collection:
> >>
> >> http://git.kernel.dk/cgit/linux-block/commit/?h=io_uring&id=158e6f42b67d0abe9ee84886b96ca8c4b3d3dfd5
> >>
> >> How does that look?
> > 
> > In a word - wrong.  Some theory: garbage collector assumes that there is
> > a subset of file references such that
> > 	* for all files with such references there's an associated unix_sock.
> > 	* all such references are stored in SCM_RIGHTS datagrams that can be
> > found by the garbage collector (currently: for data-bearing AF_UNIX sockets -
> > queued SCM_RIGHTS datagrams, for listeners - SCM_RIGHTS datagrams sent via
> > yet-to-be-accepted connections).
> > 	* there is an efficient way to count those references for given file
> > (->inflight of the corresponding unix_sock).
> > 	* removal of those references would render the graph acyclic.
> > 	* file can _NOT_ be subject to syscalls unless there are references
> > to it outside of that subset.
> 
> IOW, we cannot use fget() for registering files, and we still need fget/fput
> in the fast path to retain safe use of the file. If I'm understanding you
> correctly?

No.  *ALL* references (inflight and not) are the same for file->f_count.
unix_inflight() does not grab a new reference to file; it only says that
reference passed to it by the caller is now an in-flight one.

OK, braindump time:

Lifetime for struct file is controlled by a simple refcount.  Destructor
(__fput() and ->release() called from it) is called once the counter hits
zero.  Which is fine, except for the situations when some struct file
references are pinned down in structures reachable only via our struct file
instance.

Each file descriptor counts as a reference.  IOW, dup() will increment
the refcount by 1, close() will decrement it, fork() will increment it
by the number of descriptors in your descriptor table refering to this
struct file, desctruction of descriptor table on exit() will decrement
by the same amount, etc.

Syscalls like read() and friends turn descriptor(s) into struct file
references.  If descriptor table is shared, that counts as a new reference
that must be dropped in the end of syscall.  If it's not shared, we are
guaranteed that the reference in descriptor table will stay around until
the end of syscall, so we may use it without bumping the file refcount.
That's the difference between fget() and fdget() - the former will
bump the refcount, the latter will try to avoid that.  Of course, if
we do not intend to drop the reference we'd acquired by the end of
syscall, we want fget() - fdget() is for transient references only.

Descriptor tables (struct files) *can* be shared; several processes
(usually - threads that share VM as well, but that's not necessary)
may be working with the same instance of struct files, so e.g. open()
in one of them is seen by the others.  The same goes for close(),
dup(), dup2(), etc.

That makes for an interesting corner case - what if two threads happen
to share a descriptor table and we have close(fd) in one of them
in the middle of read(fd, ..., ...) in another?  That's one aread where
Unices differ - one variant is to abort read(), another - to have close()
wait for read() to finish, etc.  What we do is
	* close() succeeds immediately; the reference is removed from
descriptor table and dropped.
	* if close(fd) has happened before read(fd, ...) has converted
fd to struce file reference, read() will get -EBADF.
	* otherwise, read() proceeds unmolested; the reference it has
acquired is dropped in the end of syscall.  If that's the last reference
to struct file, struct file will get shut down at that point.

clone(2) will have the child sharing descriptor table of parent if
CLONE_FILES is in the flags.  Note that in this case struct file
refcounts are not modified at all - no new references to files are
created.  Without CLONE_FILES it's the same as fork() - an independent
copy of descriptor table is created and populated by copies of references
to files, each bumping file's refcount.

unshare(2) with CLONE_FILES in flags will get a copy of descriptor table
(same as done on fork(), etc.) and switch to using it; the old reference
is dropped (note: it'll only bother with that if descriptor table used
to be shared in the first place - if we hold the only reference to
descriptor table, we'll just keep using it).

execve(2) does almost the same - if descriptor table used to be shared,
it will switch to a new copy first; in case of success the reference
to original is dropped, in case of failure we revert to original and
drop the copy.  Note that handling of close-on-exec is done in the _copy_
- the original is unaffected, so failing execve() does not disrupt the
descriptor table.

exit(2) will drop the reference to descriptor table.  When the last
reference is dropped, all file references are removed from it (and dropped).

The thread's pointer to descriptor table (current->files) is never
modified by other thread; something like ls /proc/<pid>/fd will fetch
it, so stores need to be protected (by task_lock(current)), but
the only the thread itself can do them.  

Note that while extra references to descriptor table can appear at any
time (/proc/<pid>/fd accesses, for example), such references may not
be used for modifications.  In particular, you can't switch to another
thread's descriptor table, unless it had been yours at some earlier
point _and_ you've kept a reference to it.

That's about it for descriptor tables; that, by far, is the main case
of persistently held struct file references.  Transient references
are grabbed by syscalls when they resolve descriptor to struct file *,
which ought to be done once per syscall _and_ reasonably early in
it.  Unfortunately, that's not all - there are other persistent struct
file references.

The things like "LOOP_SET_FD grabs a reference to struct file and
stashes it in ->lo_backing_file" are reasonably simple - the reference
will be dropped later, either directly by LOOP_CLR_FD (if nothing else
held the damn thing open at the time) or later in lo_release().
Note that in the latter case it's possible to get "close() of
/dev/loop descriptor drops the last reference to it, triggering
bdput(), which happens to by the last thing that held block device
opened, which triggers lo_release(), which drops the reference to
underlying struct file (almost certainly the last one by that point)".
It's still not a problem - while we have the underlying struct file
pinned by something held by another struct file, the dependencies'
graph is acyclic, so plain refcounts we are using work fine.

The same goes for the things like e.g. ecryptfs opening an underlying
(encrypted) file on open() and dropping it when the last reference
to ecryptfs file is dropped - the only difference here is that the
underlying struct file is never appearing in _anyone's_ descriptor
tables.

However, in a couple of cases we do have something trickier.

Case 1: SCM_RIGHTS datagram can be sent to an AF_UNIX socket.  That
converts the caller-supplied array of descriptors into an array of
struct file references, which gets attached to the packet we queue.
When the datagram is received, the struct file references are
moved into the descriptor table of recepient or, in case of error,
dropped.  Note that sending some descriptors in an SCM_RIGHTS datagram
and closing them is perfectly legitimate - as soon as sendmsg(2)
returns you can go ahead and close the descriptors you've sent;
the references are already acquired and you don't need to wait for
the packet to be received.

That would still be simple, if not for the fact that there's nothing
to stop you from passing AF_UNIX sockets around the same way.  In fact,
that has legitimate uses and, most of the time, doesn't cause any
complications at all.  However, it is possible to get the situation
when
	* struct file instances A and B are both AF_UNIX sockets.
	* the only reference to A is in the SCM_RIGHTS packet that
sits in the receiving queue of B.
	* the only reference to B is in the SCM_RIGHTS packet that
sits in the receiving queue of A.
That, of course, is where the pure refcounting of any kind will break.

SCM_RIGHTS datagram that contains the sole reference to A can't be
received without the recepient getting hold of a reference to B.
Which cannot happen until somebody manages to receive the SCM_RIGHTS
datagram containing the sole reference to B.  Which cannot happen
until that somebody manages to get hold of a reference to A,
which cannot happen until the first SCM_RIGHTS datagram is
received.

Dropping the last reference to A would've discarded everything in
its receiving queue, including the SCM_RIGHTS that contains the
reference to B; however, that can't happen either - the other
SCM_RIGHTS datagram would have to be either received or discarded
first, etc.

Case 2: similar, with a bit of a twist.  AF_UNIX socket used for
descriptor passing is normally set up by socket(), followed by
connect().  As soon as connect() returns, one can start sending.
Note that connect() does *NOT* wait for the recepient to call
accept() - it creates the object that will serve as low-level
part of the other end of connection (complete with received
packet queue) and stashes that object into the queue of *listener*
socket.  Subsequent accept() fetches it from there and attaches
it to a new socket, completing the setup; in the meanwhile,
sending packets works fine.  Once accept() is done, it'll see
the stuff you'd sent already in the queue of the new socket and
everything works fine.

If the listening socket gets closed without accept() having
been called, its queue is flushed, discarding all pending
connection attempts, complete with _their_ queues.  Which is
the same effect as accept() + close(), so again, normally
everything just works.

However, consider the case when we have
	* struct file instances A and B being AF_UNIX sockets.
	* A is a listener
	* B is an established connection, with the other end
yet to be accepted on A
	* the only references to A and B are in an SCM_RIGHTS
datagram sent over by A.

That SCM_RIGHTS datagram could've been received, if somebody
had managed to call accept(2) on A and recvmsg(2) on the
socket created by that accept(2).  But that can't happen
without that somebody getting hold of a reference to A in
the first place, which can't happen without having received
that SCM_RIGHTS datagram.  It can't be discarded either,
since that can't happen without dropping the last reference
to A, which sits right in it.

The difference from the previous case is that there we had
	A holds unix_sock of A
	unix_sock of A holds SCM_RIGHTS with reference to B
	B holds unix_sock of B
	unix_sock of B holds SCM_RIGHTS with reference to A
and here we have
	A holds unix_sock of A
	unix_sock of A holds the packet with reference to embryonic
unix_sock created by connect()
	that embryionic unix_sock holds SCM_RIGHTS with references
to A and B.

Dependency graph is different, but the problem is the same -
unreachable loops in it.  Note that neither class of situations
would occur normally - in the best case it's "somebody had been
doing rather convoluted descriptor passing, but everyone involved
got hit with kill -9 at the wrong time; please, make sure nothing
leaks".  That can happen, but a userland race (e.g. botched protocol
handling of some sort) or a deliberate abuse are much more likely.

Catching the loop creation is hard and paying for that every time
we do descriptor-passing would be a bad idea.  Besides, the loop
per se is not fatal - e.g if in the second case the descriptor for
A had been kept around, close(accept()) would've cleaned everything
up.  Which means that we need a garbage collector to deal with
the (rare) leaks.

Note that in both cases the leaks are caused by loops passing through
some SCM_RIGHTS datagrams that could never be received.  So locating
those, removing them from the queues they sit in and then discarding
the suckers is enough to resolve the situation.

Furthermore, in both cases the loop passes through unix_sock of
something that got sent over in an SCM_RIGHTS datagram.  So we
can do the following:
	1) keep the count of references to struct file of AF_UNIX
socket held by SCM_RIGHTS (kept in unix_sock->inflight).  Any
struct unix_sock instance without such references is not a part of
unreachable loop.  Maintain the set of unix_sock that are not excluded
by that (i.e. the ones that have some of references from SCM_RIGHTS
instances).  Note that we don't need to maintain those counts in
struct file - we care only about unix_sock here.
	2) struct file of AF_UNIX socket with some references
*NOT* from SCM_RIGHTS is also not a part of unreachable loop.
	3) for each unix_sock consider the following set of
SCM_RIGHTS: everything in queue of that unix_sock if it's
a non-listener and everything in queues of all embryonic unix_sock
in queue of a listener.  Let's call those SCM_RIGHTS associated
with our unix_sock.
	4) all SCM_RIGHTS associated with a reachable unix_sock
are reachable.
	5) if some references to struct file of a unix_sock
are in reachable SCM_RIGHTS, it is reachable.

Garbage collector starts with calculating the set of potentially
unreachable unix_sock - the ones not excluded by (1,2).
No unix_sock instances outside of that set need to be considered.

If some unix_sock in that set has counter _not_ entirely covered
by SCM_RIGHTS associated with the elements of the set, we can
conclude that there are references to it in SCM_RIGHTS associated
with something outside of our set and therefore it is reachable
and can be removed from the set.

If that process converges to a non-empty set, we know that
everything left in that set is unreachable - all references
to their struct file come from _some_ SCM_RIGHTS datagrams
and all those SCM_RIGHTS datagrams are among those that can't
be received or discarded without getting hold of a reference
to struct file of something in our set.

Everything outside of that set is reachable, so taking the
SCM_RIGHTS with references to stuff in our set (all of
them to be found among those associated with elements of
our set) out of the queues they are in will break all
unreachable loops.  Discarding the collected datagrams
will do the rest - file references in those will be
dropped, etc.

One thing to keep in mind here is the locking.  What the garbage
collector relies upon is
	* changes of ->inflight are serialized with respect to
it (on unix_gc_lock; increment done by unix_inflight(),
decrement - by unix_notinflight()).
	* references cannot be extracted from SCM_RIGHTS datagrams
while the garbage collector is running (achieved by having
unix_notinflight() done before references out of SCM_RIGHTS)
	* removal of SCM_RIGHTS associated with a socket can't
be done without a reference to that socket _outside_ of any
SCM_RIGHTS (automatically true).
	* adding SCM_RIGHTS in the middle of garbage collection
is possible, but in that case it will contain no references to
anything in the initial candidate set.

The last one is delicate.  SCM_RIGHTS creation has unix_inflight()
called for each reference we put there, so it's serialized wrt
unix_gc(); however, insertion into queue is *NOT* covered by that -
queue rescans are, but each queue has a lock of its own and they
are definitely not going to be held throughout the whole thing.

So in theory it would be possible to have
	* thread A: sendmsg() has SCM_RIGHTS created and populated,
complete with file refcount and ->inflight increments implied,
at which point it gets preempted and loses the timeslice.
	* thread B: gets to run and removes all references
from descriptor table it shares with thread A.
	* on another CPU we have garbage collector triggered;
it determines the set of potentially unreachable unix_sock and
everything in our SCM_RIGHTS _is_ in that set, now that no
other references remain.
	* on the first CPU, thread A regains the timeslice
and inserts its SCM_RIGHTS into queue.  And it does contain
references to sockets from the candidate set of running
garbage collector, confusing the hell out of it.


That is avoided by a convoluted dance around the SCM_RIGHTS creation
and insertion - we use fget() to obtain struct file references,
then _duplicate_ them in SCM_RIGHTS (bumping a refcount for each, so
we are holding *two* references), do unix_inflight() on them, then
queue the damn thing, then drop each reference we got from fget().

That way everything refered to in that SCM_RIGHTS is going to have
extra struct file references (and thus be excluded from the initial
candidate set) until after it gets inserted into queue.  In other
words, if it does appear in a queue between two passes, it's
guaranteed to contain no references to anything in the initial
canidate set.

End of braindump.
===================================================================

What you are about to add is *ANOTHER* kind of loops - references
to files in the "registered" set are pinned down by owning io_uring.

That would invalidate just about every assumption made the garbage
collector - even if you forbid to register io_uring itself, you
still can register both ends of AF_UNIX socket pair, then pass
io_uring in SCM_RIGHTS over that, then close all descriptors involved.
>From the garbage collector point of view all sockets have external
references, so there's nothing to collect.  In fact those external
references are only reachable if you have a reachable reference
to io_uring, so we get a leak.

To make it work:
	* have unix_sock created for each io_uring (as your code does)
	* do *NOT* have unix_inflight() done at that point - it's
completely wrong there.
	* file set registration becomes
		* create and populate SCM_RIGHTS, with the same
fget()+grab an extra reference + unix_inflight() sequence.
Don't forget to have skb->destructor set to unix_destruct_scm
or equivalent thereof.
		* remember UNIXCB(skb).fp - that'll give you your
array of struct file *, to use in lookups.
		* queue it into your unix_sock
		* do _one_ fput() for everything you've grabbed,
dropping one of two references you've taken.
	* unregistering is simply skb_dequeue() + kfree_skb().
	* in ->release() you do sock_release(); it'll do
everything you need (including unregistering the set, etc.)

The hairiest part is the file set registration, of course -
there's almost certainly a helper or two buried in that thing;
simply exposing all the required net/unix/af_unix.c bits is
ucking fugly.

I'm not sure what you propose for non-registered descriptors -
_any_ struct file reference that outlives the return from syscall
stored in some io_uring-attached data structure is has exact same
loop (and leak) problem.  And if you mean to have it dropped before
return from syscall, I'm afraid I don't follow you.  How would
that be done?

Again, "io_uring descriptor can't be used in those requests" does
not help at all - use a socket instead, pass the io_uring fd over
it in SCM_RIGHTS and you are back to square 1.

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

^ permalink raw reply

* Re: [PATCH 3/3] mm/mincore: provide mapped status when cached status is not allowed
From: Jiri Kosina @ 2019-02-06 20:14 UTC (permalink / raw)
  To: Vlastimil Babka, Linus Torvalds
  Cc: Michal Hocko, Andrew Morton, linux-kernel, linux-mm, linux-api,
	Peter Zijlstra, Greg KH, Jann Horn, Dominique Martinet,
	Andy Lutomirski, Dave Chinner, Kevin Easton, Matthew Wilcox,
	Cyril Hrubis, Tejun Heo, Kirill A . Shutemov, Daniel Gruss,
	Josh Snyder
In-Reply-To: <e1478ab8-e009-9bdd-3866-f319bd7259a0@suse.cz>

On Fri, 1 Feb 2019, Vlastimil Babka wrote:

> > Well, but rss update will not tell you that the page has been faulted in
> > which is the most interesting part.
> 
> Sure, but the patch doesn't add back that capability neither. It allows
> to recognize page being reclaimed, and I argue you can infer that from
> rss change as well. That change is mentioned in the last paragraph in
> changelog, and I thought "add a hard to evaluate side channel" in your
> reply referred to that. It doesn't add back the "original" side channel
> to detect somebody else accessed a page.

On Fri, 1 Feb 2019, Vlastimil Babka wrote:

> > Is this really worth it? Do we know about any specific usecase that
> > would benefit from this change? TBH I would rather wait for the report
> > than add a hard to evaluate side channel.
> 
> Well it's not that complicated IMHO. Linus said it's worth trying, so
> let's see how he likes the result. The side channel exists anyway as
> long as process can e.g. check if its rss shrinked, and I doubt we are
> going to remove that possibility.

Linus, do you have any opinion here?

I have a hunch that mm maintainers are keeping this on a backburner 
because there might still open question(s) in the air.

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH 04/18] iomap: wire up the iopoll method
From: Jens Axboe @ 2019-02-06 20:05 UTC (permalink / raw)
  To: Bart Van Assche, linux-aio, linux-block, linux-api
  Cc: hch, jmoyer, avi, jannh
In-Reply-To: <1549040392.164323.2.camel@acm.org>

On 2/1/19 9:59 AM, Bart Van Assche wrote:
> On Fri, 2019-02-01 at 08:24 -0700, Jens Axboe wrote:
>> +int iomap_dio_iopoll(struct kiocb *kiocb, bool spin)
>> +{
>> +	struct request_queue *q = READ_ONCE(kiocb->private);
>> +
>> +	if (!q)
>> +		return 0;
>> +	return blk_poll(q, READ_ONCE(kiocb->ki_cookie), spin);
>> +}
>> +EXPORT_SYMBOL_GPL(iomap_dio_iopoll);
> 
> How does this interact with block device removal? My understanding is that this
> function can get called after a request has completed. Which mechanism, if any,
> prevents that request queue 'q' is removed after kiocb->private has been read
> and before blk_poll() is called?

The completion event is split from the reap, the latter being the one that
would put the file. So we're never going to call into ->iopoll() for a
kiocb where we've done the file put.

-- 
Jens Axboe

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

^ permalink raw reply

* Re: [PATCH 13/18] io_uring: add file set registration
From: Jens Axboe @ 2019-02-06 17:56 UTC (permalink / raw)
  To: Al Viro
  Cc: Jann Horn, linux-aio, linux-block, Linux API, hch, jmoyer, avi,
	linux-fsdevel
In-Reply-To: <20190206010104.GV2217@ZenIV.linux.org.uk>

On 2/5/19 6:01 PM, Al Viro wrote:
> On Tue, Feb 05, 2019 at 05:27:29PM -0700, Jens Axboe wrote:
> 
>> This should be better, passes some basic testing, too:
>>
>> http://git.kernel.dk/cgit/linux-block/commit/?h=io_uring&id=01a93aa784319a02ccfa6523371b93401c9e0073
>>
>> Verified that we're grabbing the right refs, and don't hold any
>> ourselves. For the file registration, forbid registration of the
>> io_uring fd, as that is pointless and will introduce a loop regardless
>> of fd passing.
> 
> *shrug*
> 
> So pass it to AF_UNIX socket and register _that_ - does't change the
> underlying problem.

Maybe I'm being dense here, but it's an f_op match. Should catch a
passed fd as well, correct?

With that, how can there be a loop?

-- 
Jens Axboe

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

^ permalink raw reply

* Re: [PATCH 13/18] io_uring: add file set registration
From: Jens Axboe @ 2019-02-06 13:41 UTC (permalink / raw)
  To: Al Viro
  Cc: Jann Horn, linux-aio, linux-block, Linux API, hch, jmoyer, avi,
	linux-fsdevel
In-Reply-To: <20190206005638.GU2217@ZenIV.linux.org.uk>

On 2/5/19 5:56 PM, Al Viro wrote:
> On Tue, Feb 05, 2019 at 12:08:25PM -0700, Jens Axboe wrote:
>> Proof is in the pudding, here's the main commit introducing io_uring
>> and now wiring it up to the AF_UNIX garbage collection:
>>
>> http://git.kernel.dk/cgit/linux-block/commit/?h=io_uring&id=158e6f42b67d0abe9ee84886b96ca8c4b3d3dfd5
>>
>> How does that look?
> 
> In a word - wrong.  Some theory: garbage collector assumes that there is
> a subset of file references such that
> 	* for all files with such references there's an associated unix_sock.
> 	* all such references are stored in SCM_RIGHTS datagrams that can be
> found by the garbage collector (currently: for data-bearing AF_UNIX sockets -
> queued SCM_RIGHTS datagrams, for listeners - SCM_RIGHTS datagrams sent via
> yet-to-be-accepted connections).
> 	* there is an efficient way to count those references for given file
> (->inflight of the corresponding unix_sock).
> 	* removal of those references would render the graph acyclic.
> 	* file can _NOT_ be subject to syscalls unless there are references
> to it outside of that subset.

IOW, we cannot use fget() for registering files, and we still need fget/fput
in the fast path to retain safe use of the file. If I'm understanding you
correctly?

Just trying to ensure that I understand what you're saying here, as it seems
to refer to the file registration part, not the main patch (which did get
reworked, though).

-- 
Jens Axboe

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

^ permalink raw reply

* Re: [PATCH 05/32] timerfd/timens: Take into account ns clock offsets
From: Cyrill Gorcunov @ 2019-02-06  8:55 UTC (permalink / raw)
  To: Dmitry Safonov
  Cc: linux-kernel, Andrei Vagin, Adrian Reber, Andrei Vagin,
	Andy Lutomirski, Andy Tucker, Arnd Bergmann, Christian Brauner,
	Dmitry Safonov, Eric W. Biederman, H. Peter Anvin, Ingo Molnar,
	Jeff Dike, Oleg Nesterov, Pavel Emelyanov, Shuah Khan,
	Thomas Gleixner, containers, criu, linux-api, x86
In-Reply-To: <20190206085203.GB25251@uranus>

On Wed, Feb 06, 2019 at 11:52:03AM +0300, Cyrill Gorcunov wrote:
...
> >  
> > -	if ((flags & ~TFD_SETTIME_FLAGS) ||
> > -		 !itimerspec64_valid(new))
> > -		return -EINVAL;
> 
> Please don't defer this early test of a @flags value. Otherwise
> if @flags is invalid you continue fget/put/clock-to-host even
> if result will be dropped out then.

Just to clarify -- this could be done on top of the series to
not resend the whole bunch.

^ permalink raw reply

* Re: [PATCH 05/32] timerfd/timens: Take into account ns clock offsets
From: Cyrill Gorcunov @ 2019-02-06  8:52 UTC (permalink / raw)
  To: Dmitry Safonov
  Cc: linux-kernel, Andrei Vagin, Adrian Reber, Andrei Vagin,
	Andy Lutomirski, Andy Tucker, Arnd Bergmann, Christian Brauner,
	Dmitry Safonov, Eric W. Biederman, H. Peter Anvin, Ingo Molnar,
	Jeff Dike, Oleg Nesterov, Pavel Emelyanov, Shuah Khan,
	Thomas Gleixner, containers, criu, linux-api, x86
In-Reply-To: <20190206001107.16488-6-dima@arista.com>

On Wed, Feb 06, 2019 at 12:10:39AM +0000, Dmitry Safonov wrote:
> From: Andrei Vagin <avagin@gmail.com>
> 
> Make timerfd respect timens offsets.
> Provide two helpers timens_clock_to_host() timens_clock_from_host() that
> are useful to wire up timens to different kernel subsystems.
> Following patches will use timens_clock_from_host(), added here for
> completeness.
> 
> Signed-off-by: Andrei Vagin <avagin@openvz.org>
> Co-developed-by: Dmitry Safonov <dima@arista.com>
> Signed-off-by: Dmitry Safonov <dima@arista.com>
> ---
>  fs/timerfd.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/timerfd.c b/fs/timerfd.c
> index 803ca070d42e..c7ae1e371912 100644
> --- a/fs/timerfd.c
> +++ b/fs/timerfd.c
> @@ -26,6 +26,7 @@
>  #include <linux/syscalls.h>
>  #include <linux/compat.h>
>  #include <linux/rcupdate.h>
> +#include <linux/time_namespace.h>
>  
>  struct timerfd_ctx {
>  	union {
> @@ -433,22 +434,27 @@ SYSCALL_DEFINE2(timerfd_create, int, clockid, int, flags)
>  }
>  
>  static int do_timerfd_settime(int ufd, int flags, 
> -		const struct itimerspec64 *new,
> +		struct itimerspec64 *new,
>  		struct itimerspec64 *old)
>  {
>  	struct fd f;
>  	struct timerfd_ctx *ctx;
>  	int ret;
>  
> -	if ((flags & ~TFD_SETTIME_FLAGS) ||
> -		 !itimerspec64_valid(new))
> -		return -EINVAL;

Please don't defer this early test of a @flags value. Otherwise
if @flags is invalid you continue fget/put/clock-to-host even
if result will be dropped out then.

^ permalink raw reply

* Re: [PATCH 13/18] io_uring: add file set registration
From: Al Viro @ 2019-02-06  1:01 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Jann Horn, linux-aio, linux-block, Linux API, hch, jmoyer, avi,
	linux-fsdevel
In-Reply-To: <0d2e5085-32ff-e86e-d628-6000071fd132@kernel.dk>

On Tue, Feb 05, 2019 at 05:27:29PM -0700, Jens Axboe wrote:

> This should be better, passes some basic testing, too:
> 
> http://git.kernel.dk/cgit/linux-block/commit/?h=io_uring&id=01a93aa784319a02ccfa6523371b93401c9e0073
> 
> Verified that we're grabbing the right refs, and don't hold any
> ourselves. For the file registration, forbid registration of the
> io_uring fd, as that is pointless and will introduce a loop regardless
> of fd passing.

*shrug*

So pass it to AF_UNIX socket and register _that_ - does't change the
underlying problem.

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

^ permalink raw reply

* Re: [PATCH 13/18] io_uring: add file set registration
From: Al Viro @ 2019-02-06  0:56 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Jann Horn, linux-aio, linux-block, Linux API, hch, jmoyer, avi,
	linux-fsdevel
In-Reply-To: <40b27e78-9ee8-1395-feb3-a73aac87c9a7@kernel.dk>

On Tue, Feb 05, 2019 at 12:08:25PM -0700, Jens Axboe wrote:
> Proof is in the pudding, here's the main commit introducing io_uring
> and now wiring it up to the AF_UNIX garbage collection:
> 
> http://git.kernel.dk/cgit/linux-block/commit/?h=io_uring&id=158e6f42b67d0abe9ee84886b96ca8c4b3d3dfd5
> 
> How does that look?

In a word - wrong.  Some theory: garbage collector assumes that there is
a subset of file references such that
	* for all files with such references there's an associated unix_sock.
	* all such references are stored in SCM_RIGHTS datagrams that can be
found by the garbage collector (currently: for data-bearing AF_UNIX sockets -
queued SCM_RIGHTS datagrams, for listeners - SCM_RIGHTS datagrams sent via
yet-to-be-accepted connections).
	* there is an efficient way to count those references for given file
(->inflight of the corresponding unix_sock).
	* removal of those references would render the graph acyclic.
	* file can _NOT_ be subject to syscalls unless there are references
to it outside of that subset.

unix_inflight() moves a reference into the subset
unix_notinflight() moves a reference out of the subset
activity that might add such references ought to call wait_for_unix_gc() first
(basically, to stall the massive insertions when gc is running).

Note that unix_gc() does *NOT* work in terms of dropping file references -
the primary effect is locating the SCM_RIGHTS datagrams that can be disposed
of and taking them out.  It simply won't do anything to your file references,
no matter what.  Add a printk into your ->release() and try to register io_uring
descriptor into itself, then close it.  And observe ->release() not being
called for that object.  Ever.

PS: The algorithm used by unix_gc() is basically this -

	grab unix_gc_lock (giving exclusion with unix_inflight/unix_notinflight
			   and stabilizing ->inflight counters)

	Candidates = {}
	for all unix_sock u such that u->inflight > 0
		if file corresponding to u has no other references
			Candidates += u

	/* everything else already is reachable; due to unix_gc_lock these
	   can't die or get syscall-visible references under us */
	Might_Die = Candidates

	/* invariant to maintain: for u in Candidates u->inflight will be equal
	   to the number of references from SCM_RIGHTS datagrams *except*
	   those immediately reachable from elements of Might_Die */

	for all u in Candidates
		for each file reference v in SCM_RIGHTS datagrams
					immediately reachable from u
			if v in Candidates
				v->inflight--

	To_Scan = ()	// stuff reachable from those must live
	for all u in Might_Die
		if u->inflight > 0
			queue u into To_Scan

	while To_Scan is non-empty
		u = dequeue(To_Scan)
		Might_Die -= u
		for each file reference v in SCM_RIGHTS datagrams
					immediately reachable from u
			if v in Candidates
				v->inflight++	// maintain the invariant
				if v in Might_Die
					queue v into To_Scan

	/* at that point nothing in Might_Die is reachable from the outside */

	/* restore the original values of ->inflight */
	for all u in Might_Die
		for each file reference v in SCM_RIGHTS datagrams
					immediately reachable from u
			if v in Candidates
				v->inflight++

	hitlist = ()
	for all u in Might_Die
		for each SCM_RIGHTS datagram D immediately reachable from u
			if D contains references to something in Candidates
				move D to hitlist
	/* all those datagrams would've never become reachable */

	drop unix_gc_lock

	discard all datagrams in hitlist.

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

^ permalink raw reply

* Re: [PATCH 13/18] io_uring: add file set registration
From: Jens Axboe @ 2019-02-06  0:27 UTC (permalink / raw)
  To: Al Viro, Jann Horn
  Cc: linux-aio, linux-block, Linux API, hch, jmoyer, avi,
	linux-fsdevel
In-Reply-To: <40b27e78-9ee8-1395-feb3-a73aac87c9a7@kernel.dk>

On 2/5/19 12:08 PM, Jens Axboe wrote:
> On 2/5/19 10:57 AM, Jens Axboe wrote:
>> On 2/4/19 7:19 PM, Jens Axboe wrote:
>>> On 2/3/19 7:56 PM, Al Viro wrote:
>>>> On Wed, Jan 30, 2019 at 02:29:05AM +0100, Jann Horn wrote:
>>>>> On Tue, Jan 29, 2019 at 8:27 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>> We normally have to fget/fput for each IO we do on a file. Even with
>>>>>> the batching we do, the cost of the atomic inc/dec of the file usage
>>>>>> count adds up.
>>>>>>
>>>>>> This adds IORING_REGISTER_FILES, and IORING_UNREGISTER_FILES opcodes
>>>>>> for the io_uring_register(2) system call. The arguments passed in must
>>>>>> be an array of __s32 holding file descriptors, and nr_args should hold
>>>>>> the number of file descriptors the application wishes to pin for the
>>>>>> duration of the io_uring context (or until IORING_UNREGISTER_FILES is
>>>>>> called).
>>>>>>
>>>>>> When used, the application must set IOSQE_FIXED_FILE in the sqe->flags
>>>>>> member. Then, instead of setting sqe->fd to the real fd, it sets sqe->fd
>>>>>> to the index in the array passed in to IORING_REGISTER_FILES.
>>>>>>
>>>>>> Files are automatically unregistered when the io_uring context is
>>>>>> torn down. An application need only unregister if it wishes to
>>>>>> register a new set of fds.
>>>>>
>>>>> Crazy idea:
>>>>>
>>>>> Taking a step back, at a high level, basically this patch creates sort
>>>>> of the same difference that you get when you compare the following
>>>>> scenarios for normal multithreaded I/O in userspace:
>>>>
>>>>> This kinda makes me wonder whether this is really something that
>>>>> should be implemented specifically for the io_uring API, or whether it
>>>>> would make sense to somehow handle part of this in the generic VFS
>>>>> code and give the user the ability to prepare a new files_struct that
>>>>> can then be transferred to the worker thread, or something like
>>>>> that... I'm not sure whether there's a particularly clean way to do
>>>>> that though.
>>>>
>>>> Using files_struct for that opens a can of worms you really don't
>>>> want to touch.
>>>>
>>>> Consider the following scenario with any variant of this interface:
>>>> 	* create io_uring fd.
>>>> 	* send an SCM_RIGHTS with that fd to AF_UNIX socket.
>>>> 	* add the descriptor of that AF_UNIX socket to your fd
>>>> 	* close AF_UNIX fd, close io_uring fd.
>>>> Voila - you've got a shiny leak.  No ->release() is called for
>>>> anyone (and you really don't want to do that on ->flush(), because
>>>> otherwise a library helper doing e.g. system("/bin/date") will tear
>>>> down all the io_uring in your process).  The socket is held by
>>>> the reference you've stashed into io_uring (whichever way you do
>>>> that).  io_uring is held by the reference you've stashed into
>>>> SCM_RIGHTS datagram in queue of the socket.
>>>>
>>>> No matter what, you need net/unix/garbage.c to be aware of that stuff.
>>>> And getting files_struct lifetime mixed into that would be beyond
>>>> any reason.
>>>>
>>>> The only reason for doing that as a descriptor table would be
>>>> avoiding the cost of fget() in whatever uses it, right?  Since
>>>
>>> Right, the only purpose of this patch is to avoid doing fget/fput for
>>> each IO.
>>>
>>>> those are *not* the normal syscalls (and fdget() really should not
>>>> be used anywhere other than the very top of syscall's call chain -
>>>> that's another reason why tossing file_struct around like that
>>>> is insane) and since the benefit is all due to the fact that it's
>>>> *NOT* shared, *NOT* modified in parallel, etc., allowing us to
>>>> treat file references as stable... why the hell use the descriptor
>>>> tables at all?
>>>
>>> This one is not a regular system call, since we don't do fget, then IO,
>>> then fput. We hang on to it. But for the non-registered case, it's very
>>> much just like a regular read/write system call, where we fget to do IO
>>> on it, then fput when we are done.
>>>
>>>> All you need is an array of struct file *, explicitly populated.
>>>> With net/unix/garbage.c aware of such beasts.  Guess what?  We
>>>> do have such an object already.  The one net/unix/garbage.c is
>>>> working with.  SCM_RIGHTS datagrams, that is.
>>>>
>>>> IOW, can't we give those io_uring descriptors associated struct
>>>> unix_sock?  No socket descriptors, no struct socket (probably),
>>>> just the AF_UNIX-specific part thereof.  Then teach
>>>> unix_inflight()/unix_notinflight() about getting unix_sock out
>>>> of these guys (incidentally, both would seem to benefit from
>>>> _not_ touching unix_gc_lock in case when there's no unix_sock
>>>> attached to file we are dealing with - I might be missing
>>>> something very subtle about barriers there, but it doesn't
>>>> look likely).
>>>
>>> That might be workable, though I'm not sure we currently have helpers to
>>> just explicitly create a unix_sock by itself. Not familiar with the
>>> networking bits at all, I'll take a look.
>>>
>>>> And make that (i.e. registering the descriptors) mandatory.
>>>
>>> I don't want to make it mandatory, that's very inflexible for managing
>>> tons of files. The registration is useful for specific cases where we
>>> have high frequency of operations on a set of files. Besides, it'd make
>>> the use of the API cumbersome as well for the basic case of just wanting
>>> to do async IO.
>>>
>>>> Hell, combine that with creating io_uring fd, if we really
>>>> care about the syscall count.  Benefits:
>>>
>>> We don't care about syscall count for setup as much. If you're doing
>>> registration of a file set, you're expected to do a LOT of IO to those
>>> files. Hence having an extra one for setup is not a concern. My concern
>>> is just making it mandatory to do registration, I don't think that's a
>>> workable alternative.
>>>
>>>> 	* no file_struct refcount wanking
>>>> 	* no fget()/fput() (conditional, at that) from kernel
>>>> threads
>>>> 	* no CLOEXEC-dependent anything; just the teardown
>>>> on the final fput(), whichever way it comes.
>>>> 	* no fun with duelling garbage collectors.
>>>
>>> The fget/fput from a kernel thread can be solved by just hanging on to
>>> the struct file * when we punt the IO. Right now we don't, which is a
>>> little silly, that should be changed.
>>>
>>> Getting rid of the files_struct{} is doable.
>>
>> OK, I've reworked the initial parts to wire up the io_uring fd to the
>> AF_UNIX garbage collection. As I made it to the file registration part,
>> I wanted to wire up that too. But I don't think there's a need for that
>> - if we have the io_uring fd appropriately protected, we'll be dropping
>> our struct file ** array index when the io_uring fd is released. That
>> should be adequate, we don't need the garbage collection to be aware of
>> those individually.
>>
>> The only part I had to drop for now is the sq thread polling, as that
>> depends on us carrying the files_struct. I'm going to fold that in
>> shortly, but just make it be dependent on having registered files. That
>> avoids needing to fget/fput for that case, and needing registered files
>> for the sq side submission/polling is not a usability issue like it
>> would be for the "normal" use cases.
> 
> Proof is in the pudding, here's the main commit introducing io_uring
> and now wiring it up to the AF_UNIX garbage collection:
> 
> http://git.kernel.dk/cgit/linux-block/commit/?h=io_uring&id=158e6f42b67d0abe9ee84886b96ca8c4b3d3dfd5
> 
> How does that look? Outside of the inflight hookup, we simply retain
> the file * for punting to the workqueue. This means that buffered
> retry does NOT need to do fget/fput, so we don't need a files_struct
> for that anymore.
> 
> In terms of the SQPOLL patch that's further down the series, it doesn't
> allow that mode of operation without having fixed files enabled. That
> eliminates the need for fget/fput from a kernel thread, and hence the
> need to carry a files_struct around for that as well.

This should be better, passes some basic testing, too:

http://git.kernel.dk/cgit/linux-block/commit/?h=io_uring&id=01a93aa784319a02ccfa6523371b93401c9e0073

Verified that we're grabbing the right refs, and don't hold any
ourselves. For the file registration, forbid registration of the
io_uring fd, as that is pointless and will introduce a loop regardless
of fd passing.

-- 
Jens Axboe

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

^ permalink raw reply

* [PATCH 32/32] x86/vdso: Restrict splitting VVAR VMA
From: Dmitry Safonov @ 2019-02-06  0:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: Dmitry Safonov, Adrian Reber, Andrei Vagin, Andrei Vagin,
	Andy Lutomirski, Andy Tucker, Arnd Bergmann, Christian Brauner,
	Cyrill Gorcunov, Dmitry Safonov, Eric W. Biederman,
	H. Peter Anvin, Ingo Molnar, Jeff Dike, Oleg Nesterov,
	Pavel Emelyanov, Shuah Khan, Thomas Gleixner, containers, criu,
	linux-api, x86
In-Reply-To: <20190206001107.16488-1-dima@arista.com>

Although, time namespace can work with VVAR VMA split, it seems worth
to forbid splitting VVAR resulting in stricter ABI and reducing amount
of corner-cases to consider while working further on VDSO.

I don't think there is any use-case for partial mremap() of vvar,
but it there is - this patch can be easily dropped.

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 arch/x86/entry/vdso/vma.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c
index 52c1e4c24455..dc1fca4d935a 100644
--- a/arch/x86/entry/vdso/vma.c
+++ b/arch/x86/entry/vdso/vma.c
@@ -87,6 +87,18 @@ static int vdso_mremap(const struct vm_special_mapping *sm,
 	return 0;
 }
 
+static int vvar_mremap(const struct vm_special_mapping *sm,
+		struct vm_area_struct *new_vma)
+{
+	unsigned long new_size = new_vma->vm_end - new_vma->vm_start;
+	const struct vdso_image *image = current->mm->context.vdso_image;
+
+	if (new_size != -image->sym_vvar_start)
+		return -EINVAL;
+
+	return 0;
+}
+
 static vm_fault_t vvar_fault(const struct vm_special_mapping *sm,
 		      struct vm_area_struct *vma, struct vm_fault *vmf)
 {
@@ -149,6 +161,7 @@ static const struct vm_special_mapping vdso_mapping = {
 static const struct vm_special_mapping vvar_mapping = {
 	.name = "[vvar]",
 	.fault = vvar_fault,
+	.mremap = vvar_mremap,
 };
 
 #ifdef CONFIG_TIME_NS
-- 
2.20.1

^ permalink raw reply related

* [PATCH 31/32] x86/vdso: Align VDSO functions by CPU L1 cache line
From: Dmitry Safonov @ 2019-02-06  0:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrei Vagin, Dmitry Safonov, Adrian Reber, Andrei Vagin,
	Andy Lutomirski, Andy Tucker, Arnd Bergmann, Christian Brauner,
	Cyrill Gorcunov, Dmitry Safonov, Eric W. Biederman,
	H. Peter Anvin, Ingo Molnar, Jeff Dike, Oleg Nesterov,
	Pavel Emelyanov, Shuah Khan, Thomas Gleixner, containers, criu,
	linux-api, x86
In-Reply-To: <20190206001107.16488-1-dima@arista.com>

From: Andrei Vagin <avagin@gmail.com>

After performance testing VDSO patches a noticeable 20% regression was
found on gettime_perf selftest with a cold cache.
As it turns to be, before time namespaces introduction, VDSO functions
were quite aligned to cache lines, but adding a new code to adjust
timens offset inside namespace created a small shift and vdso functions
become unaligned on cache lines.

Add align to vdso functions with gcc option to fix performance drop.

Coping the resulting numbers from cover letter:

Hot CPU cache (more gettime_perf.c cycles - the better):
        | before     | CONFIG_TIME_NS=n | host        | inside timens
--------|------------|------------------|-------------|-------------
cycles  | 139887013  | 139453003        | 139899785   | 128792458
diff (%)| 100        | 99.7             | 100         | 92

Cold cache (lesser tsc per gettime_perf_cold.c cycle - the better):
        | before     | CONFIG_TIME_NS=n | host        | inside timens
--------|------------|------------------|-------------|-------------
tsc     | 6748       | 6718             | 6862        | 12682
diff (%)| 100        | 99.6             | 101.7       | 188

Measured on Intel(R) Core(TM) i5-6300U CPU @ 2.40GHz

Co-developed-by: Dmitry Safonov <dima@arista.com>
Signed-off-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 arch/x86/entry/vdso/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index 4e1659619e7e..2cac4660db05 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -4,6 +4,7 @@
 #
 
 KBUILD_CFLAGS += $(DISABLE_LTO) -ffunction-sections
+KBUILD_CFLAGS += -falign-functions=$(CONFIG_X86_L1_CACHE_SHIFT)
 KASAN_SANITIZE			:= n
 UBSAN_SANITIZE			:= n
 OBJECT_FILES_NON_STANDARD	:= y
-- 
2.20.1

^ permalink raw reply related

* [PATCH 30/32] selftest/timens: Check that a right vdso is mapped after fork and exec
From: Dmitry Safonov @ 2019-02-06  0:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrei Vagin, Dmitry Safonov, Adrian Reber, Andrei Vagin,
	Andy Lutomirski, Andy Tucker, Arnd Bergmann, Christian Brauner,
	Cyrill Gorcunov, Dmitry Safonov, Eric W. Biederman,
	H. Peter Anvin, Ingo Molnar, Jeff Dike, Oleg Nesterov,
	Pavel Emelyanov, Shuah Khan, Thomas Gleixner, containers, criu,
	linux-api, x86
In-Reply-To: <20190206001107.16488-1-dima@arista.com>

From: Andrei Vagin <avagin@gmail.com>

Signed-off-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 tools/testing/selftests/timens/.gitignore |  1 +
 tools/testing/selftests/timens/Makefile   |  2 +-
 tools/testing/selftests/timens/exec.c     | 91 +++++++++++++++++++++++
 3 files changed, 93 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/timens/exec.c

diff --git a/tools/testing/selftests/timens/.gitignore b/tools/testing/selftests/timens/.gitignore
index 6bb90fdb4519..b08b4066f5ca 100644
--- a/tools/testing/selftests/timens/.gitignore
+++ b/tools/testing/selftests/timens/.gitignore
@@ -1,4 +1,5 @@
 clock_nanosleep
+exec
 gettime_perf
 procfs
 timens
diff --git a/tools/testing/selftests/timens/Makefile b/tools/testing/selftests/timens/Makefile
index ef65bf96b55c..9e0edf354906 100644
--- a/tools/testing/selftests/timens/Makefile
+++ b/tools/testing/selftests/timens/Makefile
@@ -1,4 +1,4 @@
-TEST_GEN_PROGS := timens timerfd timer clock_nanosleep procfs gettime_perf
+TEST_GEN_PROGS := timens timerfd timer clock_nanosleep procfs gettime_perf exec
 
 uname_M := $(shell uname -m 2>/dev/null || echo not)
 ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/i386/)
diff --git a/tools/testing/selftests/timens/exec.c b/tools/testing/selftests/timens/exec.c
new file mode 100644
index 000000000000..b3a05c41e202
--- /dev/null
+++ b/tools/testing/selftests/timens/exec.c
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <errno.h>
+#include <fcntl.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdbool.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <time.h>
+#include <unistd.h>
+#include <time.h>
+#include <string.h>
+
+#include "log.h"
+#include "timens.h"
+
+#define OFFSET (36000)
+
+int main(int argc, char *argv[])
+{
+	struct timespec now, tst;
+	int status, i;
+	pid_t pid;
+
+	if (argc > 1) {
+		if (sscanf(argv[1], "%ld", &now.tv_sec) != 1)
+			return pr_perror("sscanf");
+
+		for (i = 0; i < 2; i++) {
+			_gettime(CLOCK_MONOTONIC, &tst, i);
+			if (abs(tst.tv_sec - now.tv_sec) > 5)
+				return pr_fail("%ld %ld\n", now.tv_sec, tst.tv_sec);
+		}
+	}
+
+	nscheck();
+
+	clock_gettime(CLOCK_MONOTONIC, &now);
+
+	if (unshare(CLONE_NEWTIME))
+		return pr_perror("Can't unshare() timens");
+
+	if (_settime(CLOCK_MONOTONIC, OFFSET))
+		return 1;
+
+	for (i = 0; i < 2; i++) {
+		_gettime(CLOCK_MONOTONIC, &tst, i);
+		if (abs(tst.tv_sec - now.tv_sec) > 5)
+			return pr_fail("%ld %ld\n",
+					now.tv_sec, tst.tv_sec);
+	}
+
+	if (argc > 1)
+		return 0;
+
+	pid = fork();
+	if (pid < 0)
+		return pr_perror("fork");
+
+	if (pid == 0) {
+		char now_str[64];
+		char *cargv[] = {"exec", now_str, NULL};
+		char *cenv[] = {NULL};
+
+		/* Check that a child process is in the new timens. */
+		for (i = 0; i < 2; i++) {
+			_gettime(CLOCK_MONOTONIC, &tst, i);
+			if (abs(tst.tv_sec - now.tv_sec - OFFSET) > 5)
+				return pr_fail("%ld %ld\n",
+						now.tv_sec + OFFSET, tst.tv_sec);
+		}
+
+		/* Check that a proper vdso will be mapped after execve. */
+		snprintf(now_str, sizeof(now_str), "%ld", now.tv_sec + OFFSET);
+		execve("/proc/self/exe", cargv, cenv);
+		return pr_perror("execve");
+	}
+
+	if (waitpid(pid, &status, 0) != pid)
+		return pr_perror("waitpid");
+
+	if (status)
+		ksft_exit_fail();
+
+	ksft_test_result_pass("exec\n");
+	ksft_exit_pass();
+	return 0;
+}
-- 
2.20.1

^ permalink raw reply related

* [PATCH 29/32] selftests: Add a simple perf test for clock_gettime()
From: Dmitry Safonov @ 2019-02-06  0:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrei Vagin, Dmitry Safonov, Adrian Reber, Andrei Vagin,
	Andy Lutomirski, Andy Tucker, Arnd Bergmann, Christian Brauner,
	Cyrill Gorcunov, Dmitry Safonov, Eric W. Biederman,
	H. Peter Anvin, Ingo Molnar, Jeff Dike, Oleg Nesterov,
	Pavel Emelyanov, Shuah Khan, Thomas Gleixner, containers, criu,
	linux-api, x86
In-Reply-To: <20190206001107.16488-1-dima@arista.com>

From: Andrei Vagin <avagin@gmail.com>

Signed-off-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 tools/testing/selftests/timens/.gitignore     |  1 +
 tools/testing/selftests/timens/Makefile       |  8 +-
 tools/testing/selftests/timens/gettime_perf.c | 74 +++++++++++++++++++
 .../selftests/timens/gettime_perf_cold.c      | 63 ++++++++++++++++
 4 files changed, 145 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/timens/gettime_perf.c
 create mode 100644 tools/testing/selftests/timens/gettime_perf_cold.c

diff --git a/tools/testing/selftests/timens/.gitignore b/tools/testing/selftests/timens/.gitignore
index 3b7eda8f35ce..6bb90fdb4519 100644
--- a/tools/testing/selftests/timens/.gitignore
+++ b/tools/testing/selftests/timens/.gitignore
@@ -1,4 +1,5 @@
 clock_nanosleep
+gettime_perf
 procfs
 timens
 timer
diff --git a/tools/testing/selftests/timens/Makefile b/tools/testing/selftests/timens/Makefile
index ae1ffd24cc43..ef65bf96b55c 100644
--- a/tools/testing/selftests/timens/Makefile
+++ b/tools/testing/selftests/timens/Makefile
@@ -1,4 +1,10 @@
-TEST_GEN_PROGS := timens timerfd timer clock_nanosleep procfs
+TEST_GEN_PROGS := timens timerfd timer clock_nanosleep procfs gettime_perf
+
+uname_M := $(shell uname -m 2>/dev/null || echo not)
+ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/i386/)
+ifeq ($(ARCH),x86_64)
+TEST_GEN_PROGS += gettime_perf_cold
+endif
 
 CFLAGS := -Wall -Werror
 LDFLAGS := -lrt
diff --git a/tools/testing/selftests/timens/gettime_perf.c b/tools/testing/selftests/timens/gettime_perf.c
new file mode 100644
index 000000000000..510d77a941d9
--- /dev/null
+++ b/tools/testing/selftests/timens/gettime_perf.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <sched.h>
+#include <time.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/syscall.h>
+
+#include "log.h"
+#include "timens.h"
+
+//#define TEST_SYSCALL
+
+static void test(clock_t clockid, char *clockstr, bool in_ns)
+{
+	struct timespec tp, start;
+	long i = 0;
+	const int timeout = 3;
+
+#ifndef TEST_SYSCALL
+	clock_gettime(clockid, &start);
+#else
+	syscall(__NR_clock_gettime, clockid, &start);
+#endif
+	tp = start;
+	for (tp = start; start.tv_sec + timeout > tp.tv_sec ||
+			 (start.tv_sec + timeout == tp.tv_sec &&
+			  start.tv_nsec > tp.tv_nsec); i++) {
+#ifndef TEST_SYSCALL
+		clock_gettime(clockid, &tp);
+#else
+		syscall(__NR_clock_gettime, clockid, &tp);
+#endif
+	}
+
+	ksft_test_result_pass("%s:\tclock: %10s\tcycles:\t%10ld\n",
+			      in_ns ? "ns" : "host", clockstr, i);
+}
+
+int main(int argc, char *argv[])
+{
+	time_t offset = 10;
+	int nsfd;
+
+	test(CLOCK_MONOTONIC, "monotonic", false);
+	test(CLOCK_BOOTTIME, "boottime", false);
+
+	nscheck();
+
+	if (unshare(CLONE_NEWTIME))
+		return pr_perror("Can't unshare() timens");
+
+	nsfd = open("/proc/self/ns/time_for_children", O_RDONLY);
+	if (nsfd < 0)
+		return pr_perror("Can't open a time namespace");
+
+	if (_settime(CLOCK_MONOTONIC, offset))
+		return 1;
+	if (_settime(CLOCK_BOOTTIME, offset))
+		return 1;
+
+	if (setns(nsfd, CLONE_NEWTIME))
+		return pr_perror("setns");
+
+	test(CLOCK_MONOTONIC, "monotonic", true);
+	test(CLOCK_BOOTTIME, "boottime", true);
+
+	ksft_exit_pass();
+	return 0;
+}
diff --git a/tools/testing/selftests/timens/gettime_perf_cold.c b/tools/testing/selftests/timens/gettime_perf_cold.c
new file mode 100644
index 000000000000..f72db8a4c903
--- /dev/null
+++ b/tools/testing/selftests/timens/gettime_perf_cold.c
@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <sched.h>
+#include <time.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/syscall.h>
+#include <string.h>
+
+#include "log.h"
+#include "timens.h"
+
+static __inline__ unsigned long long rdtsc(void)
+{
+	unsigned hi, lo;
+
+	__asm__ __volatile__ ("rdtsc" : "=a"(lo), "=d"(hi));
+	return ((unsigned long long) lo) | (((unsigned long long)hi) << 32);
+}
+
+static void test(clock_t clockid, char *clockstr)
+{
+	struct timespec tp;
+	long long s, e;
+
+	s = rdtsc();
+	clock_gettime(clockid, &tp);
+	e = rdtsc();
+	printf("%lld\n", e - s);
+	return;
+}
+
+int main(int argc, char **argv)
+{
+	time_t offset = 10;
+	int nsfd;
+
+	if (argc == 1) {
+		test(CLOCK_MONOTONIC, "monotonic");
+		return 0;
+	}
+	nscheck();
+
+	if (unshare(CLONE_NEWTIME))
+		return pr_perror("Can't unshare() timens");
+
+	nsfd = open("/proc/self/ns/time_for_children", O_RDONLY);
+	if (nsfd < 0)
+		return pr_perror("Can't open a time namespace");
+
+	if (_settime(CLOCK_MONOTONIC, offset))
+		return 1;
+
+	if (setns(nsfd, CLONE_NEWTIME))
+		return pr_perror("setns");
+
+	test(CLOCK_MONOTONIC, "monotonic");
+	return 0;
+}
-- 
2.20.1

^ permalink raw reply related

* [PATCH 28/32] selftest/timens: Add timer offsets test
From: Dmitry Safonov @ 2019-02-06  0:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrei Vagin, Dmitry Safonov, Adrian Reber, Andrei Vagin,
	Andy Lutomirski, Andy Tucker, Arnd Bergmann, Christian Brauner,
	Cyrill Gorcunov, Dmitry Safonov, Eric W. Biederman,
	H. Peter Anvin, Ingo Molnar, Jeff Dike, Oleg Nesterov,
	Pavel Emelyanov, Shuah Khan, Thomas Gleixner, containers, criu,
	linux-api, x86
In-Reply-To: <20190206001107.16488-1-dima@arista.com>

From: Andrei Vagin <avagin@openvz.org>

Check that timer_create() takes into account clock offsets.

Signed-off-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 tools/testing/selftests/timens/.gitignore |   1 +
 tools/testing/selftests/timens/Makefile   |   3 +-
 tools/testing/selftests/timens/timer.c    | 115 ++++++++++++++++++++++
 3 files changed, 118 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/timens/timer.c

diff --git a/tools/testing/selftests/timens/.gitignore b/tools/testing/selftests/timens/.gitignore
index 94ffdd9cead7..3b7eda8f35ce 100644
--- a/tools/testing/selftests/timens/.gitignore
+++ b/tools/testing/selftests/timens/.gitignore
@@ -1,4 +1,5 @@
 clock_nanosleep
 procfs
 timens
+timer
 timerfd
diff --git a/tools/testing/selftests/timens/Makefile b/tools/testing/selftests/timens/Makefile
index f96f50d1fef8..ae1ffd24cc43 100644
--- a/tools/testing/selftests/timens/Makefile
+++ b/tools/testing/selftests/timens/Makefile
@@ -1,5 +1,6 @@
-TEST_GEN_PROGS := timens timerfd clock_nanosleep procfs
+TEST_GEN_PROGS := timens timerfd timer clock_nanosleep procfs
 
 CFLAGS := -Wall -Werror
+LDFLAGS := -lrt
 
 include ../lib.mk
diff --git a/tools/testing/selftests/timens/timer.c b/tools/testing/selftests/timens/timer.c
new file mode 100644
index 000000000000..aeb5623d25e4
--- /dev/null
+++ b/tools/testing/selftests/timens/timer.c
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <sched.h>
+
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <time.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <signal.h>
+#include <time.h>
+
+#include "log.h"
+#include "timens.h"
+
+int run_test(int clockid, struct timespec now)
+{
+	struct itimerspec new_value;
+	long long elapsed;
+	timer_t fd;
+	int i;
+
+	for (i = 0; i < 2; i++) {
+		struct sigevent sevp = {.sigev_notify = SIGEV_NONE};
+		int flags = 0;
+
+		new_value.it_value.tv_sec = 3600;
+		new_value.it_value.tv_nsec = 0;
+		new_value.it_interval.tv_sec = 1;
+		new_value.it_interval.tv_nsec = 0;
+
+		if (i == 1) {
+			new_value.it_value.tv_sec += now.tv_sec;
+			new_value.it_value.tv_nsec += now.tv_nsec;
+		}
+
+		if (timer_create(clockid, &sevp, &fd) == -1)
+			return pr_perror("timerfd_create");
+
+		if (i == 1)
+			flags |= TIMER_ABSTIME;
+		if (timer_settime(fd, flags, &new_value, NULL) == -1)
+			return pr_perror("timerfd_settime");
+
+		if (timer_gettime(fd, &new_value) == -1)
+			return pr_perror("timerfd_gettime");
+
+		elapsed = new_value.it_value.tv_sec;
+		if (abs(elapsed - 3600) > 60) {
+			ksft_test_result_fail("clockid: %d elapsed: %lld\n",
+					      clockid, elapsed);
+			return 1;
+		}
+	}
+
+	ksft_test_result_pass("clockid=%d\n", clockid);
+
+	return 0;
+}
+
+int main(int argc, char *argv[])
+{
+	int ret, status, len, fd;
+	char buf[4096];
+	pid_t pid;
+	struct timespec btime_now, mtime_now;
+
+	nscheck();
+
+	clock_gettime(CLOCK_MONOTONIC, &mtime_now);
+	clock_gettime(CLOCK_BOOTTIME, &btime_now);
+
+	if (unshare(CLONE_NEWTIME))
+		return pr_perror("unshare");
+
+	len = snprintf(buf, sizeof(buf), "%d %d 0\n%d %d 0",
+			CLOCK_MONOTONIC, 70 * 24 * 3600,
+			CLOCK_BOOTTIME, 9 * 24 * 3600);
+	fd = open("/proc/self/timens_offsets", O_WRONLY);
+	if (fd < 0)
+		return pr_perror("/proc/self/timens_offsets");
+
+	if (write(fd, buf, len) != len)
+		return pr_perror("/proc/self/timens_offsets");
+
+	close(fd);
+	mtime_now.tv_sec += 70 * 24 * 3600;
+	btime_now.tv_sec += 9 * 24 * 3600;
+
+	pid = fork();
+	if (pid < 0)
+		return pr_perror("Unable to fork");
+	if (pid == 0) {
+		ret = 0;
+		ret |= run_test(CLOCK_BOOTTIME, btime_now);
+		ret |= run_test(CLOCK_MONOTONIC, mtime_now);
+
+		if (ret)
+			ksft_exit_fail();
+		ksft_exit_pass();
+		return ret;
+	}
+
+	if (waitpid(pid, &status, 0) != pid)
+		return pr_perror("Unable to wait the child process");
+
+	if (WIFEXITED(status))
+		return WEXITSTATUS(status);
+
+	return 1;
+}
+
-- 
2.20.1

^ permalink raw reply related

* [PATCH 27/32] selftest/timens: Add procfs selftest
From: Dmitry Safonov @ 2019-02-06  0:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: Dmitry Safonov, Adrian Reber, Andrei Vagin, Andrei Vagin,
	Andy Lutomirski, Andy Tucker, Arnd Bergmann, Christian Brauner,
	Cyrill Gorcunov, Dmitry Safonov, Eric W. Biederman,
	H. Peter Anvin, Ingo Molnar, Jeff Dike, Oleg Nesterov,
	Pavel Emelyanov, Shuah Khan, Thomas Gleixner, containers, criu,
	linux-api, x86
In-Reply-To: <20190206001107.16488-1-dima@arista.com>

Check that /proc/uptime is correct inside a new time namespace.

Co-developed-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 tools/testing/selftests/timens/.gitignore |   1 +
 tools/testing/selftests/timens/Makefile   |   2 +-
 tools/testing/selftests/timens/procfs.c   | 142 ++++++++++++++++++++++
 3 files changed, 144 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/timens/procfs.c

diff --git a/tools/testing/selftests/timens/.gitignore b/tools/testing/selftests/timens/.gitignore
index 9b6c8ddac2c8..94ffdd9cead7 100644
--- a/tools/testing/selftests/timens/.gitignore
+++ b/tools/testing/selftests/timens/.gitignore
@@ -1,3 +1,4 @@
 clock_nanosleep
+procfs
 timens
 timerfd
diff --git a/tools/testing/selftests/timens/Makefile b/tools/testing/selftests/timens/Makefile
index 76a1dc891184..f96f50d1fef8 100644
--- a/tools/testing/selftests/timens/Makefile
+++ b/tools/testing/selftests/timens/Makefile
@@ -1,4 +1,4 @@
-TEST_GEN_PROGS := timens timerfd clock_nanosleep
+TEST_GEN_PROGS := timens timerfd clock_nanosleep procfs
 
 CFLAGS := -Wall -Werror
 
diff --git a/tools/testing/selftests/timens/procfs.c b/tools/testing/selftests/timens/procfs.c
new file mode 100644
index 000000000000..af839688ecc6
--- /dev/null
+++ b/tools/testing/selftests/timens/procfs.c
@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <errno.h>
+#include <fcntl.h>
+#include <math.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <time.h>
+#include <unistd.h>
+#include <time.h>
+
+#include "log.h"
+#include "timens.h"
+
+/*
+ * Test shouldn't be run for a day, so add 10 days to child
+ * time and check parent's time to be in the same day.
+ */
+#define MAX_TEST_TIME_SEC		(60*5)
+#define DAY_IN_SEC			(60*60*24)
+#define TEN_DAYS_IN_SEC			(10*DAY_IN_SEC)
+
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
+
+static int child_ns, parent_ns;
+
+static int switch_ns(int fd)
+{
+	if (setns(fd, CLONE_NEWTIME))
+		return pr_perror("setns()");
+
+	return 0;
+}
+
+static int init_namespaces(void)
+{
+	char path[] = "/proc/self/ns/time_for_children";
+	struct stat st1, st2;
+
+	parent_ns = open(path, O_RDONLY);
+	if (parent_ns <= 0)
+		return pr_perror("Unable to open %s", path);
+
+	if (fstat(parent_ns, &st1))
+		return pr_perror("Unable to stat the parent timens");
+
+	if (unshare(CLONE_NEWTIME))
+		return pr_perror("Can't unshare() timens");
+
+	child_ns = open(path, O_RDONLY);
+	if (child_ns <= 0)
+		return pr_perror("Unable to open %s", path);
+
+	if (fstat(child_ns, &st2))
+		return pr_perror("Unable to stat the timens");
+
+	if (st1.st_ino == st2.st_ino)
+		return pr_err("The same child_ns after CLONE_NEWTIME");
+
+	if (_settime(CLOCK_BOOTTIME, TEN_DAYS_IN_SEC))
+		return -1;
+
+	return 0;
+}
+
+static int read_proc_uptime(struct timespec *uptime)
+{
+	unsigned long up_sec, up_nsec;
+	FILE *proc;
+
+	proc = fopen("/proc/uptime", "r");
+	if (proc == NULL) {
+		pr_perror("Unable to open /proc/uptime");
+		return -1;
+	}
+
+	if (fscanf(proc, "%lu.%02lu", &up_sec, &up_nsec) != 2) {
+		if (errno) {
+			pr_perror("fscanf");
+			return -errno;
+		}
+		pr_err("failed to parse /proc/uptime");
+		return -1;
+	}
+	fclose(proc);
+
+	uptime->tv_sec = up_sec;
+	uptime->tv_nsec = up_nsec;
+	return 0;
+}
+
+static int check_uptime(void)
+{
+	struct timespec uptime_new, uptime_old;
+	time_t uptime_expected;
+	double prec = MAX_TEST_TIME_SEC;
+
+	if (switch_ns(parent_ns))
+		return pr_err("switch_ns(%d)", parent_ns);
+
+	if (read_proc_uptime(&uptime_old))
+		return 1;
+
+	if (switch_ns(child_ns))
+		return pr_err("switch_ns(%d)", child_ns);
+
+	if (read_proc_uptime(&uptime_new))
+		return 1;
+
+	uptime_expected = uptime_old.tv_sec + TEN_DAYS_IN_SEC;
+	if (fabs(difftime(uptime_new.tv_sec, uptime_expected)) > prec) {
+		pr_fail("uptime in /proc/uptime: old %ld, new %ld [%ld]",
+			uptime_old.tv_sec, uptime_new.tv_sec,
+			uptime_old.tv_sec + TEN_DAYS_IN_SEC);
+		return 1;
+	}
+
+	ksft_test_result_pass("Passed for /proc/uptime");
+	return 0;
+}
+
+int main(int argc, char *argv[])
+{
+	int ret = 0;
+
+	nscheck();
+
+	if (init_namespaces())
+		return 1;
+
+	ret |= check_uptime();
+
+	if (ret)
+		ksft_exit_fail();
+	ksft_exit_pass();
+	return ret;
+}
-- 
2.20.1

^ permalink raw reply related

* [PATCH 26/32] selftest/timens: Add a test for clock_nanosleep()
From: Dmitry Safonov @ 2019-02-06  0:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrei Vagin, Dmitry Safonov, Adrian Reber, Andrei Vagin,
	Andy Lutomirski, Andy Tucker, Arnd Bergmann, Christian Brauner,
	Cyrill Gorcunov, Dmitry Safonov, Eric W. Biederman,
	H. Peter Anvin, Ingo Molnar, Jeff Dike, Oleg Nesterov,
	Pavel Emelyanov, Shuah Khan, Thomas Gleixner, containers, criu,
	linux-api, x86
In-Reply-To: <20190206001107.16488-1-dima@arista.com>

From: Andrei Vagin <avagin@gmail.com>

Check that clock_nanosleep() takes into account clock offsets.

Signed-off-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 tools/testing/selftests/timens/.gitignore     |  1 +
 tools/testing/selftests/timens/Makefile       |  2 +-
 .../selftests/timens/clock_nanosleep.c        | 99 +++++++++++++++++++
 3 files changed, 101 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/timens/clock_nanosleep.c

diff --git a/tools/testing/selftests/timens/.gitignore b/tools/testing/selftests/timens/.gitignore
index b609f6ee9fb9..9b6c8ddac2c8 100644
--- a/tools/testing/selftests/timens/.gitignore
+++ b/tools/testing/selftests/timens/.gitignore
@@ -1,2 +1,3 @@
+clock_nanosleep
 timens
 timerfd
diff --git a/tools/testing/selftests/timens/Makefile b/tools/testing/selftests/timens/Makefile
index 66b90cd28e5c..76a1dc891184 100644
--- a/tools/testing/selftests/timens/Makefile
+++ b/tools/testing/selftests/timens/Makefile
@@ -1,4 +1,4 @@
-TEST_GEN_PROGS := timens timerfd
+TEST_GEN_PROGS := timens timerfd clock_nanosleep
 
 CFLAGS := -Wall -Werror
 
diff --git a/tools/testing/selftests/timens/clock_nanosleep.c b/tools/testing/selftests/timens/clock_nanosleep.c
new file mode 100644
index 000000000000..9a1689e5a0e1
--- /dev/null
+++ b/tools/testing/selftests/timens/clock_nanosleep.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <sched.h>
+
+#include <sys/timerfd.h>
+#include <sys/syscall.h>
+#include <time.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <stdint.h>
+
+#include "log.h"
+#include "timens.h"
+
+static long long get_elapsed_time(int clockid, struct timespec *start)
+{
+	struct timespec curr;
+	long long secs, nsecs;
+
+	if (clock_gettime(clockid, &curr) == -1)
+		return pr_perror("clock_gettime");
+
+	secs = curr.tv_sec - start->tv_sec;
+	nsecs = curr.tv_nsec - start->tv_nsec;
+	if (nsecs < 0) {
+		secs--;
+		nsecs += 1000000000;
+	}
+	if (nsecs > 1000000000) {
+		secs++;
+		nsecs -= 1000000000;
+	}
+	return secs * 1000 + nsecs / 1000000;
+}
+
+int run_test(int clockid)
+{
+	long long elapsed;
+	int i;
+
+	for (i = 0; i < 2; i++) {
+		struct timespec now = {};
+		struct timespec start;
+
+		if (clock_gettime(clockid, &start) == -1)
+			return pr_perror("clock_gettime");
+
+
+		if (i == 1) {
+			now.tv_sec = start.tv_sec;
+			now.tv_nsec = start.tv_nsec;
+		}
+
+		now.tv_sec += 2;
+		clock_nanosleep(clockid, i ? TIMER_ABSTIME : 0, &now, NULL);
+
+		elapsed = get_elapsed_time(clockid, &start);
+		if (elapsed < 1900 || elapsed > 2100) {
+			pr_fail("clockid: %d abs: %d elapsed: %lld\n",
+				clockid, i, elapsed);
+			return 1;
+		}
+		ksft_test_result_pass("clockid: %d abs:%d\n", clockid, i);
+	}
+
+	return 0;
+}
+
+int main(int argc, char *argv[])
+{
+	int ret, nsfd;
+
+	nscheck();
+
+	if (unshare(CLONE_NEWTIME))
+		return pr_perror("unshare");
+
+	if (_settime(CLOCK_MONOTONIC, 7 * 24 * 3600))
+		return 1;
+	if (_settime(CLOCK_BOOTTIME, 9 * 24 * 3600))
+		return 1;
+
+	nsfd = open("/proc/self/ns/time_for_children", O_RDONLY);
+	if (nsfd < 0)
+		return pr_perror("Unable to open timens_for_children");
+
+	if (setns(nsfd, CLONE_NEWTIME))
+		return pr_perror("Unable to set timens");
+
+	ret = 0;
+	ret |= run_test(CLOCK_MONOTONIC);
+
+	if (ret)
+		ksft_exit_fail();
+	ksft_exit_pass();
+	return ret;
+}
+
-- 
2.20.1

^ permalink raw reply related

* [PATCH 25/32] selftest/timens: Add a test for timerfd
From: Dmitry Safonov @ 2019-02-06  0:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrei Vagin, Dmitry Safonov, Adrian Reber, Andrei Vagin,
	Andy Lutomirski, Andy Tucker, Arnd Bergmann, Christian Brauner,
	Cyrill Gorcunov, Dmitry Safonov, Eric W. Biederman,
	H. Peter Anvin, Ingo Molnar, Jeff Dike, Oleg Nesterov,
	Pavel Emelyanov, Shuah Khan, Thomas Gleixner, containers, criu,
	linux-api, x86
In-Reply-To: <20190206001107.16488-1-dima@arista.com>

From: Andrei Vagin <avagin@gmail.com>

Check that timerfd_create() takes into account clock offsets.

Signed-off-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 tools/testing/selftests/timens/.gitignore |   1 +
 tools/testing/selftests/timens/Makefile   |   2 +-
 tools/testing/selftests/timens/timerfd.c  | 119 ++++++++++++++++++++++
 3 files changed, 121 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/timens/timerfd.c

diff --git a/tools/testing/selftests/timens/.gitignore b/tools/testing/selftests/timens/.gitignore
index 27a693229ce1..b609f6ee9fb9 100644
--- a/tools/testing/selftests/timens/.gitignore
+++ b/tools/testing/selftests/timens/.gitignore
@@ -1 +1,2 @@
 timens
+timerfd
diff --git a/tools/testing/selftests/timens/Makefile b/tools/testing/selftests/timens/Makefile
index b877efb78974..66b90cd28e5c 100644
--- a/tools/testing/selftests/timens/Makefile
+++ b/tools/testing/selftests/timens/Makefile
@@ -1,4 +1,4 @@
-TEST_GEN_PROGS := timens
+TEST_GEN_PROGS := timens timerfd
 
 CFLAGS := -Wall -Werror
 
diff --git a/tools/testing/selftests/timens/timerfd.c b/tools/testing/selftests/timens/timerfd.c
new file mode 100644
index 000000000000..8ec2604d26c9
--- /dev/null
+++ b/tools/testing/selftests/timens/timerfd.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <sched.h>
+
+#include <sys/timerfd.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <time.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <stdint.h>
+
+#include "log.h"
+#include "timens.h"
+
+int run_test(int clockid, struct timespec now)
+{
+	struct itimerspec new_value;
+	long long elapsed;
+	int fd, i;
+
+	if (clock_gettime(clockid, &now))
+		return pr_perror("clock_gettime");
+
+	for (i = 0; i < 2; i++) {
+		int flags = 0;
+
+		new_value.it_value.tv_sec = 3600;
+		new_value.it_value.tv_nsec = 0;
+		new_value.it_interval.tv_sec = 1;
+		new_value.it_interval.tv_nsec = 0;
+
+		if (i == 1) {
+			new_value.it_value.tv_sec += now.tv_sec;
+			new_value.it_value.tv_nsec += now.tv_nsec;
+		}
+
+		fd = timerfd_create(clockid, 0);
+		if (fd == -1)
+			return pr_perror("timerfd_create");
+
+		if (i == 1)
+			flags |= TFD_TIMER_ABSTIME;
+
+		if (timerfd_settime(fd, flags, &new_value, NULL))
+			return pr_perror("timerfd_settime");
+
+		if (timerfd_gettime(fd, &new_value))
+			return pr_perror("timerfd_gettime");
+
+		elapsed = new_value.it_value.tv_sec;
+		if (abs(elapsed - 3600) > 60) {
+			ksft_test_result_fail("clockid: %d elapsed: %lld\n",
+					      clockid, elapsed);
+			return 1;
+		}
+
+		close(fd);
+	}
+
+	ksft_test_result_pass("clockid=%d\n", clockid);
+
+	return 0;
+}
+
+int main(int argc, char *argv[])
+{
+	int ret, status, len, fd;
+	char buf[4096];
+	pid_t pid;
+	struct timespec btime_now, mtime_now;
+
+	nscheck();
+
+	clock_gettime(CLOCK_MONOTONIC, &mtime_now);
+	clock_gettime(CLOCK_BOOTTIME, &btime_now);
+
+	if (unshare(CLONE_NEWTIME))
+		return pr_perror("unshare");
+
+	len = snprintf(buf, sizeof(buf), "%d %d 0\n%d %d 0",
+			CLOCK_MONOTONIC, 70 * 24 * 3600,
+			CLOCK_BOOTTIME, 9 * 24 * 3600);
+	fd = open("/proc/self/timens_offsets", O_WRONLY);
+	if (fd < 0)
+		return pr_perror("/proc/self/timens_offsets");
+
+	if (write(fd, buf, len) != len)
+		return pr_perror("/proc/self/timens_offsets");
+
+	close(fd);
+	mtime_now.tv_sec += 70 * 24 * 3600;
+	btime_now.tv_sec += 9 * 24 * 3600;
+
+	pid = fork();
+	if (pid < 0)
+		return pr_perror("Unable to fork");
+	if (pid == 0) {
+		ret = 0;
+		ret |= run_test(CLOCK_BOOTTIME, btime_now);
+		ret |= run_test(CLOCK_MONOTONIC, mtime_now);
+
+		if (ret)
+			ksft_exit_fail();
+		ksft_exit_pass();
+		return ret;
+	}
+
+	if (waitpid(pid, &status, 0) != pid)
+		return pr_perror("Unable to wait the child process");
+
+	if (WIFEXITED(status))
+		return WEXITSTATUS(status);
+
+	return 1;
+}
+
-- 
2.20.1

^ permalink raw reply related

* [PATCH 24/32] selftest/timens: Add Time Namespace test for supported clocks
From: Dmitry Safonov @ 2019-02-06  0:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: Dmitry Safonov, Adrian Reber, Andrei Vagin, Andrei Vagin,
	Andy Lutomirski, Andy Tucker, Arnd Bergmann, Christian Brauner,
	Cyrill Gorcunov, Dmitry Safonov, Eric W. Biederman,
	H. Peter Anvin, Ingo Molnar, Jeff Dike, Oleg Nesterov,
	Pavel Emelyanov, Shuah Khan, Thomas Gleixner, containers, criu,
	linux-api, x86
In-Reply-To: <20190206001107.16488-1-dima@arista.com>

A test to check that all supported clocks work on host and inside
a new time namespace. Use both ways to get time: through VDSO and
by entering the kernel with implicit syscall.

Introduce a new timens directory in selftests framework for
the next timens tests.

Co-developed-by: Andrei Vagin <avagin@openvz.org>
Signed-off-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 tools/testing/selftests/Makefile          |   1 +
 tools/testing/selftests/timens/.gitignore |   1 +
 tools/testing/selftests/timens/Makefile   |   5 +
 tools/testing/selftests/timens/config     |   1 +
 tools/testing/selftests/timens/log.h      |  26 +++
 tools/testing/selftests/timens/timens.c   | 191 ++++++++++++++++++++++
 tools/testing/selftests/timens/timens.h   |  63 +++++++
 7 files changed, 288 insertions(+)
 create mode 100644 tools/testing/selftests/timens/.gitignore
 create mode 100644 tools/testing/selftests/timens/Makefile
 create mode 100644 tools/testing/selftests/timens/config
 create mode 100644 tools/testing/selftests/timens/log.h
 create mode 100644 tools/testing/selftests/timens/timens.c
 create mode 100644 tools/testing/selftests/timens/timens.h

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 1a2bd15c5b6e..cccbe89983fa 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -47,6 +47,7 @@ TARGETS += sysctl
 ifneq (1, $(quicktest))
 TARGETS += timers
 endif
+TARGETS += timens
 TARGETS += user
 TARGETS += vm
 TARGETS += x86
diff --git a/tools/testing/selftests/timens/.gitignore b/tools/testing/selftests/timens/.gitignore
new file mode 100644
index 000000000000..27a693229ce1
--- /dev/null
+++ b/tools/testing/selftests/timens/.gitignore
@@ -0,0 +1 @@
+timens
diff --git a/tools/testing/selftests/timens/Makefile b/tools/testing/selftests/timens/Makefile
new file mode 100644
index 000000000000..b877efb78974
--- /dev/null
+++ b/tools/testing/selftests/timens/Makefile
@@ -0,0 +1,5 @@
+TEST_GEN_PROGS := timens
+
+CFLAGS := -Wall -Werror
+
+include ../lib.mk
diff --git a/tools/testing/selftests/timens/config b/tools/testing/selftests/timens/config
new file mode 100644
index 000000000000..4480620f6f49
--- /dev/null
+++ b/tools/testing/selftests/timens/config
@@ -0,0 +1 @@
+CONFIG_TIME_NS=y
diff --git a/tools/testing/selftests/timens/log.h b/tools/testing/selftests/timens/log.h
new file mode 100644
index 000000000000..85b54bfa50c5
--- /dev/null
+++ b/tools/testing/selftests/timens/log.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __SELFTEST_TIMENS_LOG_H__
+#define __SELFTEST_TIMENS_LOG_H__
+
+#define pr_msg(fmt, lvl, ...)						\
+	ksft_print_msg("[%s] (%s:%d)\t" fmt "\n",			\
+			lvl, __FILE__, __LINE__, ##__VA_ARGS__)
+
+#define pr_p(func, fmt, ...)	func(fmt ": %m", ##__VA_ARGS__)
+
+#define pr_err(fmt, ...)						\
+	({								\
+		ksft_test_result_error(fmt, ##__VA_ARGS__);		\
+		-1;							\
+	})
+
+#define pr_fail(fmt, ...)					\
+	({							\
+		ksft_test_result_fail(fmt, ##__VA_ARGS__);	\
+		-1;						\
+	})
+
+#define pr_perror(fmt, ...)	pr_p(pr_err, fmt, ##__VA_ARGS__)
+
+#endif
diff --git a/tools/testing/selftests/timens/timens.c b/tools/testing/selftests/timens/timens.c
new file mode 100644
index 000000000000..334bdefe01a3
--- /dev/null
+++ b/tools/testing/selftests/timens/timens.c
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <errno.h>
+#include <fcntl.h>
+#include <sched.h>
+#include <stdio.h>
+#include <stdbool.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <time.h>
+#include <unistd.h>
+#include <time.h>
+#include <string.h>
+
+#include "log.h"
+#include "timens.h"
+
+/*
+ * Test shouldn't be run for a day, so add 10 days to child
+ * time and check parent's time to be in the same day.
+ */
+#define DAY_IN_SEC			(60*60*24)
+#define TEN_DAYS_IN_SEC			(10*DAY_IN_SEC)
+
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
+
+#define CLOCK_TYPES							\
+	ct(CLOCK_BOOTTIME, -1),						\
+	ct(CLOCK_MONOTONIC, -1),					\
+	ct(CLOCK_MONOTONIC_COARSE, 1),					\
+	ct(CLOCK_MONOTONIC_RAW, 1),					\
+
+
+struct test_clock {
+	clockid_t id;
+	char *name;
+	/*
+	 * off_id is -1 if a clock has own offset, or it contains an index
+	 * which contains a right offset of this clock.
+	 */
+	int off_id;
+	time_t offset;
+};
+
+#define ct(clock, off_id)	{ clock, #clock, off_id }
+static struct test_clock clocks[] = {
+	CLOCK_TYPES
+};
+#undef ct
+
+static int child_ns, parent_ns = -1;
+
+static int switch_ns(int fd)
+{
+	if (setns(fd, CLONE_NEWTIME)) {
+		pr_perror("setns()");
+		return -1;
+	}
+
+	return 0;
+}
+
+static int init_namespaces(void)
+{
+	char path[] = "/proc/self/ns/time_for_children";
+	struct stat st1, st2;
+
+	if (parent_ns == -1) {
+		parent_ns = open(path, O_RDONLY);
+		if (parent_ns <= 0)
+			return pr_perror("Unable to open %s", path);
+	}
+
+	if (fstat(parent_ns, &st1))
+		return pr_perror("Unable to stat the parent timens");
+
+	if (unshare(CLONE_NEWTIME))
+		return pr_perror("Can't unshare() timens");
+
+	child_ns = open(path, O_RDONLY);
+	if (child_ns <= 0)
+		return pr_perror("Unable to open %s", path);
+
+	if (fstat(child_ns, &st2))
+		return pr_perror("Unable to stat the timens");
+
+	if (st1.st_ino == st2.st_ino)
+		return pr_perror("The same child_ns after CLONE_NEWTIME");
+
+	return 0;
+}
+
+static int test_gettime(clockid_t clock_index, bool raw_syscall, time_t offset)
+{
+	struct timespec child_ts_new, parent_ts_old, cur_ts;
+	char *entry = raw_syscall ? "syscall" : "vdso";
+	double precision = 0.0;
+
+	switch (clocks[clock_index].id) {
+	case CLOCK_MONOTONIC_COARSE:
+	case CLOCK_MONOTONIC_RAW:
+		precision = -2.0;
+		break;
+	}
+
+	if (switch_ns(parent_ns))
+		return pr_err("switch_ns(%d)", child_ns);
+
+	if (_gettime(clocks[clock_index].id, &parent_ts_old, raw_syscall))
+		return -1;
+
+	child_ts_new.tv_nsec = parent_ts_old.tv_nsec;
+	child_ts_new.tv_sec = parent_ts_old.tv_sec + offset;
+
+	if (switch_ns(child_ns))
+		return pr_err("switch_ns(%d)", child_ns);
+
+	if (_gettime(clocks[clock_index].id, &cur_ts, raw_syscall))
+		return -1;
+
+	if (difftime(cur_ts.tv_sec, child_ts_new.tv_sec) < precision) {
+		ksft_test_result_fail(
+			"Child's %s (%s) time has not changed: %lu -> %lu [%lu]\n",
+			clocks[clock_index].name, entry, parent_ts_old.tv_sec,
+			child_ts_new.tv_sec, cur_ts.tv_sec);
+		return -1;
+	}
+
+	if (switch_ns(parent_ns))
+		return pr_err("switch_ns(%d)", parent_ns);
+
+	if (_gettime(clocks[clock_index].id, &cur_ts, raw_syscall))
+		return -1;
+
+	if (difftime(cur_ts.tv_sec, parent_ts_old.tv_sec) > DAY_IN_SEC) {
+		ksft_test_result_fail(
+			"Parent's %s (%s) time has changed: %lu -> %lu [%lu]\n",
+			clocks[clock_index].name, entry, parent_ts_old.tv_sec,
+			child_ts_new.tv_sec, cur_ts.tv_sec);
+		/* Let's play nice and put it closer to original */
+		clock_settime(clocks[clock_index].id, &cur_ts);
+		return -1;
+	}
+
+	ksft_test_result_pass("Passed for %s (%s)\n",
+				clocks[clock_index].name, entry);
+	return 0;
+}
+
+int main(int argc, char *argv[])
+{
+	unsigned int i, j;
+	int ret = 0;
+
+	nscheck();
+
+	for (j = 0; j < 2; j++) {
+		time_t offset;
+
+		if (init_namespaces())
+			return 1;
+
+		/* Offsets have to be set before tasks enter the namespace. */
+		for (i = 0; i < ARRAY_SIZE(clocks); i++) {
+			if (clocks[i].off_id != -1)
+				continue;
+			offset = TEN_DAYS_IN_SEC + i * 1000;
+			if (j > 0)
+				offset = -offset;
+			clocks[i].offset = offset;
+			if (_settime(clocks[i].id, offset))
+				return 1;
+		}
+
+		for (i = 0; i < ARRAY_SIZE(clocks); i++) {
+			if (clocks[i].off_id != -1)
+				offset = clocks[clocks[i].off_id].offset;
+			else
+				offset = clocks[i].offset;
+			ret |= test_gettime(i, true, offset);
+			ret |= test_gettime(i, false, offset);
+		}
+	}
+
+	if (ret)
+		ksft_exit_fail();
+
+	ksft_exit_pass();
+	return !!ret;
+}
diff --git a/tools/testing/selftests/timens/timens.h b/tools/testing/selftests/timens/timens.h
new file mode 100644
index 000000000000..71a0ad78c634
--- /dev/null
+++ b/tools/testing/selftests/timens/timens.h
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __TIMENS_H__
+#define __TIMENS_H__
+
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdbool.h>
+
+#include "../kselftest.h"
+
+#ifndef CLONE_NEWTIME
+# define CLONE_NEWTIME	0x00001000
+#endif
+
+static inline int _settime(clockid_t clk_id, time_t offset)
+{
+	int fd, len;
+	char buf[4096];
+
+	if (clk_id == CLOCK_MONOTONIC_COARSE || clk_id == CLOCK_MONOTONIC_RAW)
+		clk_id = CLOCK_MONOTONIC;
+
+	len = snprintf(buf, sizeof(buf), "%d %ld 0", clk_id, offset);
+
+	fd = open("/proc/self/timens_offsets", O_WRONLY);
+	if (fd < 0)
+		return pr_perror("/proc/self/timens_offsets");
+
+	if (write(fd, buf, len) != len)
+		return pr_perror("/proc/self/timens_offsets");
+
+	close(fd);
+
+	return 0;
+}
+
+static inline int _gettime(clockid_t clk_id, struct timespec *res, bool raw_syscall)
+{
+	int err;
+
+	if (!raw_syscall) {
+		if (clock_gettime(clk_id, res)) {
+			pr_perror("clock_gettime(%d)", (int)clk_id);
+			return -1;
+		}
+		return 0;
+	}
+
+	err = syscall(SYS_clock_gettime, clk_id, res);
+	if (err)
+		pr_perror("syscall(SYS_clock_gettime(%d))", (int)clk_id);
+
+	return err;
+}
+
+static inline void nscheck(void)
+{
+	if (access("/proc/self/ns/time", F_OK) < 0)
+		ksft_exit_skip("Time namespaces are not supported\n");
+}
+
+#endif
-- 
2.20.1

^ permalink raw reply related

* [PATCH 23/32] timens/fs/proc: Introduce /proc/pid/timens_offsets
From: Dmitry Safonov @ 2019-02-06  0:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrei Vagin, Dmitry Safonov, Adrian Reber, Andrei Vagin,
	Andy Lutomirski, Andy Tucker, Arnd Bergmann, Christian Brauner,
	Cyrill Gorcunov, Dmitry Safonov, Eric W. Biederman,
	H. Peter Anvin, Ingo Molnar, Jeff Dike, Oleg Nesterov,
	Pavel Emelyanov, Shuah Khan, Thomas Gleixner, containers, criu,
	linux-api, x86
In-Reply-To: <20190206001107.16488-1-dima@arista.com>

From: Andrei Vagin <avagin@gmail.com>

API to set time namespace offsets for children processes, i.e.:
echo "clockid off_ses off_nsec" > /proc/self/timens_offsets

Signed-off-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 fs/proc/base.c                 | 101 +++++++++++++++++++++++++++++++++
 include/linux/time_namespace.h |  10 ++++
 kernel/time_namespace.c        |  71 +++++++++++++++++++++++
 3 files changed, 182 insertions(+)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 633a63462573..1ba31050dcb5 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -94,6 +94,7 @@
 #include <linux/sched/stat.h>
 #include <linux/flex_array.h>
 #include <linux/posix-timers.h>
+#include <linux/time_namespace.h>
 #include <trace/events/oom.h>
 #include "internal.h"
 #include "fd.h"
@@ -1520,6 +1521,103 @@ static const struct file_operations proc_pid_sched_autogroup_operations = {
 
 #endif /* CONFIG_SCHED_AUTOGROUP */
 
+#ifdef CONFIG_TIME_NS
+static int timens_offsets_show(struct seq_file *m, void *v)
+{
+	struct inode *inode = m->private;
+	struct task_struct *p;
+
+	p = get_proc_task(inode);
+	if (!p)
+		return -ESRCH;
+	proc_timens_show_offsets(p, m);
+
+	put_task_struct(p);
+
+	return 0;
+}
+
+static ssize_t
+timens_offsets_write(struct file *file, const char __user *buf,
+	    size_t count, loff_t *ppos)
+{
+	struct inode *inode = file_inode(file);
+	struct proc_timens_offset offsets[2];
+	char *kbuf = NULL, *pos, *next_line;
+	struct task_struct *p;
+	int ret, noffsets;
+
+	/* Only allow < page size writes at the beginning of the file */
+	if ((*ppos != 0) || (count >= PAGE_SIZE))
+		return -EINVAL;
+
+	/* Slurp in the user data */
+	kbuf = memdup_user_nul(buf, count);
+	if (IS_ERR(kbuf))
+		return PTR_ERR(kbuf);
+
+	/* Parse the user data */
+	ret = -EINVAL;
+	noffsets = 0;
+	pos = kbuf;
+	for (; pos; pos = next_line) {
+		struct proc_timens_offset *off = &offsets[noffsets];
+		int err;
+
+		/* Find the end of line and ensure I don't look past it */
+		next_line = strchr(pos, '\n');
+		if (next_line) {
+			*next_line = '\0';
+			next_line++;
+			if (*next_line == '\0')
+				next_line = NULL;
+		}
+
+		err = sscanf(pos, "%u %lld %lu", &off->clockid,
+				&off->val.tv_sec, &off->val.tv_nsec);
+		if (err != 3 || off->val.tv_nsec >= NSEC_PER_SEC)
+			goto out;
+		if (noffsets++ == ARRAY_SIZE(offsets))
+			break;
+	}
+
+	ret = -ESRCH;
+	p = get_proc_task(inode);
+	if (!p)
+		goto out;
+	ret = proc_timens_set_offset(p, offsets, noffsets);
+	put_task_struct(p);
+	if (ret)
+		goto out;
+
+	ret = count;
+out:
+	kfree(kbuf);
+	return ret;
+}
+
+static int timens_offsets_open(struct inode *inode, struct file *filp)
+{
+	int ret;
+
+	ret = single_open(filp, timens_offsets_show, NULL);
+	if (!ret) {
+		struct seq_file *m = filp->private_data;
+
+		m->private = inode;
+	}
+	return ret;
+}
+
+static const struct file_operations proc_timens_offsets_operations = {
+	.open		= timens_offsets_open,
+	.read		= seq_read,
+	.write		= timens_offsets_write,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+#endif /* CONFIG_TIME_NS */
+
 static ssize_t comm_write(struct file *file, const char __user *buf,
 				size_t count, loff_t *offset)
 {
@@ -2953,6 +3051,9 @@ static const struct pid_entry tgid_base_stuff[] = {
 #endif
 #ifdef CONFIG_SCHED_AUTOGROUP
 	REG("autogroup",  S_IRUGO|S_IWUSR, proc_pid_sched_autogroup_operations),
+#endif
+#ifdef CONFIG_TIME_NS
+	REG("timens_offsets",  S_IRUGO|S_IWUSR, proc_timens_offsets_operations),
 #endif
 	REG("comm",      S_IRUGO|S_IWUSR, proc_pid_set_comm_operations),
 #ifdef CONFIG_HAVE_ARCH_TRACEHOOK
diff --git a/include/linux/time_namespace.h b/include/linux/time_namespace.h
index f1807d7f524d..c9ba7366b3d6 100644
--- a/include/linux/time_namespace.h
+++ b/include/linux/time_namespace.h
@@ -41,6 +41,16 @@ static inline void put_time_ns(struct time_namespace *ns)
 }
 
 
+extern void proc_timens_show_offsets(struct task_struct *p, struct seq_file *m);
+
+struct proc_timens_offset {
+	int clockid;
+	struct timespec64 val;
+};
+
+extern int proc_timens_set_offset(struct task_struct *p,
+				struct proc_timens_offset *offsets, int n);
+
 extern void timens_clock_to_host(int clockid, struct timespec64 *val);
 extern void timens_clock_from_host(int clockid, struct timespec64 *val);
 
diff --git a/kernel/time_namespace.c b/kernel/time_namespace.c
index 1d1d1c023ec1..6e2e6629e1ba 100644
--- a/kernel/time_namespace.c
+++ b/kernel/time_namespace.c
@@ -13,6 +13,7 @@
 #include <linux/user_namespace.h>
 #include <linux/proc_ns.h>
 #include <linux/sched/task.h>
+#include <linux/seq_file.h>
 #include <linux/mm.h>
 #include <asm/vdso.h>
 
@@ -202,6 +203,76 @@ static struct user_namespace *timens_owner(struct ns_common *ns)
 	return to_time_ns(ns)->user_ns;
 }
 
+static void show_offset(struct seq_file *m, int clockid, struct timespec64 *ts)
+{
+	seq_printf(m, "%d %lld %ld\n", clockid, ts->tv_sec, ts->tv_nsec);
+}
+
+void proc_timens_show_offsets(struct task_struct *p, struct seq_file *m)
+{
+	struct ns_common *ns;
+	struct time_namespace *time_ns;
+	struct timens_offsets *ns_offsets;
+
+	ns = timens_for_children_get(p);
+	if (!ns)
+		return;
+	time_ns = to_time_ns(ns);
+
+	if (!time_ns->offsets) {
+		put_time_ns(time_ns);
+		return;
+	}
+	ns_offsets = time_ns->offsets;
+
+	show_offset(m, CLOCK_MONOTONIC, &ns_offsets->monotonic_time_offset);
+	show_offset(m, CLOCK_BOOTTIME, &ns_offsets->monotonic_boottime_offset);
+	put_time_ns(time_ns);
+}
+
+int proc_timens_set_offset(struct task_struct *p,
+			   struct proc_timens_offset *offsets, int noffsets)
+{
+	struct ns_common *ns;
+	struct time_namespace *time_ns;
+	struct timens_offsets *ns_offsets;
+	int i, err;
+
+	ns = timens_for_children_get(p);
+	if (!ns)
+		return -ESRCH;
+	time_ns = to_time_ns(ns);
+
+	if (!time_ns->offsets || time_ns->initialized ||
+	    !ns_capable(time_ns->user_ns, CAP_SYS_TIME)) {
+		put_time_ns(time_ns);
+		return -EPERM;
+	}
+	ns_offsets = time_ns->offsets;
+
+	err = -EINVAL;
+	for (i = 0; i < noffsets; i++) {
+		struct proc_timens_offset *off = &offsets[i];
+
+		switch (off->clockid) {
+		case CLOCK_MONOTONIC:
+			ns_offsets->monotonic_time_offset = off->val;
+			break;
+		case CLOCK_BOOTTIME:
+			ns_offsets->monotonic_boottime_offset = off->val;
+			break;
+		default:
+			goto out;
+		}
+	}
+
+	err = 0;
+out:
+	put_time_ns(time_ns);
+
+	return err;
+}
+
 static void clock_timens_fixup(int clockid, struct timespec64 *val, bool to_ns)
 {
 	struct timens_offsets *ns_offsets = current->nsproxy->time_ns->offsets;
-- 
2.20.1

^ permalink raw reply related

* [PATCH 22/32] timens: Add align for timens_offsets
From: Dmitry Safonov @ 2019-02-06  0:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: Dmitry Safonov, Adrian Reber, Andrei Vagin, Andrei Vagin,
	Andy Lutomirski, Andy Tucker, Arnd Bergmann, Christian Brauner,
	Cyrill Gorcunov, Dmitry Safonov, Eric W. Biederman,
	H. Peter Anvin, Ingo Molnar, Jeff Dike, Oleg Nesterov,
	Pavel Emelyanov, Shuah Khan, Thomas Gleixner, containers, criu,
	linux-api, x86
In-Reply-To: <20190206001107.16488-1-dima@arista.com>

Align offsets so that time namespace will work for ia32 applications on
x86_64 host.

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 include/linux/timens_offsets.h | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/linux/timens_offsets.h b/include/linux/timens_offsets.h
index 777530c46852..f2a03d4f7a91 100644
--- a/include/linux/timens_offsets.h
+++ b/include/linux/timens_offsets.h
@@ -2,9 +2,17 @@
 #ifndef _LINUX_TIME_OFFSETS_H
 #define _LINUX_TIME_OFFSETS_H
 
+/*
+ * Time offsets need align as they're placed on VVAR page,
+ * which is used by x86_64 and ia32 VDSO code.
+ * On ia32 offset::tv_sec (u64) has align(4), so re-align offsets
+ * to the same positions as 64-bit offsets.
+ * On 64-bit big-endian systems VDSO should convert to timespec64
+ * to timespec because of a padding occurring between the fields.
+ */
 struct timens_offsets {
-	struct timespec64  monotonic_time_offset;
-	struct timespec64  monotonic_boottime_offset;
+	struct timespec64  monotonic_time_offset __aligned(8);
+	struct timespec64  monotonic_boottime_offset __aligned(8);
 };
 
 #endif
-- 
2.20.1

^ permalink raw reply related

* [PATCH 21/32] x86/vdso: Switch image on setns()/unshare()/clone()
From: Dmitry Safonov @ 2019-02-06  0:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: Dmitry Safonov, Adrian Reber, Andrei Vagin, Andrei Vagin,
	Andy Lutomirski, Andy Tucker, Arnd Bergmann, Christian Brauner,
	Cyrill Gorcunov, Dmitry Safonov, Eric W. Biederman,
	H. Peter Anvin, Ingo Molnar, Jeff Dike, Oleg Nesterov,
	Pavel Emelyanov, Shuah Khan, Thomas Gleixner, containers, criu,
	linux-api, x86
In-Reply-To: <20190206001107.16488-1-dima@arista.com>

As it has been discussed on timens RFC, adding a new conditional branch
`if (inside_time_ns)` on VDSO for all processes is undesirable.
It will add a penalty for everybody as branch predictor may mispredict
the jump. Also there are instruction cache lines wasted on cmp/jmp.

Those effects of introducing time namespace are very much unwanted
having in mind how much work have been spent on micro-optimisation
vdso code.

Addressing those problems, there are two versions of VDSO's .so:
for host tasks (without any penalty) and for processes inside of time
namespace with clk_to_ns() that subtracts offsets from host's time.

Whenever a user does setns()/unshare() or clone() with CLONE_TIMENS,
change VDSO image in mm and zap existing VVAR/VDSO page tables.
They will be re-faulted with corresponding image and VVAR offsets.

Co-developed-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Andrei Vagin <avagin@gmail.com>
Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 arch/x86/entry/vdso/vma.c   | 81 +++++++++++++++++++++++++++++++++++++
 arch/x86/include/asm/vdso.h |  1 +
 kernel/time_namespace.c     | 11 +++++
 3 files changed, 93 insertions(+)

diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c
index 56a62076a320..52c1e4c24455 100644
--- a/arch/x86/entry/vdso/vma.c
+++ b/arch/x86/entry/vdso/vma.c
@@ -25,6 +25,7 @@
 #include <asm/cpufeature.h>
 #include <asm/mshyperv.h>
 #include <asm/page.h>
+#include <asm/tlb.h>
 
 #if defined(CONFIG_X86_64)
 unsigned int __read_mostly vdso64_enabled = 1;
@@ -150,6 +151,84 @@ static const struct vm_special_mapping vvar_mapping = {
 	.fault = vvar_fault,
 };
 
+#ifdef CONFIG_TIME_NS
+static const struct vdso_image *timens_vdso(const struct vdso_image *old_img,
+					    bool in_ns)
+{
+#ifdef CONFIG_X86_X32_ABI
+	if (old_img == &vdso_image_x32)
+		return NULL;
+#endif
+#if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION
+	if (old_img == &vdso_image_32 || old_img == &vdso_image_32_timens)
+		return in_ns ? &vdso_image_32_timens : &vdso_image_32;
+#endif
+#ifdef CONFIG_X86_64
+	if (old_img == &vdso_image_64 || old_img == &vdso_image_64_timens)
+		return in_ns ? &vdso_image_64_timens : &vdso_image_64;
+#endif
+	return NULL;
+}
+
+static const struct vdso_image *image_to_timens(const struct vdso_image *img)
+{
+	bool in_ns = (current->nsproxy->time_ns != &init_time_ns);
+	const struct vdso_image *ns;
+
+	ns = timens_vdso(img, in_ns);
+
+	return ns ?: img;
+}
+
+int vdso_join_timens(struct task_struct *task, bool inside_ns)
+{
+	const struct vdso_image *new_image, *old_image;
+	struct mm_struct *mm = task->mm;
+	struct vm_area_struct *vma;
+	int ret = 0;
+
+	if (down_write_killable(&mm->mmap_sem))
+		return -EINTR;
+
+	old_image = mm->context.vdso_image;
+	new_image = timens_vdso(old_image, inside_ns);
+	if (!new_image) {
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
+	/* Sanity checks, shouldn't happen */
+	if (unlikely(old_image->size != new_image->size)) {
+		ret = -ENXIO;
+		goto out;
+	}
+
+	mm->context.vdso_image = new_image;
+
+	for (vma = mm->mmap; vma; vma = vma->vm_next) {
+		unsigned long size = vma->vm_end - vma->vm_start;
+
+		if (vma_is_special_mapping(vma, &vvar_mapping))
+			zap_page_range(vma, vma->vm_start, size);
+		if (vma_is_special_mapping(vma, &vdso_mapping))
+			zap_page_range(vma, vma->vm_start, size);
+	}
+
+out:
+	up_write(&mm->mmap_sem);
+	return ret;
+}
+#else /* CONFIG_TIME_NS */
+static const struct vdso_image *image_to_timens(const struct vdso_image *img)
+{
+	return img;
+}
+int vdso_join_timens(struct task_struct *task, bool inside_ns)
+{
+	return -ENXIO;
+}
+#endif
+
 /*
  * Add vdso and vvar mappings to current process.
  * @image          - blob to map
@@ -165,6 +244,8 @@ static int map_vdso(const struct vdso_image *image, unsigned long addr)
 	if (down_write_killable(&mm->mmap_sem))
 		return -EINTR;
 
+	image = image_to_timens(image);
+
 	addr = get_unmapped_area(NULL, addr,
 				 image->size - image->sym_vvar_start, 0, 0);
 	if (IS_ERR_VALUE(addr)) {
diff --git a/arch/x86/include/asm/vdso.h b/arch/x86/include/asm/vdso.h
index b6a1a028ac62..c8db853344a0 100644
--- a/arch/x86/include/asm/vdso.h
+++ b/arch/x86/include/asm/vdso.h
@@ -51,6 +51,7 @@ extern const struct vdso_image vdso_image_32_timens;
 extern void __init init_vdso_image(const struct vdso_image *image);
 
 extern int map_vdso_once(const struct vdso_image *image, unsigned long addr);
+extern int vdso_join_timens(struct task_struct *task, bool inside_ns);
 
 #endif /* __ASSEMBLER__ */
 
diff --git a/kernel/time_namespace.c b/kernel/time_namespace.c
index 36b31f234472..1d1d1c023ec1 100644
--- a/kernel/time_namespace.c
+++ b/kernel/time_namespace.c
@@ -14,6 +14,7 @@
 #include <linux/proc_ns.h>
 #include <linux/sched/task.h>
 #include <linux/mm.h>
+#include <asm/vdso.h>
 
 static struct ucounts *inc_time_namespaces(struct user_namespace *ns)
 {
@@ -155,11 +156,16 @@ static void timens_put(struct ns_common *ns)
 static int timens_install(struct nsproxy *nsproxy, struct ns_common *new)
 {
 	struct time_namespace *ns = to_time_ns(new);
+	int ret;
 
 	if (!ns_capable(ns->user_ns, CAP_SYS_ADMIN) ||
 	    !ns_capable(current_user_ns(), CAP_SYS_ADMIN))
 		return -EPERM;
 
+	ret = vdso_join_timens(current, ns != &init_time_ns);
+	if (ret)
+		return ret;
+
 	get_time_ns(ns);
 	get_time_ns(ns);
 	put_time_ns(nsproxy->time_ns);
@@ -174,10 +180,15 @@ int timens_on_fork(struct nsproxy *nsproxy, struct task_struct *tsk)
 {
 	struct ns_common *nsc = &nsproxy->time_ns_for_children->ns;
 	struct time_namespace *ns = to_time_ns(nsc);
+	int ret;
 
 	if (nsproxy->time_ns == nsproxy->time_ns_for_children)
 		return 0;
 
+	ret = vdso_join_timens(tsk, ns != &init_time_ns);
+	if (ret)
+		return ret;
+
 	get_time_ns(ns);
 	put_time_ns(nsproxy->time_ns);
 	nsproxy->time_ns = ns;
-- 
2.20.1

^ permalink raw reply related

* [PATCH 20/32] x86/vdso: Initialize timens 64-bit vdso
From: Dmitry Safonov @ 2019-02-06  0:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: Dmitry Safonov, Adrian Reber, Andrei Vagin, Andrei Vagin,
	Andy Lutomirski, Andy Tucker, Arnd Bergmann, Christian Brauner,
	Cyrill Gorcunov, Dmitry Safonov, Eric W. Biederman,
	H. Peter Anvin, Ingo Molnar, Jeff Dike, Oleg Nesterov,
	Pavel Emelyanov, Shuah Khan, Thomas Gleixner, containers, criu,
	linux-api, x86
In-Reply-To: <20190206001107.16488-1-dima@arista.com>

Initialize both 64-bit VDSO(s): host .so and timens one that has code
for adding timens offsets.

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 arch/x86/entry/vdso/vma.c   | 4 ++++
 arch/x86/include/asm/vdso.h | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c
index d1031db94093..56a62076a320 100644
--- a/arch/x86/entry/vdso/vma.c
+++ b/arch/x86/entry/vdso/vma.c
@@ -343,6 +343,10 @@ static int __init init_vdso(void)
 {
 	init_vdso_image(&vdso_image_64);
 
+#ifdef CONFIG_TIME_NS
+	init_vdso_image(&vdso_image_64_timens);
+#endif
+
 #ifdef CONFIG_X86_X32_ABI
 	init_vdso_image(&vdso_image_x32);
 #endif
diff --git a/arch/x86/include/asm/vdso.h b/arch/x86/include/asm/vdso.h
index 619322065b8e..b6a1a028ac62 100644
--- a/arch/x86/include/asm/vdso.h
+++ b/arch/x86/include/asm/vdso.h
@@ -32,6 +32,9 @@ struct vdso_image {
 
 #ifdef CONFIG_X86_64
 extern const struct vdso_image vdso_image_64;
+#ifdef CONFIG_TIME_NS
+extern const struct vdso_image vdso_image_64_timens;
+#endif
 #endif
 
 #ifdef CONFIG_X86_X32
@@ -40,6 +43,9 @@ extern const struct vdso_image vdso_image_x32;
 
 #if defined CONFIG_X86_32 || defined CONFIG_COMPAT
 extern const struct vdso_image vdso_image_32;
+#ifdef CONFIG_TIME_NS
+extern const struct vdso_image vdso_image_32_timens;
+#endif
 #endif
 
 extern void __init init_vdso_image(const struct vdso_image *image);
-- 
2.20.1

^ permalink raw reply related

* [PATCH 19/32] x86/vdso2c: Align LOCAL symbols between vdso{-timens,}.so
From: Dmitry Safonov @ 2019-02-06  0:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: Dmitry Safonov, Adrian Reber, Andrei Vagin, Andrei Vagin,
	Andy Lutomirski, Andy Tucker, Arnd Bergmann, Christian Brauner,
	Cyrill Gorcunov, Dmitry Safonov, Eric W. Biederman,
	H. Peter Anvin, Ingo Molnar, Jeff Dike, Oleg Nesterov,
	Pavel Emelyanov, Shuah Khan, Thomas Gleixner, containers, criu,
	linux-api, x86
In-Reply-To: <20190206001107.16488-1-dima@arista.com>

Align not only VDSO entries as on timens VDSO, but also addresses of
local functions. Otherwise, ld will put them after everything else
into *(.text*). That will result in common VDSO size bigger than
timens VDSO size (sic!).

Unfortunately, filtering by STB_WEAK doesn't work for ia32 VDSO:
by some reason gcc transforms weak symbols into local symbols in .so,
i.e.:
    27: 00000000   219 FUNC    WEAK   DEFAULT   12 clock_gettime
    29: 00000000    95 FUNC    WEAK   DEFAULT   14 gettimeofday
    32: 00000000    40 FUNC    WEAK   DEFAULT   16 time

become:
    20: 000006e0   219 FUNC    LOCAL  DEFAULT   12 clock_gettime
    31: 000007c0    95 FUNC    LOCAL  DEFAULT   12 gettimeofday
    33: 00000820    40 FUNC    LOCAL  DEFAULT   12 time

that results in the same align for two functions in .entries file:
		. = ABSOLUTE(0x6e0);
		*(.text.__vdso_clock_gettime*)
		. = ABSOLUTE(0x6e0);
		*(.text.clock_gettime*)

As result, ld becomes a very sad animal and refuses to cooperate:
ld:arch/x86/entry/vdso/vdso32/vdso32.lds:339 cannot move location counter backwards (from 0000000000000762 to 00000000000006e0)

Align local functions on VDSO to timens VDSO and filter weak functions
from .lds script.

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 arch/x86/entry/vdso/vdso2c.h | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/x86/entry/vdso/vdso2c.h b/arch/x86/entry/vdso/vdso2c.h
index 50566dd94451..7096710140fe 100644
--- a/arch/x86/entry/vdso/vdso2c.h
+++ b/arch/x86/entry/vdso/vdso2c.h
@@ -15,7 +15,7 @@ static void BITSFUNC(go)(void *raw_addr, size_t raw_len,
 	unsigned long mapping_size;
 	ELF(Ehdr) *hdr = (ELF(Ehdr) *)raw_addr;
 	unsigned int i, syms_nr;
-	unsigned long j;
+	unsigned long j, last_entry_addr;
 	ELF(Shdr) *symtab_hdr = NULL, *strtab_hdr, *secstrings_hdr,
 		*alt_sec = NULL;
 	ELF(Dyn) *dyn = 0, *dyn_end = 0;
@@ -121,7 +121,7 @@ static void BITSFUNC(go)(void *raw_addr, size_t raw_len,
 		if (!out_entries_lds)
 			continue;
 
-		if (ELF_FUNC(ST_BIND, sym->st_info) != STB_GLOBAL)
+		if (ELF_FUNC(ST_BIND, sym->st_info) == STB_WEAK)
 			continue;
 
 		if (ELF_FUNC(ST_TYPE, sym->st_info) != STT_FUNC)
@@ -134,8 +134,19 @@ static void BITSFUNC(go)(void *raw_addr, size_t raw_len,
 
 	qsort(entries, next_entry - entries, sizeof(*entries), entry_addr_cmp);
 
+	last_entry_addr = -1UL;
 	while (next_entry != entries && out_entries_lds) {
 		next_entry--;
+
+		/*
+		 * Unfortunately, WEAK symbols from objects are resoved
+		 * into LOCAL symbols on ia32. Filter them here, as
+		 * linker wouldn't like aligning the same symbol twice.
+		 */
+		if (last_entry_addr == next_entry->addr)
+			continue;
+		last_entry_addr = next_entry->addr;
+
 		fprintf(out_entries_lds, "\t\t. = ABSOLUTE(%#lx);\n\t\t*(.text.%s*)\n",
 			next_entry->addr, next_entry->name);
 	}
-- 
2.20.1

^ permalink raw reply related

* [PATCH 18/32] x86/vdso.lds: Align !timens (host's) vdso.so entries
From: Dmitry Safonov @ 2019-02-06  0:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: Dmitry Safonov, Adrian Reber, Andrei Vagin, Andrei Vagin,
	Andy Lutomirski, Andy Tucker, Arnd Bergmann, Christian Brauner,
	Cyrill Gorcunov, Dmitry Safonov, Eric W. Biederman,
	H. Peter Anvin, Ingo Molnar, Jeff Dike, Oleg Nesterov,
	Pavel Emelyanov, Shuah Khan, Thomas Gleixner, containers, criu,
	linux-api, x86
In-Reply-To: <20190206001107.16488-1-dima@arista.com>

As it has been discussed on timens RFC, adding a new conditional branch
`if (inside_time_ns)` on VDSO for all processes is undesirable.
It will add a penalty for everybody as branch predictor may mispredict
the jump. Also there are instruction cache lines wasted on cmp/jmp.

Those effects of introducing time namespace are very much unwanted
having in mind how much work have been spent on micro-optimisation
vdso code.

Addressing those problems, there are two versions of VDSO's .so:
for host tasks (without any penalty) and for processes inside of time
namespace with clk_to_ns() that subtracts offsets from host's time.

Unfortunately, to allow changing VDSO VMA on a running process,
the entry points to VDSO should have the same offsets (addresses).
That's needed as i.e. application that calls setns() may have already
resolved VDSO symbols in GOT/PLT.

Align VDSO entries for host with addresses generated from timens VDSO
(which is bigger as it has code for adding offsets).

Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 arch/x86/entry/vdso/vdso-layout.lds.S | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/vdso/vdso-layout.lds.S b/arch/x86/entry/vdso/vdso-layout.lds.S
index ba216527e59f..e529ee3ec9e8 100644
--- a/arch/x86/entry/vdso/vdso-layout.lds.S
+++ b/arch/x86/entry/vdso/vdso-layout.lds.S
@@ -70,7 +70,17 @@ SECTIONS
 	 * stuff that isn't used at runtime in between.
 	 */
 
-	.text		: { *(.text*) }			:text	=0x90909090,
+	.text		: {
+#if defined(CONFIG_TIME_NS) && !defined(UNALIGNED_ENTRIES)
+#ifdef BUILD_VDSO32
+# include "vdso32.entries"
+#endif
+#ifdef BUILD_VDSO64
+# include "vdso64.entries"
+#endif
+#endif
+		*(.text*)
+	}						:text	=0x90909090,
 
 	.altinstructions	: { *(.altinstructions) }	:text
 	.altinstr_replacement	: { *(.altinstr_replacement) }	:text
-- 
2.20.1

^ 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