* [PATCH] chore(rust): add opt_get! and expose some FMODE_* as Rust const @ 2023-04-27 18:02 TruongSinh Tran-Nguyen 2023-04-27 20:16 ` Kent Overstreet 0 siblings, 1 reply; 4+ messages in thread From: TruongSinh Tran-Nguyen @ 2023-04-27 18:02 UTC (permalink / raw) To: linux-bcachefs; +Cc: TruongSinh Tran-Nguyen In an effort to rewrite `bch2_read_super` from C to Rust, it is neccessary to have `opt_get!` macro defined, and some FMODE_* consts (defined as macro in `include/linux/blkdev.h`) defined as Rust const. Bindgen is currently unable to exapnd C functional macro [1], this this commit use the workaround as introduced in [2]. [1] https://github.com/rust-lang/rust-bindgen/issues/753 [2] https://github.com/rust-lang/rust-bindgen/issues/753#issuecomment-608546390 Signed-off-by: TruongSinh Tran-Nguyen <i@truongsinh.pro> --- rust-src/bch_bindgen/build.rs | 11 +++++++ .../bch_bindgen/src/libbcachefs_wrapper.h | 7 +++++ rust-src/bch_bindgen/src/opts.rs | 30 +++++++++++++++++-- 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/rust-src/bch_bindgen/build.rs b/rust-src/bch_bindgen/build.rs index f426316..92ec3ce 100644 --- a/rust-src/bch_bindgen/build.rs +++ b/rust-src/bch_bindgen/build.rs @@ -1,3 +1,12 @@ + +#[derive(Debug)] +pub struct Fix753 {} +impl bindgen::callbacks::ParseCallbacks for Fix753 { + fn item_name(&self, original_item_name: &str) -> Option<String> { + Some(original_item_name.trim_start_matches("Fix753_").to_owned()) + } +} + fn main() { use std::path::PathBuf; @@ -49,6 +58,7 @@ fn main() { .blocklist_type("srcu_struct") .allowlist_var("BCH_.*") .allowlist_var("KEY_SPEC_.*") + .allowlist_var("Fix753_FMODE_.*") .allowlist_var("bch.*") .allowlist_var("__BTREE_ITER.*") .allowlist_var("BTREE_ITER.*") @@ -69,6 +79,7 @@ fn main() { .no_partialeq("bkey") .no_partialeq("bpos") .generate_inline_functions(true) + .parse_callbacks(Box::new(Fix753 {})) .generate() .expect("BindGen Generation Failiure: [libbcachefs_wrapper]"); bindings diff --git a/rust-src/bch_bindgen/src/libbcachefs_wrapper.h b/rust-src/bch_bindgen/src/libbcachefs_wrapper.h index d1ebf4b..e7bcfcf 100644 --- a/rust-src/bch_bindgen/src/libbcachefs_wrapper.h +++ b/rust-src/bch_bindgen/src/libbcachefs_wrapper.h @@ -10,4 +10,11 @@ #include "../libbcachefs.h" #include "../crypto.h" #include "../include/linux/bio.h" +#include "../include/linux/blkdev.h" + +#define MARK_FIX_753(req_name) const fmode_t Fix753_##req_name = req_name; + +MARK_FIX_753(FMODE_READ); +MARK_FIX_753(FMODE_WRITE); +MARK_FIX_753(FMODE_EXCL); \ No newline at end of file diff --git a/rust-src/bch_bindgen/src/opts.rs b/rust-src/bch_bindgen/src/opts.rs index e226199..d38d469 100644 --- a/rust-src/bch_bindgen/src/opts.rs +++ b/rust-src/bch_bindgen/src/opts.rs @@ -3,7 +3,33 @@ macro_rules! opt_set { ($opts:ident, $n:ident, $v:expr) => { bch_bindgen::paste! { $opts.$n = $v; - $opts.[<set_ $n _defined>](1); + $opts.[<set_ $n _defined>](1) } - } + }; +} + +#[macro_export] +macro_rules! opt_defined { + ($opts:ident, $n:ident) => { + bch_bindgen::paste! { + $opts.[< $n _defined>]() + } + }; +} + +#[macro_export] +macro_rules! opt_get { + ($opts:ident, $n:ident) => { + if bch_bindgen::opt_defined!($opts, $n) == 0 { + bch_bindgen::paste! { + unsafe { + bch_bindgen::bcachefs::bch2_opts_default.$n + } + } + } else { + bch_bindgen::paste! { + $opts.$n + } + } + }; } -- 2.30.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] chore(rust): add opt_get! and expose some FMODE_* as Rust const 2023-04-27 18:02 [PATCH] chore(rust): add opt_get! and expose some FMODE_* as Rust const TruongSinh Tran-Nguyen @ 2023-04-27 20:16 ` Kent Overstreet 2023-04-27 21:17 ` TruongSinh Tran-Nguyen 0 siblings, 1 reply; 4+ messages in thread From: Kent Overstreet @ 2023-04-27 20:16 UTC (permalink / raw) To: TruongSinh Tran-Nguyen; +Cc: linux-bcachefs On Thu, Apr 27, 2023 at 11:02:00AM -0700, TruongSinh Tran-Nguyen wrote: > In an effort to rewrite `bch2_read_super` from C to Rust, > it is neccessary to have `opt_get!` macro defined, and some > FMODE_* consts (defined as macro in `include/linux/blkdev.h`) > defined as Rust const. > > Bindgen is currently unable to exapnd C functional macro [1], > this this commit use the workaround as introduced in [2]. > > [1] https://github.com/rust-lang/rust-bindgen/issues/753 > [2] https://github.com/rust-lang/rust-bindgen/issues/753#issuecomment-608546390 > > Signed-off-by: TruongSinh Tran-Nguyen <i@truongsinh.pro> > --- > rust-src/bch_bindgen/build.rs | 11 +++++++ > .../bch_bindgen/src/libbcachefs_wrapper.h | 7 +++++ > rust-src/bch_bindgen/src/opts.rs | 30 +++++++++++++++++-- > 3 files changed, 46 insertions(+), 2 deletions(-) > > diff --git a/rust-src/bch_bindgen/build.rs b/rust-src/bch_bindgen/build.rs > index f426316..92ec3ce 100644 > --- a/rust-src/bch_bindgen/build.rs > +++ b/rust-src/bch_bindgen/build.rs > @@ -1,3 +1,12 @@ > + > +#[derive(Debug)] > +pub struct Fix753 {} > +impl bindgen::callbacks::ParseCallbacks for Fix753 { > + fn item_name(&self, original_item_name: &str) -> Option<String> { > + Some(original_item_name.trim_start_matches("Fix753_").to_owned()) > + } > +} > + > fn main() { > use std::path::PathBuf; > > @@ -49,6 +58,7 @@ fn main() { > .blocklist_type("srcu_struct") > .allowlist_var("BCH_.*") > .allowlist_var("KEY_SPEC_.*") > + .allowlist_var("Fix753_FMODE_.*") > .allowlist_var("bch.*") > .allowlist_var("__BTREE_ITER.*") > .allowlist_var("BTREE_ITER.*") > @@ -69,6 +79,7 @@ fn main() { > .no_partialeq("bkey") > .no_partialeq("bpos") > .generate_inline_functions(true) > + .parse_callbacks(Box::new(Fix753 {})) > .generate() > .expect("BindGen Generation Failiure: [libbcachefs_wrapper]"); > bindings > diff --git a/rust-src/bch_bindgen/src/libbcachefs_wrapper.h b/rust-src/bch_bindgen/src/libbcachefs_wrapper.h > index d1ebf4b..e7bcfcf 100644 > --- a/rust-src/bch_bindgen/src/libbcachefs_wrapper.h > +++ b/rust-src/bch_bindgen/src/libbcachefs_wrapper.h > @@ -10,4 +10,11 @@ > #include "../libbcachefs.h" > #include "../crypto.h" > #include "../include/linux/bio.h" > +#include "../include/linux/blkdev.h" > > + > +#define MARK_FIX_753(req_name) const fmode_t Fix753_##req_name = req_name; Do we actually need to include Fix753_ in the constants we define? Why not shadow the macro names? > + > +MARK_FIX_753(FMODE_READ); > +MARK_FIX_753(FMODE_WRITE); > +MARK_FIX_753(FMODE_EXCL); > \ No newline at end of file > diff --git a/rust-src/bch_bindgen/src/opts.rs b/rust-src/bch_bindgen/src/opts.rs > index e226199..d38d469 100644 > --- a/rust-src/bch_bindgen/src/opts.rs > +++ b/rust-src/bch_bindgen/src/opts.rs > @@ -3,7 +3,33 @@ macro_rules! opt_set { > ($opts:ident, $n:ident, $v:expr) => { > bch_bindgen::paste! { > $opts.$n = $v; > - $opts.[<set_ $n _defined>](1); > + $opts.[<set_ $n _defined>](1) > } > - } > + }; > +} > + > +#[macro_export] > +macro_rules! opt_defined { > + ($opts:ident, $n:ident) => { > + bch_bindgen::paste! { > + $opts.[< $n _defined>]() > + } > + }; > +} > + > +#[macro_export] > +macro_rules! opt_get { > + ($opts:ident, $n:ident) => { > + if bch_bindgen::opt_defined!($opts, $n) == 0 { > + bch_bindgen::paste! { > + unsafe { > + bch_bindgen::bcachefs::bch2_opts_default.$n > + } > + } > + } else { > + bch_bindgen::paste! { > + $opts.$n > + } > + } > + }; This is fine, it's a direct translation of the C code. At some point we ought to do a proper Rust version of the opts struct that uses Option<>. (We'd actually want two different versions, one that has every option wrapped in Option<> and one doesn't, since e.g. c->opts doesn't have unset options.) ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] chore(rust): add opt_get! and expose some FMODE_* as Rust const 2023-04-27 20:16 ` Kent Overstreet @ 2023-04-27 21:17 ` TruongSinh Tran-Nguyen 2023-04-27 23:22 ` Kent Overstreet 0 siblings, 1 reply; 4+ messages in thread From: TruongSinh Tran-Nguyen @ 2023-04-27 21:17 UTC (permalink / raw) To: Kent Overstreet; +Cc: linux-bcachefs On Thu, 27 Apr 2023 at 13:16, Kent Overstreet <kent.overstreet@linux.dev> wrote: > > On Thu, Apr 27, 2023 at 11:02:00AM -0700, TruongSinh Tran-Nguyen wrote: > > In an effort to rewrite `bch2_read_super` from C to Rust, > > it is neccessary to have `opt_get!` macro defined, and some > > FMODE_* consts (defined as macro in `include/linux/blkdev.h`) > > defined as Rust const. > > > > Bindgen is currently unable to exapnd C functional macro [1], > > this this commit use the workaround as introduced in [2]. > > > > [1] https://github.com/rust-lang/rust-bindgen/issues/753 > > [2] https://github.com/rust-lang/rust-bindgen/issues/753#issuecomment-608546390 > > > > Signed-off-by: TruongSinh Tran-Nguyen <i@truongsinh.pro> > > --- > > rust-src/bch_bindgen/build.rs | 11 +++++++ > > .../bch_bindgen/src/libbcachefs_wrapper.h | 7 +++++ > > rust-src/bch_bindgen/src/opts.rs | 30 +++++++++++++++++-- > > 3 files changed, 46 insertions(+), 2 deletions(-) > > > > diff --git a/rust-src/bch_bindgen/build.rs b/rust-src/bch_bindgen/build.rs > > index f426316..92ec3ce 100644 > > --- a/rust-src/bch_bindgen/build.rs > > +++ b/rust-src/bch_bindgen/build.rs > > @@ -1,3 +1,12 @@ > > + > > +#[derive(Debug)] > > +pub struct Fix753 {} > > +impl bindgen::callbacks::ParseCallbacks for Fix753 { > > + fn item_name(&self, original_item_name: &str) -> Option<String> { > > + Some(original_item_name.trim_start_matches("Fix753_").to_owned()) > > + } > > +} > > + > > fn main() { > > use std::path::PathBuf; > > > > @@ -49,6 +58,7 @@ fn main() { > > .blocklist_type("srcu_struct") > > .allowlist_var("BCH_.*") > > .allowlist_var("KEY_SPEC_.*") > > + .allowlist_var("Fix753_FMODE_.*") > > .allowlist_var("bch.*") > > .allowlist_var("__BTREE_ITER.*") > > .allowlist_var("BTREE_ITER.*") > > @@ -69,6 +79,7 @@ fn main() { > > .no_partialeq("bkey") > > .no_partialeq("bpos") > > .generate_inline_functions(true) > > + .parse_callbacks(Box::new(Fix753 {})) > > .generate() > > .expect("BindGen Generation Failiure: [libbcachefs_wrapper]"); > > bindings > > diff --git a/rust-src/bch_bindgen/src/libbcachefs_wrapper.h b/rust-src/bch_bindgen/src/libbcachefs_wrapper.h > > index d1ebf4b..e7bcfcf 100644 > > --- a/rust-src/bch_bindgen/src/libbcachefs_wrapper.h > > +++ b/rust-src/bch_bindgen/src/libbcachefs_wrapper.h > > @@ -10,4 +10,11 @@ > > #include "../libbcachefs.h" > > #include "../crypto.h" > > #include "../include/linux/bio.h" > > +#include "../include/linux/blkdev.h" > > > > + > > +#define MARK_FIX_753(req_name) const fmode_t Fix753_##req_name = req_name; > > Do we actually need to include Fix753_ in the constants we define? Why > not shadow the macro names? > > > + > > +MARK_FIX_753(FMODE_READ); > > +MARK_FIX_753(FMODE_WRITE); > > +MARK_FIX_753(FMODE_EXCL); > > \ No newline at end of file > > diff --git a/rust-src/bch_bindgen/src/opts.rs b/rust-src/bch_bindgen/src/opts.rs > > index e226199..d38d469 100644 > > --- a/rust-src/bch_bindgen/src/opts.rs > > +++ b/rust-src/bch_bindgen/src/opts.rs > > @@ -3,7 +3,33 @@ macro_rules! opt_set { > > ($opts:ident, $n:ident, $v:expr) => { > > bch_bindgen::paste! { > > $opts.$n = $v; > > - $opts.[<set_ $n _defined>](1); > > + $opts.[<set_ $n _defined>](1) > > } > > - } > > + }; > > +} > > + > > +#[macro_export] > > +macro_rules! opt_defined { > > + ($opts:ident, $n:ident) => { > > + bch_bindgen::paste! { > > + $opts.[< $n _defined>]() > > + } > > + }; > > +} > > + > > +#[macro_export] > > +macro_rules! opt_get { > > + ($opts:ident, $n:ident) => { > > + if bch_bindgen::opt_defined!($opts, $n) == 0 { > > + bch_bindgen::paste! { > > + unsafe { > > + bch_bindgen::bcachefs::bch2_opts_default.$n > > + } > > + } > > + } else { > > + bch_bindgen::paste! { > > + $opts.$n > > + } > > + } > > + }; > > This is fine, it's a direct translation of the C code. > > At some point we ought to do a proper Rust version of the opts struct > that uses Option<>. (We'd actually want two different versions, one that > has every option wrapped in Option<> and one doesn't, since e.g. c->opts > doesn't have unset options.) > Do we actually need to include Fix753_ in the constants we define? Why not shadow the macro names? The prefix is during the process only, but it will be stripped out by `.parse_callbacks(Box::new(Fix753 {}))`. In other words, in Rust, it would be used like this: ``` sb.mode &= !FMODE_WRITE; ``` As for the reason we need `Fix753_` prefix even though it would not make it to the final generated `bcachefs.rs`, I guess it is rust community convention to have the workaround while waiting for the create to have the fix, and when it is fixed, remove it. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] chore(rust): add opt_get! and expose some FMODE_* as Rust const 2023-04-27 21:17 ` TruongSinh Tran-Nguyen @ 2023-04-27 23:22 ` Kent Overstreet 0 siblings, 0 replies; 4+ messages in thread From: Kent Overstreet @ 2023-04-27 23:22 UTC (permalink / raw) To: TruongSinh Tran-Nguyen; +Cc: linux-bcachefs On Thu, Apr 27, 2023 at 02:17:15PM -0700, TruongSinh Tran-Nguyen wrote: > The prefix is during the process only, but it will be stripped out by > `.parse_callbacks(Box::new(Fix753 {}))`. In other words, in Rust, > it would be used like this: > > ``` > sb.mode &= !FMODE_WRITE; > ``` Weird, but ok. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-04-27 23:22 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-04-27 18:02 [PATCH] chore(rust): add opt_get! and expose some FMODE_* as Rust const TruongSinh Tran-Nguyen 2023-04-27 20:16 ` Kent Overstreet 2023-04-27 21:17 ` TruongSinh Tran-Nguyen 2023-04-27 23:22 ` Kent Overstreet
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).