On 14.07.2018 17:55, Maxime Villard wrote:
> Le 14/07/2018 à 17:40, Kamil Rytarowski a écrit :
>> On 14.07.2018 12:52, Maxime Villard wrote:
>>> Le 14/07/2018 à 10:29, Martin Husemann a écrit :
>>>> On Fri, Jul 13, 2018 at 11:49:33PM +0200, Kamil Rytarowski wrote:
>>>>> #ifdefing it out in a non-benchmarking application (I was checking
>>>>> ones
>>>>> that do something with syscalls) is more negligible than 0,3% of
>>>>> overhead in the kernel in a loop.
>>>>
>>>> My vote is on #ifdef by a default-off kernel option as well.
>>>> Next step: optimize the switch, but that is no priority then. Can't we
>>>> use pcu(9) for that?
>>>
>>> I've written this [1], from memory I think it's enough. Kamil, please
>>> test
>>> it, because I intend to commit it soon. Same for i386.
>>>
>>> We don't update DR6/DR7 on each return to userland, and rather do it
>>> during
>>> context switches, only when one of the two LWPs is using dbregs. When
>>> none
>>> is (which always is the case), we don't touch.
>>>
>>> [1] http://m00nbsd.net/garbage/dbregs/amd64.diff
>>
>> There are panic, different to the ones that I observed in the past but
>> still there.
>>
>> [ 118.880204] panic: kernel diagnostic assertion "pcb->pcb_dbregs !=
>> NULL" failed: file "/usr/src/sys/arch/x86/x86/dbregs.c", line 13
>> 8
>> [ 118.880204] cpu0: Begin traceback...
>> [ 118.880204] vpanic() at netbsd:vpanic+0x16f
>> [ 118.880204] ch_voltag_convert_in() at netbsd:ch_voltag_convert_in
>> [ 118.880204] x86_dbregs_store_dr6() at netbsd:x86_dbregs_store_dr6+0x4c
>> [ 118.880204] trap() at netbsd:trap+0x97b
>>
>> I will try to fix the patch and be back to you.
>
> I think I missed a kpreempt_disable when we allocate, we don't want a
> preemption. Eg, in x86_dbregs_read():
>
> if (pcb->pcb_dbregs == NULL) {
> struct dbreg *dbregs = pool_get(&x86_dbregspl, PR_WAITOK);
> memcpy(dbregs, &initdbstate, sizeof(initdbstate));
> kpreempt_disable();
> pcb->pcb_dbregs = dbregs;
> kpreempt_enable();
> }
>
> Because if we get preempted before the 'memcpy' finishes, we load dr12367
> with an inconsistent state. Later it causes x86_dbregs_user_trap() to
> return true.
>
> Same in x86_dbregs_write().
I'm not sure I understand what may go wrong with dbregs there. If
memcpy() is problematic, why not to call kpreempt_disable() before it?
I've fixed the panic with clearing DB registers on g/c of a thread. I
will keep testing it under a load.
Index: sys/arch/x86/x86/vm_machdep.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/x86/vm_machdep.c,v
retrieving revision 1.35
diff -u -r1.35 vm_machdep.c
--- sys/arch/x86/x86/vm_machdep.c 1 Jul 2018 08:32:41 -0000 1.35
+++ sys/arch/x86/x86/vm_machdep.c 14 Jul 2018 16:04:40 -0000
@@ -291,6 +291,7 @@
if (pcb->pcb_dbregs) {
pool_put(&x86_dbregspl, pcb->pcb_dbregs);
pcb->pcb_dbregs = NULL;
+ x86_dbregs_clear(l);
}
}
http://netbsd.org/~kamil/patch-00066-by-maxv-dbregs.txt
Attachment:
signature.asc
Description: OpenPGP digital signature