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

Build fail on linux-next (next-20220629 tag): the name maple_enode is defined multiple times #795

Closed
mortonar opened this issue Jun 29, 2022 · 14 comments
Labels
• kbuild Related to building the kernel, `make`, `Kbuild`, `Kconfig` options...

Comments

@mortonar
Copy link

I was trying build the linux-next tree today with Rust support enabled and ran into the following issue:

$ LLVM=1 make -j8
[...]
  RUSTC L rust/kernel.o
error[E0428]: the name `maple_enode` is defined multiple times
     --> /home/mortona/workspaces/linux/kernels/next/linux/rust/bindings_generated.rs:41359:1
      |
41356 | pub struct maple_enode {
      | ---------------------- previous definition of the type `maple_enode` here
...
41359 | pub type maple_enode = *mut maple_enode;
      | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `maple_enode` redefined here
      |
      = note: `maple_enode` must be defined only once in the type namespace of this module

I've attached my .config for reference.
rust-config.txt

@mortonar mortonar added the • kbuild Related to building the kernel, `make`, `Kbuild`, `Kconfig` options... label Jun 29, 2022
@ojeda
Copy link
Member

ojeda commented Jun 29, 2022

Thanks!

(For future reference, this came from Zulip: https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/Newbie.20Corner/near/287880755)

@Richardhongyu
Copy link

Richardhongyu commented Jun 30, 2022

This bug is due to the wrong typedefs in linux/include/maple_tree.h.

It should be changed from

typedef struct maple_enode *maple_enode; /* encoded node */
typedef struct maple_pnode *maple_pnode; /* parent node */

to

typedef struct maple_enode *maple_enode_p; /* encoded node */
typedef struct maple_pnode *maple_pnode_p; /* parent node */

After solving this, there is also a bug related to file_operations. The following code should be added to the 537th line of rust/kernel/file.rs.

     uring_cmd: None,

The kernel can successfully boot after solving these two bugs.

@Richardhongyu
Copy link

It seems the linux-next git tree is not in the github. Is it proper to send the bugfix patch to the linux-next community by email?

@ojeda
Copy link
Member

ojeda commented Jun 30, 2022

This bug is due to the wrong typedefs in linux/include/maple_tree.h.

The C code seems OK (even if potentially confusing) -- the problem is that bindgen does not work when you create such a typedef.

This seems to be rust-lang/rust-bindgen#2227 (and maybe others since that one is recent and I just did a quick look). I am adding it to the wish list.

It seems the linux-next git tree is not in the github. Is it proper to send the bugfix patch to the linux-next community by email?

linux-next is recreated every day or so merging different trees. Instead, the best would be to email the respective maintainers of the tree, the LKML etc. (but I wouldn't in this case, since I don't think this is a problem on the C side).

After solving this, there is also a bug related to file_operations. The following code should be added to the 537th line of rust/kernel/file.rs.

This one was reported and fixed a while ago (in rust so far).

Thanks a lot for taking a look!

@Richardhongyu
Copy link

The C code seems OK (even if potentially confusing) -- the problem is that bindgen does not work when you create such a typedef.

This seems to be rust-lang/rust-bindgen#2227 (and maybe others since that one is recent and I just did a quick look). I am adding it to the wish list.

typedef struct foo *foo; works fine in a c project from a grammar perspective. It seems in Linux it prefers typedef struct foo *foo_p;, e.g. typedef struct cpumask *cpumask_var_t; and typedef struct cgraph_node *cgraph_node_ptr;. I use re to search in the Linux project and cannot find another example in typedef struct foo *foo; style.

However, by fixing the bug of rust-bindgen, this can be solved fundamentally. It is a better way. I will try to see whether I can fix this from rust-bindgen side.

linux-next is recreated every day or so merging different trees. Instead, the best would be to email the respective maintainers of the tree, the LKML etc. (but I wouldn't in this case, since I don't think this is a problem on the C side).

Thanks for helping!

This one was reported and fixed a while ago (in rust so far).
Thanks a lot for taking a look!

Glad to participate in :)

@ojeda
Copy link
Member

ojeda commented Jun 30, 2022

typedef struct foo *foo; works fine in a c project from a grammar perspective. It seems in Linux it prefers typedef struct foo *foo_p;, e.g. typedef struct cpumask *cpumask_var_t; and typedef struct cgraph_node *cgraph_node_ptr;. I use re to search in the Linux project and cannot find another example in typedef struct foo *foo; style.

Yeah, I think it can be worth checking with the Maple maintainer, perhaps they agree to rename it for clarity anyway. I also cannot find another example (with a pointer) from a quick grep.

@ojeda
Copy link
Member

ojeda commented Jul 16, 2022

Another report at https://lore.kernel.org/lkml/20220716124214.329949-1-conor@kernel.org/, from Conor Dooley.

@ConchuOD
Copy link

Another report at https://lore.kernel.org/lkml/20220716124214.329949-1-conor@kernel.org/, from Conor Dooley.

Sorry for the noise, I expected development/reports on the ml not on a github!

@ojeda
Copy link
Member

ojeda commented Jul 16, 2022

No worries at all Conor!

And yeah, currently most development is happening here, though things will probably change when/if we get merged. The issue tracker is nice to have and probably will remain here, especially since it allows us to easily link and ping things into different Rust projects we use etc.

intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Jul 17, 2022
The current name of the pointer to struct maple_enode is also
maple_enode. This is correct from the grammar point but can be
comfusing. Besides it seems in Linux it prefers typedef struct foo
*foo_p;, e.g. typedef struct cpumask *cpumask_var_t; and typedef
struct cgraph_node *cgraph_node_ptr;. I use re to search in the
Linux project and cannot find another example in typedef struct
foo *foo; style.

This also results in a bug in the bindings of the
rust-for-linux subsystem, which can be seen in this github issue.

Rust-for-Linux#795

The struct pointer maple_enode and maple_pnode are not used. It is
safe to change it to a new name.

Signed-off-by: Li Hongyu <lihongyu1999@bupt.edu.cn>
@ojeda
Copy link
Member

ojeda commented Jul 17, 2022

Patch submitted at https://lore.kernel.org/linux-mm/20220717120652.GA9281@38c3a67cb865/ by Li Hongyu.

@Richardhongyu
Copy link

Richardhongyu commented Jul 20, 2022

The current state of this patch is https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/maple-tree-add-new-data-structure-fix-2.patch. It will appear in the mm-unstable branch later and will be merged into linux-next branch finally.

@Richardhongyu
Copy link

The patch is merged into the linux-next branch. This issue can be closed.

@ojeda
Copy link
Member

ojeda commented Jul 27, 2022

Thanks Richard! Let's wait a bit more until the commit actually hits Linus' tree (linux-next is not a real branch, so to speak).

@ojeda
Copy link
Member

ojeda commented Oct 6, 2022

Closing this one, since the latest two patch series (v13 and v14) do not have the typedefs anymore, e.g. https://lore.kernel.org/all/20220906194824.2110408-2-Liam.Howlett@oracle.com/#Z31include:linux:maple_tree.h, so it is unlikely they will appear again even before the Maple Tree reaches Linus' tree.

@ojeda ojeda closed this as completed Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
• kbuild Related to building the kernel, `make`, `Kbuild`, `Kconfig` options...
Development

No branches or pull requests

4 participants