On Wed, 22 Nov 2017, Mathieu Desnoyers wrote:
----- On Nov 21, 2017, at 5:59 PM, Thomas Gleixner tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org wrote:
quoted
quoted
Using vDSO as an example of how things should be done is just wrong: the
vDSO interaction with debugger instruction single-stepping is broken,
as I detailed in my previous email.
Let me turn that around. You're lamenting about a conditional branch in
your rseq thing for performance reasons and at the same time you want to
force extra code into the VDSO? clock_gettime() is one of the hottest
vsyscalls in certain scenarions. So why would we want to have extra code
there? Just to make debuggers happy. You really can't be serious about
that.
There is *already* an existing branch in the clock_gettime vsyscall:
it's a loop. It won't hurt the fast-path to use that branch and
make it do something else instead. It could even help the vDSO fast-path
for some non-x86 architectures where branch prediction assumes that
backward branches are always taken (adding an unlikely() does not help
in those cases).
Yes, there is an existing branch, but forcing that thing into the real
syscall when the seqcount does not match is silly.
quoted
quoted
Thomas' proposal of handling single-stepping with a user-space locking
fallback, which is pretty much what I had in 2016, pushes a lot of
complexity to user-space, requires an extra branch in the fast-path,
as well as additional store-release/load-acquire semantics for consistency.
I don't plan going down that route.
Other than that, I have not received any concrete alternative proposal to
properly handle single-stepping.
You provided the details today. Up to that point all we had was handwaving
and inconsistent information.
I mistakenly presumed you took interest in the past 2 years discussions.
It appears I was wrong, and that information needed to be summarized in
my changelog. This was my mistake and I fixed it.
I took interest, but I did not follow all the details. And it's not about
me. Anyone who looks at those patches for whatever reason should get enough
information.
quoted
quoted
The only opposition against cpu_opv is that there *should* be an hypothetical
simpler solution. The rseq idea is not new: it's been presented by Paul Turner
in 2012 at LPC. And so far, cpu_opv is the overall simplest and most
efficient way I encountered to handle single-stepping, and it gives extra
benefits, as described in my changelog.
That's how you define it and that does not make cpu_opv less complex and
more debuggable. There is no way to debug that and still you claim that it
removes compexity from user space.
So I should ask: what kind of observability within cpu_opv() do you want ?
I can add a tracepoint for each operation, which would technically take care
of your concern. You main counter-argument seems to be a tooling issue.
Yes, it's a tooling issue and this is issue is not going to be solved
faster than gdb support for skipping rseq sections.
quoted
That ops stuff comes from user space and
is not magically constructed by the kernel. In some of your use cases it
even has different semantics than the rseq section code. So how is that
removing any complexity from user space? All it buys you is an extra branch
less in your rseq hotpath and that's your justification to shove that
thing into the kernel.
Actually, the cpu-op user-space library can hide this difference from the
user: I implemented the equivalent rseq algorithm using a compare-and-store:
Yes, you implemented it and I don't want to know how long it took to get it
right. But syscalls are not coupled to be used with a particular library.
So from a library user perspective, the fast-path and slow-path are
exactly the same.
Aside of the fact that they are not the same in terms of implementation and
semantics. I'm looking at the syscall and not reviewing your magic user
space library.
Thanks,
tglx