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

Inode::finalize() 만으로 Fstransaction이 열린 상태인지 파악하기 #290

Closed
kimjungwow opened this issue Nov 24, 2020 · 18 comments · Fixed by #555
Closed

Inode::finalize() 만으로 Fstransaction이 열린 상태인지 파악하기 #290

kimjungwow opened this issue Nov 24, 2020 · 18 comments · Fixed by #555
Labels

Comments

@kimjungwow
Copy link
Collaborator

#272 리팩토링 후 해결해야 할 이슈입니다.

impl ArenaObject for Inode 내에서 아래와 같이 finalize를 실행합니다.
이 때 InodeGuard가 아닌 Inode::finalize() 만으로 Fstransaction이 열린 상태인지 파악할 수 있어야 합니다. (run-time performance를 위해)
즉, Inode::finalize() 안에서 begin_op(), end_op()를 쓰지 않는 방법을 고민해야 합니다.

fn finalize<'s, A: Arena>(&'s mut self, guard: &'s mut A::Guard<'_>) {
if self.inner.get_mut().valid && self.inner.get_mut().nlink == 0 {
// inode has no links and no other references: truncate and free.
// TODO(rv6): must be removed.
let tx = mem::ManuallyDrop::new(FsTransaction { fs: kernel().fs() });
// self->ref == 1 means no other process can have self locked,
// so this acquiresleep() won't block (or deadlock).
let mut ip = self.lock(&tx);
A::reacquire_after(guard, move || unsafe {
ip.itrunc();
ip.deref_inner_mut().typ = 0;
ip.update();
ip.deref_inner_mut().valid = false;
drop(ip);
});
}
}

@Medowhill
Copy link
Collaborator

이 이슈는 다음의 두 문제로 나눌 수 있습니다.

  1. finalize에서 FsTransaction을 얻을 방법이 없는 문제

finalizeFsTransaction이 필요한 itrunc 등을 호출하므로 FsTransaction이 필요합니다. Arena의 구현상 finalize가 추가적인 인자를 받을 수 없기 때문에 인자로 FsTransaction을 받는 것은 불가능합니다. FsTransaction을 만드는 정상적인 방법은 begin_transaction을 호출하는 것이지만 finalize 안에서 begin_transaction을 호출하는 것 역시 불가능합니다. finalize가 호출되기 전 Arena::dealloc에서 Spinlock을 acquire하고, begin_transaction을 호출하면 Sleepablelock을 acquire하려고 하기 때문입니다. 현재는 finalize 안에서 begin_transaction 없이 FsTransaction을 임의로 만들고 사용합니다. 이 방법은 begin_op를 호출하지 않기 때문에 실질적으로 트랜잭션을 시작하지 않고 FsTransaction 값만을 만드는 것이므로 일반적으로 안전하지 않습니다. finalize가 호출되기 전에 begin_transaction을 통해 FsTransaction이 만들어진 상태라고 가정해야만 안전합니다.

  1. RcInode가 drop될 때 FsTransaction을 인자로 받지 않는 문제

RcInode가 drop되면 Arena::dealloc이 호출되고 결국 finalize가 호출됩니다. 따라서 1에서 언급한 finalize의 가정에 의해 RcInode가 drop되기 전에 begin_transaction이 호출되어야 합니다. 그러나 Rust의 drop이 인자를 받지 않으므로 begin_transaction이 drop 전에 호출되도록 강제할 방법이 없습니다. 현재는 실제로 FsTransaction 값이 전혀 사용되지 않음에도 begin_transaction을 호출하고, 값이 사용되지는 않지만 반드시 begin_transaction을 호출해야 하는 이유를 주석으로 적어 두었습니다.

Arena의 구현을 완전히 수정하거나 Arena를 사용하지 않도록 바꾸지 않는 이상 1과 2를 동시에 해결할 방법은 없어 보입니다. 어떻게 해도 finalize에 인자를 전달할 수 없기 때문입니다. 따라서 1과 2는 각각 별개의 문제로 해결해야 할 것 같습니다.

  1. finalize에서 FsTransaction을 얻을 방법이 없는 문제의 해결

현재 실행 중인 ProcFsTransaction을 저장하고 finalize 안에서 이에 접근해 사용하는 방법이 가능해 보입니다. 한 가지 단점은 FsTransaction이 필요 없어진 후에는 명시적으로 Proc에서 FsTransaction을 꺼내와 drop해야 한다는 점입니다. 결국 xv6처럼 매번 end_op를 직접 호출하는 것과 유사한 코드가 되어 Rust의 장점을 잘 살릴 수 없습니다.

  1. RcInode가 drop될 때 FsTransaction을 인자로 받지 않는 문제의 해결

RcInode를 감싸는 타입을 추가로 만들고 이 타입의 drop이 무조건 panic을 일으키게 하되, FsTransaction을 인자로 받는 safe_drop 같은 메서드를 정의하고 safe_drop 안에서 RcInode를 drop하는 것입니다. 결국 FsTransaction이 사용되지는 않겠지만 RcInode가 drop될 때는 FsTransaction이 존재하도록 강제할 수 있습니다. drop이 무조건 panic을 일으키는 것이 Rust스러운 해결 방법이라고 보기는 어렵지만, 이미 rv6의 여러 타입(Page 등)에 동일한 방법이 사용되고 있기 때문에 큰 문제는 아닐 것 같습니다.

@jeehoonkang
Copy link
Member

2.를 선호합니다.
글을 자세히 읽진 않았는데 panic drop이 더 "덜" 해로운 것 같아서요...
이건 사실 rust가 linear type을 지원 안하고 affine type만 지원해서 생기는 문제이기도 합니다.

@Medowhill
Copy link
Collaborator

제 글은 문제 1, 2가 있다는 것과 각각에 대한 해결책을 이야기한 것인데, 2를 선호하신다는 말씀은 1은 적용하지 않고 2만 적용하면 좋겠다는 뜻인가요?

@jeehoonkang
Copy link
Member

@Medowhill 아.. 제가 대충 읽어서 논지를 파악을 잘 못했네요. 기력을 회복한 후 내일 오전 전까진 다시 답변드리겠습니다. 미안합니다...

혹시 더 빨리 논의가 필요하면 전화 부탁드려요

@Medowhill
Copy link
Collaborator

지금 2에 대한 해결책을 시도해 보는 중이었는데, FileTypeRcInode를 가지고 있어서 FileType이 drop되기 전에 RcInode를 꺼내서 safe_drop을 호출해 주어야 합니다. 그러나 FileTable::alloc_file에서 Arena::alloc의 인자로 클로저를 넘기고 그 클로저가 FileType 값을 캡처하기 때문에 alloc이 실패했을 때 FileType 안에 있는 RcInodesafe_drop할 방법이 없는 것 같습니다. 해결할 좋은 방법이 딱히 생각나지 않아서 일단 보류해 두려고 합니다.

1은 아마 2와 별개로 진행할 수 있을 것 같은데, 급한 이슈는 아니니 교수님께서 편하실 때 보시고 천천히 의견 주셔도 괜찮을 것 같습니다.

@jeehoonkang
Copy link
Member

@Medowhill

  • 어떤 상황인지 잘 이해한 것 같습니다. (아마 예전에는 더 잘 이해했던 것 같은데 다 까먹었네요...)
  • 내일이나 그 이후에 30분정도 만나서 얘기하면 좋겠습니다 (offline/online). 안건은 (1) 이 제반사항에 대해 널리 공유하고, (2) 보다 넓은 시야에서 해결책을 찾아보는 것입니다.
  • 관심있는 분들은 모두 초대해서 같이 모이면 어떨가 합니다. invitation 보내주세요.

@coolofficials
Copy link
Collaborator

coolofficials commented Feb 1, 2021

@jeehoonkang 교수님,

  • (1)에서 말씀하신 '제반사항'이 현재 issue에서 다루는 FsTransaction에 국한된 제반사항을 말씀하시는건지, 아니면 Rust가 지니는 일반적인 제반사항(linear/affine type 및 Drop 관련)에 대해서 말씀하시는지 궁금합니다.
  • (2) '더 넓은 시야에서 해결책'이라고 하심은 이러한 유형의 코드들에 대한 Rust idiomatic한 새로운 디자인을 고민해보자는 의미인지, 아니면 다른 어떤 의미인지 궁금합니다.

질문을 드리는 이유는 해당 미팅에 관하여 (a) 제가 유심히 봐야할 주제인지 (b) 만약 봐야할 주제라면 어느정도의 background를 지니고 미팅에 참여해야할지 판단하기 위함입니다.

@jeehoonkang
Copy link
Member

(1) 이 이슈에 대한 얘기이고, (2) 이 이슈를 해결하기 위한 방법을 폭넓게 생각해보자는 의미였습니ㅏㄷ.

@Medowhill
Copy link
Collaborator

@jeehoonkang 오후 세 시에 미팅 만들고 초대해 드렸습니다. 저는 오프라인을 조금 더 선호하지만, 참석할 사람이 다 정해진 후에 온라인/오프라인 여부를 정하면 될 것 같습니다.

@coolofficials
Copy link
Collaborator

@Medowhill 아직 이슈에 대한 파악이 부족하여 급한 논의라면 저는 제외하고 진행하셔도 좋을 것 같습니다. 추후에 follow up 하겠습니다. @travis1829 @anemoneflower 두 분은 참석하실 계획이신가요?

@Medowhill
Copy link
Collaborator

@coolofficials 급한 이슈는 아니라고 생각합니다. 어떤 이슈인지도 간단하게 설명할 생각이라서 그냥 가볍게 듣고 가셔도 괜찮습니다.

@coolofficials
Copy link
Collaborator

@Medowhill 그렇다면 내일 최대한 코드와 이슈를 이해하고 참여하겠습니다. 저는 온라인 오프라인 둘 다 가능합니다.

@anemoneflower
Copy link
Collaborator

@Medowhill 저도 참여하겠습니다. 온라인/오프라인 둘다 가능합니다.

@travis1829
Copy link
Collaborator

travis1829 commented Feb 1, 2021

@Medowhill 그러면 저도 초대해주시면 감사하겠습니다. (온라인/오프라인 모두 가능합니다.)

@coolofficials
Copy link
Collaborator

@Medowhill 이 이슈 관련 변경점이 있었나요?

@Medowhill
Copy link
Collaborator

없습니다.

@Medowhill
Copy link
Collaborator

ArenaObject에 generic associated type Ctx를 추가하고 finalizeCtx를 인자로 받도록 했습니다.

pub trait ArenaObject {
    type Ctx<'a, 'b: 'a>;
    fn finalize<'a, 'b: 'a, A: Arena>(&mut self, ctx: Self::Ctx<'a, 'b>);
}

이에 따라 Arena::dealloc 역시 Ctx를 인자로 받습니다.

fn dealloc<'id, 'a, 'b>(
    self: ArenaRef<'id, &Self>,
    handle: Handle<'id, Self::Data>,
    ctx: <Self::Data as ArenaObject>::Ctx<'a, 'b>,
) {...}

ArenaRc는 드롭되면 패닉하며, ArenaRc::free를 호출함으로써 없어져야 합니다.

impl<A: Arena> ArenaRc<A> {
    pub fn free(mut self, ctx: <A::Data as ArenaObject>::Ctx<'_, '_>) {...}
}

BufEntry의 경우 finalize가 아무 일도 하지 않으므로 Ctx()입니다.

impl ArenaObject for BufEntry {
    type Ctx<'a, 'id: 'a> = ();
    fn finalize<'a, 'id: 'a, A: Arena>(&mut self, _: ()) {}
}

그러므로 BufEntry에 대한 ArenaRc는 자동으로 드롭되는 것이 바람직합니다. 하지만 Drop 트레잇을 특정 경우에만 구현하는 것은 불가능하므로, ArenaRc는 그대로 두고 Ctx=()인 경우에 한해 사용 가능한 DroppableRc를 추가했습니다. 올바른 구현 방법은 다음과 같습니다.

pub struct DroppableRc<T: for <'a, 'b: 'a> ArenaObject<Ctx<'a, 'b>=()>, A: Arena<Data = T>> {
    rc: ManuallyDrop<ArenaRc<A>>
}

하지만 for <'a, 'b: 'a>와 같이 HKTB에서 lifetime에 constraint를 사용할 수 없기 때문에, 위 방법은 사용할 수 없습니다. 그래서 논리적으로 이상하지만, 원하는 바를 이루기 위해 아래와 같이 할 수 있습니다.

pub struct DroppableRc<'a, 'b: 'a, T: ArenaObject<Ctx<'a, 'b>=()>, A: Arena<Data = T>> {
    rc: ManuallyDrop<ArenaRc<A>>
}

하지만 이 경우, 원하지 않는 두 개의 lifetime parameter가 DroppableRc에 생겨나 사용에 문제가 생깁니다. 그래서 더 이상하지만, 원하는 바를 이루기 위해 아래와 같이 할 수 있습니다.

pub struct DroppableRc<T: ArenaObject<Ctx<'static, 'static>=()>, A: Arena<Data = T>> {
    rc: ManuallyDrop<ArenaRc<A>>
}

이렇게 하면 컴파일이 잘 되지만, cargo fmt를 했을 때 <'static, 'static>이 지워져(위의 코드의 경우에도 cargo fmt를 하면 <'a, 'b>가 지워짐) 컴파일이 되지 않는 코드가 돼 버립니다.

결과적으로, 컴파일러의 버그로 추정되는 기능을 이용해 원하는 것을 구현했는데, rustfmt의 버그로 인해 포매팅을 하면 코드가 달라지는 상황입니다.

Medowhill added a commit to Medowhill/rv6 that referenced this issue Jun 1, 2021
@Medowhill Medowhill mentioned this issue Jun 1, 2021
@Medowhill
Copy link
Collaborator

rustc의 문제

다음 코드는 잘 컴파일됩니다.

trait C {
    type X<'a, 'b>;
}

struct S<T: for<'a, 'b> C<X<'a, 'b>=()>>(T);

impl<T: for<'a, 'b> C<X<'a, 'b>=()>> S<T> {}

또한, X'b에 upper bound로 'a를 추가하더라도 impl이 없으면 잘 컴파일됩니다.

trait C {
    type X<'a, 'b: 'a>;
}

struct S<T: for<'a, 'b> C<X<'a, 'b>=()>>(T);

하지만 X'b에 upper bound로 'a를 추가하고 impl을 넣으면 컴파일 오류가 발생합니다.

trait C {
    type X<'a, 'b: 'a>;
}

struct S<T: for<'a, 'b> C<X<'a, 'b>=()>>(T);

impl<T: for<'a, 'b> C<X<'a, 'b>=()>> S<T> {}
error[E0478]: lifetime bound not satisfied
  --> src/main.rs:9:38
  |
9 | impl<T: for<'a, 'b> C<X<'a, 'b>=()>> S<T> {
  |                                      ^^^^

for<'a, 'b: 'a> 형태로 lifetime bound를 추가하는 방식을 시도해 볼 수 있지만, 통하지 않습니다.

trait C {
    type X<'a, 'b: 'a>;
}

struct S<T: for<'a, 'b> C<X<'a, 'b>=()>>(T);

impl<T: for<'a, 'b: 'a> C<X<'a, 'b>=()>> S<T> {}
error: lifetime bounds cannot be used in this context
  --> src/main.rs:9:21
  |
9 | impl<T: for<'a, 'b: 'a> C<X<'a, 'b>=()>> S<T> {
  |                     ^^

rustfmt의 문제

trait C {
    type X<'a>;
}

struct S<T: for<'a> C<X<'a>=()>>(T);

잘 컴파일되는 위 코드를 rustfmt로 포매팅하면 컴파일되지 않는 아래 코드가 나옵니다.

trait C {
    type X<'a>;
}

struct S<T: for<'a> C<X = ()>>(T);

kaist-cp-bors bot added a commit that referenced this issue Jun 3, 2021
555: Closes #290 r=Medowhill a=Medowhill

Closes #290

Co-authored-by: Jaemin Hong <hjm0901@gmail.com>
@kaist-cp-bors kaist-cp-bors bot closed this as completed in 87314a8 Jun 3, 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

Successfully merging a pull request may close this issue.

6 participants