[DTrace-devel] [PATCH v2 4/4] EINTR: respect switchrate in the core polling loop

[Nick Alcock]

On 1 Nov 2023, Eugene Loh via DTrace-devel told this:

> Again, there are sometimes leading spaces where I think you intended leading tabs.

You're quite right: fixed.

There was at least one (harmless) debugging hack left in the test, too.

> Again, I think there is an incorrect conversion to nsecs.  Instead of
> (secs+1000000000*nsecs) it should be (1000000000*secs+nsecs).

... oh dammit now I see what you mean. Fixed, in both cases. I checked
that so many times...

Adjusted to

timeout_msec -= ((((unsigned long long) end.tv_sec * NANOSEC) + end.tv_nsec) -
		 (((unsigned long long) start.tv_sec * NANOSEC) + start.tv_nsec)) /
		MICROSEC;

which I hope is a bit less wrong?

> Again, I think MICROSEC is irrelevant;  NANOSEC/MILLISEC is more
> meaningful (and it's what is done for converting from interval to
> timeout_msec).

I honestly don't understand why that would be more *readable*, given
that you're talking about a denominator under a denominator here.
I can change it if you insist...

> Regarding the test, "wait -f" is not always available.  E.g., it seems
> not to be in OL7.9 nor in OL8.8, even if it's available in OL9.0.

Oh it's quite new, bash 4.4. Without it I don't see how to make
thiswork: wait would get woken up constantly on every signal hit, since
that constitutes a state change...

Kinda irrelevant since I don't think this patch should actually go in :)

> Importantly, it seems like the test does not do anything.  My
> impression is that the test is supposed to disrupt the loop, which
> should nonetheless take the correct amount of time due to the patch. 
> I inserted some instrumentation into the loop to report if the body of
> the while is ever encountered.  It is not.  I added some

Really? I did the same thing and it was :)

> instrumentation to report how long until the loop completes. It takes
> almost no time (consistent with the body never being entered).  So I
> think the test is passing simply because it's reporting the expected
> time is being taken even though the new code never does anything.

... hm, maybe libdtrace is explicitly blocking a bunch of signals?
That's not consistent with my observing this bug in the first place when
I resized my window at the wrong time... but no, I see no sign that we
modify the signal disposition of SIGWINCH at all.

I think this may be your terminal environment or something. Signals with
no handler are left unchanged across fork and exec, so if your
controlling shell is ignoring SIGWINCH for some reason, DTrace will also
end up ignoring it. Mine isn't doing any such thing. The Sig* entries in
/proc/self/status before you start runtest might be informative...

-- 
NULL && (void)