#70 implement ext2 #79

dust1 · 2021-11-11T16:09:29Z

Summary

it's better to merge this ext2 library once it got a MVP feature like

tasklist

defined struct
try to read super block
...
to be continue🤔

kernel/fs/mod.rs

kernel/fs/ext2/mod.rs

…,DerefMut implement

dust1 · 2021-11-12T14:24:20Z

Sorry, I accidentally uploaded the .idea folder...

nuta · 2021-11-13T03:17:51Z

I think it's better to merge this ext2 library once it got a MVP feature like, reading a file from a pseudo disk.

Could you keep working on it on this PR so I'll occasionally leave some advices?

dust1 · 2021-11-13T03:49:19Z

I think it's better to merge this ext2 library once it got a MVP feature like, reading a file from a pseudo disk.

Could you keep working on it on this PR so I'll occasionally leave some advices?

No problem at all, My original plan was to define the struct of ext2 first. What do I need to do in this PR?

nuta · 2021-11-13T08:50:34Z

The Disk trait defined in this PR looks good to me so let's move forward. The next step would be creating a pseudo ext2 image by invoking genext2fs in the testing module. Then let's try reading its superblock.

dust1 · 2021-11-13T08:51:49Z

The Disk trait defined in this PR looks good to me so let's move forward. The next step would be creating a pseudo ext2 image by invoking genext2fs in the testing module. Then let's try reading its superblock.

Ok, I'm try

dust1 · 2021-11-16T14:19:24Z

Hello, @nuta . I recently referred to the ext2 implementation in linux.
Do we need to distinguish between Big-Endian and Little-Endian of data?🤔

michalfita · 2021-11-16T14:23:27Z

@dust1 Are you implying ext2 can store data in either of them?

dust1 · 2021-11-16T14:34:27Z

@dust1 Are you implying ext2 can store data in either of them?

Sorry, does them refer to different computer hardware environments?
I don’t know much about the C language, so I can’t understand how the following definitions are represented in rust

typedef __u32 __bitwise __be32;
typedef __u64 __bitwise __le64;

dust1 · 2021-11-16T14:50:49Z

@michalfita Maybe I can replace them with u32 ?

nuta · 2021-11-16T15:13:25Z

Yes you should distinguish big/little endian fields for compatibility. It seems a popular crate byteorder provides reader/writer interfaces to handle big/little endian fields while I don't think it's not that convenient to use. How about another byte order handling library like the following? If you prefer this over byteorder, I'll implement this (BigEndianU32, ..) in kerla_utils crate.

use my_neat_byteorder_crate::{BigEndianU32, BigEndianU16};

struct MyStruct {
  foo: BigEndianU32,
  bar: BigEndianU16,
}

fn dump_my_struct() {
    let st: MyStruct = read_my_struct();
    println!("foo = {}", st.to_host());
}

dust1 · 2021-11-16T15:45:55Z

Yes you should distinguish big/little endian fields for compatibility. It seems a popular crate byteorder provides reader/writer interfaces to handle big/little endian fields while I don't think it's not that convenient to use. How about another byte order handling library like the following? If you prefer this over byteorder, I'll implement this (BigEndianU32, ..) in kerla_utils crate.
use my_neat_byteorder_crate::{BigEndianU32, BigEndianU16};

struct MyStruct {
  foo: BigEndianU32,
  bar: BigEndianU16,
}

fn dump_my_struct() {
    let st: MyStruct = read_my_struct();
    println!("foo = {}", st.to_host());
}

The BigEndianU32 and BigEndianU16 look like better.
I think of kerla as a learning project. It's interesting to reimplement something

dust1 · 2021-11-16T15:54:22Z

@michalfita I think I know them, My mind turned around

michalfita · 2021-11-16T20:22:09Z

For clarity and avoidance of doubt them meant both endianess.

