Skip to content

feat(fs): implement inotify filesystem event notification - #2164

Open
sparkzky wants to merge 1 commit into
DragonOS-Community:masterfrom
sparkzky:feat/inotify
Open

feat(fs): implement inotify filesystem event notification#2164
sparkzky wants to merge 1 commit into
DragonOS-Community:masterfrom
sparkzky:feat/inotify

Conversation

@sparkzky

@sparkzky sparkzky commented Jul 31, 2026

Copy link
Copy Markdown
Member

Summary

Implements inotify (issue #2151): filesystem event notification with inotify_init1 / inotify_add_watch / inotify_rm_watch / read, the full standard event set, epoll integration, and exact inotify_event ABI.

Design doc: docs/kernel/filesystem/inotify.md. Behavior targets Linux 6.6.

Architecture (3 layers)

  • fsnotify unified dispatch layer (fs/notify/-style): global inode_id -> Weak<mark> index, TOTAL_WATCHES atomic fast-path (zero cost when no watches), lock-family separation (global index lock / events lock / wd lock never nested). Hooks fire only after success and never alter syscall return values.
  • inotify device (inotify.rs): InotifyInode implements IndexNode + PollableInode, epoll-integrated via LockedEPItemLinkedList, exact inotify_event layout (name field aligned to sizeof(inotify_event)=16, matching the Linux 6.6 ABI).
  • VFS write-path hooks in the syscall-core layer (vcore/open/rename_utils/...), not per-filesystem: a single anchor covers ext4/tmpfs/overlayfs/fuse.

What this PR adds beyond the initial implementation

The initial implementation was put through a 3-way adversarial review (independent reviewers) + independent verification. This PR includes both the feature and the resulting fixes:

Directory watches now receive child content events (the primary inotify use case, e.g. inotifywait -m /dir). Previously only namespace events (create/delete/move) reached directory watches; IN_MODIFY/ACCESS/OPEN/CLOSE on children were silently dropped. Resolved by snapshotting the parent dir + child name once at File construction (gated by has_any_watch for zero cost when unused) and routing content events to both the parent (with name) and self watch. This also makes IN_EXCL_UNLINK effective.

Other review fixes (all verified):

  • TOTAL_WATCHES double-count on add_watch (broke the no-watch fast path).
  • add_watch TOCTOU: concurrent same-inode adds created duplicate marks.
  • IN_EXCL_UNLINK polarity was inverted.
  • fallocate: restored the offset+len overflow guard removed during the IN_MODIFY refactor.
  • rename overwriting an existing target now emits IN_DELETE/IN_DELETE_SELF (was a ghost-watch leak).
  • RENAME_EXCHANGE now emits the full 4 namespace events + 2 MOVE_SELF with two cookies.

Testing

  • dunitest regression coverage: normal/inotify_dir_watch (directory-watch child content events + self-watch sanity), added to the whitelist so CI runs it.
  • Kernel compiles clean (make kernel).
  • Known narrow limitations (documented in docs/impl-notes/): rename-after-open yields a stale parent snapshot (EXCL_UNLINK covers unlink); IN_ATTRIB-to-parent via fd-based setattr is not yet wired; unmount does not emit IN_UNMOUNT (watch reclaims on fd close).

Checklist

  • make kernel compiles
  • dunitest build + runtime (to be run on a test machine)

@github-actions github-actions Bot added the enhancement New feature or request label Jul 31, 2026
@fslongjin

Copy link
Copy Markdown
Member

needs to rebase

@sparkzky
sparkzky force-pushed the feat/inotify branch 3 times, most recently from 8fad46f to 05ce40d Compare August 1, 2026 07:01
@fslongjin

Copy link
Copy Markdown
Member

@codex review

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

Copy link
Copy Markdown

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: 05ce40d4de

ℹ️ 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".

Comment thread kernel/src/filesystem/fsnotify/mod.rs Outdated
Comment thread kernel/src/filesystem/vfs/file.rs Outdated
Comment thread kernel/src/filesystem/fsnotify/mod.rs Outdated
Comment thread kernel/src/filesystem/vfs/vcore.rs Outdated
Comment thread kernel/src/filesystem/vfs/syscall/rename_utils.rs
Comment thread kernel/src/filesystem/inotify.rs
Comment thread kernel/src/filesystem/inotify.rs Outdated
Comment thread kernel/src/filesystem/vfs/file.rs Outdated
Comment thread kernel/src/filesystem/inotify.rs
Comment thread kernel/src/filesystem/vfs/syscall/rename_utils.rs
sparkzky added a commit to sparkzky/DragonOS that referenced this pull request Aug 8, 2026
…5 P1 + 4 P2)

P1 fixes:
- Composite mark index key (inode_id, dev_id): prevents FUSE cross-mount
  event leakage when multiple mounts reuse same inode number.
- Remove has_any_watch() gate on File::inotify_parent resolution: watches
  added after open now correctly receive content events.
- Move inode-death (DELETE_SELF/UNMOUNT) destroy before mask subscription
  filter: prevents watch leak when watch doesn't subscribe to the event.
- Cache nlinks before unlink/rename-over: avoids FUSE GETATTR failure
  after namespace change; uses pre-operation link count for DELETE_SELF.
- Rename-over checks displaced nlinks: multiple hardlinks survive.

P2 fixes:
- Sync IN_ONESHOT field on IN_MASK_ADD (OR semantics).
- Dynamic O_NONBLOCK: fcntl(F_SETFL) now works on inotify fds.
- EOF reads (len==0) no longer deliver IN_ACCESS.
- Reject mask==0 and IN_MASK_ADD|IN_MASK_CREATE as EINVAL.
- Skip MOVED events on no-op rename (same dir + same name).

Verified: make kernel clean, inotify_dir_watch 2/2 PASS, fuse_core 6/6 PASS.
Signed-off-by: sparkzky <sparkhhhhhhhhhh@outlook.com>
Implements inotify (issue DragonOS-Community#2151): the fsnotify core notification layer,
the inotify pseudo-device, 4 syscalls (init/init1/add_watch/rm_watch),
and VFS write-path hooks for all standard events (create/delete/move/
modify/access/close/attrib/self events).

Architecture:
- fsnotify/ unified dispatch layer: global (inode_id, dev_id) index,
  TOTAL_WATCHES atomic fast-path (zero cost when no watches), lock-family
  separation (global index lock / events lock / wd lock never nested).
- inotify.rs device: InotifyInode implements IndexNode + PollableInode,
  epoll-integrated via LockedEPItemLinkedList, exact inotify_event layout
  (name field aligned to sizeof(inotify_event)=16, matching Linux ABI).
- VFS hooks placed in syscall-core layer (vcore/open/rename_utils/...),
  NOT per-filesystem: single anchor covers ext4/tmpfs/overlayfs/fuse.
  Hooks fire only after success and never alter syscall return values.

Review fixes incorporated:
- Directory watches receive child content events (issue B): IN_MODIFY/
  ACCESS/OPEN/CLOSE delivered to parent dir watch with child name.
  MountFSInode::as_any_ref() returns the inner inode's Any, so use
  downcast_arc instead of downcast_ref for parent resolution.
- Guard DELETE_SELF on hardlink unlink/rename-over: only emit when
  i_nlink reaches 0, matching Linux fsnotify_link_count() semantics.
- Composite mark index key (inode_id, dev_id): prevents FUSE cross-mount
  event leakage when multiple mounts reuse same inode number.
- Remove has_any_watch() gate on File::inotify_parent resolution so
  watches added after open receive content events.
- Tolerate metadata failure after unlink/rename-over (FUSE GETATTR can
  return ENOENT); cache nlinks before the namespace operation.
- SYS_INOTIFY_INIT only registered on x86_64 (generic syscall ABI uses
  inotify_init1); riscv64/loongarch64 lack the legacy init syscall.
- Skip MOVED events on no-op rename; EOF reads no longer deliver
  IN_ACCESS; reject mask==0 and IN_MASK_ADD|IN_MASK_CREATE.

Test: dunitest inotify_dir_watch (2 tests) + inotify_events (6 tests)
covering content/namespace/self events, multi-instance, and poll.

Design doc: docs/kernel/filesystem/inotify.md

Signed-off-by: sparkzky <sparkhhhhhhhhhh@outlook.com>

@fslongjin fslongjin left a comment

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.

Thank you for implementing inotify. This feature is important for skillfs, agent-memory, and configuration hot reload. The overall layering—VFS operations produce events, fsnotify dispatches them, and the inotify backend queues and exposes them—is a good direction and avoids duplicating hooks in every filesystem.

However, this review found several issues that can directly cause missing events, incorrect events, leaked watches, or significant system-wide overhead. I recommend addressing these before merging.

Required changes

1. Normal rename does not produce move events

Location: kernel/src/filesystem/vfs/syscall/rename_utils.rs:149

The event code for a normal rename(a, b) is inside the RENAME_EXCHANGE branch. As a result, normal rename does not produce:

  • IN_MOVED_FROM
  • IN_MOVED_TO
  • IN_MOVE_SELF

Conversely, RENAME_EXCHANGE sends the two expected groups and then sends an extra duplicate group for the old inode with a different cookie.

Please split normal rename and exchange rename into separate branches. A normal rename should emit one paired set of events. An exchange should emit one set for each of the two exchanged files.

2. A removed watch can still receive later events

Location: kernel/src/filesystem/fsnotify/mod.rs:282

Dispatch first takes a snapshot of the watch list and then queues events after releasing the global lock. Another thread can call inotify_rm_watch(), queue IN_IGNORED, and remove the watch while the first thread still holds a reference to it. The first thread can then queue another normal event for the removed wd.

Users can therefore observe:

  1. IN_IGNORED;
  2. then another IN_MODIFY or similar event for the same wd.

Two concurrent events can also both pass the IN_ONESHOT check and be queued.

Please add an explicit active/removed state to each watch. Removing a watch must mark it removed before cleanup. Event delivery must check and claim that state before enqueueing. For IN_ONESHOT, only one concurrent event may succeed, and IN_IGNORED must be the final event for the wd.

3. A watch may not be found or released after the file is deleted

Locations:

  • kernel/src/filesystem/fsnotify/mod.rs:185
  • kernel/src/filesystem/fsnotify/mark.rs:36

After unlink, rmdir, or an overwriting rename succeeds, fsnotify calls metadata() again to obtain the inode identity. A deleted FUSE file can return ENOENT here. Other code in this PR already recognizes that post-delete FUSE metadata may fail, but dispatch and watch removal still depend on it.

This can cause:

  • missing IN_DELETE_SELF and IN_IGNORED;
  • failure to remove the watch from the global index;
  • the watch retaining the inode and its quota until the inotify fd is closed.

Please save a stable file identity when the watch is created. Destructive operations should also capture that identity and file type before changing the namespace. Notification and cleanup must not depend on querying metadata after the file has already been deleted.

4. Events use the old directory and old name after rename

Locations: kernel/src/filesystem/vfs/file.rs:688, :1282

File stores the parent directory and file name at open time. If A/old is opened and later renamed to B/new, reads, writes, and close through the original fd still:

  • notify the watch on A;
  • report the name old;
  • fail to notify the watch on B.

The code comments already acknowledge this limitation, but it makes directory-watch results incorrect. When a file moves between directories, the old directory's watcher may also continue seeing activity for a file that is no longer there.

Please do not store a fixed copy of the open-time parent and name. File should retain path information that follows rename, and event delivery should obtain the current parent and name when the event occurs. VFS path and rename handling should maintain this consistently.

5. A directory watch misses child attribute changes

Locations: kernel/src/filesystem/vfs/open.rs:139, :250, :700

chmod, chown, and utimensat send IN_ATTRIB without a parent directory. Only a watch placed directly on the file receives the event; a watch on the parent directory does not.

Linux notifies both the file watch and the parent directory's child watch. These operations should provide the current parent and child name. Other attribute-changing paths, including xattr operations, should be covered consistently.

6. IN_EXCL_UNLINK is not updated on an existing watch

Location: kernel/src/filesystem/inotify.rs:447

When a watch already exists, the code updates only the event mask and IN_ONESHOT. IN_EXCL_UNLINK remains fixed at the value used when the watch was first created:

  • adding IN_EXCL_UNLINK with IN_MASK_ADD has no effect;
  • replacing the mask cannot clear an old IN_EXCL_UNLINK.

Please update the event mask, IN_ONESHOT, and IN_EXCL_UNLINK together. Add mode should merge them; replace mode should replace them.

7. O_PATH incorrectly produces open and close events

Locations:

  • kernel/src/filesystem/vfs/open.rs:494
  • kernel/src/filesystem/vfs/file.rs:2508

O_PATH creates a path handle; it does not open the file for normal I/O. Linux does not generate IN_OPEN or IN_CLOSE_NOWRITE for it, but the current implementation generates both.

Please exclude FMODE_PATH in both the open and close notification paths.

8. Hard-link count changes are missing IN_ATTRIB

Locations:

  • kernel/src/filesystem/vfs/syscall/link_utils.rs:137
  • kernel/src/filesystem/vfs/vcore.rs:726

Creating a hard link currently sends only IN_CREATE to the new parent directory. Removing one name while other hard links remain sends only IN_DELETE to the parent. The inode link count has changed, but a watch on the file does not receive IN_ATTRIB.

Please send IN_ATTRIB to the target inode after successful link and unlink operations. Whether to send IN_DELETE_SELF should remain a separate decision based on whether this was the final link.

9. IN_UNMOUNT is declared as supported but is not implemented

Locations:

  • kernel/src/filesystem/fsnotify/mod.rs:257
  • kernel/src/filesystem/vfs/mount/mod.rs:3541

The PR description and design document claim the full standard event set, but there is no path that produces IN_UNMOUNT. On unmount:

  • watches do not receive IN_UNMOUNT;
  • they do not receive the following IN_IGNORED;
  • references held by the watches are not released immediately.

The design document postpones this because the current ANOLISA use case does not trigger it. That does not match the claim of complete support. Please notify and remove affected watches during unmount. If this PR intentionally does not implement it, narrow the feature claim and track the missing behavior separately.

10. One watch adds substantial overhead to unrelated file operations

Locations:

  • kernel/src/filesystem/fsnotify/mod.rs:175
  • kernel/src/filesystem/vfs/file.rs:1290

The implementation has only a global watch-count shortcut. Once any watch exists anywhere in the system, every unrelated read, write, open, and close performs metadata queries, takes the same global interrupt-disabling lock, and searches the global map. FUSE metadata queries may be particularly expensive.

Even when no watch exists, every normal open still resolves the parent and allocates a saved file-name copy.

Please maintain a small per-file/per-directory indication of whether the file itself or its parent has relevant watches. Enter the global lookup only when an event can actually match. Also avoid parent/name allocation when no watch exists.

Additional compatibility and performance issues

  1. IN_DELETE_SELF and IN_MOVE_SELF for a directory should not include IN_ISDIR. Linux explicitly removes that bit for compatibility. Location: kernel/src/filesystem/fsnotify/mod.rs:277.
  2. Reading a large event queue keeps an interrupt-disabling lock while serializing and zero-padding the entire result. Pop one complete event under the lock and copy it to the read buffer after releasing the lock. Location: kernel/src/filesystem/inotify.rs:607.
  3. When the queue is already full and an overflow event is already present, every dropped event still wakes all waiters and walks the epoll list. Do not wake when the queue state did not change. Location: kernel/src/filesystem/inotify.rs:240.
  4. Instance and watch limits are system-wide. One unprivileged user can exhaust them and prevent other users from using inotify. Please account these limits per user. A separate global count may remain as a fast “no watches anywhere” check.

Suggested test coverage

Please add dunitest coverage for at least:

  • normal rename, cross-directory rename, overwrite, two hard links to the same inode, and RENAME_EXCHANGE;
  • open a file, rename it, then read/write/close through the original fd;
  • concurrent event delivery with rm_watch;
  • concurrent event delivery with IN_ONESHOT;
  • changing IN_EXCL_UNLINK on an existing watch;
  • a directory watch while chmod, chown, utimensat, and xattr are applied to a child;
  • no open/close events for O_PATH;
  • IN_ATTRIB when the hard-link count changes;
  • IN_UNMOUNT followed by IN_IGNORED;
  • queue overflow, a too-small read buffer, and changing O_NONBLOCK at runtime.

Overall recommendation

The layering can be kept, but I do not recommend adding more special cases on top of the current approach of saving the open-time parent/name, querying metadata after deletion, and using one global lookup lock.

Please first settle these three foundations:

  1. a stable file identity that remains usable after deletion;
  2. path information for an open file that follows rename;
  3. correct ordering between watch removal, one-shot watches, and event enqueueing.

Once those are in place, completing the event set and optimizing dispatch should be much safer and closer to Linux behavior.

Local make kernel passes. At the time of review, the GitHub Dunitest, Integration Test, x86_64 Build, and build-current-version checks are failing. After the changes above, please run the expanded tests inside the DragonOS guest before requesting another review.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants