Page MenuHomeFreeBSD

Stop calling set_syscall_retval() from linux_set_syscall_retval()
Needs ReviewPublic

Authored by trasz on Fri, Sep 11, 6:24 PM.

Details

Reviewers
kib
Group Reviewers
Linux Emulation
Summary

Stop calling set_syscall_retval() from linux_set_syscall_retval().
The former clobbers some registers that shouldn't be touched.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 33694
Build 30931: arc lint + arc unit

Event Timeline

trasz created this revision.Fri, Sep 11, 6:24 PM
trasz requested review of this revision.Fri, Sep 11, 6:24 PM
kib added inline comments.Tue, Sep 15, 5:43 PM
sys/amd64/linux/linux_sysvec.c
221

So why return instead of break ?

232

Why do this twice ?

252

And you are trying to remove this macro in other review ? Why do you put incomplete change to review ?

trasz added inline comments.Tue, Sep 15, 6:42 PM
sys/amd64/linux/linux_sysvec.c
221

Because that's the end of the fast path for this function. The set_syscall_retval() does the same. It makes actual sense there, since it doesn't need PCB_FULL_IRET, but I've tried to keep two routines as similar as possible.

232

This one is needed; the ones we do unconditionally are to be removed (eventually). Again, same as with set_syscall_retval().

252

It is complete - it applies to CURRENT. Otherwise this patch would depend on the other one, otherwise unrelated.

kib added inline comments.Wed, Sep 16, 3:57 PM
sys/amd64/linux/linux_sysvec.c
221

But you still set FULL_IRET, which completely undermines the meaning of the 'fast path'.

232

I do not see this patch as committable until the 'eventually' part is resolved. set_syscall_retval() does not set FULL_IRET twice.

252

I do not understand this: you cannot get proper review until one of the patches is resolved.

trasz updated this revision to Diff 77305.Mon, Sep 21, 3:58 PM
trasz marked 6 inline comments as done.

Fix multiple issues.

trasz added inline comments.Mon, Sep 21, 3:58 PM
sys/amd64/linux/linux_sysvec.c
221

True. The plan was to keep it as close to set_syscall_retval() as possible, and later just remove the problematic calls to set_pcb_flags(). But you're probably right that the code should fit the current state of things, not the one we'd like to have in the future.

kib added inline comments.Mon, Sep 21, 10:19 PM
sys/amd64/linux/linux_sysvec.c
230

This paragraph does not make much sense in the context of linux_set_syscall_retval.

234

Isn't this half of the comment relevant to all paths ending in set(FULL_IRET) ? I suggest to move it right above set_pcb_flags() instead of the wishful thinking that is currently at lines 2490250.