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

feat(debug)[WIP]: add static-keys support #1025

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Godones
Copy link
Contributor

@Godones Godones commented Oct 31, 2024

这个pr用来讨论引入static keys的支持。是tracepoint #1019 基础设施的一部分。

当前引入的crate的存在的问题:对rust版本的支持比较新,一些feature在DragonOS上还没有稳定。所以我在另一个分支上做了一点点修改。

@dragonosbot
Copy link

@Godones: no appropriate reviewer found, use r? to override

@Godones Godones requested review from Chiichen and fslongjin and removed request for Chiichen October 31, 2024 13:04
Copy link
Member

@fslongjin fslongjin left a comment

Choose a reason for hiding this comment

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

然后我看了你在你的那个仓库里面的修改,最好就是那边的更改通过条件编译的方式去做。哈哈

然后pr里面也链接一下到tracepoint的那个issue?

kernel/src/init/init.rs Show resolved Hide resolved
Comment on lines +1 to +28
#[cfg(feature = "static_keys_test")]
mod tests {
use static_keys::{define_static_key_false, static_branch_unlikely};
define_static_key_false!(MY_STATIC_KEY);
#[inline(always)]
fn foo() {
println!("Entering foo function");
if static_branch_unlikely!(MY_STATIC_KEY) {
println!("A branch");
} else {
println!("B branch");
}
}

pub(super) fn static_keys_test() {
foo();
unsafe {
MY_STATIC_KEY.enable();
}
foo();
}
}

pub fn static_keys_init() {
static_keys::global_init();
#[cfg(feature = "static_keys_test")]
tests::static_keys_test();
}
Copy link
Member

Choose a reason for hiding this comment

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

测试应当有类似assert的机制去自动化,而不是靠人工去看。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这里更多的是作为一个实例,演示该怎么用。

@Godones
Copy link
Contributor Author

Godones commented Oct 31, 2024

然后我看了你在你的那个仓库里面的修改,最好就是那边的更改通过条件编译的方式去做。哈哈

然后pr里面也链接一下到tracepoint的那个issue?

主要这些feature是跟rust版本有关,但是我尝试用那个cfg_version的feature发现不起作用

@fslongjin
Copy link
Member

然后我看了你在你的那个仓库里面的修改,最好就是那边的更改通过条件编译的方式去做。哈哈
然后pr里面也链接一下到tracepoint的那个issue?

主要这些feature是跟rust版本有关,但是我尝试用那个cfg_version的feature发现不起作用

什么版本的rust能跑?什么样的不行?那个不是unsafe{}块而已嘛

@Godones
Copy link
Contributor Author

Godones commented Oct 31, 2024

然后我看了你在你的那个仓库里面的修改,最好就是那边的更改通过条件编译的方式去做。哈哈
然后pr里面也链接一下到tracepoint的那个issue?

主要这些feature是跟rust版本有关,但是我尝试用那个cfg_version的feature发现不起作用

什么版本的rust能跑?什么样的不行?那个不是unsafe{}块而已嘛

#![feature(raw_ref_op)]
#![feature(asm_const)]

这两个feature在最新的nightly(1.84)版本上已经稳定了。dragonos的工具链是1.82,需要这两个feature。我只是不知道你说的根据条件编译是怎么做的?

@fslongjin
Copy link
Member

我只是不知道你说的根据条件编译是怎么做的?

我的意思是在static keys库那里,给那个库加feature的意思。没开feature,就走它原来的代码。开了的话就走你改过的那个版本。

@Godones
Copy link
Contributor Author

Godones commented Nov 1, 2024

我只是不知道你说的根据条件编译是怎么做的?

我的意思是在static keys库那里,给那个库加feature的意思。没开feature,就走它原来的代码。开了的话就走你改过的那个版本。

有没有可能我们升级DragonOS的工具链版本,我发现大多数依赖nightly版本的库都会跟着工具链的更新把一些不稳定的feature给删除掉,因为在最新的工具链上他们已经稳定了

@fslongjin
Copy link
Member

有没有可能我们升级DragonOS的工具链版本,我发现大多数依赖nightly版本的库都会跟着工具链的更新把一些不稳定的feature给删除掉,因为在最新的工具链上他们已经稳定了

我上个月发现如果把dragonos工具链升级到9月版本,会无法运行。原因还没调查清楚。

@Godones
Copy link
Contributor Author

Godones commented Nov 2, 2024

有没有可能我们升级DragonOS的工具链版本,我发现大多数依赖nightly版本的库都会跟着工具链的更新把一些不稳定的feature给删除掉,因为在最新的工具链上他们已经稳定了

我上个月发现如果把dragonos工具链升级到9月版本,会无法运行。原因还没调查清楚。

我试了一下用nightly-2024-10-25编译kernel没出问题

@Chiichen
Copy link
Member

Chiichen commented Nov 2, 2024

有没有可能我们升级DragonOS的工具链版本,我发现大多数依赖nightly版本的库都会跟着工具链的更新把一些不稳定的feature给删除掉,因为在最新的工具链上他们已经稳定了

我上个月发现如果把dragonos工具链升级到9月版本,会无法运行。原因还没调查清楚。

我试了一下用nightly-2024-10-25编译kernel没出问题

你看看升级版本会不会有很多修改?要是还好的话可以开一个升级版本的PR看看,修改点可以参考 #864 #870 CC@fslongjin

@fslongjin
Copy link
Member

我试了一下用nightly-2024-10-25编译kernel没出问题

要在Makefile里面改的噢,我当时在kernel/src/Makefile里面指定了工具链,也许全局搜索以下2024-07-23看看?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants