diff mbox series

[RFC,02/24] erofs: add superblock data structure in Rust

Message ID 20240916135634.98554-3-toolmanp@tlmp.cc (mailing list archive)
State New
Headers show
Series erofs: introduce Rust implementation | expand

Commit Message

Yiyang Wu Sept. 16, 2024, 1:56 p.m. UTC
This patch adds a compilable super_rs.rs and introduces superblock
data structure in Rust. Note that this patch leaves C-side code
untouched.

Signed-off-by: Yiyang Wu <toolmanp@tlmp.cc>
---
 fs/erofs/Kconfig                      |  10 ++
 fs/erofs/Makefile                     |   1 +
 fs/erofs/rust/erofs_sys.rs            |  22 +++++
 fs/erofs/rust/erofs_sys/superblock.rs | 132 ++++++++++++++++++++++++++
 fs/erofs/rust/mod.rs                  |   4 +
 fs/erofs/super_rs.rs                  |   9 ++
 6 files changed, 178 insertions(+)
 create mode 100644 fs/erofs/rust/erofs_sys.rs
 create mode 100644 fs/erofs/rust/erofs_sys/superblock.rs
 create mode 100644 fs/erofs/rust/mod.rs
 create mode 100644 fs/erofs/super_rs.rs

Comments

Greg Kroah-Hartman Sept. 16, 2024, 5:55 p.m. UTC | #1
On Mon, Sep 16, 2024 at 09:56:12PM +0800, Yiyang Wu wrote:
> diff --git a/fs/erofs/rust/erofs_sys.rs b/fs/erofs/rust/erofs_sys.rs
> new file mode 100644
> index 000000000000..0f1400175fc2
> --- /dev/null
> +++ b/fs/erofs/rust/erofs_sys.rs
> @@ -0,0 +1,22 @@
> +#![allow(dead_code)]
> +// Copyright 2024 Yiyang Wu
> +// SPDX-License-Identifier: MIT or GPL-2.0-or-later

Sorry, but I have to ask, why a dual license here?  You are only linking
to GPL-2.0-only code, so why the different license?  Especially if you
used the GPL-2.0-only code to "translate" from.

If you REALLY REALLY want to use a dual license, please get your
lawyers to document why this is needed and put it in the changelog for
the next time you submit this series when adding files with dual
licenses so I don't have to ask again :)

thanks,

greg k-h
Gao Xiang Sept. 17, 2024, 12:18 a.m. UTC | #2
Hi Greg,

On 2024/9/17 01:55, Greg KH wrote:
> On Mon, Sep 16, 2024 at 09:56:12PM +0800, Yiyang Wu wrote:
>> diff --git a/fs/erofs/rust/erofs_sys.rs b/fs/erofs/rust/erofs_sys.rs
>> new file mode 100644
>> index 000000000000..0f1400175fc2
>> --- /dev/null
>> +++ b/fs/erofs/rust/erofs_sys.rs
>> @@ -0,0 +1,22 @@
>> +#![allow(dead_code)]
>> +// Copyright 2024 Yiyang Wu
>> +// SPDX-License-Identifier: MIT or GPL-2.0-or-later
> 
> Sorry, but I have to ask, why a dual license here?  You are only linking
> to GPL-2.0-only code, so why the different license?  Especially if you
> used the GPL-2.0-only code to "translate" from.
> 
> If you REALLY REALLY want to use a dual license, please get your
> lawyers to document why this is needed and put it in the changelog for
> the next time you submit this series when adding files with dual
> licenses so I don't have to ask again :)

As a new Rust kernel developper, Yiyang is working on EROFS Rust
userspace implementation too.

I think he just would like to share the common Rust logic between
kernel and userspace.  Since for the userspace side, Apache-2.0
or even MIT is more friendly for 3rd applications (especially
cloud-native applications). So the dual license is proposed here,
if you don't have strong opinion, I will ask Yiyang document this
in the next version.  Or we're fine to drop MIT too.

