Re: [RFC PATCH 2/2] smb: client: add directory change tracking via SMB2 Change Notify

Re: [RFC PATCH 2/2] smb: client: add directory change tracking via SMB2 Change Notify

[Amir Goldstein]

On Sat, Sep 27, 2025 at 3:03 AM Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
>
> On Tue, Sep 16, 2025 at 10:35 PM Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
> >
> > Implement directory change tracking using SMB2 Change Notify protocol
> > to enable real-time monitoring of remote directories. Applications using
> > inotify now receive immediate notifications when files are created,
> > deleted, or renamed on SMB2+ shares.
> >
> > This implementation begins by detecting tracking opportunities during
> > regular SMB operations. In the current version, the readdir() serves as
> > the detection point - when directory contents are accessed on SMB2+
> > servers, we check the inode's fsnotify_mask for active inotify watches.
> > If watches exist and directory is not already being tracked, we obtain a
> > new FiieId and send a SMB2_CHANGE_NOTIFY request that waits for changes.
> >
> > Based on the server's response status, we convert CHANGE_NOTIFY response
> > to fsnotify events and deliver to application on success, mark for
> > future reconnection on network errors, or discard the response and
> > marked entries for broken. If tracking can continue, we resend
> > SMB2_CHANGE_NOTIFY for continuous monitoring.
> >
> > Entries marked for reconnection are restored during SMB2 reconnection.
> > In the current version, smb2_reconnect() serves as restoration point -
> > when connection is reestablished, we obtain new FileIds and request
> > Change Notify requests for these entries.
> >
> > Entries marked for unmount during unmount. In the current version,
> > cifs_umount() serves as unmount marking point. Entries marked for clean
> > are remove as soon as possible by the worker. Workers also run
> > periodically; currently every 30s; to check and remove untracking
> > directories.
> >
> > This feature is controlled by CONFIG_CIFS_DIR_CHANGE_TRACKING with
> > experimental.
> >
> > Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
> > ---
> >  fs/smb/client/Kconfig   |  17 ++
> >  fs/smb/client/Makefile  |   2 +
> >  fs/smb/client/connect.c |   6 +
> >  fs/smb/client/notify.c  | 535 ++++++++++++++++++++++++++++++++++++++++
> >  fs/smb/client/notify.h  |  19 ++
> >  fs/smb/client/readdir.c |   9 +
> >  fs/smb/client/smb2pdu.c |   6 +
> >  7 files changed, 594 insertions(+)
> >  create mode 100644 fs/smb/client/notify.c
> >  create mode 100644 fs/smb/client/notify.h
> >
> > diff --git a/fs/smb/client/Kconfig b/fs/smb/client/Kconfig
> > index a4c02199fef4..0e3911936e0c 100644
> > --- a/fs/smb/client/Kconfig
> > +++ b/fs/smb/client/Kconfig
> > @@ -218,4 +218,21 @@ config CIFS_COMPRESSION
> >           Say Y here if you want SMB traffic to be compressed.
> >           If unsure, say N.
> >
> > ++config CIFS_DIR_CHANGE_TRACKING
>
> I also found a typo here and I'll fix them.
>
> > +       bool "Directory change tracking (Experimental)"
> > +       depends on CIFS && FSNOTIFY=y
> > +       default n
> > +       help
> > +          Enables automatic tracking of directory changes for SMB2 or later
> > +          using the SMB2 Change Notify protocol. File managers and applications
> > +          monitoring directories via inotify will receive real-time updates
> > +          when files are created, deleted, or renamed on the server.
> > +
> > +          This feature maintains persistent connections to track changes.
> > +          Each monitored directory consumes server resources and may increase
> > +          network traffic.
> > +
> > +          Say Y here if you want real-time directory monitoring.
> > +          If unsure, say N.
> > +
> >  endif
> > diff --git a/fs/smb/client/Makefile b/fs/smb/client/Makefile
> > index 4c97b31a25c2..85253bc1b4b0 100644
> > --- a/fs/smb/client/Makefile
> > +++ b/fs/smb/client/Makefile
> > @@ -35,3 +35,5 @@ cifs-$(CONFIG_CIFS_ROOT) += cifsroot.o
> >  cifs-$(CONFIG_CIFS_ALLOW_INSECURE_LEGACY) += smb1ops.o cifssmb.o cifstransport.o
> >
> >  cifs-$(CONFIG_CIFS_COMPRESSION) += compress.o compress/lz77.o
> > +
> > +cifs-$(CONFIG_CIFS_DIR_CHANGE_TRACKING) += notify.o
> > diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> > index dd12f3eb61dc..eebf729df16a 100644
> > --- a/fs/smb/client/connect.c
> > +++ b/fs/smb/client/connect.c
> > @@ -51,6 +51,9 @@
> >  #endif
> >  #include "fs_context.h"
> >  #include "cifs_swn.h"
> > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > +#include "notify.h"
> > +#endif
> >
> >  /* FIXME: should these be tunable? */
> >  #define TLINK_ERROR_EXPIRE     (1 * HZ)
> > @@ -4154,6 +4157,9 @@ cifs_umount(struct cifs_sb_info *cifs_sb)
> >         cancel_delayed_work_sync(&cifs_sb->prune_tlinks);
> >
> >         if (cifs_sb->master_tlink) {
> > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > +               stop_track_sb_dir_changes(cifs_sb);
> > +#endif
> >                 tcon = cifs_sb->master_tlink->tl_tcon;
> >                 if (tcon) {
> >                         spin_lock(&tcon->sb_list_lock);
> > diff --git a/fs/smb/client/notify.c b/fs/smb/client/notify.c
> > new file mode 100644
> > index 000000000000..e38345965744
> > --- /dev/null
> > +++ b/fs/smb/client/notify.c
> > @@ -0,0 +1,535 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Directory change notification tracking for SMB
> > + *
> > + * Copyright (c) 2025, Sang-Heon Jeon <ekffu200098@gmail.com>
> > + *
> > + * References:
> > + * MS-SMB2 "2.2.35 SMB2 CHANGE_NOTIFY Request"
> > + * MS-SMB2 "2.2.36 SMB2 CHANGE_NOTIFY Response"
> > + * MS-SMB2 "2.7.1 FILE_NOTIFY_INFORMATION"
> > + * MS-SMB2 "3.3.5.19 Receiving and SMB2 CHANGE_NOTIFY Request"
> > + * MS-FSCC "2.6 File Attributes"
> > + */
> > +
> > +#include <linux/list.h>
> > +#include <linux/slab.h>
> > +#include <linux/fsnotify.h>
> > +#include "notify.h"
> > +#include "cifsproto.h"
> > +#include "smb2proto.h"
> > +#include "cached_dir.h"
> > +#include "cifs_debug.h"
> > +#include "cifspdu.h"
> > +#include "cifs_unicode.h"
> > +#include "../common/smb2pdu.h"
> > +#include "../common/smb2status.h"
> > +
> > +#define CLEANUP_INTERVAL (30 * HZ)
> > +#define CLEANUP_IMMEDIATE 0
> > +
> > +enum notify_state {
> > +       NOTIFY_STATE_RECONNECT = BIT(0),
> > +       NOTIFY_STATE_UMOUNT = BIT(1),
> > +       NOTIFY_STATE_NOMASK = BIT(2),
> > +       NOTIFY_STATE_BROKEN_REQ = BIT(3),
> > +       NOTIFY_STATE_BROKEN_RSP = BIT(4),
> > +};
> > +
> > +struct notify_info {
> > +       struct inode *inode;
> > +       const char *path;
> > +       __le16 *utf16_path;
> > +       struct cifs_fid cifs_fid;
> > +       atomic_t state;
> > +       struct list_head list;
> > +};
> > +
> > +static int request_change_notify(struct notify_info *info);
> > +static void notify_cleanup_worker(struct work_struct *work);
> > +
> > +static LIST_HEAD(notify_list);
> > +static DEFINE_SPINLOCK(notify_list_lock);
> > +static DECLARE_DELAYED_WORK(notify_cleanup_work, notify_cleanup_worker);
> > +
> > +static bool is_resumeable(struct notify_info *info)
> > +{
> > +       return atomic_read(&info->state) == NOTIFY_STATE_RECONNECT;
> > +}
> > +
> > +static bool is_active(struct notify_info *info)
> > +{
> > +       return atomic_read(&info->state) == 0;
> > +}
> > +
> > +static void set_state(struct notify_info *info, int state)
> > +{
> > +       atomic_or(state, &info->state);
> > +       if (!is_resumeable(info))
> > +               mod_delayed_work(cifsiod_wq, &notify_cleanup_work,
> > +                       CLEANUP_IMMEDIATE);
> > +}
> > +
> > +static void clear_state(struct notify_info *info, int state)
> > +{
> > +       atomic_and(~state, &info->state);
> > +}
> > +
> > +static int fsnotify_send(__u32 mask,
> > +                        struct inode *parent,
> > +                        struct file_notify_information *fni,
> > +                        u32 cookie)
> > +{
> > +       char *name = cifs_strndup_from_utf16(fni->FileName,
> > +                               le32_to_cpu(fni->FileNameLength), true,
> > +                               CIFS_SB(parent->i_sb)->local_nls);
> > +       struct qstr qstr;
> > +       int rc = 0;
> > +
> > +       if (!name)
> > +               return -ENOMEM;
> > +
> > +       qstr.name = (const unsigned char *)name;
> > +       qstr.len = strlen(name);
> > +
> > +       rc = fsnotify_name(mask, NULL, FSNOTIFY_EVENT_NONE, parent,
> > +                       &qstr, cookie);
> > +       cifs_dbg(FYI, "fsnotify mask=%u, name=%s, cookie=%u, w/return=%d",
> > +               mask, name, cookie, rc);
> > +       kfree(name);
> > +       return rc;
> > +}
> > +
> > +static bool is_fsnotify_masked(struct inode *inode)
> > +{
> > +       if (!inode)
> > +               return false;
> > +
> > +       /* Minimal validation of file explore inotify */
> > +       return inode->i_fsnotify_mask &
> > +               (FS_CREATE | FS_DELETE | FS_MOVED_FROM | FS_MOVED_TO);
> > +}
> > +
> > +static void handle_file_notify_information(struct notify_info *info,
> > +                                          char *buf,
> > +                                          unsigned int buf_len)
> > +{
> > +       struct file_notify_information *fni;
> > +       unsigned int next_entry_offset;
> > +       u32 cookie;
> > +       bool has_cookie = false;
> > +
> > +       do {
> > +               fni = (struct file_notify_information *)buf;
> > +               next_entry_offset = le32_to_cpu(fni->NextEntryOffset);
> > +               if (next_entry_offset > buf_len) {
> > +                       cifs_dbg(FYI, "invalid fni->NextEntryOffset=%u",
> > +                               next_entry_offset);
> > +                       break;
> > +               }
> > +
> > +               switch (le32_to_cpu(fni->Action)) {
> > +               case FILE_ACTION_ADDED:
> > +                       fsnotify_send(FS_CREATE, info->inode, fni, 0);
> > +                       break;
> > +               case FILE_ACTION_REMOVED:
> > +                       fsnotify_send(FS_DELETE, info->inode, fni, 0);
> > +                       break;
> > +               case FILE_ACTION_RENAMED_OLD_NAME:
> > +                       if (!has_cookie)
> > +                               cookie = fsnotify_get_cookie();
> > +                       has_cookie = !has_cookie;
> > +                       fsnotify_send(FS_MOVED_FROM, info->inode, fni, cookie);
> > +                       break;
> > +               case FILE_ACTION_RENAMED_NEW_NAME:
> > +                       if (!has_cookie)
> > +                               cookie = fsnotify_get_cookie();
> > +                       has_cookie = !has_cookie;
> > +                       fsnotify_send(FS_MOVED_TO, info->inode, fni, cookie);
> > +                       break;
> > +               default:
> > +                       /* Does not occur, so no need to handle */
> > +                       break;
> > +               }
> > +
> > +               buf += next_entry_offset;
> > +               buf_len -= next_entry_offset;
> > +       } while (buf_len > 0 && next_entry_offset > 0);
> > +}
> > +
> > +static void handle_smb2_change_notify_rsp(struct notify_info *info,
> > +                                         struct mid_q_entry *mid)
> > +{
> > +       struct smb2_change_notify_rsp *rsp = mid->resp_buf;
> > +       struct kvec rsp_iov;
> > +       unsigned int buf_offset, buf_len;
> > +       int rc;
> > +
> > +       switch (rsp->hdr.Status) {
> > +       case STATUS_SUCCESS:
> > +               break;
> > +       case STATUS_NOTIFY_ENUM_DIR:
> > +               goto proceed;
> > +       case STATUS_USER_SESSION_DELETED:
> > +       case STATUS_NETWORK_NAME_DELETED:
> > +       case STATUS_NETWORK_SESSION_EXPIRED:
> > +               set_state(info, NOTIFY_STATE_RECONNECT);
> > +               return;
> > +       default:
> > +               set_state(info, NOTIFY_STATE_BROKEN_RSP);
> > +               return;
> > +       }
> > +
> > +       rsp_iov.iov_base = mid->resp_buf;
> > +       rsp_iov.iov_len = mid->resp_buf_size;
> > +       buf_offset = le16_to_cpu(rsp->OutputBufferOffset);
> > +       buf_len = le32_to_cpu(rsp->OutputBufferLength);
> > +
> > +       rc = smb2_validate_iov(buf_offset, buf_len, &rsp_iov,
> > +                               sizeof(struct file_notify_information));
> > +       if (rc) {
> > +               cifs_dbg(FYI, "stay tracking, w/smb2_validate_iov=%d", rc);
> > +               goto proceed;
> > +       }
> > +
> > +       handle_file_notify_information(info,
> > +               (char *)rsp + buf_offset, buf_len);
> > +proceed:
> > +       request_change_notify(info);
> > +       return;
> > +}
> > +
> > +static void change_notify_callback(struct mid_q_entry *mid)
> > +{
> > +       struct notify_info *info = mid->callback_data;
> > +
> > +       if (!is_active(info))
> > +               return;
> > +
> > +       if (!is_fsnotify_masked(info->inode)) {
> > +               set_state(info, NOTIFY_STATE_NOMASK);
> > +               return;
> > +       }
> > +
> > +       if (!mid->resp_buf) {
> > +               if (mid->mid_state != MID_RETRY_NEEDED) {
> > +                       cifs_dbg(FYI, "stop tracking, w/mid_state=%d",
> > +                               mid->mid_state);
> > +                       set_state(info, NOTIFY_STATE_BROKEN_RSP);
> > +                       return;
> > +               }
> > +
> > +               set_state(info, NOTIFY_STATE_RECONNECT);
> > +               return;
> > +       }
> > +
> > +       handle_smb2_change_notify_rsp(info, mid);
> > +}
> > +
> > +static int request_change_notify(struct notify_info *info)
> > +{
> > +       struct cifs_sb_info *cifs_sb = CIFS_SB(info->inode->i_sb);
> > +       struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
> > +       struct smb_rqst rqst;
> > +       struct kvec iov[1];
> > +       unsigned int xid;
> > +       int rc;
> > +
> > +       if (!tcon) {
> > +               cifs_dbg(FYI, "missing tcon while request change notify");
> > +               return -EINVAL;
> > +       }
> > +
> > +       memset(&rqst, 0, sizeof(struct smb_rqst));
> > +       memset(&iov, 0, sizeof(iov));
> > +       rqst.rq_iov = iov;
> > +       rqst.rq_nvec = 1;
> > +
> > +       xid = get_xid();
> > +       rc = SMB2_notify_init(xid, &rqst, tcon, tcon->ses->server,
> > +               info->cifs_fid.persistent_fid, info->cifs_fid.volatile_fid,
> > +               FILE_NOTIFY_CHANGE_FILE_NAME | FILE_NOTIFY_CHANGE_DIR_NAME,
> > +               false);
> > +       free_xid(xid);
> > +       if (rc) {
> > +               set_state(info, NOTIFY_STATE_BROKEN_REQ);
> > +               return rc;
> > +       }
> > +
> > +       rc = cifs_call_async(tcon->ses->server, &rqst, NULL,
> > +               change_notify_callback, NULL, info, 0, NULL);
> > +       cifs_small_buf_release(rqst.rq_iov[0].iov_base);
> > +
> > +       if (rc)
> > +               set_state(info, NOTIFY_STATE_BROKEN_REQ);
> > +       return rc;
> > +}
> > +
> > +
> > +static void free_notify_info(struct notify_info *info)
> > +{
> > +       kfree(info->utf16_path);
> > +       kfree(info->path);
> > +       kfree(info);
> > +}
> > +
> > +static void cleanup_pending_mid(struct notify_info *info,
> > +                               struct cifs_tcon *tcon)
> > +{
> > +       LIST_HEAD(dispose_list);
> > +       struct TCP_Server_Info *server;
> > +       struct mid_q_entry *mid, *nmid;
> > +
> > +       if (!tcon->ses || !tcon->ses->server)
> > +               return;
> > +
> > +       server = tcon->ses->server;
> > +
> > +       spin_lock(&server->mid_queue_lock);
> > +       list_for_each_entry_safe(mid, nmid, &server->pending_mid_q, qhead) {
> > +               if (mid->callback_data == info) {
> > +                       mid->deleted_from_q = true;
> > +                       list_move(&mid->qhead, &dispose_list);
> > +               }
> > +       }
> > +       spin_unlock(&server->mid_queue_lock);
> > +
> > +       list_for_each_entry_safe(mid, nmid, &dispose_list, qhead) {
> > +               list_del_init(&mid->qhead);
> > +               release_mid(mid);
> > +       }
> > +}
> > +
> > +static void close_fid(struct notify_info *info)
> > +{
> > +       struct cifs_tcon *tcon;
> > +
> > +       unsigned int xid;
> > +       int rc;
> > +
> > +       if (!info->cifs_fid.persistent_fid && !info->cifs_fid.volatile_fid)
> > +               return;
> > +
> > +       tcon = cifs_sb_master_tcon(CIFS_SB(info->inode->i_sb));
> > +       if (!tcon) {
> > +               cifs_dbg(FYI, "missing master tcon while close");
> > +               return;
> > +       }
> > +
> > +       xid = get_xid();
> > +       rc = SMB2_close(xid, tcon, info->cifs_fid.persistent_fid,
> > +               info->cifs_fid.volatile_fid);
> > +       if (rc) {
> > +               cifs_dbg(FYI, "cleanup pending mid, w/SMB2_close=%d", rc);
> > +               cleanup_pending_mid(info, tcon);
> > +       }
> > +       free_xid(xid);
> > +}
> > +
> > +static int setup_fid(struct notify_info *info)
> > +{
> > +       struct cifs_sb_info *cifs_sb = CIFS_SB(info->inode->i_sb);
> > +       struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
> > +       struct cifs_open_parms oparms;
> > +       __u8 oplock = 0;
> > +       unsigned int xid;
> > +       int rc = 0;
> > +
> > +       if (!tcon) {
> > +               cifs_dbg(FYI, "missing master tcon while open");
> > +               return -EINVAL;
> > +       }
> > +
> > +       xid = get_xid();
> > +       oparms = (struct cifs_open_parms) {
> > +               .tcon = tcon,
> > +               .path = info->path,
> > +               .desired_access = GENERIC_READ,
> > +               .disposition = FILE_OPEN,
> > +               .create_options = cifs_create_options(cifs_sb, 0),
> > +               .fid = &info->cifs_fid,
> > +               .cifs_sb = cifs_sb,
> > +               .reconnect = false,
> > +       };
> > +       rc = SMB2_open(xid, &oparms, info->utf16_path, &oplock,
> > +                       NULL, NULL, NULL, NULL);
> > +       free_xid(xid);
> > +       return rc;
> > +}
> > +
> > +static bool is_already_tracking(struct inode *dir_inode)
> > +{
> > +       struct notify_info *entry, *nentry;
> > +
> > +       spin_lock(&notify_list_lock);
> > +       list_for_each_entry_safe(entry, nentry, &notify_list, list) {
> > +               if (is_active(entry)) {
> > +                       if (entry->inode == dir_inode) {
> > +                               spin_unlock(&notify_list_lock);
> > +                               return true;
> > +                       }
> > +
> > +                       /* Extra check since we must keep iterating */
> > +                       if (!is_fsnotify_masked(entry->inode))
> > +                               set_state(entry, NOTIFY_STATE_NOMASK);
> > +               }
> > +       }
> > +       spin_unlock(&notify_list_lock);
> > +
> > +       return false;
> > +}
> > +
> > +static bool is_tracking_supported(struct cifs_sb_info *cifs_sb)
> > +{
> > +       struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
> > +
> > +       if (!tcon->ses || !tcon->ses->server)
> > +               return false;
> > +       return tcon->ses->server->dialect >= SMB20_PROT_ID;
> > +}
> > +
> > +int start_track_dir_changes(const char *path,
> > +                           struct inode *dir_inode,
> > +                           struct cifs_sb_info *cifs_sb)
> > +{
> > +       struct notify_info *info;
> > +       int rc;
> > +
> > +       if (!is_tracking_supported(cifs_sb))
> > +               return -EINVAL;
> > +
> > +       if (!is_fsnotify_masked(dir_inode))
> > +               return -EINVAL;
> > +
> > +       if (is_already_tracking(dir_inode))
> > +               return 1;
> > +
> > +       info = kzalloc(sizeof(*info), GFP_KERNEL);
> > +       if (!info)
> > +               return -ENOMEM;
> > +
> > +       info->path = kstrdup(path, GFP_KERNEL);
> > +       if (!info->path) {
> > +               free_notify_info(info);
> > +               return -ENOMEM;
> > +       }
> > +       info->utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
> > +       if (!info->utf16_path) {
> > +               free_notify_info(info);
> > +               return -ENOMEM;
> > +       }
> > +       info->inode = dir_inode;
> > +
> > +       rc = setup_fid(info);
> > +       if (rc) {
> > +               free_notify_info(info);
> > +               return rc;
> > +       }
> > +       rc = request_change_notify(info);
> > +       if (rc) {
> > +               close_fid(info);
> > +               free_notify_info(info);
> > +               return rc;
> > +       }
> > +
> > +       spin_lock(&notify_list_lock);
> > +       list_add(&info->list, &notify_list);
> > +       spin_unlock(&notify_list_lock);
> > +       return rc;
> > +}
> > +
> > +void stop_track_sb_dir_changes(struct cifs_sb_info *cifs_sb)
> > +{
> > +       struct notify_info *entry, *nentry;
> > +
> > +       if (!list_empty(&notify_list)) {
> > +               spin_lock(&notify_list_lock);
> > +               list_for_each_entry_safe(entry, nentry, &notify_list, list) {
> > +                       if (cifs_sb == CIFS_SB(entry->inode->i_sb)) {
> > +                               set_state(entry, NOTIFY_STATE_UMOUNT);
> > +                               continue;
> > +                       }
> > +
> > +                       /* Extra check since we must keep iterating */
> > +                       if (!is_fsnotify_masked(entry->inode))
> > +                               set_state(entry, NOTIFY_STATE_NOMASK);
> > +               }
> > +               spin_unlock(&notify_list_lock);
> > +       }
> > +}
> > +
> > +void resume_track_dir_changes(void)
> > +{
> > +       LIST_HEAD(resume_list);
> > +       struct notify_info *entry, *nentry;
> > +       struct cifs_tcon *tcon;
> > +
> > +       if (list_empty(&notify_list))
> > +               return;
> > +
> > +       spin_lock(&notify_list_lock);
> > +       list_for_each_entry_safe(entry, nentry, &notify_list, list) {
> > +               if (!is_fsnotify_masked(entry->inode)) {
> > +                       set_state(entry, NOTIFY_STATE_NOMASK);
> > +                       continue;
> > +               }
> > +
> > +               if (is_resumeable(entry)) {
> > +                       tcon = cifs_sb_master_tcon(CIFS_SB(entry->inode->i_sb));
> > +                       spin_lock(&tcon->tc_lock);
> > +                       if (tcon->status == TID_GOOD) {
> > +                               spin_unlock(&tcon->tc_lock);
> > +                               list_move(&entry->list, &resume_list);
> > +                               continue;
> > +                       }
> > +                       spin_unlock(&tcon->tc_lock);
> > +               }
> > +       }
> > +       spin_unlock(&notify_list_lock);
> > +
> > +       list_for_each_entry_safe(entry, nentry, &resume_list, list) {
> > +               if (setup_fid(entry)) {
> > +                       list_del(&entry->list);
> > +                       free_notify_info(entry);
> > +                       continue;
> > +               }
> > +
> > +               if (request_change_notify(entry)) {
> > +                       list_del(&entry->list);
> > +                       close_fid(entry);
> > +                       free_notify_info(entry);
> > +                       continue;
> > +               }
> > +
> > +               clear_state(entry, NOTIFY_STATE_RECONNECT);
> > +       }
> > +
> > +       if (!list_empty(&resume_list)) {
> > +               spin_lock(&notify_list_lock);
> > +               list_splice(&resume_list, &notify_list);
> > +               spin_unlock(&notify_list_lock);
> > +       }
> > +}
> > +
> > +static void notify_cleanup_worker(struct work_struct *work)
> > +{
> > +       LIST_HEAD(cleanup_list);
> > +       struct notify_info *entry, *nentry;
> > +
> > +       if (list_empty(&notify_list))
> > +               return;
> > +
> > +       spin_lock(&notify_list_lock);
> > +       list_for_each_entry_safe(entry, nentry, &notify_list, list) {
> > +               if (!is_resumeable(entry) && !is_active(entry))
> > +                       list_move(&entry->list, &cleanup_list);
> > +       }
> > +       spin_unlock(&notify_list_lock);
> > +
> > +       list_for_each_entry_safe(entry, nentry, &cleanup_list, list) {
> > +               list_del(&entry->list);
> > +               close_fid(entry);
> > +               free_notify_info(entry);
> > +       }
> > +       mod_delayed_work(cifsiod_wq, &notify_cleanup_work, CLEANUP_INTERVAL);
> > +}
> > diff --git a/fs/smb/client/notify.h b/fs/smb/client/notify.h
> > new file mode 100644
> > index 000000000000..088efba4dce9
> > --- /dev/null
> > +++ b/fs/smb/client/notify.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Directory change notification tracking for SMB
> > + *
> > + * Copyright (c) 2025, Sang-Heon Jeon <ekffu200098@gmail.com>
> > + */
> > +
> > +#ifndef _SMB_NOTIFY_H
> > +#define _SMB_NOTIFY_H
> > +
> > +#include "cifsglob.h"
> > +
> > +int start_track_dir_changes(const char *path,
> > +                           struct inode *dir_inode,
> > +                           struct cifs_sb_info *cifs_sb);
> > +void stop_track_sb_dir_changes(struct cifs_sb_info *cifs_sb);
> > +void resume_track_dir_changes(void);
> > +
> > +#endif /* _SMB_NOTIFY_H */
> > diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
> > index 4e5460206397..455e5be37116 100644
> > --- a/fs/smb/client/readdir.c
> > +++ b/fs/smb/client/readdir.c
> > @@ -24,6 +24,9 @@
> >  #include "fs_context.h"
> >  #include "cached_dir.h"
> >  #include "reparse.h"
> > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > +#include "notify.h"
> > +#endif
> >
> >  /*
> >   * To be safe - for UCS to UTF-8 with strings loaded with the rare long
> > @@ -1070,6 +1073,9 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
> >         if (rc)
> >                 goto cache_not_found;
> >
> > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > +       start_track_dir_changes(full_path, d_inode(file_dentry(file)), cifs_sb);
> > +#endif
> >         mutex_lock(&cfid->dirents.de_mutex);
> >         /*
> >          * If this was reading from the start of the directory
> > @@ -1151,6 +1157,9 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
> >                 cifs_dbg(FYI, "Could not find entry\n");
> >                 goto rddir2_exit;
> >         }
> > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > +       start_track_dir_changes(full_path, d_inode(file_dentry(file)), cifs_sb);
> > +#endif
> >         cifs_dbg(FYI, "loop through %d times filling dir for net buf %p\n",
> >                  num_to_fill, cifsFile->srch_inf.ntwrk_buf_start);
> >         max_len = tcon->ses->server->ops->calc_smb_size(
> > diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> > index 4e922cb32110..58a1ddc39ee6 100644
> > --- a/fs/smb/client/smb2pdu.c
> > +++ b/fs/smb/client/smb2pdu.c
> > @@ -45,6 +45,9 @@
> >  #include "cached_dir.h"
> >  #include "compress.h"
> >  #include "fs_context.h"
> > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > +#include "notify.h"
> > +#endif
> >
> >  /*
> >   *  The following table defines the expected "StructureSize" of SMB2 requests
> > @@ -466,6 +469,9 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
> >                 mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
> >
> >         atomic_inc(&tconInfoReconnectCount);
> > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > +       resume_track_dir_changes();
> > +#endif
> >  out:
> >         /*
> >          * Check if handle based operation so we know whether we can continue
> > --
> > 2.43.0
> >
>
> To Reviewers.
>
> This is a gentle reminder on review requests.
>
> I would be very grateful for your feedback and am more than willing to
> revise or improve any parts as needed. And also, let me know if I
> missed anything or made mistakes.

Hi Sang,

First feedback (value):
-----------------------------
This looks very useful. this feature has been requested and
attempted several times in the past (see links below), so if you are
willing to incorporate feedback, I hope you will reach further than those
past attempts and I will certainly do my best to help you with that.

Second feedback (reviewers):
----------------------------------------
I was very surprised that your patch doesn't touch any vfs code
(more on that on design feedback), but this is not an SMB-contained
change at all.

Your patch touches the guts of the fsnotify subsystem (in a wrong way).
For the next posting please consult the MAINTAINERS entry
of the fsnotify subsystem for reviewers and list to CC (now added).

Third feedback (design):
--------------------------------
The design choice of polling i_fsnotify_mask on readdir()
is quite odd and it is not clear to me why it makes sense.
Previous discussions suggested to have a filesystem method
to update when applications setup a watch on a directory [1].
Another prior feedback was that the API should allow a clear
distinction between the REMOTE notifications and the LOCAL
notifications [2][3].

IMO it would be better to finalize the design before working on the
code, but that's up to you.

Good luck,
Amir.

[1] https://lore.kernel.org/linux-fsdevel/20211025204634.2517-4-iangelak@redhat.com/
[2] https://sourceforge.net/p/fuse/mailman/message/58719889/
[3] https://lore.kernel.org/linux-fsdevel/YhpY%2FzZe3UA2u3fj@redhat.com/

Re: [RFC PATCH 2/2] smb: client: add directory change tracking via SMB2 Change Notify

[Sang-Heon Jeon]

On Sat, Sep 27, 2025 at 5:39 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Sat, Sep 27, 2025 at 3:03 AM Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
> >
> > On Tue, Sep 16, 2025 at 10:35 PM Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
> > >
> > > Implement directory change tracking using SMB2 Change Notify protocol
> > > to enable real-time monitoring of remote directories. Applications using
> > > inotify now receive immediate notifications when files are created,
> > > deleted, or renamed on SMB2+ shares.
> > >
> > > This implementation begins by detecting tracking opportunities during
> > > regular SMB operations. In the current version, the readdir() serves as
> > > the detection point - when directory contents are accessed on SMB2+
> > > servers, we check the inode's fsnotify_mask for active inotify watches.
> > > If watches exist and directory is not already being tracked, we obtain a
> > > new FiieId and send a SMB2_CHANGE_NOTIFY request that waits for changes.
> > >
> > > Based on the server's response status, we convert CHANGE_NOTIFY response
> > > to fsnotify events and deliver to application on success, mark for
> > > future reconnection on network errors, or discard the response and
> > > marked entries for broken. If tracking can continue, we resend
> > > SMB2_CHANGE_NOTIFY for continuous monitoring.
> > >
> > > Entries marked for reconnection are restored during SMB2 reconnection.
> > > In the current version, smb2_reconnect() serves as restoration point -
> > > when connection is reestablished, we obtain new FileIds and request
> > > Change Notify requests for these entries.
> > >
> > > Entries marked for unmount during unmount. In the current version,
> > > cifs_umount() serves as unmount marking point. Entries marked for clean
> > > are remove as soon as possible by the worker. Workers also run
> > > periodically; currently every 30s; to check and remove untracking
> > > directories.
> > >
> > > This feature is controlled by CONFIG_CIFS_DIR_CHANGE_TRACKING with
> > > experimental.
> > >
> > > Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
> > > ---
> > >  fs/smb/client/Kconfig   |  17 ++
> > >  fs/smb/client/Makefile  |   2 +
> > >  fs/smb/client/connect.c |   6 +
> > >  fs/smb/client/notify.c  | 535 ++++++++++++++++++++++++++++++++++++++++
> > >  fs/smb/client/notify.h  |  19 ++
> > >  fs/smb/client/readdir.c |   9 +
> > >  fs/smb/client/smb2pdu.c |   6 +
> > >  7 files changed, 594 insertions(+)
> > >  create mode 100644 fs/smb/client/notify.c
> > >  create mode 100644 fs/smb/client/notify.h
> > >
> > > diff --git a/fs/smb/client/Kconfig b/fs/smb/client/Kconfig
> > > index a4c02199fef4..0e3911936e0c 100644
> > > --- a/fs/smb/client/Kconfig
> > > +++ b/fs/smb/client/Kconfig
> > > @@ -218,4 +218,21 @@ config CIFS_COMPRESSION
> > >           Say Y here if you want SMB traffic to be compressed.
> > >           If unsure, say N.
> > >
> > > ++config CIFS_DIR_CHANGE_TRACKING
> >
> > I also found a typo here and I'll fix them.
> >
> > > +       bool "Directory change tracking (Experimental)"
> > > +       depends on CIFS && FSNOTIFY=y
> > > +       default n
> > > +       help
> > > +          Enables automatic tracking of directory changes for SMB2 or later
> > > +          using the SMB2 Change Notify protocol. File managers and applications
> > > +          monitoring directories via inotify will receive real-time updates
> > > +          when files are created, deleted, or renamed on the server.
> > > +
> > > +          This feature maintains persistent connections to track changes.
> > > +          Each monitored directory consumes server resources and may increase
> > > +          network traffic.
> > > +
> > > +          Say Y here if you want real-time directory monitoring.
> > > +          If unsure, say N.
> > > +
> > >  endif
> > > diff --git a/fs/smb/client/Makefile b/fs/smb/client/Makefile
> > > index 4c97b31a25c2..85253bc1b4b0 100644
> > > --- a/fs/smb/client/Makefile
> > > +++ b/fs/smb/client/Makefile
> > > @@ -35,3 +35,5 @@ cifs-$(CONFIG_CIFS_ROOT) += cifsroot.o
> > >  cifs-$(CONFIG_CIFS_ALLOW_INSECURE_LEGACY) += smb1ops.o cifssmb.o cifstransport.o
> > >
> > >  cifs-$(CONFIG_CIFS_COMPRESSION) += compress.o compress/lz77.o
> > > +
> > > +cifs-$(CONFIG_CIFS_DIR_CHANGE_TRACKING) += notify.o
> > > diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> > > index dd12f3eb61dc..eebf729df16a 100644
> > > --- a/fs/smb/client/connect.c
> > > +++ b/fs/smb/client/connect.c
> > > @@ -51,6 +51,9 @@
> > >  #endif
> > >  #include "fs_context.h"
> > >  #include "cifs_swn.h"
> > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > +#include "notify.h"
> > > +#endif
> > >
> > >  /* FIXME: should these be tunable? */
> > >  #define TLINK_ERROR_EXPIRE     (1 * HZ)
> > > @@ -4154,6 +4157,9 @@ cifs_umount(struct cifs_sb_info *cifs_sb)
> > >         cancel_delayed_work_sync(&cifs_sb->prune_tlinks);
> > >
> > >         if (cifs_sb->master_tlink) {
> > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > +               stop_track_sb_dir_changes(cifs_sb);
> > > +#endif
> > >                 tcon = cifs_sb->master_tlink->tl_tcon;
> > >                 if (tcon) {
> > >                         spin_lock(&tcon->sb_list_lock);
> > > diff --git a/fs/smb/client/notify.c b/fs/smb/client/notify.c
> > > new file mode 100644
> > > index 000000000000..e38345965744
> > > --- /dev/null
> > > +++ b/fs/smb/client/notify.c
> > > @@ -0,0 +1,535 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Directory change notification tracking for SMB
> > > + *
> > > + * Copyright (c) 2025, Sang-Heon Jeon <ekffu200098@gmail.com>
> > > + *
> > > + * References:
> > > + * MS-SMB2 "2.2.35 SMB2 CHANGE_NOTIFY Request"
> > > + * MS-SMB2 "2.2.36 SMB2 CHANGE_NOTIFY Response"
> > > + * MS-SMB2 "2.7.1 FILE_NOTIFY_INFORMATION"
> > > + * MS-SMB2 "3.3.5.19 Receiving and SMB2 CHANGE_NOTIFY Request"
> > > + * MS-FSCC "2.6 File Attributes"
> > > + */
> > > +
> > > +#include <linux/list.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/fsnotify.h>
> > > +#include "notify.h"
> > > +#include "cifsproto.h"
> > > +#include "smb2proto.h"
> > > +#include "cached_dir.h"
> > > +#include "cifs_debug.h"
> > > +#include "cifspdu.h"
> > > +#include "cifs_unicode.h"
> > > +#include "../common/smb2pdu.h"
> > > +#include "../common/smb2status.h"
> > > +
> > > +#define CLEANUP_INTERVAL (30 * HZ)
> > > +#define CLEANUP_IMMEDIATE 0
> > > +
> > > +enum notify_state {
> > > +       NOTIFY_STATE_RECONNECT = BIT(0),
> > > +       NOTIFY_STATE_UMOUNT = BIT(1),
> > > +       NOTIFY_STATE_NOMASK = BIT(2),
> > > +       NOTIFY_STATE_BROKEN_REQ = BIT(3),
> > > +       NOTIFY_STATE_BROKEN_RSP = BIT(4),
> > > +};
> > > +
> > > +struct notify_info {
> > > +       struct inode *inode;
> > > +       const char *path;
> > > +       __le16 *utf16_path;
> > > +       struct cifs_fid cifs_fid;
> > > +       atomic_t state;
> > > +       struct list_head list;
> > > +};
> > > +
> > > +static int request_change_notify(struct notify_info *info);
> > > +static void notify_cleanup_worker(struct work_struct *work);
> > > +
> > > +static LIST_HEAD(notify_list);
> > > +static DEFINE_SPINLOCK(notify_list_lock);
> > > +static DECLARE_DELAYED_WORK(notify_cleanup_work, notify_cleanup_worker);
> > > +
> > > +static bool is_resumeable(struct notify_info *info)
> > > +{
> > > +       return atomic_read(&info->state) == NOTIFY_STATE_RECONNECT;
> > > +}
> > > +
> > > +static bool is_active(struct notify_info *info)
> > > +{
> > > +       return atomic_read(&info->state) == 0;
> > > +}
> > > +
> > > +static void set_state(struct notify_info *info, int state)
> > > +{
> > > +       atomic_or(state, &info->state);
> > > +       if (!is_resumeable(info))
> > > +               mod_delayed_work(cifsiod_wq, &notify_cleanup_work,
> > > +                       CLEANUP_IMMEDIATE);
> > > +}
> > > +
> > > +static void clear_state(struct notify_info *info, int state)
> > > +{
> > > +       atomic_and(~state, &info->state);
> > > +}
> > > +
> > > +static int fsnotify_send(__u32 mask,
> > > +                        struct inode *parent,
> > > +                        struct file_notify_information *fni,
> > > +                        u32 cookie)
> > > +{
> > > +       char *name = cifs_strndup_from_utf16(fni->FileName,
> > > +                               le32_to_cpu(fni->FileNameLength), true,
> > > +                               CIFS_SB(parent->i_sb)->local_nls);
> > > +       struct qstr qstr;
> > > +       int rc = 0;
> > > +
> > > +       if (!name)
> > > +               return -ENOMEM;
> > > +
> > > +       qstr.name = (const unsigned char *)name;
> > > +       qstr.len = strlen(name);
> > > +
> > > +       rc = fsnotify_name(mask, NULL, FSNOTIFY_EVENT_NONE, parent,
> > > +                       &qstr, cookie);
> > > +       cifs_dbg(FYI, "fsnotify mask=%u, name=%s, cookie=%u, w/return=%d",
> > > +               mask, name, cookie, rc);
> > > +       kfree(name);
> > > +       return rc;
> > > +}
> > > +
> > > +static bool is_fsnotify_masked(struct inode *inode)
> > > +{
> > > +       if (!inode)
> > > +               return false;
> > > +
> > > +       /* Minimal validation of file explore inotify */
> > > +       return inode->i_fsnotify_mask &
> > > +               (FS_CREATE | FS_DELETE | FS_MOVED_FROM | FS_MOVED_TO);
> > > +}
> > > +
> > > +static void handle_file_notify_information(struct notify_info *info,
> > > +                                          char *buf,
> > > +                                          unsigned int buf_len)
> > > +{
> > > +       struct file_notify_information *fni;
> > > +       unsigned int next_entry_offset;
> > > +       u32 cookie;
> > > +       bool has_cookie = false;
> > > +
> > > +       do {
> > > +               fni = (struct file_notify_information *)buf;
> > > +               next_entry_offset = le32_to_cpu(fni->NextEntryOffset);
> > > +               if (next_entry_offset > buf_len) {
> > > +                       cifs_dbg(FYI, "invalid fni->NextEntryOffset=%u",
> > > +                               next_entry_offset);
> > > +                       break;
> > > +               }
> > > +
> > > +               switch (le32_to_cpu(fni->Action)) {
> > > +               case FILE_ACTION_ADDED:
> > > +                       fsnotify_send(FS_CREATE, info->inode, fni, 0);
> > > +                       break;
> > > +               case FILE_ACTION_REMOVED:
> > > +                       fsnotify_send(FS_DELETE, info->inode, fni, 0);
> > > +                       break;
> > > +               case FILE_ACTION_RENAMED_OLD_NAME:
> > > +                       if (!has_cookie)
> > > +                               cookie = fsnotify_get_cookie();
> > > +                       has_cookie = !has_cookie;
> > > +                       fsnotify_send(FS_MOVED_FROM, info->inode, fni, cookie);
> > > +                       break;
> > > +               case FILE_ACTION_RENAMED_NEW_NAME:
> > > +                       if (!has_cookie)
> > > +                               cookie = fsnotify_get_cookie();
> > > +                       has_cookie = !has_cookie;
> > > +                       fsnotify_send(FS_MOVED_TO, info->inode, fni, cookie);
> > > +                       break;
> > > +               default:
> > > +                       /* Does not occur, so no need to handle */
> > > +                       break;
> > > +               }
> > > +
> > > +               buf += next_entry_offset;
> > > +               buf_len -= next_entry_offset;
> > > +       } while (buf_len > 0 && next_entry_offset > 0);
> > > +}
> > > +
> > > +static void handle_smb2_change_notify_rsp(struct notify_info *info,
> > > +                                         struct mid_q_entry *mid)
> > > +{
> > > +       struct smb2_change_notify_rsp *rsp = mid->resp_buf;
> > > +       struct kvec rsp_iov;
> > > +       unsigned int buf_offset, buf_len;
> > > +       int rc;
> > > +
> > > +       switch (rsp->hdr.Status) {
> > > +       case STATUS_SUCCESS:
> > > +               break;
> > > +       case STATUS_NOTIFY_ENUM_DIR:
> > > +               goto proceed;
> > > +       case STATUS_USER_SESSION_DELETED:
> > > +       case STATUS_NETWORK_NAME_DELETED:
> > > +       case STATUS_NETWORK_SESSION_EXPIRED:
> > > +               set_state(info, NOTIFY_STATE_RECONNECT);
> > > +               return;
> > > +       default:
> > > +               set_state(info, NOTIFY_STATE_BROKEN_RSP);
> > > +               return;
> > > +       }
> > > +
> > > +       rsp_iov.iov_base = mid->resp_buf;
> > > +       rsp_iov.iov_len = mid->resp_buf_size;
> > > +       buf_offset = le16_to_cpu(rsp->OutputBufferOffset);
> > > +       buf_len = le32_to_cpu(rsp->OutputBufferLength);
> > > +
> > > +       rc = smb2_validate_iov(buf_offset, buf_len, &rsp_iov,
> > > +                               sizeof(struct file_notify_information));
> > > +       if (rc) {
> > > +               cifs_dbg(FYI, "stay tracking, w/smb2_validate_iov=%d", rc);
> > > +               goto proceed;
> > > +       }
> > > +
> > > +       handle_file_notify_information(info,
> > > +               (char *)rsp + buf_offset, buf_len);
> > > +proceed:
> > > +       request_change_notify(info);
> > > +       return;
> > > +}
> > > +
> > > +static void change_notify_callback(struct mid_q_entry *mid)
> > > +{
> > > +       struct notify_info *info = mid->callback_data;
> > > +
> > > +       if (!is_active(info))
> > > +               return;
> > > +
> > > +       if (!is_fsnotify_masked(info->inode)) {
> > > +               set_state(info, NOTIFY_STATE_NOMASK);
> > > +               return;
> > > +       }
> > > +
> > > +       if (!mid->resp_buf) {
> > > +               if (mid->mid_state != MID_RETRY_NEEDED) {
> > > +                       cifs_dbg(FYI, "stop tracking, w/mid_state=%d",
> > > +                               mid->mid_state);
> > > +                       set_state(info, NOTIFY_STATE_BROKEN_RSP);
> > > +                       return;
> > > +               }
> > > +
> > > +               set_state(info, NOTIFY_STATE_RECONNECT);
> > > +               return;
> > > +       }
> > > +
> > > +       handle_smb2_change_notify_rsp(info, mid);
> > > +}
> > > +
> > > +static int request_change_notify(struct notify_info *info)
> > > +{
> > > +       struct cifs_sb_info *cifs_sb = CIFS_SB(info->inode->i_sb);
> > > +       struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
> > > +       struct smb_rqst rqst;
> > > +       struct kvec iov[1];
> > > +       unsigned int xid;
> > > +       int rc;
> > > +
> > > +       if (!tcon) {
> > > +               cifs_dbg(FYI, "missing tcon while request change notify");
> > > +               return -EINVAL;
> > > +       }
> > > +
> > > +       memset(&rqst, 0, sizeof(struct smb_rqst));
> > > +       memset(&iov, 0, sizeof(iov));
> > > +       rqst.rq_iov = iov;
> > > +       rqst.rq_nvec = 1;
> > > +
> > > +       xid = get_xid();
> > > +       rc = SMB2_notify_init(xid, &rqst, tcon, tcon->ses->server,
> > > +               info->cifs_fid.persistent_fid, info->cifs_fid.volatile_fid,
> > > +               FILE_NOTIFY_CHANGE_FILE_NAME | FILE_NOTIFY_CHANGE_DIR_NAME,
> > > +               false);
> > > +       free_xid(xid);
> > > +       if (rc) {
> > > +               set_state(info, NOTIFY_STATE_BROKEN_REQ);
> > > +               return rc;
> > > +       }
> > > +
> > > +       rc = cifs_call_async(tcon->ses->server, &rqst, NULL,
> > > +               change_notify_callback, NULL, info, 0, NULL);
> > > +       cifs_small_buf_release(rqst.rq_iov[0].iov_base);
> > > +
> > > +       if (rc)
> > > +               set_state(info, NOTIFY_STATE_BROKEN_REQ);
> > > +       return rc;
> > > +}
> > > +
> > > +
> > > +static void free_notify_info(struct notify_info *info)
> > > +{
> > > +       kfree(info->utf16_path);
> > > +       kfree(info->path);
> > > +       kfree(info);
> > > +}
> > > +
> > > +static void cleanup_pending_mid(struct notify_info *info,
> > > +                               struct cifs_tcon *tcon)
> > > +{
> > > +       LIST_HEAD(dispose_list);
> > > +       struct TCP_Server_Info *server;
> > > +       struct mid_q_entry *mid, *nmid;
> > > +
> > > +       if (!tcon->ses || !tcon->ses->server)
> > > +               return;
> > > +
> > > +       server = tcon->ses->server;
> > > +
> > > +       spin_lock(&server->mid_queue_lock);
> > > +       list_for_each_entry_safe(mid, nmid, &server->pending_mid_q, qhead) {
> > > +               if (mid->callback_data == info) {
> > > +                       mid->deleted_from_q = true;
> > > +                       list_move(&mid->qhead, &dispose_list);
> > > +               }
> > > +       }
> > > +       spin_unlock(&server->mid_queue_lock);
> > > +
> > > +       list_for_each_entry_safe(mid, nmid, &dispose_list, qhead) {
> > > +               list_del_init(&mid->qhead);
> > > +               release_mid(mid);
> > > +       }
> > > +}
> > > +
> > > +static void close_fid(struct notify_info *info)
> > > +{
> > > +       struct cifs_tcon *tcon;
> > > +
> > > +       unsigned int xid;
> > > +       int rc;
> > > +
> > > +       if (!info->cifs_fid.persistent_fid && !info->cifs_fid.volatile_fid)
> > > +               return;
> > > +
> > > +       tcon = cifs_sb_master_tcon(CIFS_SB(info->inode->i_sb));
> > > +       if (!tcon) {
> > > +               cifs_dbg(FYI, "missing master tcon while close");
> > > +               return;
> > > +       }
> > > +
> > > +       xid = get_xid();
> > > +       rc = SMB2_close(xid, tcon, info->cifs_fid.persistent_fid,
> > > +               info->cifs_fid.volatile_fid);
> > > +       if (rc) {
> > > +               cifs_dbg(FYI, "cleanup pending mid, w/SMB2_close=%d", rc);
> > > +               cleanup_pending_mid(info, tcon);
> > > +       }
> > > +       free_xid(xid);
> > > +}
> > > +
> > > +static int setup_fid(struct notify_info *info)
> > > +{
> > > +       struct cifs_sb_info *cifs_sb = CIFS_SB(info->inode->i_sb);
> > > +       struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
> > > +       struct cifs_open_parms oparms;
> > > +       __u8 oplock = 0;
> > > +       unsigned int xid;
> > > +       int rc = 0;
> > > +
> > > +       if (!tcon) {
> > > +               cifs_dbg(FYI, "missing master tcon while open");
> > > +               return -EINVAL;
> > > +       }
> > > +
> > > +       xid = get_xid();
> > > +       oparms = (struct cifs_open_parms) {
> > > +               .tcon = tcon,
> > > +               .path = info->path,
> > > +               .desired_access = GENERIC_READ,
> > > +               .disposition = FILE_OPEN,
> > > +               .create_options = cifs_create_options(cifs_sb, 0),
> > > +               .fid = &info->cifs_fid,
> > > +               .cifs_sb = cifs_sb,
> > > +               .reconnect = false,
> > > +       };
> > > +       rc = SMB2_open(xid, &oparms, info->utf16_path, &oplock,
> > > +                       NULL, NULL, NULL, NULL);
> > > +       free_xid(xid);
> > > +       return rc;
> > > +}
> > > +
> > > +static bool is_already_tracking(struct inode *dir_inode)
> > > +{
> > > +       struct notify_info *entry, *nentry;
> > > +
> > > +       spin_lock(&notify_list_lock);
> > > +       list_for_each_entry_safe(entry, nentry, &notify_list, list) {
> > > +               if (is_active(entry)) {
> > > +                       if (entry->inode == dir_inode) {
> > > +                               spin_unlock(&notify_list_lock);
> > > +                               return true;
> > > +                       }
> > > +
> > > +                       /* Extra check since we must keep iterating */
> > > +                       if (!is_fsnotify_masked(entry->inode))
> > > +                               set_state(entry, NOTIFY_STATE_NOMASK);
> > > +               }
> > > +       }
> > > +       spin_unlock(&notify_list_lock);
> > > +
> > > +       return false;
> > > +}
> > > +
> > > +static bool is_tracking_supported(struct cifs_sb_info *cifs_sb)
> > > +{
> > > +       struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
> > > +
> > > +       if (!tcon->ses || !tcon->ses->server)
> > > +               return false;
> > > +       return tcon->ses->server->dialect >= SMB20_PROT_ID;
> > > +}
> > > +
> > > +int start_track_dir_changes(const char *path,
> > > +                           struct inode *dir_inode,
> > > +                           struct cifs_sb_info *cifs_sb)
> > > +{
> > > +       struct notify_info *info;
> > > +       int rc;
> > > +
> > > +       if (!is_tracking_supported(cifs_sb))
> > > +               return -EINVAL;
> > > +
> > > +       if (!is_fsnotify_masked(dir_inode))
> > > +               return -EINVAL;
> > > +
> > > +       if (is_already_tracking(dir_inode))
> > > +               return 1;
> > > +
> > > +       info = kzalloc(sizeof(*info), GFP_KERNEL);
> > > +       if (!info)
> > > +               return -ENOMEM;
> > > +
> > > +       info->path = kstrdup(path, GFP_KERNEL);
> > > +       if (!info->path) {
> > > +               free_notify_info(info);
> > > +               return -ENOMEM;
> > > +       }
> > > +       info->utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
> > > +       if (!info->utf16_path) {
> > > +               free_notify_info(info);
> > > +               return -ENOMEM;
> > > +       }
> > > +       info->inode = dir_inode;
> > > +
> > > +       rc = setup_fid(info);
> > > +       if (rc) {
> > > +               free_notify_info(info);
> > > +               return rc;
> > > +       }
> > > +       rc = request_change_notify(info);
> > > +       if (rc) {
> > > +               close_fid(info);
> > > +               free_notify_info(info);
> > > +               return rc;
> > > +       }
> > > +
> > > +       spin_lock(&notify_list_lock);
> > > +       list_add(&info->list, &notify_list);
> > > +       spin_unlock(&notify_list_lock);
> > > +       return rc;
> > > +}
> > > +
> > > +void stop_track_sb_dir_changes(struct cifs_sb_info *cifs_sb)
> > > +{
> > > +       struct notify_info *entry, *nentry;
> > > +
> > > +       if (!list_empty(&notify_list)) {
> > > +               spin_lock(&notify_list_lock);
> > > +               list_for_each_entry_safe(entry, nentry, &notify_list, list) {
> > > +                       if (cifs_sb == CIFS_SB(entry->inode->i_sb)) {
> > > +                               set_state(entry, NOTIFY_STATE_UMOUNT);
> > > +                               continue;
> > > +                       }
> > > +
> > > +                       /* Extra check since we must keep iterating */
> > > +                       if (!is_fsnotify_masked(entry->inode))
> > > +                               set_state(entry, NOTIFY_STATE_NOMASK);
> > > +               }
> > > +               spin_unlock(&notify_list_lock);
> > > +       }
> > > +}
> > > +
> > > +void resume_track_dir_changes(void)
> > > +{
> > > +       LIST_HEAD(resume_list);
> > > +       struct notify_info *entry, *nentry;
> > > +       struct cifs_tcon *tcon;
> > > +
> > > +       if (list_empty(&notify_list))
> > > +               return;
> > > +
> > > +       spin_lock(&notify_list_lock);
> > > +       list_for_each_entry_safe(entry, nentry, &notify_list, list) {
> > > +               if (!is_fsnotify_masked(entry->inode)) {
> > > +                       set_state(entry, NOTIFY_STATE_NOMASK);
> > > +                       continue;
> > > +               }
> > > +
> > > +               if (is_resumeable(entry)) {
> > > +                       tcon = cifs_sb_master_tcon(CIFS_SB(entry->inode->i_sb));
> > > +                       spin_lock(&tcon->tc_lock);
> > > +                       if (tcon->status == TID_GOOD) {
> > > +                               spin_unlock(&tcon->tc_lock);
> > > +                               list_move(&entry->list, &resume_list);
> > > +                               continue;
> > > +                       }
> > > +                       spin_unlock(&tcon->tc_lock);
> > > +               }
> > > +       }
> > > +       spin_unlock(&notify_list_lock);
> > > +
> > > +       list_for_each_entry_safe(entry, nentry, &resume_list, list) {
> > > +               if (setup_fid(entry)) {
> > > +                       list_del(&entry->list);
> > > +                       free_notify_info(entry);
> > > +                       continue;
> > > +               }
> > > +
> > > +               if (request_change_notify(entry)) {
> > > +                       list_del(&entry->list);
> > > +                       close_fid(entry);
> > > +                       free_notify_info(entry);
> > > +                       continue;
> > > +               }
> > > +
> > > +               clear_state(entry, NOTIFY_STATE_RECONNECT);
> > > +       }
> > > +
> > > +       if (!list_empty(&resume_list)) {
> > > +               spin_lock(&notify_list_lock);
> > > +               list_splice(&resume_list, &notify_list);
> > > +               spin_unlock(&notify_list_lock);
> > > +       }
> > > +}
> > > +
> > > +static void notify_cleanup_worker(struct work_struct *work)
> > > +{
> > > +       LIST_HEAD(cleanup_list);
> > > +       struct notify_info *entry, *nentry;
> > > +
> > > +       if (list_empty(&notify_list))
> > > +               return;
> > > +
> > > +       spin_lock(&notify_list_lock);
> > > +       list_for_each_entry_safe(entry, nentry, &notify_list, list) {
> > > +               if (!is_resumeable(entry) && !is_active(entry))
> > > +                       list_move(&entry->list, &cleanup_list);
> > > +       }
> > > +       spin_unlock(&notify_list_lock);
> > > +
> > > +       list_for_each_entry_safe(entry, nentry, &cleanup_list, list) {
> > > +               list_del(&entry->list);
> > > +               close_fid(entry);
> > > +               free_notify_info(entry);
> > > +       }
> > > +       mod_delayed_work(cifsiod_wq, &notify_cleanup_work, CLEANUP_INTERVAL);
> > > +}
> > > diff --git a/fs/smb/client/notify.h b/fs/smb/client/notify.h
> > > new file mode 100644
> > > index 000000000000..088efba4dce9
> > > --- /dev/null
> > > +++ b/fs/smb/client/notify.h
> > > @@ -0,0 +1,19 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/*
> > > + * Directory change notification tracking for SMB
> > > + *
> > > + * Copyright (c) 2025, Sang-Heon Jeon <ekffu200098@gmail.com>
> > > + */
> > > +
> > > +#ifndef _SMB_NOTIFY_H
> > > +#define _SMB_NOTIFY_H
> > > +
> > > +#include "cifsglob.h"
> > > +
> > > +int start_track_dir_changes(const char *path,
> > > +                           struct inode *dir_inode,
> > > +                           struct cifs_sb_info *cifs_sb);
> > > +void stop_track_sb_dir_changes(struct cifs_sb_info *cifs_sb);
> > > +void resume_track_dir_changes(void);
> > > +
> > > +#endif /* _SMB_NOTIFY_H */
> > > diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
> > > index 4e5460206397..455e5be37116 100644
> > > --- a/fs/smb/client/readdir.c
> > > +++ b/fs/smb/client/readdir.c
> > > @@ -24,6 +24,9 @@
> > >  #include "fs_context.h"
> > >  #include "cached_dir.h"
> > >  #include "reparse.h"
> > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > +#include "notify.h"
> > > +#endif
> > >
> > >  /*
> > >   * To be safe - for UCS to UTF-8 with strings loaded with the rare long
> > > @@ -1070,6 +1073,9 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
> > >         if (rc)
> > >                 goto cache_not_found;
> > >
> > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > +       start_track_dir_changes(full_path, d_inode(file_dentry(file)), cifs_sb);
> > > +#endif
> > >         mutex_lock(&cfid->dirents.de_mutex);
> > >         /*
> > >          * If this was reading from the start of the directory
> > > @@ -1151,6 +1157,9 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
> > >                 cifs_dbg(FYI, "Could not find entry\n");
> > >                 goto rddir2_exit;
> > >         }
> > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > +       start_track_dir_changes(full_path, d_inode(file_dentry(file)), cifs_sb);
> > > +#endif
> > >         cifs_dbg(FYI, "loop through %d times filling dir for net buf %p\n",
> > >                  num_to_fill, cifsFile->srch_inf.ntwrk_buf_start);
> > >         max_len = tcon->ses->server->ops->calc_smb_size(
> > > diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> > > index 4e922cb32110..58a1ddc39ee6 100644
> > > --- a/fs/smb/client/smb2pdu.c
> > > +++ b/fs/smb/client/smb2pdu.c
> > > @@ -45,6 +45,9 @@
> > >  #include "cached_dir.h"
> > >  #include "compress.h"
> > >  #include "fs_context.h"
> > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > +#include "notify.h"
> > > +#endif
> > >
> > >  /*
> > >   *  The following table defines the expected "StructureSize" of SMB2 requests
> > > @@ -466,6 +469,9 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
> > >                 mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
> > >
> > >         atomic_inc(&tconInfoReconnectCount);
> > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > +       resume_track_dir_changes();
> > > +#endif
> > >  out:
> > >         /*
> > >          * Check if handle based operation so we know whether we can continue
> > > --
> > > 2.43.0
> > >
> >
> > To Reviewers.
> >
> > This is a gentle reminder on review requests.
> >
> > I would be very grateful for your feedback and am more than willing to
> > revise or improve any parts as needed. And also, let me know if I
> > missed anything or made mistakes.
>
> Hi Sang,

Hello, Amir

> First feedback (value):
> -----------------------------
> This looks very useful. this feature has been requested and
> attempted several times in the past (see links below), so if you are
> willing to incorporate feedback, I hope you will reach further than those
> past attempts and I will certainly do my best to help you with that.

Thanks for your kind comment. I'm really glad to hear that.

> Second feedback (reviewers):
> ----------------------------------------
> I was very surprised that your patch doesn't touch any vfs code
> (more on that on design feedback), but this is not an SMB-contained
> change at all.

I agree with your last comment. I think it might not be easy;
honestly, I may know less than
Ioannis or Vivek; but I'm fully committed to giving it a try, no
matter the challenge.

> Your patch touches the guts of the fsnotify subsystem (in a wrong way).
> For the next posting please consult the MAINTAINERS entry
> of the fsnotify subsystem for reviewers and list to CC (now added).

I see. I'll keep it in my mind.

> Third feedback (design):
> --------------------------------
> The design choice of polling i_fsnotify_mask on readdir()
> is quite odd and it is not clear to me why it makes sense.
> Previous discussions suggested to have a filesystem method
> to update when applications setup a watch on a directory [1].
> Another prior feedback was that the API should allow a clear
> distinction between the REMOTE notifications and the LOCAL
> notifications [2][3].

Current design choice is a workaround for setting an appropriate add
watch point (as well as remove). I don't want to stick to the RFC
design. Also, The point that I considered important is similar to
Ioannis' one: compatible with existing applications.

> IMO it would be better to finalize the design before working on the
> code, but that's up to you.

I agree, although it's quite hard to create a perfect blueprint, but
it might be better to draw to some extent.

Based on my current understanding, I think we need to do the following things.
- design more compatible and general fsnotify API for all network fs;
should process LOCAL and REMOTE both smoothly.
- expand inotify (if needed, fanotify both) flow with new fsnotify API
- replace SMB2 change_notify start/end point to new API

Let me know if I missed or misunderstood something. And also please
give me some time to read attached threads more deeply and clean up my
thoughts and questions.

> Good luck,
> Amir.
>
> [1] https://lore.kernel.org/linux-fsdevel/20211025204634.2517-4-iangelak@redhat.com/
> [2] https://sourceforge.net/p/fuse/mailman/message/58719889/
> [3] https://lore.kernel.org/linux-fsdevel/YhpY%2FzZe3UA2u3fj@redhat.com/

Best Regards,
Sang-Heon Jeon

Re: [RFC PATCH 2/2] smb: client: add directory change tracking via SMB2 Change Notify

[Amir Goldstein]

On Mon, Sep 29, 2025 at 6:51 PM Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
>
> On Sat, Sep 27, 2025 at 5:39 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > On Sat, Sep 27, 2025 at 3:03 AM Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
> > >
> > > On Tue, Sep 16, 2025 at 10:35 PM Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
> > > >
> > > > Implement directory change tracking using SMB2 Change Notify protocol
> > > > to enable real-time monitoring of remote directories. Applications using
> > > > inotify now receive immediate notifications when files are created,
> > > > deleted, or renamed on SMB2+ shares.
> > > >
> > > > This implementation begins by detecting tracking opportunities during
> > > > regular SMB operations. In the current version, the readdir() serves as
> > > > the detection point - when directory contents are accessed on SMB2+
> > > > servers, we check the inode's fsnotify_mask for active inotify watches.
> > > > If watches exist and directory is not already being tracked, we obtain a
> > > > new FiieId and send a SMB2_CHANGE_NOTIFY request that waits for changes.
> > > >
> > > > Based on the server's response status, we convert CHANGE_NOTIFY response
> > > > to fsnotify events and deliver to application on success, mark for
> > > > future reconnection on network errors, or discard the response and
> > > > marked entries for broken. If tracking can continue, we resend
> > > > SMB2_CHANGE_NOTIFY for continuous monitoring.
> > > >
> > > > Entries marked for reconnection are restored during SMB2 reconnection.
> > > > In the current version, smb2_reconnect() serves as restoration point -
> > > > when connection is reestablished, we obtain new FileIds and request
> > > > Change Notify requests for these entries.
> > > >
> > > > Entries marked for unmount during unmount. In the current version,
> > > > cifs_umount() serves as unmount marking point. Entries marked for clean
> > > > are remove as soon as possible by the worker. Workers also run
> > > > periodically; currently every 30s; to check and remove untracking
> > > > directories.
> > > >
> > > > This feature is controlled by CONFIG_CIFS_DIR_CHANGE_TRACKING with
> > > > experimental.
> > > >
> > > > Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
> > > > ---
> > > >  fs/smb/client/Kconfig   |  17 ++
> > > >  fs/smb/client/Makefile  |   2 +
> > > >  fs/smb/client/connect.c |   6 +
> > > >  fs/smb/client/notify.c  | 535 ++++++++++++++++++++++++++++++++++++++++
> > > >  fs/smb/client/notify.h  |  19 ++
> > > >  fs/smb/client/readdir.c |   9 +
> > > >  fs/smb/client/smb2pdu.c |   6 +
> > > >  7 files changed, 594 insertions(+)
> > > >  create mode 100644 fs/smb/client/notify.c
> > > >  create mode 100644 fs/smb/client/notify.h
> > > >
> > > > diff --git a/fs/smb/client/Kconfig b/fs/smb/client/Kconfig
> > > > index a4c02199fef4..0e3911936e0c 100644
> > > > --- a/fs/smb/client/Kconfig
> > > > +++ b/fs/smb/client/Kconfig
> > > > @@ -218,4 +218,21 @@ config CIFS_COMPRESSION
> > > >           Say Y here if you want SMB traffic to be compressed.
> > > >           If unsure, say N.
> > > >
> > > > ++config CIFS_DIR_CHANGE_TRACKING
> > >
> > > I also found a typo here and I'll fix them.
> > >
> > > > +       bool "Directory change tracking (Experimental)"
> > > > +       depends on CIFS && FSNOTIFY=y
> > > > +       default n
> > > > +       help
> > > > +          Enables automatic tracking of directory changes for SMB2 or later
> > > > +          using the SMB2 Change Notify protocol. File managers and applications
> > > > +          monitoring directories via inotify will receive real-time updates
> > > > +          when files are created, deleted, or renamed on the server.
> > > > +
> > > > +          This feature maintains persistent connections to track changes.
> > > > +          Each monitored directory consumes server resources and may increase
> > > > +          network traffic.
> > > > +
> > > > +          Say Y here if you want real-time directory monitoring.
> > > > +          If unsure, say N.
> > > > +
> > > >  endif
> > > > diff --git a/fs/smb/client/Makefile b/fs/smb/client/Makefile
> > > > index 4c97b31a25c2..85253bc1b4b0 100644
> > > > --- a/fs/smb/client/Makefile
> > > > +++ b/fs/smb/client/Makefile
> > > > @@ -35,3 +35,5 @@ cifs-$(CONFIG_CIFS_ROOT) += cifsroot.o
> > > >  cifs-$(CONFIG_CIFS_ALLOW_INSECURE_LEGACY) += smb1ops.o cifssmb.o cifstransport.o
> > > >
> > > >  cifs-$(CONFIG_CIFS_COMPRESSION) += compress.o compress/lz77.o
> > > > +
> > > > +cifs-$(CONFIG_CIFS_DIR_CHANGE_TRACKING) += notify.o
> > > > diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> > > > index dd12f3eb61dc..eebf729df16a 100644
> > > > --- a/fs/smb/client/connect.c
> > > > +++ b/fs/smb/client/connect.c
> > > > @@ -51,6 +51,9 @@
> > > >  #endif
> > > >  #include "fs_context.h"
> > > >  #include "cifs_swn.h"
> > > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > > +#include "notify.h"
> > > > +#endif
> > > >
> > > >  /* FIXME: should these be tunable? */
> > > >  #define TLINK_ERROR_EXPIRE     (1 * HZ)
> > > > @@ -4154,6 +4157,9 @@ cifs_umount(struct cifs_sb_info *cifs_sb)
> > > >         cancel_delayed_work_sync(&cifs_sb->prune_tlinks);
> > > >
> > > >         if (cifs_sb->master_tlink) {
> > > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > > +               stop_track_sb_dir_changes(cifs_sb);
> > > > +#endif
> > > >                 tcon = cifs_sb->master_tlink->tl_tcon;
> > > >                 if (tcon) {
> > > >                         spin_lock(&tcon->sb_list_lock);
> > > > diff --git a/fs/smb/client/notify.c b/fs/smb/client/notify.c
> > > > new file mode 100644
> > > > index 000000000000..e38345965744
> > > > --- /dev/null
> > > > +++ b/fs/smb/client/notify.c
> > > > @@ -0,0 +1,535 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Directory change notification tracking for SMB
> > > > + *
> > > > + * Copyright (c) 2025, Sang-Heon Jeon <ekffu200098@gmail.com>
> > > > + *
> > > > + * References:
> > > > + * MS-SMB2 "2.2.35 SMB2 CHANGE_NOTIFY Request"
> > > > + * MS-SMB2 "2.2.36 SMB2 CHANGE_NOTIFY Response"
> > > > + * MS-SMB2 "2.7.1 FILE_NOTIFY_INFORMATION"
> > > > + * MS-SMB2 "3.3.5.19 Receiving and SMB2 CHANGE_NOTIFY Request"
> > > > + * MS-FSCC "2.6 File Attributes"
> > > > + */
> > > > +
> > > > +#include <linux/list.h>
> > > > +#include <linux/slab.h>
> > > > +#include <linux/fsnotify.h>
> > > > +#include "notify.h"
> > > > +#include "cifsproto.h"
> > > > +#include "smb2proto.h"
> > > > +#include "cached_dir.h"
> > > > +#include "cifs_debug.h"
> > > > +#include "cifspdu.h"
> > > > +#include "cifs_unicode.h"
> > > > +#include "../common/smb2pdu.h"
> > > > +#include "../common/smb2status.h"
> > > > +
> > > > +#define CLEANUP_INTERVAL (30 * HZ)
> > > > +#define CLEANUP_IMMEDIATE 0
> > > > +
> > > > +enum notify_state {
> > > > +       NOTIFY_STATE_RECONNECT = BIT(0),
> > > > +       NOTIFY_STATE_UMOUNT = BIT(1),
> > > > +       NOTIFY_STATE_NOMASK = BIT(2),
> > > > +       NOTIFY_STATE_BROKEN_REQ = BIT(3),
> > > > +       NOTIFY_STATE_BROKEN_RSP = BIT(4),
> > > > +};
> > > > +
> > > > +struct notify_info {
> > > > +       struct inode *inode;
> > > > +       const char *path;
> > > > +       __le16 *utf16_path;
> > > > +       struct cifs_fid cifs_fid;
> > > > +       atomic_t state;
> > > > +       struct list_head list;
> > > > +};
> > > > +
> > > > +static int request_change_notify(struct notify_info *info);
> > > > +static void notify_cleanup_worker(struct work_struct *work);
> > > > +
> > > > +static LIST_HEAD(notify_list);
> > > > +static DEFINE_SPINLOCK(notify_list_lock);
> > > > +static DECLARE_DELAYED_WORK(notify_cleanup_work, notify_cleanup_worker);
> > > > +
> > > > +static bool is_resumeable(struct notify_info *info)
> > > > +{
> > > > +       return atomic_read(&info->state) == NOTIFY_STATE_RECONNECT;
> > > > +}
> > > > +
> > > > +static bool is_active(struct notify_info *info)
> > > > +{
> > > > +       return atomic_read(&info->state) == 0;
> > > > +}
> > > > +
> > > > +static void set_state(struct notify_info *info, int state)
> > > > +{
> > > > +       atomic_or(state, &info->state);
> > > > +       if (!is_resumeable(info))
> > > > +               mod_delayed_work(cifsiod_wq, &notify_cleanup_work,
> > > > +                       CLEANUP_IMMEDIATE);
> > > > +}
> > > > +
> > > > +static void clear_state(struct notify_info *info, int state)
> > > > +{
> > > > +       atomic_and(~state, &info->state);
> > > > +}
> > > > +
> > > > +static int fsnotify_send(__u32 mask,
> > > > +                        struct inode *parent,
> > > > +                        struct file_notify_information *fni,
> > > > +                        u32 cookie)
> > > > +{
> > > > +       char *name = cifs_strndup_from_utf16(fni->FileName,
> > > > +                               le32_to_cpu(fni->FileNameLength), true,
> > > > +                               CIFS_SB(parent->i_sb)->local_nls);
> > > > +       struct qstr qstr;
> > > > +       int rc = 0;
> > > > +
> > > > +       if (!name)
> > > > +               return -ENOMEM;
> > > > +
> > > > +       qstr.name = (const unsigned char *)name;
> > > > +       qstr.len = strlen(name);
> > > > +
> > > > +       rc = fsnotify_name(mask, NULL, FSNOTIFY_EVENT_NONE, parent,
> > > > +                       &qstr, cookie);
> > > > +       cifs_dbg(FYI, "fsnotify mask=%u, name=%s, cookie=%u, w/return=%d",
> > > > +               mask, name, cookie, rc);
> > > > +       kfree(name);
> > > > +       return rc;
> > > > +}
> > > > +
> > > > +static bool is_fsnotify_masked(struct inode *inode)
> > > > +{
> > > > +       if (!inode)
> > > > +               return false;
> > > > +
> > > > +       /* Minimal validation of file explore inotify */
> > > > +       return inode->i_fsnotify_mask &
> > > > +               (FS_CREATE | FS_DELETE | FS_MOVED_FROM | FS_MOVED_TO);
> > > > +}
> > > > +
> > > > +static void handle_file_notify_information(struct notify_info *info,
> > > > +                                          char *buf,
> > > > +                                          unsigned int buf_len)
> > > > +{
> > > > +       struct file_notify_information *fni;
> > > > +       unsigned int next_entry_offset;
> > > > +       u32 cookie;
> > > > +       bool has_cookie = false;
> > > > +
> > > > +       do {
> > > > +               fni = (struct file_notify_information *)buf;
> > > > +               next_entry_offset = le32_to_cpu(fni->NextEntryOffset);
> > > > +               if (next_entry_offset > buf_len) {
> > > > +                       cifs_dbg(FYI, "invalid fni->NextEntryOffset=%u",
> > > > +                               next_entry_offset);
> > > > +                       break;
> > > > +               }
> > > > +
> > > > +               switch (le32_to_cpu(fni->Action)) {
> > > > +               case FILE_ACTION_ADDED:
> > > > +                       fsnotify_send(FS_CREATE, info->inode, fni, 0);
> > > > +                       break;
> > > > +               case FILE_ACTION_REMOVED:
> > > > +                       fsnotify_send(FS_DELETE, info->inode, fni, 0);
> > > > +                       break;
> > > > +               case FILE_ACTION_RENAMED_OLD_NAME:
> > > > +                       if (!has_cookie)
> > > > +                               cookie = fsnotify_get_cookie();
> > > > +                       has_cookie = !has_cookie;
> > > > +                       fsnotify_send(FS_MOVED_FROM, info->inode, fni, cookie);
> > > > +                       break;
> > > > +               case FILE_ACTION_RENAMED_NEW_NAME:
> > > > +                       if (!has_cookie)
> > > > +                               cookie = fsnotify_get_cookie();
> > > > +                       has_cookie = !has_cookie;
> > > > +                       fsnotify_send(FS_MOVED_TO, info->inode, fni, cookie);
> > > > +                       break;
> > > > +               default:
> > > > +                       /* Does not occur, so no need to handle */
> > > > +                       break;
> > > > +               }
> > > > +
> > > > +               buf += next_entry_offset;
> > > > +               buf_len -= next_entry_offset;
> > > > +       } while (buf_len > 0 && next_entry_offset > 0);
> > > > +}
> > > > +
> > > > +static void handle_smb2_change_notify_rsp(struct notify_info *info,
> > > > +                                         struct mid_q_entry *mid)
> > > > +{
> > > > +       struct smb2_change_notify_rsp *rsp = mid->resp_buf;
> > > > +       struct kvec rsp_iov;
> > > > +       unsigned int buf_offset, buf_len;
> > > > +       int rc;
> > > > +
> > > > +       switch (rsp->hdr.Status) {
> > > > +       case STATUS_SUCCESS:
> > > > +               break;
> > > > +       case STATUS_NOTIFY_ENUM_DIR:
> > > > +               goto proceed;
> > > > +       case STATUS_USER_SESSION_DELETED:
> > > > +       case STATUS_NETWORK_NAME_DELETED:
> > > > +       case STATUS_NETWORK_SESSION_EXPIRED:
> > > > +               set_state(info, NOTIFY_STATE_RECONNECT);
> > > > +               return;
> > > > +       default:
> > > > +               set_state(info, NOTIFY_STATE_BROKEN_RSP);
> > > > +               return;
> > > > +       }
> > > > +
> > > > +       rsp_iov.iov_base = mid->resp_buf;
> > > > +       rsp_iov.iov_len = mid->resp_buf_size;
> > > > +       buf_offset = le16_to_cpu(rsp->OutputBufferOffset);
> > > > +       buf_len = le32_to_cpu(rsp->OutputBufferLength);
> > > > +
> > > > +       rc = smb2_validate_iov(buf_offset, buf_len, &rsp_iov,
> > > > +                               sizeof(struct file_notify_information));
> > > > +       if (rc) {
> > > > +               cifs_dbg(FYI, "stay tracking, w/smb2_validate_iov=%d", rc);
> > > > +               goto proceed;
> > > > +       }
> > > > +
> > > > +       handle_file_notify_information(info,
> > > > +               (char *)rsp + buf_offset, buf_len);
> > > > +proceed:
> > > > +       request_change_notify(info);
> > > > +       return;
> > > > +}
> > > > +
> > > > +static void change_notify_callback(struct mid_q_entry *mid)
> > > > +{
> > > > +       struct notify_info *info = mid->callback_data;
> > > > +
> > > > +       if (!is_active(info))
> > > > +               return;
> > > > +
> > > > +       if (!is_fsnotify_masked(info->inode)) {
> > > > +               set_state(info, NOTIFY_STATE_NOMASK);
> > > > +               return;
> > > > +       }
> > > > +
> > > > +       if (!mid->resp_buf) {
> > > > +               if (mid->mid_state != MID_RETRY_NEEDED) {
> > > > +                       cifs_dbg(FYI, "stop tracking, w/mid_state=%d",
> > > > +                               mid->mid_state);
> > > > +                       set_state(info, NOTIFY_STATE_BROKEN_RSP);
> > > > +                       return;
> > > > +               }
> > > > +
> > > > +               set_state(info, NOTIFY_STATE_RECONNECT);
> > > > +               return;
> > > > +       }
> > > > +
> > > > +       handle_smb2_change_notify_rsp(info, mid);
> > > > +}
> > > > +
> > > > +static int request_change_notify(struct notify_info *info)
> > > > +{
> > > > +       struct cifs_sb_info *cifs_sb = CIFS_SB(info->inode->i_sb);
> > > > +       struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
> > > > +       struct smb_rqst rqst;
> > > > +       struct kvec iov[1];
> > > > +       unsigned int xid;
> > > > +       int rc;
> > > > +
> > > > +       if (!tcon) {
> > > > +               cifs_dbg(FYI, "missing tcon while request change notify");
> > > > +               return -EINVAL;
> > > > +       }
> > > > +
> > > > +       memset(&rqst, 0, sizeof(struct smb_rqst));
> > > > +       memset(&iov, 0, sizeof(iov));
> > > > +       rqst.rq_iov = iov;
> > > > +       rqst.rq_nvec = 1;
> > > > +
> > > > +       xid = get_xid();
> > > > +       rc = SMB2_notify_init(xid, &rqst, tcon, tcon->ses->server,
> > > > +               info->cifs_fid.persistent_fid, info->cifs_fid.volatile_fid,
> > > > +               FILE_NOTIFY_CHANGE_FILE_NAME | FILE_NOTIFY_CHANGE_DIR_NAME,
> > > > +               false);
> > > > +       free_xid(xid);
> > > > +       if (rc) {
> > > > +               set_state(info, NOTIFY_STATE_BROKEN_REQ);
> > > > +               return rc;
> > > > +       }
> > > > +
> > > > +       rc = cifs_call_async(tcon->ses->server, &rqst, NULL,
> > > > +               change_notify_callback, NULL, info, 0, NULL);
> > > > +       cifs_small_buf_release(rqst.rq_iov[0].iov_base);
> > > > +
> > > > +       if (rc)
> > > > +               set_state(info, NOTIFY_STATE_BROKEN_REQ);
> > > > +       return rc;
> > > > +}
> > > > +
> > > > +
> > > > +static void free_notify_info(struct notify_info *info)
> > > > +{
> > > > +       kfree(info->utf16_path);
> > > > +       kfree(info->path);
> > > > +       kfree(info);
> > > > +}
> > > > +
> > > > +static void cleanup_pending_mid(struct notify_info *info,
> > > > +                               struct cifs_tcon *tcon)
> > > > +{
> > > > +       LIST_HEAD(dispose_list);
> > > > +       struct TCP_Server_Info *server;
> > > > +       struct mid_q_entry *mid, *nmid;
> > > > +
> > > > +       if (!tcon->ses || !tcon->ses->server)
> > > > +               return;
> > > > +
> > > > +       server = tcon->ses->server;
> > > > +
> > > > +       spin_lock(&server->mid_queue_lock);
> > > > +       list_for_each_entry_safe(mid, nmid, &server->pending_mid_q, qhead) {
> > > > +               if (mid->callback_data == info) {
> > > > +                       mid->deleted_from_q = true;
> > > > +                       list_move(&mid->qhead, &dispose_list);
> > > > +               }
> > > > +       }
> > > > +       spin_unlock(&server->mid_queue_lock);
> > > > +
> > > > +       list_for_each_entry_safe(mid, nmid, &dispose_list, qhead) {
> > > > +               list_del_init(&mid->qhead);
> > > > +               release_mid(mid);
> > > > +       }
> > > > +}
> > > > +
> > > > +static void close_fid(struct notify_info *info)
> > > > +{
> > > > +       struct cifs_tcon *tcon;
> > > > +
> > > > +       unsigned int xid;
> > > > +       int rc;
> > > > +
> > > > +       if (!info->cifs_fid.persistent_fid && !info->cifs_fid.volatile_fid)
> > > > +               return;
> > > > +
> > > > +       tcon = cifs_sb_master_tcon(CIFS_SB(info->inode->i_sb));
> > > > +       if (!tcon) {
> > > > +               cifs_dbg(FYI, "missing master tcon while close");
> > > > +               return;
> > > > +       }
> > > > +
> > > > +       xid = get_xid();
> > > > +       rc = SMB2_close(xid, tcon, info->cifs_fid.persistent_fid,
> > > > +               info->cifs_fid.volatile_fid);
> > > > +       if (rc) {
> > > > +               cifs_dbg(FYI, "cleanup pending mid, w/SMB2_close=%d", rc);
> > > > +               cleanup_pending_mid(info, tcon);
> > > > +       }
> > > > +       free_xid(xid);
> > > > +}
> > > > +
> > > > +static int setup_fid(struct notify_info *info)
> > > > +{
> > > > +       struct cifs_sb_info *cifs_sb = CIFS_SB(info->inode->i_sb);
> > > > +       struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
> > > > +       struct cifs_open_parms oparms;
> > > > +       __u8 oplock = 0;
> > > > +       unsigned int xid;
> > > > +       int rc = 0;
> > > > +
> > > > +       if (!tcon) {
> > > > +               cifs_dbg(FYI, "missing master tcon while open");
> > > > +               return -EINVAL;
> > > > +       }
> > > > +
> > > > +       xid = get_xid();
> > > > +       oparms = (struct cifs_open_parms) {
> > > > +               .tcon = tcon,
> > > > +               .path = info->path,
> > > > +               .desired_access = GENERIC_READ,
> > > > +               .disposition = FILE_OPEN,
> > > > +               .create_options = cifs_create_options(cifs_sb, 0),
> > > > +               .fid = &info->cifs_fid,
> > > > +               .cifs_sb = cifs_sb,
> > > > +               .reconnect = false,
> > > > +       };
> > > > +       rc = SMB2_open(xid, &oparms, info->utf16_path, &oplock,
> > > > +                       NULL, NULL, NULL, NULL);
> > > > +       free_xid(xid);
> > > > +       return rc;
> > > > +}
> > > > +
> > > > +static bool is_already_tracking(struct inode *dir_inode)
> > > > +{
> > > > +       struct notify_info *entry, *nentry;
> > > > +
> > > > +       spin_lock(&notify_list_lock);
> > > > +       list_for_each_entry_safe(entry, nentry, &notify_list, list) {
> > > > +               if (is_active(entry)) {
> > > > +                       if (entry->inode == dir_inode) {
> > > > +                               spin_unlock(&notify_list_lock);
> > > > +                               return true;
> > > > +                       }
> > > > +
> > > > +                       /* Extra check since we must keep iterating */
> > > > +                       if (!is_fsnotify_masked(entry->inode))
> > > > +                               set_state(entry, NOTIFY_STATE_NOMASK);
> > > > +               }
> > > > +       }
> > > > +       spin_unlock(&notify_list_lock);
> > > > +
> > > > +       return false;
> > > > +}
> > > > +
> > > > +static bool is_tracking_supported(struct cifs_sb_info *cifs_sb)
> > > > +{
> > > > +       struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
> > > > +
> > > > +       if (!tcon->ses || !tcon->ses->server)
> > > > +               return false;
> > > > +       return tcon->ses->server->dialect >= SMB20_PROT_ID;
> > > > +}
> > > > +
> > > > +int start_track_dir_changes(const char *path,
> > > > +                           struct inode *dir_inode,
> > > > +                           struct cifs_sb_info *cifs_sb)
> > > > +{
> > > > +       struct notify_info *info;
> > > > +       int rc;
> > > > +
> > > > +       if (!is_tracking_supported(cifs_sb))
> > > > +               return -EINVAL;
> > > > +
> > > > +       if (!is_fsnotify_masked(dir_inode))
> > > > +               return -EINVAL;
> > > > +
> > > > +       if (is_already_tracking(dir_inode))
> > > > +               return 1;
> > > > +
> > > > +       info = kzalloc(sizeof(*info), GFP_KERNEL);
> > > > +       if (!info)
> > > > +               return -ENOMEM;
> > > > +
> > > > +       info->path = kstrdup(path, GFP_KERNEL);
> > > > +       if (!info->path) {
> > > > +               free_notify_info(info);
> > > > +               return -ENOMEM;
> > > > +       }
> > > > +       info->utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
> > > > +       if (!info->utf16_path) {
> > > > +               free_notify_info(info);
> > > > +               return -ENOMEM;
> > > > +       }
> > > > +       info->inode = dir_inode;
> > > > +
> > > > +       rc = setup_fid(info);
> > > > +       if (rc) {
> > > > +               free_notify_info(info);
> > > > +               return rc;
> > > > +       }
> > > > +       rc = request_change_notify(info);
> > > > +       if (rc) {
> > > > +               close_fid(info);
> > > > +               free_notify_info(info);
> > > > +               return rc;
> > > > +       }
> > > > +
> > > > +       spin_lock(&notify_list_lock);
> > > > +       list_add(&info->list, &notify_list);
> > > > +       spin_unlock(&notify_list_lock);
> > > > +       return rc;
> > > > +}
> > > > +
> > > > +void stop_track_sb_dir_changes(struct cifs_sb_info *cifs_sb)
> > > > +{
> > > > +       struct notify_info *entry, *nentry;
> > > > +
> > > > +       if (!list_empty(&notify_list)) {
> > > > +               spin_lock(&notify_list_lock);
> > > > +               list_for_each_entry_safe(entry, nentry, &notify_list, list) {
> > > > +                       if (cifs_sb == CIFS_SB(entry->inode->i_sb)) {
> > > > +                               set_state(entry, NOTIFY_STATE_UMOUNT);
> > > > +                               continue;
> > > > +                       }
> > > > +
> > > > +                       /* Extra check since we must keep iterating */
> > > > +                       if (!is_fsnotify_masked(entry->inode))
> > > > +                               set_state(entry, NOTIFY_STATE_NOMASK);
> > > > +               }
> > > > +               spin_unlock(&notify_list_lock);
> > > > +       }
> > > > +}
> > > > +
> > > > +void resume_track_dir_changes(void)
> > > > +{
> > > > +       LIST_HEAD(resume_list);
> > > > +       struct notify_info *entry, *nentry;
> > > > +       struct cifs_tcon *tcon;
> > > > +
> > > > +       if (list_empty(&notify_list))
> > > > +               return;
> > > > +
> > > > +       spin_lock(&notify_list_lock);
> > > > +       list_for_each_entry_safe(entry, nentry, &notify_list, list) {
> > > > +               if (!is_fsnotify_masked(entry->inode)) {
> > > > +                       set_state(entry, NOTIFY_STATE_NOMASK);
> > > > +                       continue;
> > > > +               }
> > > > +
> > > > +               if (is_resumeable(entry)) {
> > > > +                       tcon = cifs_sb_master_tcon(CIFS_SB(entry->inode->i_sb));
> > > > +                       spin_lock(&tcon->tc_lock);
> > > > +                       if (tcon->status == TID_GOOD) {
> > > > +                               spin_unlock(&tcon->tc_lock);
> > > > +                               list_move(&entry->list, &resume_list);
> > > > +                               continue;
> > > > +                       }
> > > > +                       spin_unlock(&tcon->tc_lock);
> > > > +               }
> > > > +       }
> > > > +       spin_unlock(&notify_list_lock);
> > > > +
> > > > +       list_for_each_entry_safe(entry, nentry, &resume_list, list) {
> > > > +               if (setup_fid(entry)) {
> > > > +                       list_del(&entry->list);
> > > > +                       free_notify_info(entry);
> > > > +                       continue;
> > > > +               }
> > > > +
> > > > +               if (request_change_notify(entry)) {
> > > > +                       list_del(&entry->list);
> > > > +                       close_fid(entry);
> > > > +                       free_notify_info(entry);
> > > > +                       continue;
> > > > +               }
> > > > +
> > > > +               clear_state(entry, NOTIFY_STATE_RECONNECT);
> > > > +       }
> > > > +
> > > > +       if (!list_empty(&resume_list)) {
> > > > +               spin_lock(&notify_list_lock);
> > > > +               list_splice(&resume_list, &notify_list);
> > > > +               spin_unlock(&notify_list_lock);
> > > > +       }
> > > > +}
> > > > +
> > > > +static void notify_cleanup_worker(struct work_struct *work)
> > > > +{
> > > > +       LIST_HEAD(cleanup_list);
> > > > +       struct notify_info *entry, *nentry;
> > > > +
> > > > +       if (list_empty(&notify_list))
> > > > +               return;
> > > > +
> > > > +       spin_lock(&notify_list_lock);
> > > > +       list_for_each_entry_safe(entry, nentry, &notify_list, list) {
> > > > +               if (!is_resumeable(entry) && !is_active(entry))
> > > > +                       list_move(&entry->list, &cleanup_list);
> > > > +       }
> > > > +       spin_unlock(&notify_list_lock);
> > > > +
> > > > +       list_for_each_entry_safe(entry, nentry, &cleanup_list, list) {
> > > > +               list_del(&entry->list);
> > > > +               close_fid(entry);
> > > > +               free_notify_info(entry);
> > > > +       }
> > > > +       mod_delayed_work(cifsiod_wq, &notify_cleanup_work, CLEANUP_INTERVAL);
> > > > +}
> > > > diff --git a/fs/smb/client/notify.h b/fs/smb/client/notify.h
> > > > new file mode 100644
> > > > index 000000000000..088efba4dce9
> > > > --- /dev/null
> > > > +++ b/fs/smb/client/notify.h
> > > > @@ -0,0 +1,19 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > +/*
> > > > + * Directory change notification tracking for SMB
> > > > + *
> > > > + * Copyright (c) 2025, Sang-Heon Jeon <ekffu200098@gmail.com>
> > > > + */
> > > > +
> > > > +#ifndef _SMB_NOTIFY_H
> > > > +#define _SMB_NOTIFY_H
> > > > +
> > > > +#include "cifsglob.h"
> > > > +
> > > > +int start_track_dir_changes(const char *path,
> > > > +                           struct inode *dir_inode,
> > > > +                           struct cifs_sb_info *cifs_sb);
> > > > +void stop_track_sb_dir_changes(struct cifs_sb_info *cifs_sb);
> > > > +void resume_track_dir_changes(void);
> > > > +
> > > > +#endif /* _SMB_NOTIFY_H */
> > > > diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
> > > > index 4e5460206397..455e5be37116 100644
> > > > --- a/fs/smb/client/readdir.c
> > > > +++ b/fs/smb/client/readdir.c
> > > > @@ -24,6 +24,9 @@
> > > >  #include "fs_context.h"
> > > >  #include "cached_dir.h"
> > > >  #include "reparse.h"
> > > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > > +#include "notify.h"
> > > > +#endif
> > > >
> > > >  /*
> > > >   * To be safe - for UCS to UTF-8 with strings loaded with the rare long
> > > > @@ -1070,6 +1073,9 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
> > > >         if (rc)
> > > >                 goto cache_not_found;
> > > >
> > > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > > +       start_track_dir_changes(full_path, d_inode(file_dentry(file)), cifs_sb);
> > > > +#endif
> > > >         mutex_lock(&cfid->dirents.de_mutex);
> > > >         /*
> > > >          * If this was reading from the start of the directory
> > > > @@ -1151,6 +1157,9 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
> > > >                 cifs_dbg(FYI, "Could not find entry\n");
> > > >                 goto rddir2_exit;
> > > >         }
> > > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > > +       start_track_dir_changes(full_path, d_inode(file_dentry(file)), cifs_sb);
> > > > +#endif
> > > >         cifs_dbg(FYI, "loop through %d times filling dir for net buf %p\n",
> > > >                  num_to_fill, cifsFile->srch_inf.ntwrk_buf_start);
> > > >         max_len = tcon->ses->server->ops->calc_smb_size(
> > > > diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> > > > index 4e922cb32110..58a1ddc39ee6 100644
> > > > --- a/fs/smb/client/smb2pdu.c
> > > > +++ b/fs/smb/client/smb2pdu.c
> > > > @@ -45,6 +45,9 @@
> > > >  #include "cached_dir.h"
> > > >  #include "compress.h"
> > > >  #include "fs_context.h"
> > > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > > +#include "notify.h"
> > > > +#endif
> > > >
> > > >  /*
> > > >   *  The following table defines the expected "StructureSize" of SMB2 requests
> > > > @@ -466,6 +469,9 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
> > > >                 mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
> > > >
> > > >         atomic_inc(&tconInfoReconnectCount);
> > > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > > +       resume_track_dir_changes();
> > > > +#endif
> > > >  out:
> > > >         /*
> > > >          * Check if handle based operation so we know whether we can continue
> > > > --
> > > > 2.43.0
> > > >
> > >
> > > To Reviewers.
> > >
> > > This is a gentle reminder on review requests.
> > >
> > > I would be very grateful for your feedback and am more than willing to
> > > revise or improve any parts as needed. And also, let me know if I
> > > missed anything or made mistakes.
> >
> > Hi Sang,
>
> Hello, Amir
>
> > First feedback (value):
> > -----------------------------
> > This looks very useful. this feature has been requested and
> > attempted several times in the past (see links below), so if you are
> > willing to incorporate feedback, I hope you will reach further than those
> > past attempts and I will certainly do my best to help you with that.
>
> Thanks for your kind comment. I'm really glad to hear that.
>
> > Second feedback (reviewers):
> > ----------------------------------------
> > I was very surprised that your patch doesn't touch any vfs code
> > (more on that on design feedback), but this is not an SMB-contained
> > change at all.
>
> I agree with your last comment. I think it might not be easy;
> honestly, I may know less than
> Ioannis or Vivek; but I'm fully committed to giving it a try, no
> matter the challenge.
>
> > Your patch touches the guts of the fsnotify subsystem (in a wrong way).
> > For the next posting please consult the MAINTAINERS entry
> > of the fsnotify subsystem for reviewers and list to CC (now added).
>
> I see. I'll keep it in my mind.
>
> > Third feedback (design):
> > --------------------------------
> > The design choice of polling i_fsnotify_mask on readdir()
> > is quite odd and it is not clear to me why it makes sense.
> > Previous discussions suggested to have a filesystem method
> > to update when applications setup a watch on a directory [1].
> > Another prior feedback was that the API should allow a clear
> > distinction between the REMOTE notifications and the LOCAL
> > notifications [2][3].
>
> Current design choice is a workaround for setting an appropriate add
> watch point (as well as remove). I don't want to stick to the RFC
> design. Also, The point that I considered important is similar to
> Ioannis' one: compatible with existing applications.
>
> > IMO it would be better to finalize the design before working on the
> > code, but that's up to you.
>
> I agree, although it's quite hard to create a perfect blueprint, but
> it might be better to draw to some extent.
>
> Based on my current understanding, I think we need to do the following things.
> - design more compatible and general fsnotify API for all network fs;
> should process LOCAL and REMOTE both smoothly.
> - expand inotify (if needed, fanotify both) flow with new fsnotify API
> - replace SMB2 change_notify start/end point to new API
>

Yap, that's about it.
All the rest is the details...

> Let me know if I missed or misunderstood something. And also please
> give me some time to read attached threads more deeply and clean up my
> thoughts and questions.
>

Take your time.
It's good to understand the concerns of previous attempts to
avoid hitting the same roadblocks.

Thanks,
Amir.

Re: [RFC PATCH 2/2] smb: client: add directory change tracking via SMB2 Change Notify

[Sang-Heon Jeon]

Hello, Amir

On Tue, Sep 30, 2025 at 2:06 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Mon, Sep 29, 2025 at 6:51 PM Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
> >
> > On Sat, Sep 27, 2025 at 5:39 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > On Sat, Sep 27, 2025 at 3:03 AM Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
> > > >
> > > > On Tue, Sep 16, 2025 at 10:35 PM Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
> > > > >
> > > > > Implement directory change tracking using SMB2 Change Notify protocol
> > > > > to enable real-time monitoring of remote directories. Applications using
> > > > > inotify now receive immediate notifications when files are created,
> > > > > deleted, or renamed on SMB2+ shares.
> > > > >
> > > > > This implementation begins by detecting tracking opportunities during
> > > > > regular SMB operations. In the current version, the readdir() serves as
> > > > > the detection point - when directory contents are accessed on SMB2+
> > > > > servers, we check the inode's fsnotify_mask for active inotify watches.
> > > > > If watches exist and directory is not already being tracked, we obtain a
> > > > > new FiieId and send a SMB2_CHANGE_NOTIFY request that waits for changes.
> > > > >
> > > > > Based on the server's response status, we convert CHANGE_NOTIFY response
> > > > > to fsnotify events and deliver to application on success, mark for
> > > > > future reconnection on network errors, or discard the response and
> > > > > marked entries for broken. If tracking can continue, we resend
> > > > > SMB2_CHANGE_NOTIFY for continuous monitoring.
> > > > >
> > > > > Entries marked for reconnection are restored during SMB2 reconnection.
> > > > > In the current version, smb2_reconnect() serves as restoration point -
> > > > > when connection is reestablished, we obtain new FileIds and request
> > > > > Change Notify requests for these entries.
> > > > >
> > > > > Entries marked for unmount during unmount. In the current version,
> > > > > cifs_umount() serves as unmount marking point. Entries marked for clean
> > > > > are remove as soon as possible by the worker. Workers also run
> > > > > periodically; currently every 30s; to check and remove untracking
> > > > > directories.
> > > > >
> > > > > This feature is controlled by CONFIG_CIFS_DIR_CHANGE_TRACKING with
> > > > > experimental.
> > > > >
> > > > > Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
> > > > > ---
> > > > >  fs/smb/client/Kconfig   |  17 ++
> > > > >  fs/smb/client/Makefile  |   2 +
> > > > >  fs/smb/client/connect.c |   6 +
> > > > >  fs/smb/client/notify.c  | 535 ++++++++++++++++++++++++++++++++++++++++
> > > > >  fs/smb/client/notify.h  |  19 ++
> > > > >  fs/smb/client/readdir.c |   9 +
> > > > >  fs/smb/client/smb2pdu.c |   6 +
> > > > >  7 files changed, 594 insertions(+)
> > > > >  create mode 100644 fs/smb/client/notify.c
> > > > >  create mode 100644 fs/smb/client/notify.h
> > > > >
> > > > > diff --git a/fs/smb/client/Kconfig b/fs/smb/client/Kconfig
> > > > > index a4c02199fef4..0e3911936e0c 100644
> > > > > --- a/fs/smb/client/Kconfig
> > > > > +++ b/fs/smb/client/Kconfig
> > > > > @@ -218,4 +218,21 @@ config CIFS_COMPRESSION
> > > > >           Say Y here if you want SMB traffic to be compressed.
> > > > >           If unsure, say N.
> > > > >
> > > > > ++config CIFS_DIR_CHANGE_TRACKING
> > > >
> > > > I also found a typo here and I'll fix them.
> > > >
> > > > > +       bool "Directory change tracking (Experimental)"
> > > > > +       depends on CIFS && FSNOTIFY=y
> > > > > +       default n
> > > > > +       help
> > > > > +          Enables automatic tracking of directory changes for SMB2 or later
> > > > > +          using the SMB2 Change Notify protocol. File managers and applications
> > > > > +          monitoring directories via inotify will receive real-time updates
> > > > > +          when files are created, deleted, or renamed on the server.
> > > > > +
> > > > > +          This feature maintains persistent connections to track changes.
> > > > > +          Each monitored directory consumes server resources and may increase
> > > > > +          network traffic.
> > > > > +
> > > > > +          Say Y here if you want real-time directory monitoring.
> > > > > +          If unsure, say N.
> > > > > +
> > > > >  endif
> > > > > diff --git a/fs/smb/client/Makefile b/fs/smb/client/Makefile
> > > > > index 4c97b31a25c2..85253bc1b4b0 100644
> > > > > --- a/fs/smb/client/Makefile
> > > > > +++ b/fs/smb/client/Makefile
> > > > > @@ -35,3 +35,5 @@ cifs-$(CONFIG_CIFS_ROOT) += cifsroot.o
> > > > >  cifs-$(CONFIG_CIFS_ALLOW_INSECURE_LEGACY) += smb1ops.o cifssmb.o cifstransport.o
> > > > >
> > > > >  cifs-$(CONFIG_CIFS_COMPRESSION) += compress.o compress/lz77.o
> > > > > +
> > > > > +cifs-$(CONFIG_CIFS_DIR_CHANGE_TRACKING) += notify.o
> > > > > diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> > > > > index dd12f3eb61dc..eebf729df16a 100644
> > > > > --- a/fs/smb/client/connect.c
> > > > > +++ b/fs/smb/client/connect.c
> > > > > @@ -51,6 +51,9 @@
> > > > >  #endif
> > > > >  #include "fs_context.h"
> > > > >  #include "cifs_swn.h"
> > > > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > > > +#include "notify.h"
> > > > > +#endif
> > > > >
> > > > >  /* FIXME: should these be tunable? */
> > > > >  #define TLINK_ERROR_EXPIRE     (1 * HZ)
> > > > > @@ -4154,6 +4157,9 @@ cifs_umount(struct cifs_sb_info *cifs_sb)
> > > > >         cancel_delayed_work_sync(&cifs_sb->prune_tlinks);
> > > > >
> > > > >         if (cifs_sb->master_tlink) {
> > > > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > > > +               stop_track_sb_dir_changes(cifs_sb);
> > > > > +#endif
> > > > >                 tcon = cifs_sb->master_tlink->tl_tcon;
> > > > >                 if (tcon) {
> > > > >                         spin_lock(&tcon->sb_list_lock);
> > > > > diff --git a/fs/smb/client/notify.c b/fs/smb/client/notify.c
> > > > > new file mode 100644
> > > > > index 000000000000..e38345965744
> > > > > --- /dev/null
> > > > > +++ b/fs/smb/client/notify.c
> > > > > @@ -0,0 +1,535 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * Directory change notification tracking for SMB
> > > > > + *
> > > > > + * Copyright (c) 2025, Sang-Heon Jeon <ekffu200098@gmail.com>
> > > > > + *
> > > > > + * References:
> > > > > + * MS-SMB2 "2.2.35 SMB2 CHANGE_NOTIFY Request"
> > > > > + * MS-SMB2 "2.2.36 SMB2 CHANGE_NOTIFY Response"
> > > > > + * MS-SMB2 "2.7.1 FILE_NOTIFY_INFORMATION"
> > > > > + * MS-SMB2 "3.3.5.19 Receiving and SMB2 CHANGE_NOTIFY Request"
> > > > > + * MS-FSCC "2.6 File Attributes"
> > > > > + */
> > > > > +
> > > > > +#include <linux/list.h>
> > > > > +#include <linux/slab.h>
> > > > > +#include <linux/fsnotify.h>
> > > > > +#include "notify.h"
> > > > > +#include "cifsproto.h"
> > > > > +#include "smb2proto.h"
> > > > > +#include "cached_dir.h"
> > > > > +#include "cifs_debug.h"
> > > > > +#include "cifspdu.h"
> > > > > +#include "cifs_unicode.h"
> > > > > +#include "../common/smb2pdu.h"
> > > > > +#include "../common/smb2status.h"
> > > > > +
> > > > > +#define CLEANUP_INTERVAL (30 * HZ)
> > > > > +#define CLEANUP_IMMEDIATE 0
> > > > > +
> > > > > +enum notify_state {
> > > > > +       NOTIFY_STATE_RECONNECT = BIT(0),
> > > > > +       NOTIFY_STATE_UMOUNT = BIT(1),
> > > > > +       NOTIFY_STATE_NOMASK = BIT(2),
> > > > > +       NOTIFY_STATE_BROKEN_REQ = BIT(3),
> > > > > +       NOTIFY_STATE_BROKEN_RSP = BIT(4),
> > > > > +};
> > > > > +
> > > > > +struct notify_info {
> > > > > +       struct inode *inode;
> > > > > +       const char *path;
> > > > > +       __le16 *utf16_path;
> > > > > +       struct cifs_fid cifs_fid;
> > > > > +       atomic_t state;
> > > > > +       struct list_head list;
> > > > > +};
> > > > > +
> > > > > +static int request_change_notify(struct notify_info *info);
> > > > > +static void notify_cleanup_worker(struct work_struct *work);
> > > > > +
> > > > > +static LIST_HEAD(notify_list);
> > > > > +static DEFINE_SPINLOCK(notify_list_lock);
> > > > > +static DECLARE_DELAYED_WORK(notify_cleanup_work, notify_cleanup_worker);
> > > > > +
> > > > > +static bool is_resumeable(struct notify_info *info)
> > > > > +{
> > > > > +       return atomic_read(&info->state) == NOTIFY_STATE_RECONNECT;
> > > > > +}
> > > > > +
> > > > > +static bool is_active(struct notify_info *info)
> > > > > +{
> > > > > +       return atomic_read(&info->state) == 0;
> > > > > +}
> > > > > +
> > > > > +static void set_state(struct notify_info *info, int state)
> > > > > +{
> > > > > +       atomic_or(state, &info->state);
> > > > > +       if (!is_resumeable(info))
> > > > > +               mod_delayed_work(cifsiod_wq, &notify_cleanup_work,
> > > > > +                       CLEANUP_IMMEDIATE);
> > > > > +}
> > > > > +
> > > > > +static void clear_state(struct notify_info *info, int state)
> > > > > +{
> > > > > +       atomic_and(~state, &info->state);
> > > > > +}
> > > > > +
> > > > > +static int fsnotify_send(__u32 mask,
> > > > > +                        struct inode *parent,
> > > > > +                        struct file_notify_information *fni,
> > > > > +                        u32 cookie)
> > > > > +{
> > > > > +       char *name = cifs_strndup_from_utf16(fni->FileName,
> > > > > +                               le32_to_cpu(fni->FileNameLength), true,
> > > > > +                               CIFS_SB(parent->i_sb)->local_nls);
> > > > > +       struct qstr qstr;
> > > > > +       int rc = 0;
> > > > > +
> > > > > +       if (!name)
> > > > > +               return -ENOMEM;
> > > > > +
> > > > > +       qstr.name = (const unsigned char *)name;
> > > > > +       qstr.len = strlen(name);
> > > > > +
> > > > > +       rc = fsnotify_name(mask, NULL, FSNOTIFY_EVENT_NONE, parent,
> > > > > +                       &qstr, cookie);
> > > > > +       cifs_dbg(FYI, "fsnotify mask=%u, name=%s, cookie=%u, w/return=%d",
> > > > > +               mask, name, cookie, rc);
> > > > > +       kfree(name);
> > > > > +       return rc;
> > > > > +}
> > > > > +
> > > > > +static bool is_fsnotify_masked(struct inode *inode)
> > > > > +{
> > > > > +       if (!inode)
> > > > > +               return false;
> > > > > +
> > > > > +       /* Minimal validation of file explore inotify */
> > > > > +       return inode->i_fsnotify_mask &
> > > > > +               (FS_CREATE | FS_DELETE | FS_MOVED_FROM | FS_MOVED_TO);
> > > > > +}
> > > > > +
> > > > > +static void handle_file_notify_information(struct notify_info *info,
> > > > > +                                          char *buf,
> > > > > +                                          unsigned int buf_len)
> > > > > +{
> > > > > +       struct file_notify_information *fni;
> > > > > +       unsigned int next_entry_offset;
> > > > > +       u32 cookie;
> > > > > +       bool has_cookie = false;
> > > > > +
> > > > > +       do {
> > > > > +               fni = (struct file_notify_information *)buf;
> > > > > +               next_entry_offset = le32_to_cpu(fni->NextEntryOffset);
> > > > > +               if (next_entry_offset > buf_len) {
> > > > > +                       cifs_dbg(FYI, "invalid fni->NextEntryOffset=%u",
> > > > > +                               next_entry_offset);
> > > > > +                       break;
> > > > > +               }
> > > > > +
> > > > > +               switch (le32_to_cpu(fni->Action)) {
> > > > > +               case FILE_ACTION_ADDED:
> > > > > +                       fsnotify_send(FS_CREATE, info->inode, fni, 0);
> > > > > +                       break;
> > > > > +               case FILE_ACTION_REMOVED:
> > > > > +                       fsnotify_send(FS_DELETE, info->inode, fni, 0);
> > > > > +                       break;
> > > > > +               case FILE_ACTION_RENAMED_OLD_NAME:
> > > > > +                       if (!has_cookie)
> > > > > +                               cookie = fsnotify_get_cookie();
> > > > > +                       has_cookie = !has_cookie;
> > > > > +                       fsnotify_send(FS_MOVED_FROM, info->inode, fni, cookie);
> > > > > +                       break;
> > > > > +               case FILE_ACTION_RENAMED_NEW_NAME:
> > > > > +                       if (!has_cookie)
> > > > > +                               cookie = fsnotify_get_cookie();
> > > > > +                       has_cookie = !has_cookie;
> > > > > +                       fsnotify_send(FS_MOVED_TO, info->inode, fni, cookie);
> > > > > +                       break;
> > > > > +               default:
> > > > > +                       /* Does not occur, so no need to handle */
> > > > > +                       break;
> > > > > +               }
> > > > > +
> > > > > +               buf += next_entry_offset;
> > > > > +               buf_len -= next_entry_offset;
> > > > > +       } while (buf_len > 0 && next_entry_offset > 0);
> > > > > +}
> > > > > +
> > > > > +static void handle_smb2_change_notify_rsp(struct notify_info *info,
> > > > > +                                         struct mid_q_entry *mid)
> > > > > +{
> > > > > +       struct smb2_change_notify_rsp *rsp = mid->resp_buf;
> > > > > +       struct kvec rsp_iov;
> > > > > +       unsigned int buf_offset, buf_len;
> > > > > +       int rc;
> > > > > +
> > > > > +       switch (rsp->hdr.Status) {
> > > > > +       case STATUS_SUCCESS:
> > > > > +               break;
> > > > > +       case STATUS_NOTIFY_ENUM_DIR:
> > > > > +               goto proceed;
> > > > > +       case STATUS_USER_SESSION_DELETED:
> > > > > +       case STATUS_NETWORK_NAME_DELETED:
> > > > > +       case STATUS_NETWORK_SESSION_EXPIRED:
> > > > > +               set_state(info, NOTIFY_STATE_RECONNECT);
> > > > > +               return;
> > > > > +       default:
> > > > > +               set_state(info, NOTIFY_STATE_BROKEN_RSP);
> > > > > +               return;
> > > > > +       }
> > > > > +
> > > > > +       rsp_iov.iov_base = mid->resp_buf;
> > > > > +       rsp_iov.iov_len = mid->resp_buf_size;
> > > > > +       buf_offset = le16_to_cpu(rsp->OutputBufferOffset);
> > > > > +       buf_len = le32_to_cpu(rsp->OutputBufferLength);
> > > > > +
> > > > > +       rc = smb2_validate_iov(buf_offset, buf_len, &rsp_iov,
> > > > > +                               sizeof(struct file_notify_information));
> > > > > +       if (rc) {
> > > > > +               cifs_dbg(FYI, "stay tracking, w/smb2_validate_iov=%d", rc);
> > > > > +               goto proceed;
> > > > > +       }
> > > > > +
> > > > > +       handle_file_notify_information(info,
> > > > > +               (char *)rsp + buf_offset, buf_len);
> > > > > +proceed:
> > > > > +       request_change_notify(info);
> > > > > +       return;
> > > > > +}
> > > > > +
> > > > > +static void change_notify_callback(struct mid_q_entry *mid)
> > > > > +{
> > > > > +       struct notify_info *info = mid->callback_data;
> > > > > +
> > > > > +       if (!is_active(info))
> > > > > +               return;
> > > > > +
> > > > > +       if (!is_fsnotify_masked(info->inode)) {
> > > > > +               set_state(info, NOTIFY_STATE_NOMASK);
> > > > > +               return;
> > > > > +       }
> > > > > +
> > > > > +       if (!mid->resp_buf) {
> > > > > +               if (mid->mid_state != MID_RETRY_NEEDED) {
> > > > > +                       cifs_dbg(FYI, "stop tracking, w/mid_state=%d",
> > > > > +                               mid->mid_state);
> > > > > +                       set_state(info, NOTIFY_STATE_BROKEN_RSP);
> > > > > +                       return;
> > > > > +               }
> > > > > +
> > > > > +               set_state(info, NOTIFY_STATE_RECONNECT);
> > > > > +               return;
> > > > > +       }
> > > > > +
> > > > > +       handle_smb2_change_notify_rsp(info, mid);
> > > > > +}
> > > > > +
> > > > > +static int request_change_notify(struct notify_info *info)
> > > > > +{
> > > > > +       struct cifs_sb_info *cifs_sb = CIFS_SB(info->inode->i_sb);
> > > > > +       struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
> > > > > +       struct smb_rqst rqst;
> > > > > +       struct kvec iov[1];
> > > > > +       unsigned int xid;
> > > > > +       int rc;
> > > > > +
> > > > > +       if (!tcon) {
> > > > > +               cifs_dbg(FYI, "missing tcon while request change notify");
> > > > > +               return -EINVAL;
> > > > > +       }
> > > > > +
> > > > > +       memset(&rqst, 0, sizeof(struct smb_rqst));
> > > > > +       memset(&iov, 0, sizeof(iov));
> > > > > +       rqst.rq_iov = iov;
> > > > > +       rqst.rq_nvec = 1;
> > > > > +
> > > > > +       xid = get_xid();
> > > > > +       rc = SMB2_notify_init(xid, &rqst, tcon, tcon->ses->server,
> > > > > +               info->cifs_fid.persistent_fid, info->cifs_fid.volatile_fid,
> > > > > +               FILE_NOTIFY_CHANGE_FILE_NAME | FILE_NOTIFY_CHANGE_DIR_NAME,
> > > > > +               false);
> > > > > +       free_xid(xid);
> > > > > +       if (rc) {
> > > > > +               set_state(info, NOTIFY_STATE_BROKEN_REQ);
> > > > > +               return rc;
> > > > > +       }
> > > > > +
> > > > > +       rc = cifs_call_async(tcon->ses->server, &rqst, NULL,
> > > > > +               change_notify_callback, NULL, info, 0, NULL);
> > > > > +       cifs_small_buf_release(rqst.rq_iov[0].iov_base);
> > > > > +
> > > > > +       if (rc)
> > > > > +               set_state(info, NOTIFY_STATE_BROKEN_REQ);
> > > > > +       return rc;
> > > > > +}
> > > > > +
> > > > > +
> > > > > +static void free_notify_info(struct notify_info *info)
> > > > > +{
> > > > > +       kfree(info->utf16_path);
> > > > > +       kfree(info->path);
> > > > > +       kfree(info);
> > > > > +}
> > > > > +
> > > > > +static void cleanup_pending_mid(struct notify_info *info,
> > > > > +                               struct cifs_tcon *tcon)
> > > > > +{
> > > > > +       LIST_HEAD(dispose_list);
> > > > > +       struct TCP_Server_Info *server;
> > > > > +       struct mid_q_entry *mid, *nmid;
> > > > > +
> > > > > +       if (!tcon->ses || !tcon->ses->server)
> > > > > +               return;
> > > > > +
> > > > > +       server = tcon->ses->server;
> > > > > +
> > > > > +       spin_lock(&server->mid_queue_lock);
> > > > > +       list_for_each_entry_safe(mid, nmid, &server->pending_mid_q, qhead) {
> > > > > +               if (mid->callback_data == info) {
> > > > > +                       mid->deleted_from_q = true;
> > > > > +                       list_move(&mid->qhead, &dispose_list);
> > > > > +               }
> > > > > +       }
> > > > > +       spin_unlock(&server->mid_queue_lock);
> > > > > +
> > > > > +       list_for_each_entry_safe(mid, nmid, &dispose_list, qhead) {
> > > > > +               list_del_init(&mid->qhead);
> > > > > +               release_mid(mid);
> > > > > +       }
> > > > > +}
> > > > > +
> > > > > +static void close_fid(struct notify_info *info)
> > > > > +{
> > > > > +       struct cifs_tcon *tcon;
> > > > > +
> > > > > +       unsigned int xid;
> > > > > +       int rc;
> > > > > +
> > > > > +       if (!info->cifs_fid.persistent_fid && !info->cifs_fid.volatile_fid)
> > > > > +               return;
> > > > > +
> > > > > +       tcon = cifs_sb_master_tcon(CIFS_SB(info->inode->i_sb));
> > > > > +       if (!tcon) {
> > > > > +               cifs_dbg(FYI, "missing master tcon while close");
> > > > > +               return;
> > > > > +       }
> > > > > +
> > > > > +       xid = get_xid();
> > > > > +       rc = SMB2_close(xid, tcon, info->cifs_fid.persistent_fid,
> > > > > +               info->cifs_fid.volatile_fid);
> > > > > +       if (rc) {
> > > > > +               cifs_dbg(FYI, "cleanup pending mid, w/SMB2_close=%d", rc);
> > > > > +               cleanup_pending_mid(info, tcon);
> > > > > +       }
> > > > > +       free_xid(xid);
> > > > > +}
> > > > > +
> > > > > +static int setup_fid(struct notify_info *info)
> > > > > +{
> > > > > +       struct cifs_sb_info *cifs_sb = CIFS_SB(info->inode->i_sb);
> > > > > +       struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
> > > > > +       struct cifs_open_parms oparms;
> > > > > +       __u8 oplock = 0;
> > > > > +       unsigned int xid;
> > > > > +       int rc = 0;
> > > > > +
> > > > > +       if (!tcon) {
> > > > > +               cifs_dbg(FYI, "missing master tcon while open");
> > > > > +               return -EINVAL;
> > > > > +       }
> > > > > +
> > > > > +       xid = get_xid();
> > > > > +       oparms = (struct cifs_open_parms) {
> > > > > +               .tcon = tcon,
> > > > > +               .path = info->path,
> > > > > +               .desired_access = GENERIC_READ,
> > > > > +               .disposition = FILE_OPEN,
> > > > > +               .create_options = cifs_create_options(cifs_sb, 0),
> > > > > +               .fid = &info->cifs_fid,
> > > > > +               .cifs_sb = cifs_sb,
> > > > > +               .reconnect = false,
> > > > > +       };
> > > > > +       rc = SMB2_open(xid, &oparms, info->utf16_path, &oplock,
> > > > > +                       NULL, NULL, NULL, NULL);
> > > > > +       free_xid(xid);
> > > > > +       return rc;
> > > > > +}
> > > > > +
> > > > > +static bool is_already_tracking(struct inode *dir_inode)
> > > > > +{
> > > > > +       struct notify_info *entry, *nentry;
> > > > > +
> > > > > +       spin_lock(&notify_list_lock);
> > > > > +       list_for_each_entry_safe(entry, nentry, &notify_list, list) {
> > > > > +               if (is_active(entry)) {
> > > > > +                       if (entry->inode == dir_inode) {
> > > > > +                               spin_unlock(&notify_list_lock);
> > > > > +                               return true;
> > > > > +                       }
> > > > > +
> > > > > +                       /* Extra check since we must keep iterating */
> > > > > +                       if (!is_fsnotify_masked(entry->inode))
> > > > > +                               set_state(entry, NOTIFY_STATE_NOMASK);
> > > > > +               }
> > > > > +       }
> > > > > +       spin_unlock(&notify_list_lock);
> > > > > +
> > > > > +       return false;
> > > > > +}
> > > > > +
> > > > > +static bool is_tracking_supported(struct cifs_sb_info *cifs_sb)
> > > > > +{
> > > > > +       struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
> > > > > +
> > > > > +       if (!tcon->ses || !tcon->ses->server)
> > > > > +               return false;
> > > > > +       return tcon->ses->server->dialect >= SMB20_PROT_ID;
> > > > > +}
> > > > > +
> > > > > +int start_track_dir_changes(const char *path,
> > > > > +                           struct inode *dir_inode,
> > > > > +                           struct cifs_sb_info *cifs_sb)
> > > > > +{
> > > > > +       struct notify_info *info;
> > > > > +       int rc;
> > > > > +
> > > > > +       if (!is_tracking_supported(cifs_sb))
> > > > > +               return -EINVAL;
> > > > > +
> > > > > +       if (!is_fsnotify_masked(dir_inode))
> > > > > +               return -EINVAL;
> > > > > +
> > > > > +       if (is_already_tracking(dir_inode))
> > > > > +               return 1;
> > > > > +
> > > > > +       info = kzalloc(sizeof(*info), GFP_KERNEL);
> > > > > +       if (!info)
> > > > > +               return -ENOMEM;
> > > > > +
> > > > > +       info->path = kstrdup(path, GFP_KERNEL);
> > > > > +       if (!info->path) {
> > > > > +               free_notify_info(info);
> > > > > +               return -ENOMEM;
> > > > > +       }
> > > > > +       info->utf16_path = cifs_convert_path_to_utf16(path, cifs_sb);
> > > > > +       if (!info->utf16_path) {
> > > > > +               free_notify_info(info);
> > > > > +               return -ENOMEM;
> > > > > +       }
> > > > > +       info->inode = dir_inode;
> > > > > +
> > > > > +       rc = setup_fid(info);
> > > > > +       if (rc) {
> > > > > +               free_notify_info(info);
> > > > > +               return rc;
> > > > > +       }
> > > > > +       rc = request_change_notify(info);
> > > > > +       if (rc) {
> > > > > +               close_fid(info);
> > > > > +               free_notify_info(info);
> > > > > +               return rc;
> > > > > +       }
> > > > > +
> > > > > +       spin_lock(&notify_list_lock);
> > > > > +       list_add(&info->list, &notify_list);
> > > > > +       spin_unlock(&notify_list_lock);
> > > > > +       return rc;
> > > > > +}
> > > > > +
> > > > > +void stop_track_sb_dir_changes(struct cifs_sb_info *cifs_sb)
> > > > > +{
> > > > > +       struct notify_info *entry, *nentry;
> > > > > +
> > > > > +       if (!list_empty(&notify_list)) {
> > > > > +               spin_lock(&notify_list_lock);
> > > > > +               list_for_each_entry_safe(entry, nentry, &notify_list, list) {
> > > > > +                       if (cifs_sb == CIFS_SB(entry->inode->i_sb)) {
> > > > > +                               set_state(entry, NOTIFY_STATE_UMOUNT);
> > > > > +                               continue;
> > > > > +                       }
> > > > > +
> > > > > +                       /* Extra check since we must keep iterating */
> > > > > +                       if (!is_fsnotify_masked(entry->inode))
> > > > > +                               set_state(entry, NOTIFY_STATE_NOMASK);
> > > > > +               }
> > > > > +               spin_unlock(&notify_list_lock);
> > > > > +       }
> > > > > +}
> > > > > +
> > > > > +void resume_track_dir_changes(void)
> > > > > +{
> > > > > +       LIST_HEAD(resume_list);
> > > > > +       struct notify_info *entry, *nentry;
> > > > > +       struct cifs_tcon *tcon;
> > > > > +
> > > > > +       if (list_empty(&notify_list))
> > > > > +               return;
> > > > > +
> > > > > +       spin_lock(&notify_list_lock);
> > > > > +       list_for_each_entry_safe(entry, nentry, &notify_list, list) {
> > > > > +               if (!is_fsnotify_masked(entry->inode)) {
> > > > > +                       set_state(entry, NOTIFY_STATE_NOMASK);
> > > > > +                       continue;
> > > > > +               }
> > > > > +
> > > > > +               if (is_resumeable(entry)) {
> > > > > +                       tcon = cifs_sb_master_tcon(CIFS_SB(entry->inode->i_sb));
> > > > > +                       spin_lock(&tcon->tc_lock);
> > > > > +                       if (tcon->status == TID_GOOD) {
> > > > > +                               spin_unlock(&tcon->tc_lock);
> > > > > +                               list_move(&entry->list, &resume_list);
> > > > > +                               continue;
> > > > > +                       }
> > > > > +                       spin_unlock(&tcon->tc_lock);
> > > > > +               }
> > > > > +       }
> > > > > +       spin_unlock(&notify_list_lock);
> > > > > +
> > > > > +       list_for_each_entry_safe(entry, nentry, &resume_list, list) {
> > > > > +               if (setup_fid(entry)) {
> > > > > +                       list_del(&entry->list);
> > > > > +                       free_notify_info(entry);
> > > > > +                       continue;
> > > > > +               }
> > > > > +
> > > > > +               if (request_change_notify(entry)) {
> > > > > +                       list_del(&entry->list);
> > > > > +                       close_fid(entry);
> > > > > +                       free_notify_info(entry);
> > > > > +                       continue;
> > > > > +               }
> > > > > +
> > > > > +               clear_state(entry, NOTIFY_STATE_RECONNECT);
> > > > > +       }
> > > > > +
> > > > > +       if (!list_empty(&resume_list)) {
> > > > > +               spin_lock(&notify_list_lock);
> > > > > +               list_splice(&resume_list, &notify_list);
> > > > > +               spin_unlock(&notify_list_lock);
> > > > > +       }
> > > > > +}
> > > > > +
> > > > > +static void notify_cleanup_worker(struct work_struct *work)
> > > > > +{
> > > > > +       LIST_HEAD(cleanup_list);
> > > > > +       struct notify_info *entry, *nentry;
> > > > > +
> > > > > +       if (list_empty(&notify_list))
> > > > > +               return;
> > > > > +
> > > > > +       spin_lock(&notify_list_lock);
> > > > > +       list_for_each_entry_safe(entry, nentry, &notify_list, list) {
> > > > > +               if (!is_resumeable(entry) && !is_active(entry))
> > > > > +                       list_move(&entry->list, &cleanup_list);
> > > > > +       }
> > > > > +       spin_unlock(&notify_list_lock);
> > > > > +
> > > > > +       list_for_each_entry_safe(entry, nentry, &cleanup_list, list) {
> > > > > +               list_del(&entry->list);
> > > > > +               close_fid(entry);
> > > > > +               free_notify_info(entry);
> > > > > +       }
> > > > > +       mod_delayed_work(cifsiod_wq, &notify_cleanup_work, CLEANUP_INTERVAL);
> > > > > +}
> > > > > diff --git a/fs/smb/client/notify.h b/fs/smb/client/notify.h
> > > > > new file mode 100644
> > > > > index 000000000000..088efba4dce9
> > > > > --- /dev/null
> > > > > +++ b/fs/smb/client/notify.h
> > > > > @@ -0,0 +1,19 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > > +/*
> > > > > + * Directory change notification tracking for SMB
> > > > > + *
> > > > > + * Copyright (c) 2025, Sang-Heon Jeon <ekffu200098@gmail.com>
> > > > > + */
> > > > > +
> > > > > +#ifndef _SMB_NOTIFY_H
> > > > > +#define _SMB_NOTIFY_H
> > > > > +
> > > > > +#include "cifsglob.h"
> > > > > +
> > > > > +int start_track_dir_changes(const char *path,
> > > > > +                           struct inode *dir_inode,
> > > > > +                           struct cifs_sb_info *cifs_sb);
> > > > > +void stop_track_sb_dir_changes(struct cifs_sb_info *cifs_sb);
> > > > > +void resume_track_dir_changes(void);
> > > > > +
> > > > > +#endif /* _SMB_NOTIFY_H */
> > > > > diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
> > > > > index 4e5460206397..455e5be37116 100644
> > > > > --- a/fs/smb/client/readdir.c
> > > > > +++ b/fs/smb/client/readdir.c
> > > > > @@ -24,6 +24,9 @@
> > > > >  #include "fs_context.h"
> > > > >  #include "cached_dir.h"
> > > > >  #include "reparse.h"
> > > > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > > > +#include "notify.h"
> > > > > +#endif
> > > > >
> > > > >  /*
> > > > >   * To be safe - for UCS to UTF-8 with strings loaded with the rare long
> > > > > @@ -1070,6 +1073,9 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
> > > > >         if (rc)
> > > > >                 goto cache_not_found;
> > > > >
> > > > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > > > +       start_track_dir_changes(full_path, d_inode(file_dentry(file)), cifs_sb);
> > > > > +#endif
> > > > >         mutex_lock(&cfid->dirents.de_mutex);
> > > > >         /*
> > > > >          * If this was reading from the start of the directory
> > > > > @@ -1151,6 +1157,9 @@ int cifs_readdir(struct file *file, struct dir_context *ctx)
> > > > >                 cifs_dbg(FYI, "Could not find entry\n");
> > > > >                 goto rddir2_exit;
> > > > >         }
> > > > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > > > +       start_track_dir_changes(full_path, d_inode(file_dentry(file)), cifs_sb);
> > > > > +#endif
> > > > >         cifs_dbg(FYI, "loop through %d times filling dir for net buf %p\n",
> > > > >                  num_to_fill, cifsFile->srch_inf.ntwrk_buf_start);
> > > > >         max_len = tcon->ses->server->ops->calc_smb_size(
> > > > > diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> > > > > index 4e922cb32110..58a1ddc39ee6 100644
> > > > > --- a/fs/smb/client/smb2pdu.c
> > > > > +++ b/fs/smb/client/smb2pdu.c
> > > > > @@ -45,6 +45,9 @@
> > > > >  #include "cached_dir.h"
> > > > >  #include "compress.h"
> > > > >  #include "fs_context.h"
> > > > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > > > +#include "notify.h"
> > > > > +#endif
> > > > >
> > > > >  /*
> > > > >   *  The following table defines the expected "StructureSize" of SMB2 requests
> > > > > @@ -466,6 +469,9 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
> > > > >                 mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
> > > > >
> > > > >         atomic_inc(&tconInfoReconnectCount);
> > > > > +#ifdef CONFIG_CIFS_DIR_CHANGE_TRACKING
> > > > > +       resume_track_dir_changes();
> > > > > +#endif
> > > > >  out:
> > > > >         /*
> > > > >          * Check if handle based operation so we know whether we can continue
> > > > > --
> > > > > 2.43.0
> > > > >
> > > >
> > > > To Reviewers.
> > > >
> > > > This is a gentle reminder on review requests.
> > > >
> > > > I would be very grateful for your feedback and am more than willing to
> > > > revise or improve any parts as needed. And also, let me know if I
> > > > missed anything or made mistakes.
> > >
> > > Hi Sang,
> >
> > Hello, Amir
> >
> > > First feedback (value):
> > > -----------------------------
> > > This looks very useful. this feature has been requested and
> > > attempted several times in the past (see links below), so if you are
> > > willing to incorporate feedback, I hope you will reach further than those
> > > past attempts and I will certainly do my best to help you with that.
> >
> > Thanks for your kind comment. I'm really glad to hear that.
> >
> > > Second feedback (reviewers):
> > > ----------------------------------------
> > > I was very surprised that your patch doesn't touch any vfs code
> > > (more on that on design feedback), but this is not an SMB-contained
> > > change at all.
> >
> > I agree with your last comment. I think it might not be easy;
> > honestly, I may know less than
> > Ioannis or Vivek; but I'm fully committed to giving it a try, no
> > matter the challenge.
> >
> > > Your patch touches the guts of the fsnotify subsystem (in a wrong way).
> > > For the next posting please consult the MAINTAINERS entry
> > > of the fsnotify subsystem for reviewers and list to CC (now added).
> >
> > I see. I'll keep it in my mind.
> >
> > > Third feedback (design):
> > > --------------------------------
> > > The design choice of polling i_fsnotify_mask on readdir()
> > > is quite odd and it is not clear to me why it makes sense.
> > > Previous discussions suggested to have a filesystem method
> > > to update when applications setup a watch on a directory [1].
> > > Another prior feedback was that the API should allow a clear
> > > distinction between the REMOTE notifications and the LOCAL
> > > notifications [2][3].
> >
> > Current design choice is a workaround for setting an appropriate add
> > watch point (as well as remove). I don't want to stick to the RFC
> > design. Also, The point that I considered important is similar to
> > Ioannis' one: compatible with existing applications.
> >
> > > IMO it would be better to finalize the design before working on the
> > > code, but that's up to you.
> >
> > I agree, although it's quite hard to create a perfect blueprint, but
> > it might be better to draw to some extent.
> >
> > Based on my current understanding, I think we need to do the following things.
> > - design more compatible and general fsnotify API for all network fs;
> > should process LOCAL and REMOTE both smoothly.
> > - expand inotify (if needed, fanotify both) flow with new fsnotify API
> > - replace SMB2 change_notify start/end point to new API
> >
>
> Yap, that's about it.
> All the rest is the details...
>
> > Let me know if I missed or misunderstood something. And also please
> > give me some time to read attached threads more deeply and clean up my
> > thoughts and questions.
> >
>
> Take your time.
> It's good to understand the concerns of previous attempts to
> avoid hitting the same roadblocks.

Good to see you again!

I read and try to understand previous discussions that you attached. I
would like to ask for your opinion about my current step.
I considered different places for new fsnotify API. I came to the same
conclusion that you already suggested to Inoannis [1]
After adding new API to `struct super_operations`, I tried to find the
right place for API calls that would not break existing systems and
have compatibility with inotify and fanotify.

From my current perspective, I think the outside of fsnotify (like
inotify_user.c/fanotify_user.c) is a better place to call new API.
Also, it might lead some duplicate code with inotify and fanotify, but
it seems difficult to create one unified logic that covers both
inotify and fanotify. With this approach, we could start inotify first
and then fanotify second that Inoannis and Vivek already attempted.
Even if unified logic is possible, I don't think it is not difficult
to merge and move them into inside of fsnotify (like mark.c)

Also, I have concerns when to call the new API. I think after updating
the mark is a good moment to call API if we decide to ignore errors
from new API; now, to me, it is affordable in terms of minimizing side
effect and lower risk with user spaces. However, eventually, I believe
the user should be able to decide whether to ignore the error or not
of new API, maybe by config or flag else. In that case, we need to
rollback update of fsnotify when new API fails. but it is not
supported yet. Could you share your thoughts on this, too?

If my inspection is wrong or you might have better idea, please let me
know about it. TBH, understanding new things is hard, but it's also a
happy moment to me.

[1] https://lore.kernel.org/linux-fsdevel/CAOQ4uxjvbcM9GKgs=KPNcH9PmtFJEiLY9O_ZHS7qeXrtUn=4yw@mail.gmail.com/

> Thanks,
> Amir.

Best Regards.
Sang-Heon Jeon

Re: [RFC PATCH 2/2] smb: client: add directory change tracking via SMB2 Change Notify

[Amir Goldstein]

...
> > > Hello, Amir
> > >
> > > > First feedback (value):
> > > > -----------------------------
> > > > This looks very useful. this feature has been requested and
> > > > attempted several times in the past (see links below), so if you are
> > > > willing to incorporate feedback, I hope you will reach further than those
> > > > past attempts and I will certainly do my best to help you with that.
> > >
> > > Thanks for your kind comment. I'm really glad to hear that.
> > >
> > > > Second feedback (reviewers):
> > > > ----------------------------------------
> > > > I was very surprised that your patch doesn't touch any vfs code
> > > > (more on that on design feedback), but this is not an SMB-contained
> > > > change at all.
> > >
> > > I agree with your last comment. I think it might not be easy;
> > > honestly, I may know less than
> > > Ioannis or Vivek; but I'm fully committed to giving it a try, no
> > > matter the challenge.
> > >
> > > > Your patch touches the guts of the fsnotify subsystem (in a wrong way).
> > > > For the next posting please consult the MAINTAINERS entry
> > > > of the fsnotify subsystem for reviewers and list to CC (now added).
> > >
> > > I see. I'll keep it in my mind.
> > >
> > > > Third feedback (design):
> > > > --------------------------------
> > > > The design choice of polling i_fsnotify_mask on readdir()
> > > > is quite odd and it is not clear to me why it makes sense.
> > > > Previous discussions suggested to have a filesystem method
> > > > to update when applications setup a watch on a directory [1].
> > > > Another prior feedback was that the API should allow a clear
> > > > distinction between the REMOTE notifications and the LOCAL
> > > > notifications [2][3].
> > >
> > > Current design choice is a workaround for setting an appropriate add
> > > watch point (as well as remove). I don't want to stick to the RFC
> > > design. Also, The point that I considered important is similar to
> > > Ioannis' one: compatible with existing applications.
> > >
> > > > IMO it would be better to finalize the design before working on the
> > > > code, but that's up to you.
> > >
> > > I agree, although it's quite hard to create a perfect blueprint, but
> > > it might be better to draw to some extent.
> > >
> > > Based on my current understanding, I think we need to do the following things.
> > > - design more compatible and general fsnotify API for all network fs;
> > > should process LOCAL and REMOTE both smoothly.
> > > - expand inotify (if needed, fanotify both) flow with new fsnotify API
> > > - replace SMB2 change_notify start/end point to new API
> > >
> >
> > Yap, that's about it.
> > All the rest is the details...
> >
> > > Let me know if I missed or misunderstood something. And also please
> > > give me some time to read attached threads more deeply and clean up my
> > > thoughts and questions.
> > >
> >
> > Take your time.
> > It's good to understand the concerns of previous attempts to
> > avoid hitting the same roadblocks.
>
> Good to see you again!
>
> I read and try to understand previous discussions that you attached. I
> would like to ask for your opinion about my current step.
> I considered different places for new fsnotify API. I came to the same
> conclusion that you already suggested to Inoannis [1]
> After adding new API to `struct super_operations`, I tried to find the
> right place for API calls that would not break existing systems and
> have compatibility with inotify and fanotify.
>
> From my current perspective, I think the outside of fsnotify (like
> inotify_user.c/fanotify_user.c) is a better place to call new API.
> Also, it might lead some duplicate code with inotify and fanotify, but
> it seems difficult to create one unified logic that covers both
> inotify and fanotify.


Personally, I don't mind duplicating this call in the inotify and
fanotify backends.
Not sure if this feature is relevant to other backends like nfsd and audit.

I do care about making this feature opt-in, which goes a bit against your
requirement that existing applications will get the REMOTE notifications
without opting in for them and without the notifications being clearly marked
as REMOTE notifications.

If you do not make this feature opt-in (e.g. by fanotify flag FAN_MARK_REMOTE)
then it's a change of behavior that could be desired to some and surprising to
others.

Also when performing an operation on the local smb client (e.g. unlink)
you would get two notifications, one the LOCAL and one the REMOTE,
not being able to distinguish between them or request just one of them
is going to be confusing.

> With this approach, we could start inotify first
> and then fanotify second that Inoannis and Vivek already attempted.
> Even if unified logic is possible, I don't think it is not difficult
> to merge and move them into inside of fsnotify (like mark.c)
>

For all the reasons above I would prefer to support fanotify first
(with opt-in flag) and not support inotify at all, but if you want to
support inotify, better have some method to opt-in at least.
Could be a global inotify kob for all I care, as long as the default
does not take anyone by surprise.

> Also, I have concerns when to call the new API. I think after updating
> the mark is a good moment to call API if we decide to ignore errors
> from new API; now, to me, it is affordable in terms of minimizing side
> effect and lower risk with user spaces. However, eventually, I believe
> the user should be able to decide whether to ignore the error or not
> of new API, maybe by config or flag else. In that case, we need to
> rollback update of fsnotify when new API fails. but it is not
> supported yet. Could you share your thoughts on this, too?
>

If you update remote mask with explicit FAN_MARK_REMOTE
and update local mask without FAN_MARK_REMOTE, then
there is no problem is there?

Either FAN_MARK_REMOTE succeeded or not.
If it did, remote fs could be expected to generate remote events.

> If my inspection is wrong or you might have better idea, please let me
> know about it. TBH, understanding new things is hard, but it's also a
> happy moment to me.
>

I am relying on what I think you mean, but I may misunderstand you
because you did not sketch any code samples, so I don't believe
I fully understand all your concerns and ideas.

Even an untested patch could help me understand if we are on the same page.

Thanks,
Amir.

Re: [RFC PATCH 2/2] smb: client: add directory change tracking via SMB2 Change Notify

[Sang-Heon Jeon]

[-- Attachment #1: Type: text/plain, Size: 7973 bytes --]

On Fri, Oct 24, 2025 at 12:30 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> ...
> > > > Hello, Amir
> > > >
> > > > > First feedback (value):
> > > > > -----------------------------
> > > > > This looks very useful. this feature has been requested and
> > > > > attempted several times in the past (see links below), so if you are
> > > > > willing to incorporate feedback, I hope you will reach further than those
> > > > > past attempts and I will certainly do my best to help you with that.
> > > >
> > > > Thanks for your kind comment. I'm really glad to hear that.
> > > >
> > > > > Second feedback (reviewers):
> > > > > ----------------------------------------
> > > > > I was very surprised that your patch doesn't touch any vfs code
> > > > > (more on that on design feedback), but this is not an SMB-contained
> > > > > change at all.
> > > >
> > > > I agree with your last comment. I think it might not be easy;
> > > > honestly, I may know less than
> > > > Ioannis or Vivek; but I'm fully committed to giving it a try, no
> > > > matter the challenge.
> > > >
> > > > > Your patch touches the guts of the fsnotify subsystem (in a wrong way).
> > > > > For the next posting please consult the MAINTAINERS entry
> > > > > of the fsnotify subsystem for reviewers and list to CC (now added).
> > > >
> > > > I see. I'll keep it in my mind.
> > > >
> > > > > Third feedback (design):
> > > > > --------------------------------
> > > > > The design choice of polling i_fsnotify_mask on readdir()
> > > > > is quite odd and it is not clear to me why it makes sense.
> > > > > Previous discussions suggested to have a filesystem method
> > > > > to update when applications setup a watch on a directory [1].
> > > > > Another prior feedback was that the API should allow a clear
> > > > > distinction between the REMOTE notifications and the LOCAL
> > > > > notifications [2][3].
> > > >
> > > > Current design choice is a workaround for setting an appropriate add
> > > > watch point (as well as remove). I don't want to stick to the RFC
> > > > design. Also, The point that I considered important is similar to
> > > > Ioannis' one: compatible with existing applications.
> > > >
> > > > > IMO it would be better to finalize the design before working on the
> > > > > code, but that's up to you.
> > > >
> > > > I agree, although it's quite hard to create a perfect blueprint, but
> > > > it might be better to draw to some extent.
> > > >
> > > > Based on my current understanding, I think we need to do the following things.
> > > > - design more compatible and general fsnotify API for all network fs;
> > > > should process LOCAL and REMOTE both smoothly.
> > > > - expand inotify (if needed, fanotify both) flow with new fsnotify API
> > > > - replace SMB2 change_notify start/end point to new API
> > > >
> > >
> > > Yap, that's about it.
> > > All the rest is the details...
> > >
> > > > Let me know if I missed or misunderstood something. And also please
> > > > give me some time to read attached threads more deeply and clean up my
> > > > thoughts and questions.
> > > >
> > >
> > > Take your time.
> > > It's good to understand the concerns of previous attempts to
> > > avoid hitting the same roadblocks.
> >
> > Good to see you again!
> >
> > I read and try to understand previous discussions that you attached. I
> > would like to ask for your opinion about my current step.
> > I considered different places for new fsnotify API. I came to the same
> > conclusion that you already suggested to Inoannis [1]
> > After adding new API to `struct super_operations`, I tried to find the
> > right place for API calls that would not break existing systems and
> > have compatibility with inotify and fanotify.
> >
> > From my current perspective, I think the outside of fsnotify (like
> > inotify_user.c/fanotify_user.c) is a better place to call new API.
> > Also, it might lead some duplicate code with inotify and fanotify, but
> > it seems difficult to create one unified logic that covers both
> > inotify and fanotify.
>
>
> Personally, I don't mind duplicating this call in the inotify and
> fanotify backends.
> Not sure if this feature is relevant to other backends like nfsd and audit.
>
> I do care about making this feature opt-in, which goes a bit against your
> requirement that existing applications will get the REMOTE notifications
> without opting in for them and without the notifications being clearly marked
> as REMOTE notifications.
>
> If you do not make this feature opt-in (e.g. by fanotify flag FAN_MARK_REMOTE)
> then it's a change of behavior that could be desired to some and surprising to
> others.

You're right, Upon further reflection, my previous approach may create
unexpected effects to the user program. But to achieve my requirement,
inotify should be supported (also safely). I will revisit inotify with
opt-in method after finishing the discussion about fanotify.

> Also when performing an operation on the local smb client (e.g. unlink)
> you would get two notifications, one the LOCAL and one the REMOTE,
> not being able to distinguish between them or request just one of them
> is going to be confusing.
>
> > With this approach, we could start inotify first
> > and then fanotify second that Inoannis and Vivek already attempted.
> > Even if unified logic is possible, I don't think it is not difficult
> > to merge and move them into inside of fsnotify (like mark.c)
> >
>
> For all the reasons above I would prefer to support fanotify first
> (with opt-in flag) and not support inotify at all, but if you want to
> support inotify, better have some method to opt-in at least.
> Could be a global inotify kob for all I care, as long as the default
> does not take anyone by surprise.

Thanks for the detailed description. I understand the point of
distinguishing remote and local notification better. And the way you
prefer (fanotify first) is also reasonable to me because implementing
fanotify would also help support inotify more safely.

> > Also, I have concerns when to call the new API. I think after updating
> > the mark is a good moment to call API if we decide to ignore errors
> > from new API; now, to me, it is affordable in terms of minimizing side
> > effect and lower risk with user spaces. However, eventually, I believe
> > the user should be able to decide whether to ignore the error or not
> > of new API, maybe by config or flag else. In that case, we need to
> > rollback update of fsnotify when new API fails. but it is not
> > supported yet. Could you share your thoughts on this, too?
> >
>
> If you update remote mask with explicit FAN_MARK_REMOTE
> and update local mask without FAN_MARK_REMOTE, then
> there is no problem is there?
>
> Either FAN_MARK_REMOTE succeeded or not.
> If it did, remote fs could be expected to generate remote events.

I understand you mean splitting mask into a local and remote
notification instead of sharing, is it right?
TBH, I never thought of that solution but it's quite clear and looks good to me.
If I misunderstand, could you please explain a bit more?

> > If my inspection is wrong or you might have better idea, please let me
> > know about it. TBH, understanding new things is hard, but it's also a
> > happy moment to me.
> >
>
> I am relying on what I think you mean, but I may misunderstand you
> because you did not sketch any code samples, so I don't believe
> I fully understand all your concerns and ideas.
>
> Even an untested patch could help me understand if we are on the same page.

Thanks for your advice. I think we're getting closer to the same page.
I also attached patch of my current sketch (not tested and cleaned),
feel free to give your opinions.

Thanks for your consideration as well.

> Thanks,
> Amir.

Best Regards.
Sang-Heon Jeon

[-- Attachment #2: 0001-fanotify-introduce-remote-mask.patch --]
[-- Type: text/x-patch, Size: 9583 bytes --]

From 7fe87dc4cf41193d280cee08ea995e9991e885b5 Mon Sep 17 00:00:00 2001
From: Sang-Heon Jeon <ekffu200098@gmail.com>
Date: Wed, 29 Oct 2025 00:46:02 +0900
Subject: [PATCH] fanotify: introduce remote mask

Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
---
 fs/inode.c                         |  1 +
 fs/notify/fanotify/fanotify_user.c | 54 +++++++++++++++++++++++-------
 fs/notify/mark.c                   | 23 ++++++++++++-
 include/linux/fanotify.h           |  3 +-
 include/linux/fs.h                 |  1 +
 include/linux/fsnotify_backend.h   | 22 ++++++++++++
 6 files changed, 89 insertions(+), 15 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index ec9339024ac3..76b68f7fed3d 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -300,6 +300,7 @@ int inode_init_always_gfp(struct super_block *sb, struct inode *inode, gfp_t gfp
 
 #ifdef CONFIG_FSNOTIFY
 	inode->i_fsnotify_mask = 0;
+	inode->i_fsnotify_remote_mask = 0;
 #endif
 	inode->i_flctx = NULL;
 
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 1dadda82cae5..49f723836839 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1247,20 +1247,29 @@ static __u32 fanotify_mark_remove_from_mask(struct fsnotify_mark *fsn_mark,
 	/* umask bits cannot be removed by user */
 	mask &= ~umask;
 	spin_lock(&fsn_mark->lock);
-	oldmask = fsnotify_calc_mask(fsn_mark);
-	if (!(flags & FANOTIFY_MARK_IGNORE_BITS)) {
-		fsn_mark->mask &= ~mask;
+	if (flags & FAN_MARK_REMOTE) {
+		oldmask = fsnotify_calc_remote_mask(fsn_mark);
+		if (!(flags & FANOTIFY_MARK_IGNORE_BITS))
+			fsn_mark->remote_mask &= ~mask;
+		else
+			fsn_mark->remote_ignore_mask &= ~mask;
+		newmask = fsnotify_calc_remote_mask(fsn_mark);
 	} else {
-		fsn_mark->ignore_mask &= ~mask;
+		oldmask = fsnotify_calc_mask(fsn_mark);
+		if (!(flags & FANOTIFY_MARK_IGNORE_BITS))
+			fsn_mark->mask &= ~mask;
+		else
+			fsn_mark->ignore_mask &= ~mask;
+		newmask = fsnotify_calc_mask(fsn_mark);
 	}
-	newmask = fsnotify_calc_mask(fsn_mark);
 	/*
 	 * We need to keep the mark around even if remaining mask cannot
 	 * result in any events (e.g. mask == FAN_ONDIR) to support incremenal
 	 * changes to the mask.
 	 * Destroy mark when only umask bits remain.
 	 */
-	*destroy = !((fsn_mark->mask | fsn_mark->ignore_mask) & ~umask);
+	*destroy = !((fsn_mark->remote_mask | fsn_mark->remote_ignore_mask |
+		fsn_mark->mask | fsn_mark->ignore_mask) & ~umask);
 	spin_unlock(&fsn_mark->lock);
 
 	return oldmask & ~newmask;
@@ -1283,8 +1292,13 @@ static int fanotify_remove_mark(struct fsnotify_group *group,
 
 	removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags,
 						 umask, &destroy_mark);
-	if (removed & fsnotify_conn_mask(fsn_mark->connector))
-		fsnotify_recalc_mask(fsn_mark->connector);
+	if (flags & FAN_MARK_REMOTE) {
+		if (removed & fsnotify_conn_remote_mask(fsn_mark->connector))
+			fsnotify_recalc_remote_mask(fsn_mark->connector);
+	} else {
+		if (removed & fsnotify_conn_mask(fsn_mark->connector))
+			fsnotify_recalc_mask(fsn_mark->connector);
+	}
 	if (destroy_mark)
 		fsnotify_detach_mark(fsn_mark);
 	fsnotify_group_unlock(group);
@@ -1320,8 +1334,12 @@ static bool fanotify_mark_update_flags(struct fsnotify_mark *fsn_mark,
 	if (ignore && (fan_flags & FAN_MARK_IGNORED_SURV_MODIFY) &&
 	    !(fsn_mark->flags & FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY)) {
 		fsn_mark->flags |= FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY;
-		if (!(fsn_mark->mask & FS_MODIFY))
-			recalc = true;
+		if (!(fsn_mark->mask & FS_MODIFY) &&
+			!(fan_flags & FAN_MARK_REMOTE))
+				recalc = true;
+		if (!(fsn_mark->remote_mask & FS_MODIFY) &&
+			(fan_flags & FAN_MARK_REMOTE))
+				recalc = true;
 	}
 
 	if (fsn_mark->connector->type != FSNOTIFY_OBJ_TYPE_INODE ||
@@ -1345,9 +1363,15 @@ static bool fanotify_mark_add_to_mask(struct fsnotify_mark *fsn_mark,
 
 	spin_lock(&fsn_mark->lock);
 	if (!(fan_flags & FANOTIFY_MARK_IGNORE_BITS))
-		fsn_mark->mask |= mask;
+		if (fan_flags & FAN_MARK_REMOTE)
+			fsn_mark->remote_mask |= mask;
+		else
+			fsn_mark->mask |= mask;
 	else
-		fsn_mark->ignore_mask |= mask;
+		if (fan_flags & FAN_MARK_REMOTE)
+			fsn_mark->remote_ignore_mask |= mask;
+		else
+			fsn_mark->ignore_mask |= mask;
 
 	recalc = fsnotify_calc_mask(fsn_mark) &
 		~fsnotify_conn_mask(fsn_mark->connector);
@@ -1509,7 +1533,11 @@ static int fanotify_may_update_existing_mark(struct fsnotify_mark *fsn_mark,
 		return -EEXIST;
 
 	/* For now pre-content events are not generated for directories */
-	mask |= fsn_mark->mask;
+	if (fan_flags & FAN_MARK_REMOTE)
+		mask |= fsn_mark->remote_mask;
+	else
+		mask |= fsn_mark->mask;
+
 	if (mask & FANOTIFY_PRE_CONTENT_EVENTS && mask & FAN_ONDIR)
 		return -EEXIST;
 
diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index 55a03bb05aa1..1f030d91fcc5 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -127,6 +127,14 @@ static __u32 *fsnotify_conn_mask_p(struct fsnotify_mark_connector *conn)
 	return NULL;
 }
 
+static __u32 *fsnotify_conn_remote_mask_p(struct fsnotify_mark_connector *conn)
+{
+	if (conn->type == FSNOTIFY_OBJ_TYPE_INODE)
+		return &fsnotify_conn_inode(conn)->i_fsnotify_remote_mask;
+
+	return NULL;
+}
+
 __u32 fsnotify_conn_mask(struct fsnotify_mark_connector *conn)
 {
 	if (WARN_ON(!fsnotify_valid_obj_type(conn->type)))
@@ -135,6 +143,14 @@ __u32 fsnotify_conn_mask(struct fsnotify_mark_connector *conn)
 	return READ_ONCE(*fsnotify_conn_mask_p(conn));
 }
 
+__u32 fsnotify_conn_remote_mask(struct fsnotify_mark_connector *conn)
+{
+	if (conn->type != FSNOTIFY_OBJ_TYPE_INODE)
+		return 0;
+
+	return READ_ONCE(*fsnotify_conn_remote_mask_p(conn));
+}
+
 static void fsnotify_get_sb_watched_objects(struct super_block *sb)
 {
 	atomic_long_inc(fsnotify_sb_watched_objects(sb));
@@ -239,9 +255,10 @@ static struct inode *fsnotify_update_iref(struct fsnotify_mark_connector *conn,
 
 static void *__fsnotify_recalc_mask(struct fsnotify_mark_connector *conn)
 {
-	u32 new_mask = 0;
+	u32 new_mask = 0, new_remote_mask = 0;
 	bool want_iref = false;
 	struct fsnotify_mark *mark;
+	u32 *remote_mask_p;
 
 	assert_spin_locked(&conn->lock);
 	/* We can get detached connector here when inode is getting unlinked. */
@@ -251,6 +268,7 @@ static void *__fsnotify_recalc_mask(struct fsnotify_mark_connector *conn)
 		if (!(mark->flags & FSNOTIFY_MARK_FLAG_ATTACHED))
 			continue;
 		new_mask |= fsnotify_calc_mask(mark);
+		new_remote_mask |= fsnotify_calc_remote_mask(mark);
 		if (conn->type == FSNOTIFY_OBJ_TYPE_INODE &&
 		    !(mark->flags & FSNOTIFY_MARK_FLAG_NO_IREF))
 			want_iref = true;
@@ -260,6 +278,9 @@ static void *__fsnotify_recalc_mask(struct fsnotify_mark_connector *conn)
 	 * confusing readers not holding conn->lock with partial updates.
 	 */
 	WRITE_ONCE(*fsnotify_conn_mask_p(conn), new_mask);
+	remote_mask_p = fsnotify_conn_remote_mask_p(conn);
+	if (remote_mask_p)
+		WRITE_ONCE(*remote_mask_p, new_remote_mask);
 
 	return fsnotify_update_iref(conn, want_iref);
 }
diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
index 879cff5eccd4..34be292eec2f 100644
--- a/include/linux/fanotify.h
+++ b/include/linux/fanotify.h
@@ -72,7 +72,8 @@
 				 FAN_MARK_DONT_FOLLOW | \
 				 FAN_MARK_ONLYDIR | \
 				 FAN_MARK_IGNORED_SURV_MODIFY | \
-				 FAN_MARK_EVICTABLE)
+				 FAN_MARK_EVICTABLE | \
+				 FAN_MARK_REMOTE)
 
 /*
  * Events that can be reported with data type FSNOTIFY_EVENT_PATH.
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 68c4a59ec8fb..0ff83a01f711 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -895,6 +895,7 @@ struct inode {
 
 #ifdef CONFIG_FSNOTIFY
 	__u32			i_fsnotify_mask; /* all events this inode cares about */
+	__u32			i_fsnotify_remote_mask;
 	/* 32-bit hole reserved for expanding i_fsnotify_mask */
 	struct fsnotify_mark_connector __rcu	*i_fsnotify_marks;
 #endif
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 0d954ea7b179..b81877dc6b72 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -635,6 +635,9 @@ struct fsnotify_mark {
 #define FSNOTIFY_MARK_FLAG_HAS_FSID		0x0800
 #define FSNOTIFY_MARK_FLAG_WEAK_FSID		0x1000
 	unsigned int flags;		/* flags [mark->lock] */
+
+	__u32 remote_mask;
+	__u32 remote_ignore_mask;
 };
 
 #ifdef CONFIG_FSNOTIFY
@@ -799,6 +802,11 @@ static inline __u32 fsnotify_ignored_events(struct fsnotify_mark *mark)
 	return mark->ignore_mask & ALL_FSNOTIFY_EVENTS;
 }
 
+static inline __u32 fsnotify_remote_ignored_events(struct fsnotify_mark *mark)
+{
+	return mark->remote_ignore_mask & ALL_FSNOTIFY_EVENTS;
+}
+
 /*
  * Check if mask (or ignore mask) should be applied depending if victim is a
  * directory and whether it is reported to a watching parent.
@@ -860,8 +868,22 @@ static inline __u32 fsnotify_calc_mask(struct fsnotify_mark *mark)
 	return mask | mark->ignore_mask;
 }
 
+static inline __u32 fsnotify_calc_remote_mask(struct fsnotify_mark *mark)
+{
+	__u32 mask = mark->remote_mask;
+
+	if (!fsnotify_remote_ignored_events(mark))
+		return mask;
+
+	if (!(mark->flags & FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY))
+		mask |= FS_MODIFY;
+
+	return mask | mark->remote_ignore_mask;
+}
+
 /* Get mask of events for a list of marks */
 extern __u32 fsnotify_conn_mask(struct fsnotify_mark_connector *conn);
+extern __u32 fsnotify_conn_remote_mask(struct fsnotify_mark_connector *conn);
 /* Calculate mask of events for a list of marks */
 extern void fsnotify_recalc_mask(struct fsnotify_mark_connector *conn);
 extern void fsnotify_init_mark(struct fsnotify_mark *mark,
-- 
2.43.0

Re: [RFC PATCH 2/2] smb: client: add directory change tracking via SMB2 Change Notify

[Amir Goldstein]

On Wed, Oct 29, 2025 at 01:13:31AM +0900, Sang-Heon Jeon wrote:
> On Fri, Oct 24, 2025 at 12:30 AM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > ...
> > > > > Hello, Amir
> > > > >
> > > > > > First feedback (value):
> > > > > > -----------------------------
> > > > > > This looks very useful. this feature has been requested and
> > > > > > attempted several times in the past (see links below), so if you are
> > > > > > willing to incorporate feedback, I hope you will reach further than those
> > > > > > past attempts and I will certainly do my best to help you with that.
> > > > >
> > > > > Thanks for your kind comment. I'm really glad to hear that.
> > > > >
> > > > > > Second feedback (reviewers):
> > > > > > ----------------------------------------
> > > > > > I was very surprised that your patch doesn't touch any vfs code
> > > > > > (more on that on design feedback), but this is not an SMB-contained
> > > > > > change at all.
> > > > >
> > > > > I agree with your last comment. I think it might not be easy;
> > > > > honestly, I may know less than
> > > > > Ioannis or Vivek; but I'm fully committed to giving it a try, no
> > > > > matter the challenge.
> > > > >
> > > > > > Your patch touches the guts of the fsnotify subsystem (in a wrong way).
> > > > > > For the next posting please consult the MAINTAINERS entry
> > > > > > of the fsnotify subsystem for reviewers and list to CC (now added).
> > > > >
> > > > > I see. I'll keep it in my mind.
> > > > >
> > > > > > Third feedback (design):
> > > > > > --------------------------------
> > > > > > The design choice of polling i_fsnotify_mask on readdir()
> > > > > > is quite odd and it is not clear to me why it makes sense.
> > > > > > Previous discussions suggested to have a filesystem method
> > > > > > to update when applications setup a watch on a directory [1].
> > > > > > Another prior feedback was that the API should allow a clear
> > > > > > distinction between the REMOTE notifications and the LOCAL
> > > > > > notifications [2][3].
> > > > >
> > > > > Current design choice is a workaround for setting an appropriate add
> > > > > watch point (as well as remove). I don't want to stick to the RFC
> > > > > design. Also, The point that I considered important is similar to
> > > > > Ioannis' one: compatible with existing applications.
> > > > >
> > > > > > IMO it would be better to finalize the design before working on the
> > > > > > code, but that's up to you.
> > > > >
> > > > > I agree, although it's quite hard to create a perfect blueprint, but
> > > > > it might be better to draw to some extent.
> > > > >
> > > > > Based on my current understanding, I think we need to do the following things.
> > > > > - design more compatible and general fsnotify API for all network fs;
> > > > > should process LOCAL and REMOTE both smoothly.
> > > > > - expand inotify (if needed, fanotify both) flow with new fsnotify API
> > > > > - replace SMB2 change_notify start/end point to new API
> > > > >
> > > >
> > > > Yap, that's about it.
> > > > All the rest is the details...
> > > >
> > > > > Let me know if I missed or misunderstood something. And also please
> > > > > give me some time to read attached threads more deeply and clean up my
> > > > > thoughts and questions.
> > > > >
> > > >
> > > > Take your time.
> > > > It's good to understand the concerns of previous attempts to
> > > > avoid hitting the same roadblocks.
> > >
> > > Good to see you again!
> > >
> > > I read and try to understand previous discussions that you attached. I
> > > would like to ask for your opinion about my current step.
> > > I considered different places for new fsnotify API. I came to the same
> > > conclusion that you already suggested to Inoannis [1]
> > > After adding new API to `struct super_operations`, I tried to find the
> > > right place for API calls that would not break existing systems and
> > > have compatibility with inotify and fanotify.
> > >
> > > From my current perspective, I think the outside of fsnotify (like
> > > inotify_user.c/fanotify_user.c) is a better place to call new API.
> > > Also, it might lead some duplicate code with inotify and fanotify, but
> > > it seems difficult to create one unified logic that covers both
> > > inotify and fanotify.
> >
> >
> > Personally, I don't mind duplicating this call in the inotify and
> > fanotify backends.
> > Not sure if this feature is relevant to other backends like nfsd and audit.
> >
> > I do care about making this feature opt-in, which goes a bit against your
> > requirement that existing applications will get the REMOTE notifications
> > without opting in for them and without the notifications being clearly marked
> > as REMOTE notifications.
> >
> > If you do not make this feature opt-in (e.g. by fanotify flag FAN_MARK_REMOTE)
> > then it's a change of behavior that could be desired to some and surprising to
> > others.
> 
> You're right, Upon further reflection, my previous approach may create
> unexpected effects to the user program. But to achieve my requirement,
> inotify should be supported (also safely). I will revisit inotify with
> opt-in method after finishing the discussion about fanotify.
> 
> > Also when performing an operation on the local smb client (e.g. unlink)
> > you would get two notifications, one the LOCAL and one the REMOTE,
> > not being able to distinguish between them or request just one of them
> > is going to be confusing.
> >
> > > With this approach, we could start inotify first
> > > and then fanotify second that Inoannis and Vivek already attempted.
> > > Even if unified logic is possible, I don't think it is not difficult
> > > to merge and move them into inside of fsnotify (like mark.c)
> > >
> >
> > For all the reasons above I would prefer to support fanotify first
> > (with opt-in flag) and not support inotify at all, but if you want to
> > support inotify, better have some method to opt-in at least.
> > Could be a global inotify kob for all I care, as long as the default
> > does not take anyone by surprise.
> 
> Thanks for the detailed description. I understand the point of
> distinguishing remote and local notification better. And the way you
> prefer (fanotify first) is also reasonable to me because implementing
> fanotify would also help support inotify more safely.
> 
> > > Also, I have concerns when to call the new API. I think after updating
> > > the mark is a good moment to call API if we decide to ignore errors
> > > from new API; now, to me, it is affordable in terms of minimizing side
> > > effect and lower risk with user spaces. However, eventually, I believe
> > > the user should be able to decide whether to ignore the error or not
> > > of new API, maybe by config or flag else. In that case, we need to
> > > rollback update of fsnotify when new API fails. but it is not
> > > supported yet. Could you share your thoughts on this, too?
> > >
> >
> > If you update remote mask with explicit FAN_MARK_REMOTE
> > and update local mask without FAN_MARK_REMOTE, then
> > there is no problem is there?
> >
> > Either FAN_MARK_REMOTE succeeded or not.
> > If it did, remote fs could be expected to generate remote events.
> 
> I understand you mean splitting mask into a local and remote
> notification instead of sharing, is it right?

No, that's no what I meant.

> TBH, I never thought of that solution but it's quite clear and looks good to me.
> If I misunderstand, could you please explain a bit more?
> 

It is indeed clear, but inode space is quite expensive, so we cannot
add i_fsnotify_remote_mask field for all inodes and also its not
necessary.

TBH, I do have a final picture of the opt-in API and there are several
shapes that it could take.

I think that we anyway need an "event mask flag" FAN_REMOTE,
similar to FAN_ONDIR.

A remote notification is generated by the filesystem and this source
of notification always sets the FS_REMOTE in the event mask, which is
visible to users reading the events.

This makes it natural to also opt-in for remote events via the mask,
some as is done with FAN_ONDIR.

However, I would like to avoid the complexities involved with flipping
this FS_REMOTE bit in event mask, that's why I was thinking about
FAN_MARK_REMOTE that sets a flag in the mark, like FAN_MARK_EVICTABLE
and forces all mark updates to use the same flag.

But for your first RFC, I suggest that you make it simple and use
a group flag to request only remote notifications at fanotify_init
time.

This way you can take the existing cifs implementation of remote
notifications using an ioctl and "bind" it to use the fanotify API
with as little interferance with local notifications as possible.

> > > If my inspection is wrong or you might have better idea, please let me
> > > know about it. TBH, understanding new things is hard, but it's also a
> > > happy moment to me.
> > >
> >
> > I am relying on what I think you mean, but I may misunderstand you
> > because you did not sketch any code samples, so I don't believe
> > I fully understand all your concerns and ideas.
> >
> > Even an untested patch could help me understand if we are on the same page.
> 
> Thanks for your advice. I think we're getting closer to the same page.

Not quite there yet :)

> I also attached patch of my current sketch (not tested and cleaned),
> feel free to give your opinions.

This is not what I was looking for.

What I am looking for is a POC patch of binding cifs notifications
to fsnotify/fanotify:

- fanotify_mask() calls filesystem method to update server event mask
- filesystem calls fnsotify() hook with FS_REMOTE flag
- groups or marks that did not opt-in for remote notifications
  would drop this event

First milestone: be able to write program that listens to remote
notifications only.

Same program can use two groups remote and local with mask per type
to request a mixture of local and remote events.

Honestly, I don't think we need more than that?

Thanks,
Amir.

Re: [RFC PATCH 2/2] smb: client: add directory change tracking via SMB2 Change Notify

[Sang-Heon Jeon]

On Wed, Oct 29, 2025 at 9:39 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Wed, Oct 29, 2025 at 01:13:31AM +0900, Sang-Heon Jeon wrote:
> > On Fri, Oct 24, 2025 at 12:30 AM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > ...
> > > > > > Hello, Amir
> > > > > >
> > > > > > > First feedback (value):
> > > > > > > -----------------------------
> > > > > > > This looks very useful. this feature has been requested and
> > > > > > > attempted several times in the past (see links below), so if you are
> > > > > > > willing to incorporate feedback, I hope you will reach further than those
> > > > > > > past attempts and I will certainly do my best to help you with that.
> > > > > >
> > > > > > Thanks for your kind comment. I'm really glad to hear that.
> > > > > >
> > > > > > > Second feedback (reviewers):
> > > > > > > ----------------------------------------
> > > > > > > I was very surprised that your patch doesn't touch any vfs code
> > > > > > > (more on that on design feedback), but this is not an SMB-contained
> > > > > > > change at all.
> > > > > >
> > > > > > I agree with your last comment. I think it might not be easy;
> > > > > > honestly, I may know less than
> > > > > > Ioannis or Vivek; but I'm fully committed to giving it a try, no
> > > > > > matter the challenge.
> > > > > >
> > > > > > > Your patch touches the guts of the fsnotify subsystem (in a wrong way).
> > > > > > > For the next posting please consult the MAINTAINERS entry
> > > > > > > of the fsnotify subsystem for reviewers and list to CC (now added).
> > > > > >
> > > > > > I see. I'll keep it in my mind.
> > > > > >
> > > > > > > Third feedback (design):
> > > > > > > --------------------------------
> > > > > > > The design choice of polling i_fsnotify_mask on readdir()
> > > > > > > is quite odd and it is not clear to me why it makes sense.
> > > > > > > Previous discussions suggested to have a filesystem method
> > > > > > > to update when applications setup a watch on a directory [1].
> > > > > > > Another prior feedback was that the API should allow a clear
> > > > > > > distinction between the REMOTE notifications and the LOCAL
> > > > > > > notifications [2][3].
> > > > > >
> > > > > > Current design choice is a workaround for setting an appropriate add
> > > > > > watch point (as well as remove). I don't want to stick to the RFC
> > > > > > design. Also, The point that I considered important is similar to
> > > > > > Ioannis' one: compatible with existing applications.
> > > > > >
> > > > > > > IMO it would be better to finalize the design before working on the
> > > > > > > code, but that's up to you.
> > > > > >
> > > > > > I agree, although it's quite hard to create a perfect blueprint, but
> > > > > > it might be better to draw to some extent.
> > > > > >
> > > > > > Based on my current understanding, I think we need to do the following things.
> > > > > > - design more compatible and general fsnotify API for all network fs;
> > > > > > should process LOCAL and REMOTE both smoothly.
> > > > > > - expand inotify (if needed, fanotify both) flow with new fsnotify API
> > > > > > - replace SMB2 change_notify start/end point to new API
> > > > > >
> > > > >
> > > > > Yap, that's about it.
> > > > > All the rest is the details...
> > > > >
> > > > > > Let me know if I missed or misunderstood something. And also please
> > > > > > give me some time to read attached threads more deeply and clean up my
> > > > > > thoughts and questions.
> > > > > >
> > > > >
> > > > > Take your time.
> > > > > It's good to understand the concerns of previous attempts to
> > > > > avoid hitting the same roadblocks.
> > > >
> > > > Good to see you again!
> > > >
> > > > I read and try to understand previous discussions that you attached. I
> > > > would like to ask for your opinion about my current step.
> > > > I considered different places for new fsnotify API. I came to the same
> > > > conclusion that you already suggested to Inoannis [1]
> > > > After adding new API to `struct super_operations`, I tried to find the
> > > > right place for API calls that would not break existing systems and
> > > > have compatibility with inotify and fanotify.
> > > >
> > > > From my current perspective, I think the outside of fsnotify (like
> > > > inotify_user.c/fanotify_user.c) is a better place to call new API.
> > > > Also, it might lead some duplicate code with inotify and fanotify, but
> > > > it seems difficult to create one unified logic that covers both
> > > > inotify and fanotify.
> > >
> > >
> > > Personally, I don't mind duplicating this call in the inotify and
> > > fanotify backends.
> > > Not sure if this feature is relevant to other backends like nfsd and audit.
> > >
> > > I do care about making this feature opt-in, which goes a bit against your
> > > requirement that existing applications will get the REMOTE notifications
> > > without opting in for them and without the notifications being clearly marked
> > > as REMOTE notifications.
> > >
> > > If you do not make this feature opt-in (e.g. by fanotify flag FAN_MARK_REMOTE)
> > > then it's a change of behavior that could be desired to some and surprising to
> > > others.
> >
> > You're right, Upon further reflection, my previous approach may create
> > unexpected effects to the user program. But to achieve my requirement,
> > inotify should be supported (also safely). I will revisit inotify with
> > opt-in method after finishing the discussion about fanotify.
> >
> > > Also when performing an operation on the local smb client (e.g. unlink)
> > > you would get two notifications, one the LOCAL and one the REMOTE,
> > > not being able to distinguish between them or request just one of them
> > > is going to be confusing.
> > >
> > > > With this approach, we could start inotify first
> > > > and then fanotify second that Inoannis and Vivek already attempted.
> > > > Even if unified logic is possible, I don't think it is not difficult
> > > > to merge and move them into inside of fsnotify (like mark.c)
> > > >
> > >
> > > For all the reasons above I would prefer to support fanotify first
> > > (with opt-in flag) and not support inotify at all, but if you want to
> > > support inotify, better have some method to opt-in at least.
> > > Could be a global inotify kob for all I care, as long as the default
> > > does not take anyone by surprise.
> >
> > Thanks for the detailed description. I understand the point of
> > distinguishing remote and local notification better. And the way you
> > prefer (fanotify first) is also reasonable to me because implementing
> > fanotify would also help support inotify more safely.
> >
> > > > Also, I have concerns when to call the new API. I think after updating
> > > > the mark is a good moment to call API if we decide to ignore errors
> > > > from new API; now, to me, it is affordable in terms of minimizing side
> > > > effect and lower risk with user spaces. However, eventually, I believe
> > > > the user should be able to decide whether to ignore the error or not
> > > > of new API, maybe by config or flag else. In that case, we need to
> > > > rollback update of fsnotify when new API fails. but it is not
> > > > supported yet. Could you share your thoughts on this, too?
> > > >
> > >
> > > If you update remote mask with explicit FAN_MARK_REMOTE
> > > and update local mask without FAN_MARK_REMOTE, then
> > > there is no problem is there?
> > >
> > > Either FAN_MARK_REMOTE succeeded or not.
> > > If it did, remote fs could be expected to generate remote events.
> >
> > I understand you mean splitting mask into a local and remote
> > notification instead of sharing, is it right?
>
> No, that's no what I meant.
>
> > TBH, I never thought of that solution but it's quite clear and looks good to me.
> > If I misunderstand, could you please explain a bit more?
> >
>
> It is indeed clear, but inode space is quite expensive, so we cannot
> add i_fsnotify_remote_mask field for all inodes and also its not
> necessary.

Agree, especially the point that remote masks are not for every inode.

> TBH, I do have a final picture of the opt-in API and there are several
> shapes that it could take.
>
> I think that we anyway need an "event mask flag" FAN_REMOTE,
> similar to FAN_ONDIR.
>
> A remote notification is generated by the filesystem and this source
> of notification always sets the FS_REMOTE in the event mask, which is
> visible to users reading the events.
>
> This makes it natural to also opt-in for remote events via the mask,
> some as is done with FAN_ONDIR.
>
> However, I would like to avoid the complexities involved with flipping
> this FS_REMOTE bit in event mask, that's why I was thinking about
> FAN_MARK_REMOTE that sets a flag in the mark, like FAN_MARK_EVICTABLE
> and forces all mark updates to use the same flag.

I also considered setting flag inside of mark, and using that to
distinguish local and remote.
The reason why I split mask was that the local and remote masks might
differ. However, I don't yet have any clear use case in my mind. So I
will follow your suggestion now.

> But for your first RFC, I suggest that you make it simple and use
> a group flag to request only remote notifications at fanotify_init
> time.
>
> This way you can take the existing cifs implementation of remote
> notifications using an ioctl and "bind" it to use the fanotify API
> with as little interferance with local notifications as possible.

Currently, I also think that the REMOTE flag should be attached to
mark(or similar level) in the end. But it's good to develop RFC step
by step. I'll follow your guidance as well.

> > > > If my inspection is wrong or you might have better idea, please let me
> > > > know about it. TBH, understanding new things is hard, but it's also a
> > > > happy moment to me.
> > > >
> > >
> > > I am relying on what I think you mean, but I may misunderstand you
> > > because you did not sketch any code samples, so I don't believe
> > > I fully understand all your concerns and ideas.
> > >
> > > Even an untested patch could help me understand if we are on the same page.
> >
> > Thanks for your advice. I think we're getting closer to the same page.
>
> Not quite there yet :)
>
> > I also attached patch of my current sketch (not tested and cleaned),
> > feel free to give your opinions.
>
> This is not what I was looking for.
>
> What I am looking for is a POC patch of binding cifs notifications
> to fsnotify/fanotify:
>
> - fanotify_mask() calls filesystem method to update server event mask
> - filesystem calls fnsotify() hook with FS_REMOTE flag
> - groups or marks that did not opt-in for remote notifications
>   would drop this event
>
> First milestone: be able to write program that listens to remote
> notifications only.
>
> Same program can use two groups remote and local with mask per type
> to request a mixture of local and remote events.
>
> Honestly, I don't think we need more than that?

Yeah.. No more this time. While digging about fsnotify, I missed some
important and origin interests. Thanks for reminding me through
setting the first milestone. I think it would be better to discuss
next topics after reaching the first milestone.

If there are no further questions (and hopefully there aren't), I'll
make and send the RFC patch.
Please bear with me if it takes some time and also thank you so much
for all your help.

> Thanks,
> Amir.
>
>

Best Regards.
Sang-Heon Jeon