VFS/inotify: send netlink messages when an inotify watch has been set or removed.

from: Stef Bon

I would like to apply a patch to the kernel to enable the sending of a netlink message when setting or removing an inotify watch. My goal is to make FUSE filesystems notify aware.

Since inotify works in the kernel space, and FUSE filesystems are in userspace, FUSE fs's do not "know" when a watch has been set or removed.

I think it's a good thing the fs "knows" about a watch, since it can then set a backend specific notify watch on the backend, and report anything back to the kernel when something changes.

The new netlink.c file is almost a copy of the netlink.c file in fs/quota.

I've got a testprogram which receives messages in the group GENERIC/VFS_INOTIFY.
If you want to test please email me. It just gives the information a watch has been set (by who, and where, which mask) and when removed (pid/fd/wd).

---

index b981fc0..07f9bfc 100644
@@ -15,3 +15,11 @@ config INOTIFY_USER
          For more information, see

<file:Documentation/filesystems/inotify.txt>

If unsure, say Y.
+
+config INOTIFY_USER_NETLINK_INTERFACE
+ bool "Report inotify add/remove watch messages through netlink
interface"
+ depends on INOTIFY_USER && NET
+ ---help---
+ If you say Y here, inotify messages (about a watch being set
or removed
+ , not the events!) will be reported through netlink interface.
If unsure,
+ say Y.

index a380dab..f977eaa 100644
@@ -1 +1,2 @@
 obj-$(CONFIG_INOTIFY_USER)     += inotify_fsnotify.o inotify_user.o
+obj-$(CONFIG_INOTIFY_USER_NETLINK_INTERFACE)   += netlink.o
\ No newline at end of file

b/fs/notify/inotify/inotify_user.c
index 8445fbc..3f056b9 100644

@@ -786,6 +786,9 @@ SYSCALL_DEFINE3(inotify_add_watch, int, fd, const

char __user *, pathname,

/* create/update an inode mark */
ret = inotify_update_watch(group, inode,
mask);
+

+ if ( ret>=0 ) inotify_send_add_message(fd, ret, mask,
pathname);
+

path_put(&path);

fput_and_out:

        fput_light(filp, fput_needed);
@@ -822,6 +825,8 @@ SYSCALL_DEFINE2(inotify_rm_watch, int, fd, __s32, wd)
        /* match ref taken by inotify_idr_find */
        fsnotify_put_mark(&i_mark->fsn_mark);

+ inotify_send_remove_message(fd, wd);
+
out:

        fput_light(filp, fput_needed);
        return ret;

new file mode 100644
index 0000000..b965c7f
@@ -0,0 +1,151 @@
+
+#include <linux/cred.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/inotify.h>
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+
+/* Netlink family structure for inotify */
+static struct genl_family inotify_genl_family = {
+       .id = GENL_ID_GENERATE,
+       .hdrsize = 0,
+       .name = "VFS_INOTIFY",
+       .version = 1,
+       .maxattr = INOTIFY_NL_A_MAX,
+};
+
+/**
+ * inotify_send_add_message
+ * @fd: inotify fd
+ * @wd: watch descriptor
+ * @mask: the mask
+ * @path: path
+ *
+ */
+
+void inotify_send_add_message(u32 fd, u32 wd, u32 mask, const char *path)
+{
+       static atomic_t seq;
+       struct sk_buff *skb;
+       void *msg_head;
+       int ret;
+       int msg_size = 4 * nla_total_size(sizeof(u32)) +

nla_total_size(strlen(path)) + 1;
+
+ skb = genlmsg_new(msg_size, GFP_KERNEL);
+
+ if (!skb) {
+
+ printk(KERN_ERR "VFS: Not enough memory to compose
inotify add wd netlink message.\n");
+ return;
+
+ }
+
+ msg_head = genlmsg_put(skb, 0, atomic_add_return(1, &seq),
+ &inotify_genl_family, 0, INOTIFY_NL_C_ADD_MESSAGE);
+
+ if (!msg_head) {
+
+ printk(KERN_ERR "VFS: Cannot store netlink header in
inotify add wd netlink message.\n");
+ goto err_out;
+
+ }
+
+ ret = nla_put_u32(skb, INOTIFY_NL_A_PID, current->pid);
+ if (ret) goto attr_err_out;
+
+ ret = nla_put_u32(skb, INOTIFY_NL_A_FD, fd);
+ if (ret) goto attr_err_out;
+
+ ret = nla_put_u32(skb, INOTIFY_NL_A_WD, wd);
+ if (ret) goto attr_err_out;
+
+
+ ret = nla_put_u32(skb, INOTIFY_NL_A_MASK, mask);
+ if (ret) goto attr_err_out;
+
+ ret = nla_put_string(skb, INOTIFY_NL_A_PATH, path);
+ if (ret) goto attr_err_out;
+
+ genlmsg_end(skb, msg_head);
+
+ genlmsg_multicast(skb, 0, inotify_genl_family.id, GFP_KERNEL);
+
+ return;
+attr_err_out:
+ printk(KERN_ERR "VFS: Error when writing attributes to inotify
add wd netlink message!\n");
+err_out:
+ kfree_skb(skb);
+}
+EXPORT_SYMBOL(inotify_send_add_message);
+
+/**
+ * inotify_send_remove_message
+ * @fd: inotify fd
+ * @wd: watch descriptor
+ *
+ */
+
+void inotify_send_remove_message(u32 fd, u32 wd)
+{

+       static atomic_t seq;
+       struct sk_buff *skb;
+       void *msg_head;
+       int ret;
+       int msg_size = 3 * nla_total_size(sizeof(u32));

+
+ skb = genlmsg_new(msg_size, GFP_KERNEL);
+
+ if (!skb) {
+
+ printk(KERN_ERR "VFS: Not enough memory to send inotify
remove wd netlink message.\n");
+ return;
+
+ }
+
+ msg_head = genlmsg_put(skb, 0, atomic_add_return(1, &seq),
+ &inotify_genl_family, 0,
INOTIFY_NL_C_REMOVE_MESSAGE);
+
+ if (!msg_head) {
+
+ printk(KERN_ERR "VFS: Cannot store netlink header in
inotify remove wd netlink message.\n");
+ goto err_out;
+
+ }
+
+ ret = nla_put_u32(skb, INOTIFY_NL_A_PID, current->pid);
+ if (ret) goto attr_err_out;
+
+ ret = nla_put_u32(skb, INOTIFY_NL_A_FD, fd);
+ if (ret) goto attr_err_out;
+
+ ret = nla_put_u32(skb, INOTIFY_NL_A_WD, wd);
+ if (ret) goto attr_err_out;
+
+ genlmsg_end(skb, msg_head);
+
+ genlmsg_multicast(skb, 0, inotify_genl_family.id, GFP_KERNEL);
+
+ return;
+attr_err_out:
+ printk(KERN_ERR "VFS: Error when writing attributes to inotify
remove wd netlink message\n");
+err_out:
+ kfree_skb(skb);
+}
+EXPORT_SYMBOL(inotify_send_remove_message);
+
+
+
+
+static int __init inotify_init(void)
+{
+ if (genl_register_family(&inotify_genl_family) != 0)
+ printk(KERN_ERR"VFS: Failed to create inotify_user
netlink interface.\n");
+ return 0;
+};
+
+module_init(inotify_init);

index d33041e..4ad0b4f 100644
@@ -84,4 +84,44 @@ extern struct ctl_table inotify_table[]; /* for sysctl */
 
 #endif
 
+/* commands */
+
+enum {
+       INOTIFY_NL_C_UNSPEC,
+       INOTIFY_NL_C_ADD_MESSAGE,
+       INOTIFY_NL_C_REMOVE_MESSAGE,
+       __INOTIFY_NL_C_MAX,
+};
+
+#define INOTIFY_NL_C_MAX (__INOTIFY_NL_C_MAX - 1)
+
+/* attributes */
+
+enum {
+       INOTIFY_NL_A_UNSPEC,
+       INOTIFY_NL_A_PID,
+       INOTIFY_NL_A_FD,
+       INOTIFY_NL_A_WD,
+       INOTIFY_NL_A_MASK,
+       INOTIFY_NL_A_PATH,
+       __INOTIFY_NL_A_MAX,
+};
+
+#define INOTIFY_NL_A_MAX (__INOTIFY_NL_A_MAX - 1)
+
+
+#ifdef CONFIG_INOTIFY_USER_NETLINK_INTERFACE
+extern void inotify_send_add_message(u32 fd, u32 wd, u32 mask, const

char *path);
+extern void inotify_send_remove_message(u32 fd, u32 wd); +#else
+static inline void inotify_send_add_message(u32 fd, u32 wd, u32 mask, const char *path)
+{
+ return;
+}
+static inline void inotify_send_remove_message(u32 fd, u32 wd); +{
+ return;
+}
+#endif /* CONFIG_INOTIFY_USER_NETLINK_INTERFACE */
+
#endif /* _LINUX_INOTIFY_H */

And what, pray tell, is the recepient to do with the contents of that packet? Pardon the bluntness, but how the hell is your FUSE fs supposed to tell that pathname has anything to do with it, nevermind telling which object would it relate to? Especially when it has no way to tell at which locations the damn thing happens to be mounted from the caller POV.

NAK.

2012/1/7 Al Viro <[email protected]>:

About bluntness, yes I've received blunt reactions from you in the past. We're working on the same goal here, and nobody is trying to misbehave, sabotage or do other nasty things so please be a bit more gentle. Maybe I'm not that into the kernel programming as you, but that is not a reason to get rude.

Futher, the FUSE fs knows it's own mountpoint. (ignore the submounts on a FUSE fs here), it can filter out the watches which are set on the fs.
This is not so hard right? What problems with "no way to tell at which locations the damn thing happens to" you are pointing at??

(When another fs is mounted on a directory part of a FUSE fs this is making things complicated, but not impossible, the fs needs to have a table of all mounted filesystems to determine a watch is set on a inode part of the fs or of another fs).

When it is set on an object part of the fs, the fs can then translate the path relative to the mountpoint to an inode used internally by the fs or any other reference used to identify objects. This has to exist.

The FUSE fs can then set a backend specific watch on the inode, and report anything back to the kernel when anything "happens", using the fuse_lowlevel_notify_.. calls.

I haven't tested this through and through, so I do not know yet the inotify subsystem will pick up the changes reported by the fuse_lowlevel_notify_..
calls.

Stef Bon

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Filesystem may be mounted at many places. The set of those locations depends on the process. It may change at any time. With no way for the filesystem to know about that (nor should it care, actually).

See mount(2). Pay particular attention to MS_MOVE and MS_BIND flags (== --move and --bind in mount(8)).

Moreover, it's not just maintaining the table of what is mounted where; different processes may bloody well have different sets of mounts.

And even assuming we started sending notifications on *everything*, it still would not be enough - how is your FUSE server supposed to distinguish between
* "add watch" notification sent
* pathname argument of inotify_add_watch() resolved (down in
inotify_find_inode())
* "mount --move" notification sent, mounted subtree moved
and
* "add watch" notification sent
* "mount --move" notification sent, mounted subtree moved
* pathname argument of inotify_add_watch() resolved (down in
inotify_find_inode())
when the netlink traffic it sees is identical in both cases? And results are very much _not_ the same...

IOW, what you are proposing is inherently racy.

BTW, how does FUSE fs know its mountpoint? Other than "my fs won't be mounted other than <here> and if you mount it elsewhere, it's your problem" I don't see any possible way to do that... Note that no information about the intended mountpoint reaches fuse_mount() (and it would've been useless there anyway, since mountpoint can change at literally zero notice, so the userland side of FUSE would have no use for it), which leaves only the apriori knowledge ("my code assumes that it'll be mounted here, violate that assumption at your peril")...

2012/1/7 Al Viro <[email protected]>:

Yes you've got a point here. Absolutly, the moving of a subtree is not noticed by the fuse fs, and therefore cannot be taken into account. And yes, I agree, that the only way to deal with that is "if you do this with the fuse fs, strange things can happen type of warning". I'm happy we agree on something!

But can you see a better way of making a FUSE fs (i)notify aware?? In my opinion FUSE fs's become so much more "responsive" when aware.

And in near future I think that such filesystems will be used/developed to access filestorage (cloudlike such as Google docs) on the Internet, next to access to it via the browser. Not to replace existing sollutions, but to add more functionality. When moving a lot of documents (or any relevant data/files) doing that with the browser is silly and unhandy. And I like it (and I think many do) when I just can browse through the files using my favorite file browser.

Access to the same files with a filesystem is then the solution.

And it will be very likely a FUSE fs, since it has to use an API which will very likely not be in the kernel. And doing so, it would be a very big plus when this filesystem is "(i)notify aware", you as user want to see a file shows up in your filebrowser when you've just created it using the webbrowser.

(there are already several filesystems to access webdav (oneCloud) for example, or google docs... I think there are more to come...)

Maybe you can do a suggestion howto solve this (thus making a FUSE fs inotify aware) better?

Stef Bon
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

There are almost certainly some things we agree on, but that's not the *only* way to deal with that. "Don't use inotify" is another.

And it's not just moving a subtree. mount --bind followed by umount of the original is another obvious example. So's having per-user namespaces, etc.

Moreover, exact same example of a race would still apply without mount being involved - have user pass /proc/6969/fd/42/bar/barf to inotify_add_watch() and replace mount --move with dup2().

Passing the pathname is *hopeless* here. And IMNSHO so's relying on inotify in general - it's not a general-purpose API and any program relying on it being usable on an arbitrary part of tree is buggy.

2012/1/7 Al Viro <[email protected]>:

Ok, but (i)notify is part of the kernel, so I do not have a choice here. One has to deal with it, if you like or not.

Pathname not good, what about an unique identifier per mountpoint and the inode?

Stef
Stef
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html