From e082d4eaa68aff1df8bf379c0dfbe8aeb52ee70b Mon Sep 17 00:00:00 2001 From: Ruihan Li Date: Mon, 5 Jan 2026 11:00:02 +0800 Subject: [PATCH] Respect `InotifyControls::ONESHOT` --- .../file-systems-and-mount-control/README.md | 1 - .../inotify_add_watch.scml | 2 +- kernel/src/fs/notify/inotify.rs | 89 +++++++++++++------ kernel/src/fs/notify/mod.rs | 57 +++++++++--- .../syscall/gvisor/blocklists/inotify_test | 3 - 5 files changed, 106 insertions(+), 46 deletions(-) diff --git a/book/src/kernel/linux-compatibility/syscall-flag-coverage/file-systems-and-mount-control/README.md b/book/src/kernel/linux-compatibility/syscall-flag-coverage/file-systems-and-mount-control/README.md index f271ee2cc..7b644d7f3 100644 --- a/book/src/kernel/linux-compatibility/syscall-flag-coverage/file-systems-and-mount-control/README.md +++ b/book/src/kernel/linux-compatibility/syscall-flag-coverage/file-systems-and-mount-control/README.md @@ -90,7 +90,6 @@ Unsupported event flags: Unsupported control flags: * `IN_EXCL_UNLINK` - Events on unlinked files are not excluded -* `IN_ONESHOT` - Watches are not automatically removed after the first event For more information, see [the man page](https://man7.org/linux/man-pages/man7/inotify.7.html). diff --git a/book/src/kernel/linux-compatibility/syscall-flag-coverage/file-systems-and-mount-control/inotify_add_watch.scml b/book/src/kernel/linux-compatibility/syscall-flag-coverage/file-systems-and-mount-control/inotify_add_watch.scml index 4343b3794..de4e381e4 100644 --- a/book/src/kernel/linux-compatibility/syscall-flag-coverage/file-systems-and-mount-control/inotify_add_watch.scml +++ b/book/src/kernel/linux-compatibility/syscall-flag-coverage/file-systems-and-mount-control/inotify_add_watch.scml @@ -3,7 +3,7 @@ inotify_events = IN_ACCESS | IN_MODIFY | IN_ATTRIB | IN_CLOSE_WRITE | IN_DELETE_SELF | IN_CLOSE; inotify_controls = IN_ONLYDIR | IN_DONT_FOLLOW | IN_MASK_CREATE | - IN_MASK_ADD | IN_ISDIR; + IN_MASK_ADD | IN_ISDIR | IN_ONESHOT; // Add a watch to an initialized inotify instance inotify_add_watch(fd, pathname, mask = | ); diff --git a/kernel/src/fs/notify/inotify.rs b/kernel/src/fs/notify/inotify.rs index 548e04771..3fabc0304 100644 --- a/kernel/src/fs/notify/inotify.rs +++ b/kernel/src/fs/notify/inotify.rs @@ -139,15 +139,19 @@ impl InotifyFile { // Try to find and update the existing subscriber first. let inode_weak = Arc::downgrade(path.inode()); - for (wd, entry) in watch_map.iter() { + let mut watch_iter = watch_map.iter(); + while let Some((wd, entry)) = watch_iter.next() { if !Weak::ptr_eq(&entry.inode, &inode_weak) { continue; } - // The inode has been unlinked and the subscriber is dead. We shouldn't need to update - // since no new events can occur. + // The subscriber is dead because it's a one-shot subscriber or the inode is dead. + // The watch is considered removed. let Some(subscriber) = entry.subscriber.upgrade() else { - return Ok(*wd); + let wd = *wd; + watch_map.remove(&wd); + watch_iter = watch_map.iter(); + continue; }; subscriber.update(interesting, options)?; @@ -200,8 +204,8 @@ impl InotifyFile { let (inode, subscriber) = match (entry.inode.upgrade(), entry.subscriber.upgrade()) { (Some(inode), Some(subscriber)) => (inode, subscriber), - // The inode has been unlinked and the subscriber is dead. The watch is considered - // removed, so we return an error. + // The subscriber is dead because it's a one-shot subscriber or the inode is dead. + // The watch is considered removed. _ => return_errno_with_message!(Errno::EINVAL, "the inotify watch does not exist"), }; @@ -221,12 +225,7 @@ impl InotifyFile { /// The event will be queued and can be read by users. /// If the event can be merged with the last event in the queue, it will be merged. /// The event is only queued if it matches one of the subscriber's interesting events. - fn receive_event(&self, subscriber: &InotifySubscriber, event: FsEvents, name: Option) { - if !event.contains(FsEvents::IN_IGNORED) && !subscriber.is_interesting(event) { - return; - } - - let wd = subscriber.wd(); + fn receive_event(&self, wd: u32, event: FsEvents, name: Option) { let new_event = InotifyEvent::new(wd, event, 0, name); 'notify: { @@ -445,6 +444,8 @@ pub struct InotifySubscriber { // This field is packed into a `u64`: the high 32 bits store options, // and the low 32 bits store interesting events. interesting_and_controls: AtomicU64, + // Whether the subscriber is one-shot and has been dead. + is_dead: AtomicBool, // Watch descriptor. wd: u32, // Reference to the owning inotify file. @@ -461,6 +462,7 @@ impl InotifySubscriber { let wd = inotify_file.alloc_wd()?; let this = Arc::new(Self { interesting_and_controls: AtomicU64::new(0), + is_dead: AtomicBool::new(false), wd, inotify_file, }); @@ -473,14 +475,13 @@ impl InotifySubscriber { self.wd } - fn interesting(&self) -> InotifyEvents { + /// Loads the interesting events and options atomically. + fn interesting_and_controls(&self) -> (InotifyEvents, InotifyControls) { let flags = self.interesting_and_controls.load(Ordering::Relaxed); - InotifyEvents::from_bits_truncate((flags & 0xFFFFFFFF) as u32) - } - - fn options(&self) -> InotifyControls { - let flags = self.interesting_and_controls.load(Ordering::Relaxed); - InotifyControls::from_bits_truncate((flags >> 32) as u32) + ( + InotifyEvents::from_bits_truncate((flags & 0xFFFFFFFF) as u32), + InotifyControls::from_bits_truncate((flags >> 32) as u32), + ) } pub fn inotify_file(&self) -> &Arc { @@ -497,8 +498,10 @@ impl InotifySubscriber { let mut merged_options = options; if options.contains(InotifyControls::MASK_ADD) { - merged_interesting |= self.interesting(); - merged_options |= self.options(); + // There are no races because `update()` is only called under the `watch_map` lock. + let (old_interesting, old_options) = self.interesting_and_controls(); + merged_interesting |= old_interesting; + merged_options |= old_options; } merged_options.remove(InotifyControls::MASK_ADD); @@ -513,25 +516,53 @@ impl InotifySubscriber { self.interesting_and_controls .store(new_flags, Ordering::Relaxed); } - - /// Checks if the event matches the subscriber's interesting events. - fn is_interesting(&self, event: FsEvents) -> bool { - self.interesting().bits() & event.bits() != 0 - } } impl FsEventSubscriber for InotifySubscriber { /// Sends FS events to the inotify file. - fn deliver_event(&self, event: FsEvents, name: Option) { + fn deliver_event(&self, event: FsEvents, name: Option) -> bool { + let (interesting, options) = self.interesting_and_controls(); + + if !event.contains(FsEvents::IN_IGNORED) && !is_interesting(interesting, event) { + return false; + } + + let (is_dead, is_oneshot) = if options.contains(InotifyControls::ONESHOT) { + (self.is_dead.swap(true, Ordering::Relaxed), true) + } else { + (self.is_dead.load(Ordering::Relaxed), false) + }; + if is_dead { + return false; + } + let inotify_file = self.inotify_file(); - inotify_file.receive_event(self, event, name); + let wd = self.wd(); + + inotify_file.receive_event(wd, event, name); + + if !event.contains(FsEvents::IN_IGNORED) && is_oneshot { + inotify_file.receive_event(wd, FsEvents::IN_IGNORED, None); + } + + is_oneshot } /// Returns the events that this subscriber is interested in. fn interesting_events(&self) -> FsEvents { - let inotify_events = self.interesting(); + let inotify_events = self.interesting_and_controls().0; FsEvents::from_bits_truncate(inotify_events.bits()) } + + /// Returns whether the subscriber is one-shot and dead. + fn is_oneshot_and_dead(&self) -> bool { + self.is_dead.load(Ordering::Relaxed) + } +} + +/// Checks if the event matches the subscriber's interesting events. +fn is_interesting(interesting: InotifyEvents, event: FsEvents) -> bool { + interesting.bits() & event.bits() != 0 } /// Represents an inotify event that can be read by users. diff --git a/kernel/src/fs/notify/mod.rs b/kernel/src/fs/notify/mod.rs index 39b05a5b6..3e5150c3a 100644 --- a/kernel/src/fs/notify/mod.rs +++ b/kernel/src/fs/notify/mod.rs @@ -80,12 +80,11 @@ impl FsEventPublisher { let mut subscribers = self.subscribers.write(); let orig_len = subscribers.len(); - subscribers.retain(|m| !Arc::ptr_eq(m, subscriber)); + self.retain_and_recalc_events(&mut subscribers, |m| !Arc::ptr_eq(m, subscriber)); let removed = subscribers.len() != orig_len; if removed { subscriber.deliver_event(FsEvents::IN_IGNORED, None); - self.recalc_interesting_events(&subscribers); } removed @@ -126,8 +125,17 @@ impl FsEventPublisher { } let subscribers = self.subscribers.read(); + let mut has_oneshot = false; for subscriber in subscribers.iter() { - subscriber.deliver_event(events, name.clone()); + has_oneshot |= subscriber.deliver_event(events, name.clone()); + } + drop(subscribers); + + if has_oneshot { + let mut subscribers = self.subscribers.write(); + // The `deliver_event()` method should already deliver the `FsEvents::IN_IGNORED` + // events for one-shot subscribers. Here, we simply remove them. + self.retain_and_recalc_events(&mut subscribers, |m| !m.is_oneshot_and_dead()); } } @@ -135,16 +143,28 @@ impl FsEventPublisher { pub fn update_subscriber_events(&self) { // Take a write lock to avoid race conditions that may change `all_interesting_events` to // an outdated value. - let subscribers = self.subscribers.write(); - self.recalc_interesting_events(&subscribers); + let mut subscribers = self.subscribers.write(); + self.retain_and_recalc_events(&mut subscribers, |_| true); } - /// Recalculates the aggregated interesting events from all subscribers. - fn recalc_interesting_events(&self, subscribers: &[Arc]) { + /// Retains only the subscribers specified by the predicate and recalculates the aggregated + /// interesting events. + fn retain_and_recalc_events( + &self, + subscribers: &mut Vec>, + mut pred: F, + ) where + F: FnMut(&Arc) -> bool, + { let mut new_events = FsEvents::empty(); - for subscriber in subscribers.iter() { - new_events |= subscriber.interesting_events(); - } + subscribers.retain(|subscriber| { + if pred(subscriber) { + new_events |= subscriber.interesting_events(); + true + } else { + false + } + }); self.all_interesting_events .store(new_events, Ordering::Relaxed); } @@ -179,11 +199,25 @@ impl FsEventPublisher { pub trait FsEventSubscriber: Any + Send + Sync { /// Delivers a filesystem event notification to the subscriber. /// + /// Returns whether the subscriber is a one-shot subscriber and the event has been + /// delivered. If there are no one-shot subscribers, simply return `false` here. + /// Otherwise, [`Self::is_oneshot_and_dead`] should be implemented correspondingly. + /// /// Invariant: This method must not sleep or perform blocking operations. The publisher /// may hold a spin lock when calling this method. - fn deliver_event(&self, events: FsEvents, name: Option); + fn deliver_event(&self, events: FsEvents, name: Option) -> bool; + /// Returns the events that this subscriber is interested in. fn interesting_events(&self) -> FsEvents; + + /// Returns whether the subscriber is a one-shot subscriber and an event has been + /// delivered. + /// + /// This method should return `true` if and only if a previous call to + /// [`Self::deliver_event`] has already returned `true`. + fn is_oneshot_and_dead(&self) -> bool { + false + } } bitflags! { @@ -280,7 +314,6 @@ pub fn on_delete( { return; } - if inode.type_() == InodeType::Dir { notify_inode_with_name(dir_inode, FsEvents::DELETE | FsEvents::ISDIR, name) } else { diff --git a/test/initramfs/src/syscall/gvisor/blocklists/inotify_test b/test/initramfs/src/syscall/gvisor/blocklists/inotify_test index d73b24304..bf342e4fa 100644 --- a/test/initramfs/src/syscall/gvisor/blocklists/inotify_test +++ b/test/initramfs/src/syscall/gvisor/blocklists/inotify_test @@ -21,9 +21,6 @@ Inotify.ExcludeUnlinkMultipleChildren_NoRandomSave Inotify.MoveGeneratesEvents Inotify.MoveWatchedTargetGeneratesEvents -# TODO: Support the `ONESHOT` flag. -Inotify.OneShot - # TODO: Support the `splice()` system call. Inotify.SpliceOnInotifyFD Inotify.SpliceOnWatchTarget