Skip to content

feat(fcntl): implement F_SETSIG/F_GETSIG#1880

Open
jiuyue486 wants to merge 2 commits intoDragonOS-Community:masterfrom
jiuyue486:claude/fix-pr-issues-eSVIq
Open

feat(fcntl): implement F_SETSIG/F_GETSIG#1880
jiuyue486 wants to merge 2 commits intoDragonOS-Community:masterfrom
jiuyue486:claude/fix-pr-issues-eSVIq

Conversation

@jiuyue486
Copy link
Copy Markdown

Implement the F_SETSIG/F_GETSIG fcntl commands so userspace can pick
the signal delivered for asynchronous I/O notifications. arg == 0
keeps the SIGIO default; values in (0, SIGRTMAX] override it.

  • Add a per-File fasync_signum (AtomicI32), preserved across
    try_clone.
  • Validate the raw usize arg against SIGRTMAX to avoid 64-bit
    values whose low 32 bits accidentally fall in [0, SIGRTMAX] from
    being silently accepted. Look up the fd first so EBADF wins over
    EINVAL, matching Linux fs/fcntl.c.
  • Cover defaults, set/get round-trip, the SIGRTMAX boundary, invalid
    values, and invalid fd in a new fcntl_signal dunitest.

flynojy and others added 2 commits April 26, 2026 07:49
Implement the F_SETSIG/F_GETSIG fcntl commands so userspace can pick
the signal delivered for asynchronous I/O notifications. arg == 0
keeps the SIGIO default; values in (0, SIGRTMAX] override it.

- Add a per-File `fasync_signum` (AtomicI32), preserved across
  `try_clone`.
- Validate the raw `usize` arg against `SIGRTMAX` to avoid 64-bit
  values whose low 32 bits accidentally fall in [0, SIGRTMAX] from
  being silently accepted. Look up the fd first so EBADF wins over
  EINVAL, matching Linux `fs/fcntl.c`.
- Cover defaults, set/get round-trip, the SIGRTMAX boundary, invalid
  values, and invalid fd in a new `fcntl_signal` dunitest.
@github-actions github-actions Bot added enhancement New feature or request test Unitest/User space test labels Apr 26, 2026
ra_state: Mutex<FileReadaheadState>,
/// 异步 I/O 通知信号编号(F_SETSIG/F_GETSIG)。
/// 0 表示默认 SIGIO;范围 [0, SIGRTMAX]。
fasync_signum: AtomicI32,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这个不应该直接放到 File 结构体下 用原子变量保护吧
应该做一个 FileOwner 结构体,和 pid 一起用 Mutex 保护

参考Linux的fown_struct

@fslongjin
Copy link
Copy Markdown
Member

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e0b2863a4e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

if arg > Signal::SIGRTMAX as usize {
return Err(SystemError::EINVAL);
}
file.set_fasync_signum(arg as i32);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Wire F_SETSIG value into async signal delivery

F_SETSIG now returns success and persists arg via file.set_fasync_signum(...), but the notification path still hardcodes SIGIO_OR_POLL in FAsyncItems::send_sigio_to_process (kernel/src/filesystem/vfs/fasync.rs). As a result, programs that set a nonzero signal (especially realtime signals with SA_SIGINFO) will continue to receive SIGIO instead of the configured signal, so the new API behavior is observably incorrect despite passing F_GETSIG round-trips.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request test Unitest/User space test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants