-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WaitChannel
design
#241
Comments
저는 2가 어떨까 합니다. 되도록 xv6의 의도를 따라가면서도 safe api를 지원하면 좋겠어서 그렇습니다. 1은 unsafe, 3은 xv6의 의도와 다름. |
혹시 급한(쓰시는 논문에 중요하게 다룰 예정인) refactoring point가 아니라면 시험 이후에 제가 좀 봐도 괜찮을까요? @efenniht 이유는
|
네, 부탁드립니다 🙏 |
현재는 |
@travis1829 혹시 진행중인 PR이 있나요? |
아직 PR은 없습니다! |
349: Fix WaitChannel's API r=efenniht a=travis1829 #241 작업을 하기 전에 먼저 `WaitChannel`의 지저분한 부분을 조금 다듬어봤습니다. * 기존에는 `WaitChannel`에 대해서 sleep을 하기 위해서는 `SpinlockGuard`는 `sleep()`로, `SleepablelockGuard`는 `sleep_sleepable()`로, 그리고 `SpinlockProtected`는 `sleep_raw()`로 **여러가지로 API가 나눠져있는 걸 하나로 합쳤습니다.** * 이를 위해, `Guard`라는 trait을 추가했습니다. (현재 `SpinlockGuard`, `SpinlockProtectedGuard`, `SleepablelockGuard`이 이걸 impl합니다.) * 다만, 별개로, **현재 `SleepablelockGuard::sleep()`이 `unsafe`이 아닌데, 이래도 괜찮은지 잘 모르겠습니다.** * 먼저, `WaitChannel::sleep()`이 `unsafe`한 이유는 `guard.sched()`을 call할때는 항상 `p->lock`이외에는 어떠한 RawSpinlock도 acquire한 상태여선 안되기 때문입니다. (`guard.sched()`내에서도 `assert_eq!((*kernel().mycpu()).noff, 1, "sched locks");`로 이걸 확인합니다). 그러나 `SleepablelockGuard::sleep()`은 내부에서 `WaitChannel::sleep()`을 사용하는데도 unsafe이 아닙니다. Co-authored-by: travis1829 <[email protected]>
349: Fix WaitChannel's API r=efenniht a=travis1829 #241 작업을 하기 전에 먼저 `WaitChannel`의 지저분한 부분을 조금 다듬어봤습니다. * 기존에는 `WaitChannel`에 대해서 sleep을 하기 위해서는 `SpinlockGuard`는 `sleep()`로, `SleepablelockGuard`는 `sleep_sleepable()`로, 그리고 `SpinlockProtected`는 `sleep_raw()`로 **여러가지로 API가 나눠져있는 걸 하나로 합쳤습니다.** * 이를 위해, `Guard`라는 trait을 추가했습니다. (현재 `SpinlockGuard`, `SpinlockProtectedGuard`, `SleepablelockGuard`이 이걸 impl합니다.) * 다만, 별개로, **현재 `SleepablelockGuard::sleep()`이 `unsafe`이 아닌데, 이래도 괜찮은지 잘 모르겠습니다.** * 먼저, `WaitChannel::sleep()`이 `unsafe`한 이유는 `guard.sched()`을 call할때는 항상 `p->lock`이외에는 어떠한 RawSpinlock도 acquire한 상태여선 안되기 때문입니다. (`guard.sched()`내에서도 `assert_eq!((*kernel().mycpu()).noff, 1, "sched locks");`로 이걸 확인합니다). 그러나 `SleepablelockGuard::sleep()`은 내부에서 `WaitChannel::sleep()`을 사용하는데도 unsafe이 아닙니다. Co-authored-by: travis1829 <[email protected]>
WaitChannel을 어떻게 바꾸면 좋을지에 대해 의문사항이 좀 있어서 comment을 답니다.
참고: 혹시, 제가 Pin API에 대해 잘못 이해하고 있는게 있다면 알려주세요. |
WaitChannel과 이를 포함하는 struct (Pipe, Proc) 모두 안움직이도록 강제할 수도 있을까요? 제가 Pin에 대해 까먹었는데 가령 |
예, API가 EDIT: 다만, 애초에 |
|
(아직 작성중...)
|
이 issue를 해결하기 위해서는 크게 3가지 방법이 있는 것 같습니다.
혹시 이 부분에 대해 어떻게 생각하시나요? @jeehoonkang @efenniht |
저는 장기적으로 1번이 맞아보이긴 합니다. 지금 object가 옮기면 안된다는 invariant를 가정하는 부분이 상당히 많은데 (waitchannel이 아니더라도) 따라서 (너무 비용이 크면 3도 좋은 선택이라고 생각합니다.) |
교수님께서 말씀하신 대로 이미 Pin 이 필요한 부분이 많습니다. 당장 생각나는 것만 해도:
이 정도인데 이런 것들을 지원하려면 |
그러면 1번을 이용해보도록 하겠습니다!
일단, 제가 간단히 알아본걸 미리 말씀드리자면,
|
fn move_pinned_ref<T>(mut a: T, mut b: T) {
unsafe {
let p: Pin<&mut T> = Pin::new_unchecked(&mut a);
// This should mean the pointee `a` can never move again.
}
mem::swap(&mut a, &mut b);
// The address of `a` changed to `b`'s stack slot, so `a` got moved even
// though we have previously pinned it! We have violated the pinning API contract.
}
|
"더 큰 struct"도 pin으로 접근하면 안되나요? |
장기적으로는 그런식으로 해야 합니다. 즉, struct가 move되서는 안되는 field를 직접적으로, 또는 다른 field의 하위 field로 간접적으로 가지고 있으면, 항상 pin으로 접근해야 합니다. |
|
superseded by #378 |
WaitChannel
에 대해서 고민해봤는데, 이런 형태의 디자인이 과연 올바른 지 궁금합니다. @jeehoonkangWaitChannel
을 들고 있는 타입은 크기가 커집니다. 그렇다고WaitChannel<T>
같은 타입은 더 이상한 것 같습니다.WaitChannel
은 주소가 wait 대상의 identifier 가 되므로 move 되면 스레드를 깨워도 일어나지 않거나 이상한 스레드가 깨는 등 의도되지 않은 일이 일어날 수 있습니다. 따라서 올바른 API는sleep(self: Pin<&mut Self>, ...)
이 되어야 합니다. 다만 이것은 safety invariant 는 아니므로 주석만 쓰고 넘길 수도 있습니다.WaitChannel
의 멤버로 들고 있습니다. 이러면 channel 이 move 되어도 문제가 없습니다.어떻게 수정하는 것이 옳을까요?
Pin
넣고 타입 대공사Originally posted by @efenniht in #236 (comment)
The text was updated successfully, but these errors were encountered: