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

[Closes #371] Use const Default trait in arena. #469

Merged
merged 2 commits into from
Apr 1, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions kernel-rs/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion kernel-rs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ scopeguard = { version = "1.1.0", default-features = false }
arrayvec = { version = "0.5.2", default-features = false }
cstr_core = { version = "0.2.2", default-features = false }
spin = { version = "0.7.1", default-features = false }
array-macro = "2.0.0"
array-macro = "2.1.0"
static_assertions = "1.1.0"
itertools = { version = "0.10.0", default-features = false }
pin-project = "1"
Expand Down
41 changes: 32 additions & 9 deletions kernel-rs/src/arena.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use core::mem::{self, ManuallyDrop};
use core::ops::Deref;
use core::pin::Pin;

use array_macro::array;
use pin_project::pin_project;

use crate::list::*;
Expand Down Expand Up @@ -124,9 +125,21 @@ pub struct Rc<A: Arena> {
unsafe impl<T: Sync, A: Arena<Data = T>> Send for Rc<A> {}

impl<T, const CAPACITY: usize> ArrayArena<T, CAPACITY> {
// TODO(https://github.com/kaist-cp/rv6/issues/371): unsafe...
pub const fn new(entries: [RcCell<T>; CAPACITY]) -> Self {
Self { entries }
/// Returns an `ArrayArena` of size `CAPACITY` that is wrapped by a `Spinlock`
/// and filled with `D`'s const default value. Note that `D` must `impl const [Default]`.
///
/// # Examples
///
/// ```rust,no_run
/// let arr_arena = ArrayArena::<D, 100>::new_locked("arr_arena");
/// ```
pub const fn new_locked<D: Default>(name: &'static str) -> Spinlock<ArrayArena<D, CAPACITY>> {
travis1829 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new가 아니라 new_locked를 만드는 이유는?

Copy link
Collaborator Author

@travis1829 travis1829 Mar 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • ...Arena 자체를 접근하는건 arena.rs 내부에서만 해야하고, 외부에서는 항상 Spinlock에 감싸진 형태로 접근해야합니다.
  • 추가로, 외부에서는 ...Arena를 가지고서는 할 수 있는게 없습니다. (사용할 수 있는 API가 없습니다.)

newSelf를 리턴하는 것보다 Spinlock<Self>를 리턴하는게 위 사항을 더 잘 나타낸다고 생각하여 변경했습니다.
다만, 혹시 이전으로 다시 되돌리기를 원하시면 말씀해주세요.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

* 외부에서는 항상 `Spinlock`에 감싸진 형태로 접근해야합니다.

왜 그런가요? Arena 자체는 spinlock 없이도 API를 잘 만들 수 있는게 아닌가 싶어서 질문드립니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다시 new로 변경했습니다.

지금의 ArrayArena/MruArena는 multi thread에서는 사용할 수 없으므로, 항상 Spinlock에 감싸져있어야해서 이렇게 생각했는데, 말씀하신것처럼 arena를 single thread에서 사용할 수도 있고 이미 MruArena::init같은 API가 존재하므로, 다시 변경했습니다.

Spinlock::new(
name,
ArrayArena {
entries: array![_ => RcCell::new(Default::default()); CAPACITY],
},
)
}
}

Expand Down Expand Up @@ -246,12 +259,22 @@ unsafe impl<T> ListNode for MruEntry<T> {
}

impl<T, const CAPACITY: usize> MruArena<T, CAPACITY> {
// TODO(https://github.com/kaist-cp/rv6/issues/371): unsafe...
pub const fn new(entries: [MruEntry<T>; CAPACITY]) -> Self {
Self {
entries,
list: unsafe { List::new() },
}
/// Returns an `MruArena` of size `CAPACITY` that is wrapped by a `Spinlock`
/// and filled with `D`'s const default value. Note that `D` must `impl const [Default]`.
///
/// # Examples
///
/// ```rust,no_run
/// let mru_arena = MruArena::<D, 100>::new_locked("mru_arena");
/// ```
pub const fn new_locked<D: Default>(name: &'static str) -> Spinlock<MruArena<D, CAPACITY>> {
Spinlock::new(
name,
MruArena {
entries: array![_ => MruEntry::new(Default::default()); CAPACITY],
list: unsafe { List::new() },
},
)
}

pub fn init(self: Pin<&mut Self>) {
Expand Down
18 changes: 9 additions & 9 deletions kernel-rs/src/bio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@
use core::mem::{self, ManuallyDrop};
use core::ops::{Deref, DerefMut};

use array_macro::array;

use crate::{
arena::{Arena, ArenaObject, MruArena, MruEntry, Rc},
arena::{Arena, ArenaObject, MruArena, Rc},
lock::{Sleeplock, Spinlock},
param::{BSIZE, NBUF},
proc::WaitChannel,
Expand All @@ -44,6 +42,13 @@ impl BufEntry {
}
}

#[rustfmt::skip] // Need this to use #![feature(const_trait_impl)]
travis1829 marked this conversation as resolved.
Show resolved Hide resolved
impl const Default for BufEntry {
fn default() -> Self {
Self::zero()
}
}

impl ArenaObject for BufEntry {
fn finalize<'s, A: Arena>(&'s mut self, _guard: &'s mut A::Guard<'_>) {
// The buffer contents should have been written. Does nothing.
Expand Down Expand Up @@ -149,12 +154,7 @@ impl Bcache {
///
/// The caller should make sure that `Bcache` never gets moved.
pub const unsafe fn zero() -> Self {
unsafe {
Spinlock::new(
"BCACHE",
MruArena::new(array![_ => MruEntry::new(BufEntry::zero()); NBUF]),
)
}
MruArena::<BufEntry, NBUF>::new_locked("BCACHE")
}

/// Return a unlocked buf with the contents of the indicated block.
Expand Down
15 changes: 8 additions & 7 deletions kernel-rs/src/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

use core::{cell::UnsafeCell, cmp, mem, ops::Deref, ops::DerefMut};

use array_macro::array;

use crate::{
arena::{Arena, ArenaObject, ArrayArena, Rc},
fs::{FileSystem, InodeGuard, RcInode},
Expand All @@ -12,7 +10,6 @@ use crate::{
param::{BSIZE, MAXOPBLOCKS, NFILE},
pipe::AllocatedPipe,
proc::CurrentProc,
rc_cell::RcCell,
vm::UVAddr,
};

Expand Down Expand Up @@ -201,6 +198,13 @@ impl File {
}
}

#[rustfmt::skip] // Need this to use #![feature(const_trait_impl)]
impl const Default for File {
fn default() -> Self {
Self::zero()
}
}

impl ArenaObject for File {
fn finalize<'s, A: Arena>(&'s mut self, guard: &'s mut A::Guard<'_>) {
// SAFETY: `FileTable` does not use `Arena::find_or_alloc`.
Expand Down Expand Up @@ -235,10 +239,7 @@ impl ArenaObject for File {

impl FileTable {
pub const fn zero() -> Self {
Spinlock::new(
"FTABLE",
ArrayArena::new(array![_ => RcCell::new(File::zero()); NFILE]),
)
ArrayArena::<File, NFILE>::new_locked("FTABLE")
}

/// Allocate a file structure.
Expand Down
14 changes: 8 additions & 6 deletions kernel-rs/src/fs/inode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ use core::{
ptr,
};

use array_macro::array;
use static_assertions::const_assert;

use super::{FileName, IPB, MAXFILE, NDIRECT, NINDIRECT};
Expand All @@ -87,7 +86,6 @@ use crate::{
param::ROOTDEV,
param::{BSIZE, NINODE},
proc::CurrentProc,
rc_cell::RcCell,
stat::Stat,
vm::UVAddr,
};
Expand Down Expand Up @@ -675,6 +673,13 @@ impl InodeGuard<'_> {
}
}

#[rustfmt::skip] // Need this to use #![feature(const_trait_impl)]
impl const Default for Inode {
fn default() -> Self {
Self::zero()
}
}

impl ArenaObject for Inode {
/// Drop a reference to an in-memory inode.
/// If that was the last reference, the inode table entry can
Expand Down Expand Up @@ -810,10 +815,7 @@ impl Inode {

impl Itable {
pub const fn zero() -> Self {
Spinlock::new(
"ITABLE",
ArrayArena::new(array![_ => RcCell::new(Inode::zero()); NINODE]),
)
ArrayArena::<Inode, NINODE>::new_locked("ITABLE")
}

/// Find the inode with number inum on device dev
Expand Down
2 changes: 2 additions & 0 deletions kernel-rs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@
#![feature(array_value_iter)]
#![feature(const_fn)]
#![feature(const_fn_union)]
#![feature(const_trait_impl)]
#![feature(const_precise_live_drops)]
#![feature(maybe_uninit_extra)]
#![feature(generic_associated_types)]
#![feature(unsafe_block_in_unsafe_fn)]
Expand Down