Skip to content
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

Pin을 이용하여 struct가 move되는 것을 방지 #378

Closed
9 tasks done
travis1829 opened this issue Jan 27, 2021 · 4 comments
Closed
9 tasks done

Pin을 이용하여 struct가 move되는 것을 방지 #378

travis1829 opened this issue Jan 27, 2021 · 4 comments
Labels

Comments

@travis1829
Copy link
Collaborator

travis1829 commented Jan 27, 2021

교수님께서 말씀하신 대로 이미 Pin 이 필요한 부분이 많습니다. 당장 생각나는 것만 해도:

  • intrusive linked list 를 사용하는 코드 (MruArena)
  • virtqueue 에 접근하는 Disk
  • parent process 의 포인터를 저장하는 parent field

이 정도인데 이런 것들을 지원하려면 Kernel 이 pin 이 되어야 합니다.

Originally posted by @efenniht in #241 (comment)

이와 같은 struct들에 대하여, Pin을 사용하여 struct들이 move되는 것을 방지해야 합니다. 즉, !Unpin으로 표시해야 하며, 또, &mut T 대신 Pin<&mut T>를 사용해야 합니다.

  • Kernel
  • Hal
  • Ufs
  • Disk
  • List
  • Kmem
  • MruArena
  • ArrayArena
  • Procs
ghost pushed a commit that referenced this issue Feb 4, 2021
394: [Sub-issue of #378] Add `PinnedKernel`, and use `Pin` in locks, `MruArena`, and `MruEntry` r=jeehoonkang a=travis1829

Partially resolves # 378.
* `T: Unpin`인 경우에만 (즉, `T`가 move되어도 상관없는 경우에만) `Spinlock`, `SpinlockGuard`와 같은 lock 및 guard들이 `get_mut()`, `get_mut_unchecked()`, `DerefMut`을 impl하도록 바꿨습니다. 그 대신, `get_pin()`을 추가했습니다.
  * 추가적으로 ,다른 PR에서 lock과 guard들을 더 정리할 계획입니다.
* `ListEntry` API가 `Pin`을 사용하게끔 바꿨습니다.
* `MruArena`, `MruEntry`에서 mutable reference가 필요할 때는 항상 `Pin<&mut ...>`을 사용하게끔 바꿨습니다.
  * 편의를 위해 pin_project crate를 사용했습니다. move되선 안되는 field에는 #[pin]을 붙여서 `Pin`으로만 접근하도록 제한했습니다. 다만 pin_project가 procedural macro를 사용해서인지 dependency를 약 10개나 늘려버립니다. 참고로, procedural macro를 쓰지 않는 pin_project_lite도 있기는 한데 이건 기능이 제한적입니다.

EDIT:
* 특히, `Pin`의 안전성을 깔끔하게 보장하기 위해서 `PinnedKernel`이라는 struct 및 static 변수를 추가했습니다. 지금은 `BcacheInner`만 들어있지만 추후 `Pin`이 필요한 struct들을 `Kernel`에서 여기로 옮길 계획입니다.
  * #394 (comment)

Co-authored-by: travis1829 <travis1829@naver.com>
@travis1829
Copy link
Collaborator Author

@travis1829 travis1829 changed the title Pin API를 사용하여 struct가 move되는 것을 방지 Pin을 이용하여 struct가 move되는 것을 방지 Mar 3, 2021
@jeehoonkang
Copy link
Member

https://github.com/jeehoonkang/rv6/tree/pin-2021-05-01 에서 진행중입니다.

@jeehoonkang jeehoonkang self-assigned this Apr 30, 2021
@Medowhill Medowhill added D-hard and removed D-mid labels Jun 2, 2021
@Medowhill
Copy link
Collaborator

Future 등에서 init할 때만 Pin을 받는지, 아니면 다른 메서드에서도 Pin을 받는지 확인해 보기.

@Medowhill
Copy link
Collaborator

https://github.com/rust-lang/rust/blob/c4f186f0ea443db4aacdd90a2515632c20ccd3fe/library/std/src/sys_common/remutex.rs#L49-L54

https://github.com/rust-lang/rust/blob/c4f186f0ea443db4aacdd90a2515632c20ccd3fe/library/std/src/sys_common/remutex.rs#L68

https://github.com/rust-lang/rust/blob/c4f186f0ea443db4aacdd90a2515632c20ccd3fe/library/std/src/sys_common/remutex.rs#L84

impl<T> ReentrantMutex<T> {
    /// # Unsafety
    ///
    /// This function is unsafe because it is required that `init` is called
    /// once this mutex is in its final resting place, and only then are the
    /// lock/unlock methods safe.
    pub const unsafe fn new(t: T) -> ReentrantMutex<T> {...}

    pub unsafe fn init(self: Pin<&mut Self>) {...}

    pub fn lock(self: Pin<&Self>) -> ReentrantMutexGuard<'_, T> {...}

    ...
}

Rust standard libraryReentrantMutex 구현에서 정확히 저희가 필요한 예시를 찾았습니다. ReentrantMutexinit된 이후에만 사용될 수 있으며, Pin된 상태에서만 사용될 수 있습니다. newinit은 unsafe 메서드이며, initlockPin을 인자로 받습니다. 같은 방식을 rv6에서 이 이슈와 #437 을 해결하는 데 사용하면 될 것 같습니다.

kaist-cp-bors bot added a commit that referenced this issue Jun 4, 2021
561: Closes #378 r=Medowhill a=Medowhill



Co-authored-by: Jaemin Hong <hjm0901@gmail.com>
@kaist-cp-bors kaist-cp-bors bot closed this as completed in 4e2add1 Jun 4, 2021
Gabriel4256 pushed a commit to Gabriel4256/rv6 that referenced this issue Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants