rv6 icon indicating copy to clipboard operation
rv6 copied to clipboard

Reasoning safety of `self.get_ctx()` in `KernelRef::kernel_trap`

Open Medowhill opened this issue 3 years ago • 2 comments

https://github.com/kaist-cp/rv6/blob/b946a9463ccee7f23328514d570de1c4ab926a28/kernel-rs/src/trap.rs#L200-L201

Medowhill avatar Apr 30 '21 05:04 Medowhill

음... 전 사실 이게 왜 safe한지 모르겠습니다. ctx 잡고있는데 kernel trap 밟아서 다시 ctx 생성될 수 있는거같아서요. 재민님, 혹시 왜 safe한지 아시나요?

jeehoonkang avatar Apr 30 '21 16:04 jeehoonkang

KernelCtx를 두 개 만들면 안 되는 이유는, 그 자체로 위험해서가 아니라, KernelCtx가 두 개이면 하나의 ProcData를 가리키는 두 개의 &mut을 만들 수 있기 때문입니다. kernel_trap에서는 KernelCtx를 만들며, cpu_yield를 호출하는데, cpu_yield 안에서 ProcData 내부의 Context에 접근하기 위해 &mut ProcData를 얻습니다. 그래서 이론상으로는 하나의 Context에 대한 두 &mut Context가 만들어질 수 있습니다. 그러나 아마도 실제로는 Context가 필요한 경우가 context switch를 할 때뿐이기 때문에, kernel trap이 발생하기 전에 커널 모드에서 user trap을 처리하고 있을 때는 Context에 접근하고 있지 않았을 것 같습니다. 그래서 kernel_trap에서 &mut Context에 접근하더라도 실질적으로는 unique access가 아닌가 합니다. Safety를 따지기기 상당히 복잡한 것 같습니다..

그리고 바로 이어지는 코드에도 unsafe block이 있는데, safety가 주석으로 써 있기는 하지만 설명이 잘 이해되지는 않습니다.

https://github.com/kaist-cp/rv6/blob/9f0a531cfd4e7187809fbea13c9f3afc41acf4de/kernel-rs/src/trap.rs#L200-L207

아래의 논의에서 나온 내용을 주석으로 쓴 것 같은데 왜 안전한 것인지 잘 모르겠습니다.

https://github.com/kaist-cp/rv6/pull/401#discussion_r571761982

일단 CurrentProc의 invariant를 정의할 때 state가 Running이라는 내용을 넣었기 때문에, 해당 invariant가 틀린 것이 아니라면 굳이 state가 Running인지 검사할 필요가 없습니다. 지금 해당 검사를 지우고 무조건 cpu_yield를 호출하게 수정해 보았는데, 테스트는 무사히 통과하는 것 같습니다. 해당 검사는 xv6에부터 있던 것인데, xv6가 불필요한 검사를 넣은 것일 수도 있을 것 같습니다.

Medowhill avatar May 01 '21 06:05 Medowhill