public inbox for linux-um@lists.infradead.org
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Benjamin Beichler <Benjamin.Beichler@uni-rostock.de>,
	Richard Weinberger <richard@nod.at>,
	Anton Ivanov <anton.ivanov@cambridgegreys.com>
Cc: linux-um@lists.infradead.org
Subject: Re: [PATCH RFC 10/11] um: Delay timer_read only in possible busy loops in TT-mode
Date: Fri, 10 Nov 2023 18:29:42 +0100	[thread overview]
Message-ID: <f3ab3489db72e21f877df82b8fec4fabcb5b2fdf.camel@sipsolutions.net> (raw)
In-Reply-To: <867df5dc-8e78-48de-aa1a-da7c8fedb5e6@uni-rostock.de>

On Fri, 2023-11-10 at 16:54 +0100, Benjamin Beichler wrote:
> Am 06.11.2023 um 21:51 schrieb Johannes Berg:
> > On Fri, 2023-11-03 at 16:41 +0000, Benjamin Beichler wrote:
> > > This slows down external TT-mode as more simulation roundtrips are
> > > required, and it unnecessarily affects the determinism and accuracy of
> > > the simulation.
> > I still don't think this is really true, it doesn't really affect
> > determinism? It makes it ... different, sure, but not non-deterministic?
> I intentionally kept it vague, but what I meant is that it's 
> unnecessarily challenging to determine.

Yeah, ok, fair enough.

> Perhaps I should mention that I'm running an unmodified Ubuntu rootfs 
> with systemd, which starts many daemons and other processes.

That sounds like a bit of a nightmare, to be honest, wouldn't you want
to keep things under tighter control? But I guess it really depends on
what you're trying to achieve.

> To me, it seems illogical to delay everything just because one process 
> is waiting for a timestamp.

Yeah I guess we'll just have to disagree ;-) You're running some
process, so you've kind of decided to "give it time" of sorts, and in a
normal system reading time will always take time, just like everything
else :)

But anyway, I'm not really opposed to this patch, it's just ... not
great, I guess? And like I said, makes more sense to squash 9 and 10?

> At the moment, we haven't patched the random device that fetches random 
> bytes from the host (do you already have a patch for this?),
> so complete repeatability isn't guaranteed at the moment. However, that 
> could be a logical next step.
> > > +static const int suspicious_busy_loop_syscalls[] = {
> > > +     36, //sys_getitimer
> > > +     96, //sys_gettimeofday
> > > +     201, //sys_time
> > > +     224, //sys_timer_gettime
> > > +     228, //sys_clock_gettime
> > > +     287, //sys_timerfd_gettime
> > > +};
> > That's kind of awful. Surely we can use __NR_timer_gettime etc. here at
> > least?
> Actually, this was a quick attempt to address the issue, and during that 
> period, I couldn't locate the appropriate macros.
> 
> These numbers are generated from arch/x86/entry/syscalls/syscall_64.tbl 
> (or 32 if configured in that manner).
> 
> I might be overlooking something, but it seems that __NR_timer_gettime 
> isn't defined in the kernel. If you have a better reference for this 
> translation, I'd appreciate it.

Look at the arch/x86/include/generated/uapi/asm/unistd*.h files after
you build the tree. How do they actually get generated? Beats me.

> > > +static bool suspicious_busy_loop(void)
> > > +{
> > > +     int i;
> > > +     int syscall = syscall_get_nr(current, task_pt_regs(current));
> > > +
> > > +     for (i = 0; i < ARRAY_SIZE(suspicious_busy_loop_syscalls); i++) {
> > > +             if (suspicious_busy_loop_syscalls[i] == syscall)
> > > +                     return true;
> > > +     }
> > Might also be faster to have a bitmap? But ... also kind of awkward I
> > guess.
> Actually, a short fixed size array should be optimized quite well with 
> loop unrolling or other stuff, isn't it? I could also do a switch with 
> all calls, but this loop seems for me the easiest.

Yeah, maybe. I haven't checked the output.

> > I dunno. I'm not even sure what you're trying to achieve - apart from
> > "determinism" which seems odd or even wrong, and speed, which is
> > probably easier done with a better free-until and the shared memory
> > calendar we have been working on.
> In my perspective, delaying get_timer only serves as a tie-breaker for 
> poorly behaving software that resorts to a busy-loop while waiting for 
> time to advance.

Yeah I guess that's true.

> While this behavior might not be uncommon, why penalize all processes 
> for it?

Well I think we have a different sense of "penalize", I wouldn't say
that. I mean, you can't reasonably expect getting a timestamp doesn't
take any time at all, that's just not how physical reality works? Now
we're bending the rules here in that a lot of things that normally take
time suddenly don't, but I guess I don't fully understand why you're so
keen on bending the rules _all the way_.

But I think that's this:

> Consider an experiment where I aim to measure the impact of network 
> latency on software. Sometimes, the response latency fluctuates
> because a background task was scheduled randomly but deterministically 

"randomly but deterministically" is kind of fun 😂️

> at the same time, obtaining a timestamp that blocks all other
> processes and advances simulation time. This action simply undermines 
> the utility of the time travel mode unnecessarily.
> 
> However, software actively waiting for time advancement in a busy loop 
> achieves its goal. It’s almost a win-win situation, isn't it?

Fair enough.

johannes

_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

  parent reply	other threads:[~2023-11-10 17:29 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-03 16:41 [PATCH RFC 00/11] Several Time Travel Mode Enhancements Benjamin Beichler
2023-11-03 16:41 ` [PATCH RFC 01/11] um: Make UBD requests synchronous in TT ext/infcpu mode Benjamin Beichler
2023-11-06 20:24   ` Johannes Berg
2023-11-03 16:41 ` [PATCH RFC 02/11] um: add a simple time_travel_handler implementation Benjamin Beichler
2023-11-03 16:41 ` [PATCH RFC 03/11] um: Use a simple time travel handler for line interrupts Benjamin Beichler
2023-11-06 20:26   ` Johannes Berg
2023-11-10 16:53     ` Benjamin Beichler
2023-11-10 17:16       ` Johannes Berg
2023-11-13 11:46         ` Benjamin Beichler
2023-11-13 21:22           ` Johannes Berg
2023-11-13 21:57             ` Johannes Berg
2023-11-20 13:42               ` Benjamin Beichler
2023-11-24 14:53                 ` Johannes Berg
2023-11-03 16:41 ` [PATCH RFC 04/11] um: Handle UM_TIMETRAVEL_RUN only in idle loop, signal success in ACK Benjamin Beichler
2023-11-06 20:53   ` Johannes Berg
2023-11-10 18:41   ` Johannes Berg
2023-11-03 16:41 ` [PATCH RFC 05/11] um: Add final request time to TT wait message Benjamin Beichler
2023-11-06 20:29   ` Johannes Berg
2023-11-03 16:41 ` [PATCH RFC 06/11] um: always send UM_TIMETRAVEL_REQUEST from ISRs Benjamin Beichler
2023-11-06 20:32   ` Johannes Berg
2023-11-03 16:41 ` [PATCH RFC 07/11] um: add TIMETRAVEL_REQUEST handler to request latest event Benjamin Beichler
2023-11-06 20:33   ` Johannes Berg
2023-11-10 16:23     ` Benjamin Beichler
2023-11-10 17:19       ` Johannes Berg
2023-11-03 16:41 ` [PATCH RFC 08/11] um: Protect accesses to the timetravel event list Benjamin Beichler
2023-11-06 20:37   ` Johannes Berg
2023-11-03 16:41 ` [PATCH RFC 09/11] um: Delay timer_read in time travel mode only after consecutive reads Benjamin Beichler
2023-11-06 20:40   ` Johannes Berg
2023-11-03 16:41 ` [PATCH RFC 10/11] um: Delay timer_read only in possible busy loops in TT-mode Benjamin Beichler
2023-11-06 20:51   ` Johannes Berg
2023-11-10 15:54     ` Benjamin Beichler
2023-11-10 16:39       ` Benjamin Berg
2023-11-10 17:29       ` Johannes Berg [this message]
2023-11-03 16:41 ` [PATCH RFC 11/11] um: Remove all TSC flags when using Time Travel Mode Benjamin Beichler
2023-11-03 18:45   ` Anton Ivanov
2023-11-06 20:52   ` Johannes Berg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f3ab3489db72e21f877df82b8fec4fabcb5b2fdf.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=Benjamin.Beichler@uni-rostock.de \
    --cc=anton.ivanov@cambridgegreys.com \
    --cc=linux-um@lists.infradead.org \
    --cc=richard@nod.at \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox