Skip to content

Commit ce68875

Browse files
committed
fix: wire F_SETSIG into fasync delivery
1 parent dd0c44c commit ce68875

17 files changed

Lines changed: 605 additions & 76 deletions

File tree

AGENTS.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,13 @@ DragonOS是一个面向云计算轻量化场景的,完全自主内核的,提
3535

3636
- 符合Linux 6.6的语义
3737
- 结合测例报错、测例代码、DragonOS代码、Linux行为实现来深入分析
38+
- 修复gVisor系统调用兼容问题时,不能只让接口返回值通过;必须检查接口背后的真实内核状态变化与可观测副作用(例如信号投递、poll/epoll通知、fd继承/dup语义)。
39+
- 涉及异步I/O通知(`O_ASYNC`/`FASYNC`/`F_SETOWN`/`F_SETSIG`/`F_GETSIG`)时,owner pid 与通知信号必须作为同一一致性域建模,参考Linux `fown_struct`;不得用分散的独立原子字段保存相关状态。
40+
- 涉及 `fcntl(F_SETFL, O_ASYNC)``ioctl(FIOASYNC)` 时,两条入口必须复用同一套fasync注册/注销逻辑,避免只更新 `FileFlags` 而没有更新通知链路。
41+
- 涉及 `F_SETSIG` 的修复必须验证真实异步通知路径:默认 `0` 发送 `SIGIO`,非0值发送用户指定信号,并在支持 `SA_SIGINFO` 时维护 `si_fd`/`si_band`
3842

3943
### 开发时的一些常见命令
4044

4145
- 格式化代码:在项目根目录下运行`make fmt`,会自动格式化,并且运行clippy检查
4246
- 编译内核:在项目根目录下运行`make kernel`. 当你想检查你编辑的代码有没有语法错误的时候,请执行这个命令
4347

44-
Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
# F_SETSIG/F_GETSIG 与 fasync 修复方案
2+
3+
## 1. 背景
4+
5+
Issue: https://github.com/DragonOS-Community/DragonOS/issues/1841
6+
7+
PR: https://github.com/DragonOS-Community/DragonOS/pull/1880
8+
9+
目标是在 DragonOS 中实现 Linux 兼容的 `fcntl(F_SETSIG)``fcntl(F_GETSIG)` 语义,使异步 I/O 通知不只支持默认 `SIGIO`,还支持用户通过 `F_SETSIG` 设置的标准信号或实时信号。
10+
11+
PR 1880 已经补充了 `FcntlCommand::SetSig``FcntlCommand::GetSig` 以及基础的 set/get 测试,但 review 指出了两个关键问题:
12+
13+
- `F_SETSIG` 的值只被保存,没有接入 `fasync` 实际投递路径。
14+
- signal owner 状态不应作为独立原子变量直接放在 `File` 中,应与 owner pid 一起建模并使用同一把锁保护。
15+
16+
## 2. 测试范围理解
17+
18+
gVisor `test/syscalls/linux/fcntl.cc` 中相关测试覆盖两类行为:
19+
20+
| 测试范围 | 期望语义 |
21+
|---------|---------|
22+
| `FcntlTest.SetSig*` | `F_SETSIG` 校验信号值并持久化,`F_GETSIG` 返回当前设置,非法值不覆盖旧值 |
23+
| `FcntlSignalTest.SetSigDefault` | `signum == 0` 表示默认行为,I/O ready 时投递 `SIGIO` |
24+
| `FcntlSignalTest.SetSigCustom` | 非 0 signum 应投递用户指定信号,并在 `siginfo_t` 中携带 `si_fd``si_band` |
25+
| `FcntlSignalTest.SetSigWithSigioStillGetsSiginfo` | 显式设置 `SIGIO` 与默认 `0` 不等价;前者应携带 `siginfo_t` |
26+
| `FcntlSignalTest.SetSigDup*` | `dup` 后共享同一个 open file description,但 fasync 注册项记录触发通知时使用的 fd |
27+
| `FcntlSignalTest.SetSigDupUnregister*` | 重新设置 `F_SETSIG(0)` 后回到默认 `SIGIO` 行为 |
28+
| `FcntlSignalTest.ConcurrentSetSigSetOwnSetOAsync` | `F_SETOWN``F_SETSIG``F_SETFL(O_ASYNC)` 并发更新不能产生不一致状态 |
29+
30+
因此,修复不能只做 `F_SETSIG/F_GETSIG` round-trip,还必须打通真实异步通知链路。
31+
32+
## 3. 内核现状
33+
34+
| 位置 | 现状 | 问题 |
35+
|-----|------|------|
36+
| `kernel/src/filesystem/vfs/fcntl.rs` | 当前 master 尚无 `SetSig = 10``GetSig = 11` | 命令无法被 `FcntlCommand::from_u32` 识别 |
37+
| `kernel/src/filesystem/vfs/file.rs` | `File` 使用 `pid: Mutex<Option<Arc<ProcessControlBlock>>>` 记录异步通知 owner | owner pid 与 signum 分散存储会产生状态一致性问题 |
38+
| `kernel/src/filesystem/vfs/fasync.rs` | `send_sigio()` 固定发送 `Signal::SIGIO_OR_POLL` | 忽略 `F_SETSIG` 设置,用户仍只能收到默认 `SIGIO` |
39+
| `kernel/src/filesystem/vfs/fasync.rs` | `FAsyncItem` 只保存 `Weak<File>` | 无法按 Linux `fasync_struct::fa_fd` 语义返回注册 fd |
40+
| `kernel/src/filesystem/vfs/syscall/sys_fcntl.rs` | `F_SETFL` 只更新 `FileFlags::FASYNC` | 通过 `fcntl(F_SETFL, O_ASYNC)` 不会注册/注销 `FAsyncItem` |
41+
| `kernel/src/filesystem/vfs/syscall/sys_ioctl.rs` | `FIOASYNC` 内联完成 flags 更新和 fasync 注册 | 可复用逻辑没有抽成 VFS helper,容易与 `F_SETFL` 行为分叉 |
42+
| `kernel/src/ipc/signal_types.rs` | `PosixSiginfoSigpoll` 已有 `si_band/si_fd` 布局,但 `SigType` 没有 sigpoll 变体 | 无法构造符合 `SA_SIGINFO` 的异步 I/O 信号 |
43+
44+
## 4. 根因分析
45+
46+
| 测试点 | Linux 期望 | DragonOS 实际 | 差距 |
47+
|-------|------------|---------------|------|
48+
| `F_GETSIG/F_SETSIG` 命令识别 | `F_GETSIG = 11` 返回 `file.f_owner.signum``F_SETSIG = 10` 校验后更新 | 当前 master 未识别命令;PR 1880 只补了存取 | 缺少完整命令实现 |
49+
| owner/signum 状态 | Linux `struct fown_struct` 同时保存 pid、pid type、uid/euid、signum,并通过 lock 保护 owner 字段 | DragonOS 只有 `pid`,PR 1880 将 `signum` 作为独立原子字段 | owner 和 signum 不是同一一致性域 |
50+
| fasync 投递 | Linux `kill_fasync``fa_file->f_owner` 读取 signum,并调用 `send_sigio(fown, fa_fd, band)` | DragonOS `FAsyncItems::send_sigio()` 固定发送 `SIGIO_OR_POLL` | `F_SETSIG` 不影响真实通知 |
51+
| `siginfo_t` | 显式 `F_SETSIG(nonzero)` 投递 queued SIGIO 风格信号,携带 `si_fd``si_band` | 当前信号路径只构造 kill/user 类型信息 | `SA_SIGINFO` 测试无法通过 |
52+
| `F_SETFL(O_ASYNC)` | 设置/清除 `O_ASYNC` 时更新 fasync 注册状态 | `F_SETFL` 只改 flags;`FIOASYNC` 才注册 | gVisor `RegisterFD()` 使用 `F_SETFL`,真实投递链路缺失 |
53+
54+
## 5. 修复方案
55+
56+
### 5.1 关键改动
57+
58+
| 文件 | 改动 | 原因 |
59+
|-----|------|------|
60+
| `kernel/src/filesystem/vfs/fcntl.rs` | 增加 `SetSig = 10``GetSig = 11` | 识别 Linux 标准 fcntl 命令 |
61+
| `kernel/src/filesystem/vfs/file.rs` | 引入 `FileOwner`,用 `Mutex<FileOwner>` 替代 `pid: Mutex<Option<Arc<ProcessControlBlock>>>` | 对齐 Linux `fown_struct` 的 owner/signum 一致性域 |
62+
| `kernel/src/filesystem/vfs/file.rs` | 提供 `owner_snapshot()``set_owner()``set_owner_signum()``owner_signum()` 等方法 | 避免调用者直接拼装或拆分 owner 状态 |
63+
| `kernel/src/filesystem/vfs/fasync.rs` | `FAsyncItem` 保存注册 fd,并在发送时使用 owner signum 与 fd | 支持 `si_fd` 和 dup 场景 |
64+
| `kernel/src/filesystem/vfs/fasync.rs` | 将固定 `SIGIO` 发送改为 `signum == 0` 发默认 `SIGIO_OR_POLL`,非 0 发指定信号 |`F_SETSIG` 影响真实投递 |
65+
| `kernel/src/ipc/pipe.rs` | 为 pipe 维护读端/写端两组 fasync 注册队列 | 避免写端收到可读通知,读端收到可写通知 |
66+
| `kernel/src/ipc/signal_types.rs` | 增加 `SigType::SigPoll { fd, band }` 并转换到 `_sigpoll` | 支持 `SA_SIGINFO` 下的 `si_fd/si_band` |
67+
| `kernel/src/filesystem/vfs/syscall/sys_fcntl.rs` | 实现 `F_SETSIG/F_GETSIG`,并复用统一 fasync 切换 helper | 保持 `F_SETFL(O_ASYNC)``FIOASYNC` 行为一致 |
68+
| `kernel/src/filesystem/vfs/syscall/sys_ioctl.rs` | `FIOASYNC` 改为调用统一 helper | 避免两条入口语义分叉 |
69+
70+
### 5.2 推荐数据结构
71+
72+
`file.rs` 中新增 owner 状态对象:
73+
74+
```rust
75+
#[derive(Clone, Debug)]
76+
pub struct FileOwnerSnapshot {
77+
pub pcb: Option<Arc<ProcessControlBlock>>,
78+
pub signum: i32,
79+
}
80+
81+
#[derive(Debug)]
82+
struct FileOwner {
83+
pcb: Option<Arc<ProcessControlBlock>>,
84+
signum: i32,
85+
}
86+
```
87+
88+
`File` 中使用:
89+
90+
```rust
91+
owner: Mutex<FileOwner>,
92+
```
93+
94+
约束:
95+
96+
- `signum == 0` 表示默认 `SIGIO` 行为。
97+
- `F_SETSIG` 只接受 `0..=Signal::SIGRTMAX`
98+
- 不允许新增独立 `AtomicI32` 保存 `F_SETSIG` 状态。
99+
- `F_SETOWN``F_SETSIG` 必须通过 `FileOwner` 方法更新,避免调用者绕过锁。
100+
101+
### 5.3 fasync helper
102+
103+
建议新增一个统一 helper,供 `F_SETFL``FIOASYNC` 同时调用:
104+
105+
```rust
106+
pub fn set_file_fasync(file: &Arc<File>, fd: i32, enabled: bool) -> Result<(), SystemError>
107+
```
108+
109+
职责:
110+
111+
- 更新 `FileFlags::FASYNC`。
112+
- `enabled == true` 时创建或更新 `FAsyncItem`,记录 `Weak<File>` 与注册 fd
113+
- `enabled == false` 时移除当前 file 对应注册项。
114+
- 对不支持 `PollableInode::add_fasync` 的 inode 保持现有兼容策略:flags 可更新,注册失败不应破坏普通 `F_SETFL` 语义,但需要在代码注释中说明。
115+
116+
### 5.4 信号投递策略
117+
118+
`FAsyncItems::send_sigio()` 应改为按事件类型传递 band
119+
120+
```rust
121+
pub fn send_sigio(&self, band: i32)
122+
```
123+
124+
初期可先在调用点传入常用值:
125+
126+
- 可读事件:`POLL_IN` 对应 `EPOLLIN | EPOLLRDNORM`。
127+
- 可写事件:`POLL_OUT` 对应 `EPOLLOUT | EPOLLWRNORM | EPOLLWRBAND`。
128+
- 暂时无法区分的旧调用点可先传入 `POLL_IN`,但必须在文档或注释中标明后续要按事件类型细分。
129+
130+
发送规则:
131+
132+
- `signum == 0`:发送默认 `Signal::SIGIO_OR_POLL`,`siginfo_t` 内容可不保证。
133+
- `signum != 0`:发送指定信号,并构造 `SigInfo { sig_code: SigCode::SigIO, sig_type: SigType::SigPoll { fd, band } }`。
134+
- 如果 DragonOS 信号队列暂不支持 Linuxqueued signal fallback,可先不实现 fallback,但要在测试计划中明确风险。
135+
136+
## 6. 分阶段实施计划
137+
138+
### 阶段一:补齐 fcntl 命令与状态建模
139+
140+
1. 在 `FcntlCommand` 中增加 `SetSig`、`GetSig`。
141+
2. 将 `File::pid` 重构为 `File::owner: Mutex<FileOwner>`。
142+
3. 更新 `set_owner()`、`owner()`、`get_owner()` 等接口。
143+
4. 在 `sys_fcntl.rs` 增加 `F_SETSIG/F_GETSIG` 分支。
144+
5. 保留 `EBADF` 优先级:先查 fd,再校验 `arg`。
145+
146+
验收标准:
147+
148+
- `F_GETSIG` 默认返回 0
149+
- `F_SETSIG(SIGUSR1)` 后 `F_GETSIG` 返回 `SIGUSR1`。
150+
- `F_SETSIG(SIGRTMAX + 1)` 返回 `EINVAL`,且不覆盖旧值。
151+
- 无效 fd 返回 `EBADF`。
152+
153+
### 阶段二:统一 O_ASYNC/fasync 注册
154+
155+
1. 抽出 `set_file_fasync()` helper
156+
2. `ioctl(FIOASYNC)` 调用 helper
157+
3. `fcntl(F_SETFL)` 检测 `FASYNC` 位变化并调用 helper
158+
4. `FAsyncItem` 增加 fd 字段,注册重复项时更新 fd 而不是重复 push
159+
5. pipe 按 `FilePrivateData::Pipefs(flags)` 将读端注册到 read fasync 队列,将写端注册到 write fasync 队列。
160+
161+
验收标准:
162+
163+
- `fcntl(F_SETFL, old | O_ASYNC)` 能注册 fasync
164+
- `fcntl(F_SETFL, old & ~O_ASYNC)` 能注销 fasync
165+
- `ioctl(FIOASYNC)` 与 `F_SETFL(O_ASYNC)` 行为一致。
166+
- pipe 写入只通知读端,pipe 读取释放空间只通知写端。
167+
168+
### 阶段三:接入 signumsiginfo 投递
169+
170+
1. `FAsyncItems::send_sigio()` 使用 `FileOwnerSnapshot`。
171+
2. `signum == 0` 走默认 `SIGIO_OR_POLL`。
172+
3. `signum != 0` 发送指定信号。
173+
4. 增加 `SigType::SigPoll`,转换到 `PosixSiginfoSigpoll`。
174+
5. 按调用点传入 read/write 对应 band
175+
176+
验收标准:
177+
178+
- 设置 `F_SETSIG(SIGUSR1)` 后,pipe/socket ready 时收到 `SIGUSR1`。
179+
- `SA_SIGINFO` handler 中 `si_signo == SIGUSR1`。
180+
- `si_fd` 等于注册 `O_ASYNC` 的 fd
181+
- `si_band` 至少覆盖 `EPOLLIN | EPOLLRDNORM`。
182+
183+
### 阶段四:补充测试与回归
184+
185+
1. 扩展 `user/apps/tests/dunitest/suites/normal/fcntl_signal.cc`。
186+
2. 覆盖 set/get、非法参数、默认 `SIGIO`、自定义信号、dupfd 语义。
187+
3. 若 gVisor runner 可用,重点跑 `fcntl.cc` 中 `FcntlTest.SetSig*` 与 `FcntlSignalTest.*`。
188+
4. 执行 `make kernel` 检查内核编译。
189+
5. 最后执行 `make fmt`。
190+
191+
## 7. 风险与注意事项
192+
193+
- 当前 `F_SETOWN` 只支持按 pid 查找,尚未完整支持 Linux 的负 pid/process group 语义;本次修复不要扩大范围,避免和 `F_SETOWN_EX` 语义混在一起。
194+
- `dup` 共享同一个 `Arc<File>`,owner/signum 应共享;但 fasync 注册项的 fd 应是注册 `O_ASYNC` 时的 fd
195+
- 非实时普通信号在 DragonOS 中可能被合并,gVisor 的并发测试需要关注队列语义差异。
196+
- `F_SETFL` 更新 flagsfasync 注册之间应避免长时间持有 fd table 锁,防止调度或锁顺序问题。
197+
- 若某个 inode 不支持 `PollableInode::add_fasync`,不要为了通过测试写 workaround;应明确返回或兼容策略并保持 Linux 语义优先。

docs/kernel/filesystem/vfs/index.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ VFS是DragonOS文件系统的核心,它提供了一套统一的文件系统接
2121
:caption: 目录
2222

2323
design
24+
fcntl_fasync_signal_plan
2425
mount_propagation
2526
api
2627
mountable_fs
27-

kernel/src/filesystem/vfs/fasync.rs

Lines changed: 97 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,24 +10,38 @@ use alloc::{sync::Weak, vec::Vec};
1010
use core::sync::atomic::compiler_fence;
1111

1212
use crate::{
13-
arch::ipc::signal::Signal, ipc::kill::send_signal_to_pcb, libs::mutex::Mutex,
14-
process::ProcessControlBlock,
13+
arch::ipc::signal::Signal,
14+
ipc::signal_types::{SigCode, SigInfo, SigType},
15+
libs::mutex::Mutex,
16+
process::pid::PidType,
1517
};
1618
use alloc::sync::Arc;
19+
use system_error::SystemError;
1720

18-
use super::file::File;
21+
use super::file::{File, FileFlags};
22+
23+
pub const FASYNC_POLL_IN: i64 = 0x00000001 | 0x00000040;
24+
pub const FASYNC_POLL_OUT: i64 = 0x00000004 | 0x00000100 | 0x00000200;
25+
26+
struct FAsyncSignalTarget {
27+
pcb: Arc<crate::process::ProcessControlBlock>,
28+
signum: i32,
29+
fd: i32,
30+
band: i64,
31+
}
1932

2033
/// FAsyncItem represents a file that wants to receive SIGIO signals
2134
/// when IO events occur on the underlying inode.
22-
#[derive(Debug)]
35+
#[derive(Clone, Debug)]
2336
pub struct FAsyncItem {
2437
/// Weak reference to the file
2538
file: Weak<File>,
39+
fd: i32,
2640
}
2741

2842
impl FAsyncItem {
29-
pub fn new(file: Weak<File>) -> Self {
30-
Self { file }
43+
pub fn new(file: Weak<File>, fd: i32) -> Self {
44+
Self { file, fd }
3145
}
3246

3347
/// Get the file reference
@@ -45,10 +59,18 @@ impl FAsyncItem {
4559
pub fn file_weak(&self) -> &Weak<File> {
4660
&self.file
4761
}
62+
63+
pub fn fd(&self) -> i32 {
64+
self.fd
65+
}
66+
67+
pub fn set_fd(&mut self, fd: i32) {
68+
self.fd = fd;
69+
}
4870
}
4971

5072
/// List of FAsyncItems for an inode
51-
pub type LockedFAsyncItemList = Mutex<Vec<Arc<FAsyncItem>>>;
73+
pub type LockedFAsyncItemList = Mutex<Vec<FAsyncItem>>;
5274

5375
/// FAsyncItems manages the list of files that want SIGIO notifications
5476
#[derive(Debug)]
@@ -71,8 +93,15 @@ impl FAsyncItems {
7193
}
7294

7395
/// Add a FAsyncItem
74-
pub fn add(&self, item: Arc<FAsyncItem>) {
75-
self.items.lock().push(item);
96+
pub fn add(&self, item: FAsyncItem) {
97+
let mut guard = self.items.lock();
98+
for old_item in guard.iter_mut() {
99+
if Weak::ptr_eq(old_item.file_weak(), item.file_weak()) {
100+
old_item.set_fd(item.fd());
101+
return;
102+
}
103+
}
104+
guard.push(item);
76105
}
77106

78107
/// Remove a FAsyncItem by file reference
@@ -89,7 +118,8 @@ impl FAsyncItems {
89118

90119
/// Send SIGIO to all registered file owners
91120
/// This should be called when IO events occur (e.g., data becomes readable)
92-
pub fn send_sigio(&self) {
121+
pub fn send_sigio(&self, band: i64) {
122+
let mut targets = Vec::new();
93123
let guard = self.items.lock();
94124
for item in guard.iter() {
95125
if let Some(file) = item.file() {
@@ -98,24 +128,73 @@ impl FAsyncItems {
98128
continue;
99129
}
100130

101-
// Get the owner process
102-
let owner = file.get_owner();
103-
if let Some(pcb) = owner {
104-
// Send SIGIO to the owner
105-
Self::send_sigio_to_process(pcb);
131+
let owner = file.owner_snapshot();
132+
if let Some(pcb) = owner.pcb {
133+
targets.push(FAsyncSignalTarget {
134+
pcb,
135+
signum: owner.signum,
136+
fd: item.fd(),
137+
band,
138+
});
106139
}
107140
}
108141
}
142+
drop(guard);
143+
144+
for target in targets {
145+
Self::send_sigio_to_process(target.pcb, target.signum, target.fd, target.band);
146+
}
109147
}
110148

111149
/// Send SIGIO signal to a process
112-
fn send_sigio_to_process(pcb: Arc<ProcessControlBlock>) {
113-
let sig = Signal::SIGIO_OR_POLL;
150+
fn send_sigio_to_process(
151+
pcb: Arc<crate::process::ProcessControlBlock>,
152+
signum: i32,
153+
fd: i32,
154+
band: i64,
155+
) {
156+
let sig = if signum == 0 {
157+
Signal::SIGIO_OR_POLL
158+
} else {
159+
Signal::from(signum)
160+
};
161+
162+
if sig == Signal::INVALID {
163+
return;
164+
}
114165

115166
compiler_fence(core::sync::atomic::Ordering::SeqCst);
116167

117-
let _ = send_signal_to_pcb(pcb, sig);
168+
if signum == 0 {
169+
let _ = sig.send_signal_info_to_pcb(None, pcb, PidType::TGID);
170+
} else {
171+
let mut info = SigInfo::new(sig, 0, SigCode::SigIO, SigType::SigPoll { fd, band });
172+
let _ = sig.send_signal_info_to_pcb(Some(&mut info), pcb, PidType::TGID);
173+
}
118174

119175
compiler_fence(core::sync::atomic::Ordering::SeqCst);
120176
}
121177
}
178+
179+
pub fn set_file_fasync(file: &Arc<File>, fd: i32, enabled: bool) -> Result<(), SystemError> {
180+
let mut flags = file.flags();
181+
if enabled {
182+
flags.insert(FileFlags::FASYNC);
183+
} else {
184+
flags.remove(FileFlags::FASYNC);
185+
}
186+
187+
file.set_flags(flags)?;
188+
189+
if let Ok(pollable) = file.inode().as_pollable_inode() {
190+
let private_data = file.private_data.lock();
191+
if enabled {
192+
let item = FAsyncItem::new(Arc::downgrade(file), fd);
193+
let _ = pollable.add_fasync(item, &private_data);
194+
} else {
195+
let _ = pollable.remove_fasync(&Arc::downgrade(file), &private_data);
196+
}
197+
}
198+
199+
Ok(())
200+
}

0 commit comments

Comments
 (0)