@nuta Do we really need to reinvent the wheel in the light of existing solutions, even if they're not greatest? Maybe contribution upstream would help. Linux Kernel implemented absolutely everything for itself and don't have dependencies - that's astonishing achievement, but we have to look at the history of it. From software engineering perspective rewriting potentially faulty code in 2021 is shooting in the foot and increasing of the maintenance burden. ’byteorder’ crate is widely used in embedded software.

nuta · 2021-11-17T15:47:39Z

I agree but we still sometimes need to reinvent a wheel if existing wheels are not suitable for us. Of course I'd use byteorder crate but reading structures with reader interfaces (instead of something like BigEndianU32 I posted above) look not that convenient to me.

I don't have any strong preferences on this at all so would like @dust1 to chose the approach. By the way, it would be helpful to take a look at famous libraries dealing with endianness, for example, scroll looks interesting.

michalfita · 2021-11-17T16:01:56Z

reading structures with reader interfaces (instead of something like BigEndianU32 I posted above) look not that convenient to me

The reason behind this is they keep numbers in their original layout and help with reading and writing buffers - moreover, from what I've read actually plenty of what they're doing can nicely optimized.

You can always stick to:

from_be()
from_le()
flom_le_bytes()
etc. and don't rely on that crate.

Eventually, maybe simple_endian would satisfy you?

dust1 · 2021-11-18T01:06:14Z

When defining the attribute, the number type can be seen intuitively is BigEndian or LittleEndian better for me.
the simple_endian seems to meet out needs?

nuta · 2021-11-18T11:02:37Z

@dust1 I can't guarantee because I haven't used it before but it's worth trying 😃

dust1 · 2021-11-18T11:05:52Z

@dust1 I can't guarantee because I haven't used it before but it's worth trying 😃

Let me try as a guinea pig

dust1 · 2021-11-18T14:41:56Z

@dust1 I can't guarantee because I haven't used it before but it's worth trying 😃

I tried to define the structure with simple_endian, when i run make check, it will throw a exception:

error[E0463]: can't find crate for `std`
  |
  = note: the `x64-18354856424868835069` target may not support the standard library
  = note: `std` is required by `simple_endian` because it does not declare `#![no_std]`
  = help: consider building the standard library from source with `cargo build -Zbuild-std`

For more information about this error, try `rustc --explain E0463`.
error: could not compile `simple_endian` due to previous error
warning: build failed, waiting for other jobs to finish...
error: build failed
make: *** [Makefile:142: check] Error 101

I have run cargo check successfully in the libs/ext2 directory, I checked the environment and confirmed that it is under no_std.
if i use :

[dependencies]
simple_endian = {version = "0.2.1", default-features = false}

it compile fail in libs/ext2 dir.

error[E0277]: the trait bound `u32: SpecificEndian<u32>` is not satisfied
   --> libs/ext2/ext2_h.rs:6:21
    |
6   |     s_inodes_count: u32le,      /* Inodes count */
    |                     ^^^^^ the trait `SpecificEndian<u32>` is not implemented for `u32`
    |
note: required by a bound in `LittleEndian`
   --> /home/yoke/.cargo/registry/src/mirrors.tuna.tsinghua.edu.cn-df7c3c540f42cdbd/simple_endian-0.2.1/src/specific_endian.rs:138:28

Then I tried using byteorder. I found that I cannot specify whether the parameters are big-endianu32 or big-endianu16 when they are defined.
For a newbee like me, the more information a data structure can reflect, the better.
If we want to guarantee the quality of the project, I tend to wrap byteorder to BigEndianU32/LittleEndianU32😢

michalfita · 2021-11-19T18:19:19Z

OK, the fix is publicly available. To used it with your crate put in Cargo.toml:

[dependencies]
simple_endian = { git = "https://github.com/michalfita/simple-endian-rs", branch ="bugfix/4/fix-no_std-support-right-way", default-features = false, features = ["big_endian", "little_endian", "byte_impls", "integer_impls", "comparisons", "format"] }

Warning: above line doesn't exhaust all available features, so please double check if you need anything else. I assumed we don't need floating point, but may missed something else.

The issue is raised as rexlunae/simple-endian-rs#4 and fix PRed as rexlunae/simple-endian-rs#5. Looking at author's forks he's interested in OS development, so hopefully we get his attention soon.

dust1 · 2021-11-20T04:24:01Z

The problem is temporarily solved! thank you @michalfita !!!

dust1 · 2021-11-21T10:28:54Z

Hello, @nuta
I think I have implemented the process of reading the super block from disk.
The main problem I encountered was that when deserializing a binary file, I did not find an efficient conversion crate.
so i use impl Ext2SuperBlock::by_binary, Although it works fine, it looks very ugly.
I used serde in the beginning, but the bincode looks like no support #[!(no_std)].
In the end, I implemented it manually using type aliases.😀

michalfita · 2021-11-21T12:47:08Z

bincode 2.0.0 (in alpha) has no_std support. Haven't checked earlier versions.

dust1 · 2021-11-21T13:02:27Z

bincode 2.0.0 (in alpha) has no_std support. Haven't checked earlier versions.

v2.0.0-alpha.1 is ok, i will edit my code later, thanks

dust1 · 2021-11-29T13:46:47Z

I modified the code related to serialization. and re-modified the directory structure.
But I haven't added test cases. Later I plan to add a new ext2_fs_fuse module specifically for testing

dust1 · 2021-11-30T14:19:20Z

I have added the test and completed the reading of SuperBlock. Next, I plan to implement the structure of SuperBlock in memory, or should I say block cache?

nuta · 2021-12-04T16:04:57Z

Great! Before moving forward, let's discuss the current implementation.

.idea/.gitignore

nuta · 2021-12-04T16:07:34Z

libs/ext2_fs/src/layout.rs

+
+/// Structure of the super block
+/// disk layout
+#[derive(Serialize, Deserialize, Debug, Eq, PartialEq)]


Why did you end up with serde?

This is the serialization tool used in another rust project I am studying, and after searching, I found that the article about this tool is relatively rich.

I'm not sure if we can use serde for our usage as well. How do you plan to deal with endianess?

nuta · 2021-12-04T16:08:33Z

libs/ext2_fs/src/super_block.rs

+    if data.len() < 2048 {
+        return Err(FileSysError::Eof);
+    }
+    let super_block_data = &data[1024..2048];


Why do you skip the first 1024 bytes?

I have read the entire image file in, so i read skip the boot block, it was 1024 bytes.

Out of curiosity, how did you created a test image?

michalfita · 2021-12-04T16:37:26Z

@dust1 If you committed something, adding that to .gitignore isn't enough to get rid of that.

Use of serde in the filesystem implementation bothers me as well.

dust1 · 2021-12-04T16:45:28Z

@dust1 If you committed something, adding that to .gitignore isn't enough to get rid of that.

Use of serde in the filesystem implementation bothers me as well.

I just learned about serde so I used it😂
Recently I was looking at the ext2 implementation of Linux. The flexible operation of the C on memory is really amazing.
But I can't find the corresponding example in rust.

michalfita · 2021-12-04T16:57:32Z

I just learned about serde so I used it

And that's even more worrying. I'm not convinced it's the right tool for the job here. Serde is a serialiser, turns structures of data into a stream of values in particular format. That's pretty compute intensive process.

dust1 · 2021-12-05T12:10:12Z

I just learned about serde so I used it

And that's even more worrying. I'm not convinced it's the right tool for the job here. Serde is a serialiser, turns structures of data into a stream of values in particular format. That's pretty compute intensive process.

Can we implement a simple serialization tool by ourselves? Due to my misconfiguration, I removed the simple_endian you modified before, I'm sorry.

michalfita · 2021-12-05T12:26:54Z

I'm puzzled, @dust1. Why do you insist on serialisation for filesystem implementation? For performance purpose structure in the memory shall be 1:1 with structure on the device. I don't expect ext2 to use odd number of bits anywhere, so things shouldn't be complicated on that matter.

dust1 · 2021-12-05T12:36:31Z

I'm puzzled, @dust1. Why do you insist on serialisation for filesystem implementation? For performance purpose structure in the memory shall be 1:1 with structure on the device. I don't expect ext2 to use odd number of bits anywhere, so things shouldn't be complicated on that matter.

because in my opinion, after reading binary data through BlockDrive, they need to be deserialized into a structure. I read the existing implementation of ext2 and found that they are mapped from disk to data structure according to 1:1. But I don’t know how to implement it in rust. All I know is to serialize and deserialize it through other tools. (When I first saw Linux's operation between memory and structure, I say: wtf?!)

michalfita · 2021-12-05T13:07:08Z

So, you have to learn about data representation and packing, first in C then in Rust. That's formally part of FFI but it's important part low level and embedded programming. Sorry to be blunt, @dust1, but without these foundations you have nothing to do in the filesystem programming. I/O don't have time for serialisation and deserialisation in the kernel.

dust1 · 2021-12-05T13:15:37Z

So, you have to learn about data representation and packing, first in C then in Rust. That's formally part of FFI but it's important part low level and embedded programming. Sorry to be blunt, @dust1, but without these foundations you have nothing to do in the filesystem programming. I/O don't have time for serialisation and deserialisation in the kernel.

This is exactly what I lack, there are not many courses in this area in my undergraduate studies. so I want to make up for my lack of knowledge in this area through this challenging job. Can this PR remain open for a long time. I will supplement this knowledge in my free time and try to apply it to this file system

nuta · 2021-12-06T10:40:03Z

Serde is one of the reasons why I prefer Rust and it's indeed the best way to handle popular data formats like JSON, however, to read a binary format I believe there's a better way, for example, scroll. Since implementing file system in Rust is not that common, you'll need to find best practices on it, and that's what you're currently doing right now!

@dust1, you don't need to hurry at all! Let's gradually get used to the kernel development and enjoy exploring best practices on writing OS in Rust 😃

dust1 · 2021-12-06T11:22:36Z

Serde is one of the reasons why I prefer Rust and it's indeed the best way to handle popular data formats like JSON, however, to read a binary format I believe there's a better way, for example, scroll. Since implementing file system in Rust is not that common, you'll need to find best practices on it, and that's what you're currently doing right now!

@dust1, you don't need to hurry at all! Let's gradually get used to the kernel development and enjoy exploring best practices on writing OS in Rust 😃

thank you, using serde in the OS is indeed a big mistake, next i will edit it.

dust1 · 2021-12-06T13:39:32Z

I find it! Rust can also achieve its purpose through pointer type conversion. Although I don’t have a deep understanding yet, this should be the best solution.

Block replacement algorithm is similar to FIFO

dust1 · 2021-12-20T13:32:28Z

@nuta Sorry it took a long time.
I add block cache. next, should I implement manage the block content in the file system?

nuta · 2021-12-22T10:31:26Z

@dust1 I think you're unsure what you'll do next. How about writing a design proposal (e.g. in Google Doc) first to discuss high-level design of the ext2 support?

dust1 · 2021-12-22T11:06:36Z

ok, @nuta
Recently, I encountered many basic problems in the process of reading ext2, and I am currently learning the file system implemented by other people using rust.
I think I did a wrong demonstration . I should not act so quickly. The lack of preliminary preparation has caused my progress to be very slow or almost zero.
I'm sorry for wasting your time and energy and @michalfita .
Maybe this PR should be closed. I'll resubmit when I'm ready. 😥

nuta · 2021-12-23T01:14:22Z

@dust1 You don't need to hesitate about contributing at all! I would be happy if this becomes the chance for you to dive into the kernel development 😃

There're a lot of work towards Kerla's 1.0.0 release. If you find something interesting for you, feel free to open an issue or PR to ask more from me. By the way, talking about Kerla or operating system concepts is not waste of time for me (rather I'd like to). Please ask me anything 👍

dust1 and others added 4 commits November 11, 2021 23:15

add ext2 mod and add Disk trait with unit test

e73cc23

fix warning and error

f6a2ab6

edit desc

200a290

fix warnings

206e627

dust1 marked this pull request as ready for review November 11, 2021 16:14

nuta requested changes Nov 12, 2021

View reviewed changes

kernel/fs/mod.rs Outdated Show resolved Hide resolved

kernel/fs/ext2/mod.rs Outdated Show resolved Hide resolved

kernel/fs/ext2/mod.rs Outdated Show resolved Hide resolved

kernel/fs/ext2/mod.rs Outdated Show resolved Hide resolved

dust1 added 2 commits November 12, 2021 22:21

move kernel/fs/ext2 to libs/ext2; remove clone an usize; remove Deref…

57395b2

…,DerefMut implement

add gitignore to ignore .idea

ab2250a

dust1 requested a review from nuta November 12, 2021 15:51

dust1 changed the title ~~#70 ext2: add reader/writer library~~ #70 implement ext2 Nov 15, 2021

Merge branch 'nuta:main' into main

899e4f9

implement read super block in file system image

7478ba5

add serde by serialize/deserialize

8411f66

add ext2_fs_fuse by test. and finish it

2c94890

nuta requested changes Dec 4, 2021

View reviewed changes

remove serde, and get struct from binary by pointer

1792668

add block_cache。

4eb13fa

Block replacement algorithm is similar to FIFO

dust1 closed this Dec 23, 2021

#70 implement ext2 #79

#70 implement ext2 #79

Conversation

dust1 commented Nov 11, 2021 • edited Loading

Summary

tasklist

dust1 commented Nov 12, 2021

nuta commented Nov 13, 2021

dust1 commented Nov 13, 2021

nuta commented Nov 13, 2021 • edited Loading

dust1 commented Nov 13, 2021

dust1 commented Nov 16, 2021

michalfita commented Nov 16, 2021

dust1 commented Nov 16, 2021

dust1 commented Nov 16, 2021

nuta commented Nov 16, 2021

dust1 commented Nov 16, 2021

dust1 commented Nov 16, 2021

michalfita commented Nov 16, 2021

nuta commented Nov 17, 2021

michalfita commented Nov 17, 2021

dust1 commented Nov 18, 2021

nuta commented Nov 18, 2021

dust1 commented Nov 18, 2021

dust1 commented Nov 18, 2021 • edited Loading

michalfita commented Nov 19, 2021

dust1 commented Nov 20, 2021

dust1 commented Nov 21, 2021

michalfita commented Nov 21, 2021

dust1 commented Nov 21, 2021

dust1 commented Nov 29, 2021

dust1 commented Nov 30, 2021

nuta commented Dec 4, 2021

nuta Dec 4, 2021

Choose a reason for hiding this comment

dust1 Dec 4, 2021

Choose a reason for hiding this comment

nuta Dec 5, 2021

Choose a reason for hiding this comment

nuta Dec 4, 2021

Choose a reason for hiding this comment

dust1 Dec 4, 2021

Choose a reason for hiding this comment

nuta Dec 5, 2021

Choose a reason for hiding this comment

michalfita commented Dec 4, 2021

dust1 commented Dec 4, 2021

michalfita commented Dec 4, 2021

dust1 commented Dec 5, 2021

michalfita commented Dec 5, 2021

dust1 commented Dec 5, 2021

michalfita commented Dec 5, 2021

dust1 commented Dec 5, 2021

nuta commented Dec 6, 2021

dust1 commented Dec 6, 2021

dust1 commented Dec 6, 2021

dust1 commented Dec 20, 2021

nuta commented Dec 22, 2021

dust1 commented Dec 22, 2021

nuta commented Dec 23, 2021

dust1 commented Nov 11, 2021 •

edited

Loading

nuta commented Nov 13, 2021 •

edited

Loading

dust1 commented Nov 18, 2021 •

edited

Loading