Thanks,
Gao Xiang

> 
> thanks,
> 
> greg k-h
Yiyang Wu Sept. 17, 2024, 5:27 a.m. UTC | #3
On Mon, Sep 16, 2024 at 07:55:43PM GMT, Greg KH wrote:
> On Mon, Sep 16, 2024 at 09:56:12PM +0800, Yiyang Wu wrote:
> > diff --git a/fs/erofs/rust/erofs_sys.rs b/fs/erofs/rust/erofs_sys.rs
> > new file mode 100644
> > index 000000000000..0f1400175fc2
> > --- /dev/null
> > +++ b/fs/erofs/rust/erofs_sys.rs
> > @@ -0,0 +1,22 @@
> > +#![allow(dead_code)]
> > +// Copyright 2024 Yiyang Wu
> > +// SPDX-License-Identifier: MIT or GPL-2.0-or-later
> 
> Sorry, but I have to ask, why a dual license here?  You are only linking
> to GPL-2.0-only code, so why the different license?  Especially if you
> used the GPL-2.0-only code to "translate" from.
> 
> If you REALLY REALLY want to use a dual license, please get your
> lawyers to document why this is needed and put it in the changelog for
> the next time you submit this series when adding files with dual
> licenses so I don't have to ask again :)
> 
> thanks,
> 
> greg k-h

C'mon, I just don't want this discussion to be heated.

I mean my original code is licensed under MIT and I've already learned
that Linux is under GPL-2.0. So i originally thought modifying it to
dual licenses can help address incompatiblity issues. According to
wikipedia, may I quote: "When software is multi-licensed, recipients
can typically choose the terms under which they want to use or
distribute the software, but the simple presence of multiple licenses
in a software package or library does not necessarily indicate that
the recipient can freely choose one or the other."[1], so according
to this, I believe putting these under a GPL-2.0 project should be
OK, since it will be forcily licensed **only** under GPL-2.0.

Since I wasn't involved in Kernel Development before, 
I just don't know you guys attitudes towards this kind of stuff.
If you guys are not pretty happy with this, I can just switch back to
GPL-2.0 and it's a big business for me.

Best Regards,
Yiyang Wu.
Greg Kroah-Hartman Sept. 17, 2024, 5:34 a.m. UTC | #4
On Tue, Sep 17, 2024 at 08:18:06AM +0800, Gao Xiang wrote:
> Hi Greg,
> 
> On 2024/9/17 01:55, Greg KH wrote:
> > On Mon, Sep 16, 2024 at 09:56:12PM +0800, Yiyang Wu wrote:
> > > diff --git a/fs/erofs/rust/erofs_sys.rs b/fs/erofs/rust/erofs_sys.rs
> > > new file mode 100644
> > > index 000000000000..0f1400175fc2
> > > --- /dev/null
> > > +++ b/fs/erofs/rust/erofs_sys.rs
> > > @@ -0,0 +1,22 @@
> > > +#![allow(dead_code)]
> > > +// Copyright 2024 Yiyang Wu
> > > +// SPDX-License-Identifier: MIT or GPL-2.0-or-later
> > 
> > Sorry, but I have to ask, why a dual license here?  You are only linking
> > to GPL-2.0-only code, so why the different license?  Especially if you
> > used the GPL-2.0-only code to "translate" from.
> > 
> > If you REALLY REALLY want to use a dual license, please get your
> > lawyers to document why this is needed and put it in the changelog for
> > the next time you submit this series when adding files with dual
> > licenses so I don't have to ask again :)
> 
> As a new Rust kernel developper, Yiyang is working on EROFS Rust
> userspace implementation too.
> 
> I think he just would like to share the common Rust logic between
> kernel and userspace.

Is that actually possible here?  This is very kernel-specific code from
what I can tell, and again, it's based on the existing GPL-v2 code, so
you are kind of changing the license in the transformation to a
different language, right?

> Since for the userspace side, Apache-2.0
> or even MIT is more friendly for 3rd applications (especially
> cloud-native applications). So the dual license is proposed here,
> if you don't have strong opinion, I will ask Yiyang document this
> in the next version.  Or we're fine to drop MIT too.

If you do not have explicit reasons to do this, AND legal approval with
the understanding of how to do dual license kernel code properly, I
would not do it at all as it's a lot of extra work.  Again, talk to your
lawyers about this please.  And if you come up with the "we really want
to do this," great, just document it properly as to what is going on
here and why this decision is made.

thanks,

greg k-h
Yiyang Wu Sept. 17, 2024, 5:39 a.m. UTC | #5
On Mon, Sep 16, 2024 at 07:55:43PM GMT, Greg KH wrote:
> On Mon, Sep 16, 2024 at 09:56:12PM +0800, Yiyang Wu wrote:
> > diff --git a/fs/erofs/rust/erofs_sys.rs b/fs/erofs/rust/erofs_sys.rs
> > new file mode 100644
> > index 000000000000..0f1400175fc2
> > --- /dev/null
> > +++ b/fs/erofs/rust/erofs_sys.rs
> > @@ -0,0 +1,22 @@
> > +#![allow(dead_code)]
> > +// Copyright 2024 Yiyang Wu
> > +// SPDX-License-Identifier: MIT or GPL-2.0-or-later
> 
> Sorry, but I have to ask, why a dual license here?  You are only linking
> to GPL-2.0-only code, so why the different license?  Especially if you
> used the GPL-2.0-only code to "translate" from.
> 
> If you REALLY REALLY want to use a dual license, please get your
> lawyers to document why this is needed and put it in the changelog for
> the next time you submit this series when adding files with dual
> licenses so I don't have to ask again :)
> 
> thanks,
> 
> greg k-h

C'Mon, I have no intension to make this discussion look heated.

I mean what I original code is under MIT and i've learned that Linux
is GPL-2.0, so I naively thought it's OK to dual licensed this to
support flexibility according the Wikipedia, should I quote: "When
software is multi-licensed, recipients can typically choose the terms
under which they want to use or distribute the software, but the simple
presence of multiple licenses in a software package or library does not
necessarily indicate that the recipient can 
freely choose one or the other. "[1]. Since it says multiple licenses
does not necessarily indicate that the recipient can freely choose one
or other,I thought the strictest license applies here and it should
GPL-2.0-only in this case.

I don't have any previous experience in Kernel Development so I really
just have no ideas about you guys attitude towards this kind of issue.
If insisted on switching back to GPL-2.0-only code, It's fine for me
and i'llchange this in the next version. Again I don't have this kind
of knowledge in advance, and if multi-license is inspected case-by-case,
project-by-project, then I will take notes and never make this
kind of mistakes again.

Best Regards,
Yiyang Wu.
Gao Xiang Sept. 17, 2024, 5:45 a.m. UTC | #6
On 2024/9/17 13:34, Greg KH wrote:
> On Tue, Sep 17, 2024 at 08:18:06AM +0800, Gao Xiang wrote:
>> Hi Greg,
>>
>> On 2024/9/17 01:55, Greg KH wrote:
>>> On Mon, Sep 16, 2024 at 09:56:12PM +0800, Yiyang Wu wrote:
>>>> diff --git a/fs/erofs/rust/erofs_sys.rs b/fs/erofs/rust/erofs_sys.rs
>>>> new file mode 100644
>>>> index 000000000000..0f1400175fc2
>>>> --- /dev/null
>>>> +++ b/fs/erofs/rust/erofs_sys.rs
>>>> @@ -0,0 +1,22 @@
>>>> +#![allow(dead_code)]
>>>> +// Copyright 2024 Yiyang Wu
>>>> +// SPDX-License-Identifier: MIT or GPL-2.0-or-later
>>>
>>> Sorry, but I have to ask, why a dual license here?  You are only linking
>>> to GPL-2.0-only code, so why the different license?  Especially if you
>>> used the GPL-2.0-only code to "translate" from.
>>>
>>> If you REALLY REALLY want to use a dual license, please get your
>>> lawyers to document why this is needed and put it in the changelog for
>>> the next time you submit this series when adding files with dual
>>> licenses so I don't have to ask again :)
>>
>> As a new Rust kernel developper, Yiyang is working on EROFS Rust
>> userspace implementation too.
>>
>> I think he just would like to share the common Rust logic between
>> kernel and userspace.
> 
> Is that actually possible here?  This is very kernel-specific code from
> what I can tell, and again, it's based on the existing GPL-v2 code, so
> you are kind of changing the license in the transformation to a
> different language, right?

It's possible, Yiyang implemented a total userspace Rust crates
to parse EROFS format with limited APIs:

https://github.com/ToolmanP/erofs-rs

Also take another C example, kernel XFS (fs/libxfs) and xfsprogs
(userspace) use the same codebase.  Although they both use GPL
license only.

> 
>> Since for the userspace side, Apache-2.0
>> or even MIT is more friendly for 3rd applications (especially
>> cloud-native applications). So the dual license is proposed here,
>> if you don't have strong opinion, I will ask Yiyang document this
>> in the next version.  Or we're fine to drop MIT too.
> 
> If you do not have explicit reasons to do this, AND legal approval with
> the understanding of how to do dual license kernel code properly, I
> would not do it at all as it's a lot of extra work.  Again, talk to your
> lawyers about this please.  And if you come up with the "we really want
> to do this," great, just document it properly as to what is going on
> here and why this decision is made.

Ok, then let's stay with GPL only.  Although as I mentioned,
cloud-native applications are happy with Apache-2.0 or MIT, which
means there could be diverged for kernel and userspace on the Rust
side too.

Thanks,
Gao Xiang

> 
> thanks,
> 
> greg k-h
diff mbox series

Patch

diff --git a/fs/erofs/Kconfig b/fs/erofs/Kconfig
index 6ea60661fa55..e2883efbf497 100644
--- a/fs/erofs/Kconfig
+++ b/fs/erofs/Kconfig
@@ -178,3 +178,13 @@  config EROFS_FS_PCPU_KTHREAD_HIPRI
 	  at higher priority.
 
 	  If unsure, say N.
+
+config EROFS_FS_RUST
+	bool "EROFS use RUST Replacement (EXPERIMENTAL)"
+	depends on EROFS_FS && RUST
+	help
+	  This permits EROFS to use EXPERIMENTAL Rust implementation
+	  for EROFS. This should be considered as an experimental
+	  feature for now.
+
+	  If unsure, say N.
diff --git a/fs/erofs/Makefile b/fs/erofs/Makefile
index 4331d53c7109..fb46a2c7fb50 100644
--- a/fs/erofs/Makefile
+++ b/fs/erofs/Makefile
@@ -9,3 +9,4 @@  erofs-$(CONFIG_EROFS_FS_ZIP_DEFLATE) += decompressor_deflate.o
 erofs-$(CONFIG_EROFS_FS_ZIP_ZSTD) += decompressor_zstd.o
 erofs-$(CONFIG_EROFS_FS_BACKED_BY_FILE) += fileio.o
 erofs-$(CONFIG_EROFS_FS_ONDEMAND) += fscache.o
+erofs-$(CONFIG_EROFS_FS_RUST) += super_rs.o
diff --git a/fs/erofs/rust/erofs_sys.rs b/fs/erofs/rust/erofs_sys.rs
new file mode 100644
index 000000000000..0f1400175fc2
--- /dev/null
+++ b/fs/erofs/rust/erofs_sys.rs
@@ -0,0 +1,22 @@ 
+#![allow(dead_code)]
+// Copyright 2024 Yiyang Wu
+// SPDX-License-Identifier: MIT or GPL-2.0-or-later
+
+//! A pure Rust implementation of the EROFS filesystem.
+//! Technical Details are documented in the [EROFS Documentation](https://erofs.docs.kernel.org/en/latest/)
+
+// It's unavoidable to import alloc here. Since there are so many backends there and if we want to
+// to use trait object to export Filesystem pointer. The alloc crate here is necessary.
+
+#[cfg(not(CONFIG_EROFS_FS = "y"))]
+extern crate alloc;
+
+/// Erofs requires block index to a 32 bit unsigned integer.
+pub(crate) type Blk = u32;
+/// Erofs requires normal offset to be a 64bit unsigned integer.
+pub(crate) type Off = u64;
+/// Erofs requires inode nid to be a 64bit unsigned integer.
+pub(crate) type Nid = u64;
+/// Erofs Super Offset to read the ondisk superblock
+pub(crate) const EROFS_SUPER_OFFSET: Off = 1024;
+pub(crate) mod superblock;
diff --git a/fs/erofs/rust/erofs_sys/superblock.rs b/fs/erofs/rust/erofs_sys/superblock.rs
new file mode 100644
index 000000000000..213be6dbc553
--- /dev/null
+++ b/fs/erofs/rust/erofs_sys/superblock.rs
@@ -0,0 +1,132 @@ 
+// Copyright 2024 Yiyang Wu
+// SPDX-License-Identifier: MIT or GPL-2.0-or-later
+
+use super::*;
+use core::mem::size_of;
+
+/// The ondisk superblock structure.
+#[derive(Debug, Clone, Copy, Default)]
+#[repr(C)]
+pub(crate) struct SuperBlock {
+    pub(crate) magic: u32,
+    pub(crate) checksum: i32,
+    pub(crate) feature_compat: i32,
+    pub(crate) blkszbits: u8,
+    pub(crate) sb_extslots: u8,
+    pub(crate) root_nid: i16,
+    pub(crate) inos: i64,
+    pub(crate) build_time: i64,
+    pub(crate) build_time_nsec: i32,
+    pub(crate) blocks: i32,
+    pub(crate) meta_blkaddr: u32,
+    pub(crate) xattr_blkaddr: u32,
+    pub(crate) uuid: [u8; 16],
+    pub(crate) volume_name: [u8; 16],
+    pub(crate) feature_incompat: i32,
+    pub(crate) compression: i16,
+    pub(crate) extra_devices: i16,
+    pub(crate) devt_slotoff: i16,
+    pub(crate) dirblkbits: u8,
+    pub(crate) xattr_prefix_count: u8,
+    pub(crate) xattr_prefix_start: i32,
+    pub(crate) packed_nid: i64,
+    pub(crate) xattr_filter_reserved: u8,
+    pub(crate) reserved: [u8; 23],
+}
+
+impl TryFrom<&[u8]> for SuperBlock {
+    type Error = core::array::TryFromSliceError;
+    fn try_from(value: &[u8]) -> Result<Self, Self::Error> {
+        value[0..128].try_into()
+    }
+}
+
+impl From<[u8; 128]> for SuperBlock {
+    fn from(value: [u8; 128]) -> Self {
+        Self {
+            magic: u32::from_le_bytes([value[0], value[1], value[2], value[3]]),
+            checksum: i32::from_le_bytes([value[4], value[5], value[6], value[7]]),
+            feature_compat: i32::from_le_bytes([value[8], value[9], value[10], value[11]]),
+            blkszbits: value[12],
+            sb_extslots: value[13],
+            root_nid: i16::from_le_bytes([value[14], value[15]]),
+            inos: i64::from_le_bytes([
+                value[16], value[17], value[18], value[19], value[20], value[21], value[22],
+                value[23],
+            ]),
+            build_time: i64::from_le_bytes([
+                value[24], value[25], value[26], value[27], value[28], value[29], value[30],
+                value[31],
+            ]),
+            build_time_nsec: i32::from_le_bytes([value[32], value[33], value[34], value[35]]),
+            blocks: i32::from_le_bytes([value[36], value[37], value[38], value[39]]),
+            meta_blkaddr: u32::from_le_bytes([value[40], value[41], value[42], value[43]]),
+            xattr_blkaddr: u32::from_le_bytes([value[44], value[45], value[46], value[47]]),
+            uuid: value[48..64].try_into().unwrap(),
+            volume_name: value[64..80].try_into().unwrap(),
+            feature_incompat: i32::from_le_bytes([value[80], value[81], value[82], value[83]]),
+            compression: i16::from_le_bytes([value[84], value[85]]),
+            extra_devices: i16::from_le_bytes([value[86], value[87]]),
+            devt_slotoff: i16::from_le_bytes([value[88], value[89]]),
+            dirblkbits: value[90],
+            xattr_prefix_count: value[91],
+            xattr_prefix_start: i32::from_le_bytes([value[92], value[93], value[94], value[95]]),
+            packed_nid: i64::from_le_bytes([
+                value[96], value[97], value[98], value[99], value[100], value[101], value[102],
+                value[103],
+            ]),
+            xattr_filter_reserved: value[104],
+            reserved: value[105..128].try_into().unwrap(),
+        }
+    }
+}
+
+pub(crate) type SuperBlockBuf = [u8; size_of::<SuperBlock>()];
+pub(crate) const SUPERBLOCK_EMPTY_BUF: SuperBlockBuf = [0; size_of::<SuperBlock>()];
+
+/// Used for external address calculation.
+pub(crate) struct Accessor {
+    pub(crate) base: Off,
+    pub(crate) off: Off,
+    pub(crate) len: Off,
+    pub(crate) nr: Off,
+}
+
+impl Accessor {
+    pub(crate) fn new(address: Off, bits: Off) -> Self {
+        let sz = 1 << bits;
+        let mask = sz - 1;
+        Accessor {
+            base: (address >> bits) << bits,
+            off: address & mask,
+            len: sz - (address & mask),
+            nr: address >> bits,
+        }
+    }
+}
+
+impl SuperBlock {
+    pub(crate) fn blk_access(&self, address: Off) -> Accessor {
+        Accessor::new(address, self.blkszbits as Off)
+    }
+
+    pub(crate) fn blknr(&self, pos: Off) -> Blk {
+        (pos >> self.blkszbits) as Blk
+    }
+
+    pub(crate) fn blkpos(&self, blk: Blk) -> Off {
+        (blk as Off) << self.blkszbits
+    }
+
+    pub(crate) fn blksz(&self) -> Off {
+        1 << self.blkszbits
+    }
+
+    pub(crate) fn blk_round_up(&self, addr: Off) -> Blk {
+        ((addr + self.blksz() - 1) >> self.blkszbits) as Blk
+    }
+
+    pub(crate) fn iloc(&self, nid: Nid) -> Off {
+        self.blkpos(self.meta_blkaddr) + ((nid as Off) << (5 as Off))
+    }
+}
diff --git a/fs/erofs/rust/mod.rs b/fs/erofs/rust/mod.rs
new file mode 100644
index 000000000000..e6c0731f2533
--- /dev/null
+++ b/fs/erofs/rust/mod.rs
@@ -0,0 +1,4 @@ 
+// Copyright 2024 Yiyang Wu
+// SPDX-License-Identifier: MIT or GPL-2.0-or-later
+
+pub(crate) mod erofs_sys;
diff --git a/fs/erofs/super_rs.rs b/fs/erofs/super_rs.rs
new file mode 100644
index 000000000000..4b8cbef507e3
--- /dev/null
+++ b/fs/erofs/super_rs.rs
@@ -0,0 +1,9 @@ 
+// Copyright 2024 Yiyang Wu
+// SPDX-License-Identifier: MIT or GPL-2.0-or-later
+
+//! EROFS Rust Kernel Module Helpers Implementation
+//! This is only for experimental purpose. Feedback is always welcome.
+
+#[allow(dead_code)]
+#[allow(missing_docs)]
+pub(crate) mod rust;