LWN: Comments on "watch_mount(), watch_sb(), and fsinfo() (again)"

watch_mount(), watch_sb(), and fsinfo() (again)

dhowells — Fri, 28 Feb 2020 14:43:54 +0000

I'm trying to make a generic notification system, with such properties as:
- messages can be longer than 8 bytes (say up to 128 bytes);
- message queueing requires no on-the-spot allocation and can be done from softirq/irq context or inside spinlocks;
- messages can come from a variety of sources (e.g. mount(2), add_key(2), keyctl(2), sb errors, usb notifications, ...);
- messages from different sources can be in the same queue;
- messages may need copying into multiple queues;
- filters can be easily employed;
- message loss reporting.

As has been mentioned, there are existing solutions:
- Netlink:
- would make the core VFS dependent on the networking code.
- would require GFP_ATOMIC message allocation at the point of generation (at least sometimes).
- doesn't seem to make it easy to do message loss reporting.
- epoll is for dealing with file descriptors - but we're not dealing with fd events.
- Not all sources I need notifications for can be addressed with fanotify.
- eventfd() has 8-byte messages.

Pipes have disadvantages too: no message loss reporting (I'm having to add it); pipe ring buffer metadata elements generally larger than common messages (might be able to optimise in future to store small data in the ring). Note that I didn't want to use pipes either: I was originally using a mappable chardev ring buffer but Linus said I had to use pipes instead.

watch_mount(), watch_sb(), and fsinfo() (again)

dhowells — Fri, 28 Feb 2020 14:15:44 +0000

eventfd is limited to 8-byte messages. That is not sufficient.

watch_mount(), watch_sb(), and fsinfo() (again)

kugel — Thu, 27 Feb 2020 14:47:39 +0000

There is also eventfd(2)

A pipe seems really awkward for one-way event notifications.

watch_mount(), watch_sb(), and fsinfo() (again)

Paf — Wed, 26 Feb 2020 14:25:01 +0000

This doesn’t really seem like a big deal - novel and different mechanisms are a problem when they’re, well, novel and different. This is just a pipe, and unless I’ve missed something, it’s not requiring implementing anything.

I mean maybe netlink would be superior, but this seems nicely trivial. It’s not like they’re knocking up a netlink competitor, just using a trivial interface where they can.

watch_mount(), watch_sb(), and fsinfo() (again)

maxfragg — Wed, 26 Feb 2020 07:24:36 +0000

not sure if the fact that this existed so long without a lot of people noticing makes it better or worse.
But I have to agree, having one encouraged interface (probably netlink, or maybe even epoll here?) to be used for any new interface of this type would be a good idea

watch_mount(), watch_sb(), and fsinfo() (again)

cyphar — Wed, 26 Feb 2020 02:19:49 +0000

Indeed, that was the entire point of copy_struct_from_user() -- so that new syscalls can be extensible and avoid the need to predict how many future fields you might want (I gave a talk about this at LCA this year). I've sent a suggestion to the ML, though there is some discussion on the latest thread to remove the need for fsinfo(2) entirely.

watch_mount(), watch_sb(), and fsinfo() (again)

neilbrown — Tue, 25 Feb 2020 22:36:23 +0000

This *is* an existing mechanism. autofs has been using a pipe for notification for decades.

watch_mount(), watch_sb(), and fsinfo() (again)

alison — Tue, 25 Feb 2020 14:38:17 +0000

>Why not extend fanotify?

The author of "man fanotify" appears to have the same question:

The fanotify API provides notification and interception of filesystem events.
Use cases include virus scanning and hierarchical storage management. **Cur‐
rently, only a limited set of events is supported.**

watch_mount(), watch_sb(), and fsinfo() (again)

Cyberax — Tue, 25 Feb 2020 10:11:57 +0000

Yes, I actually wanted to ask the same. Why not extend fanotify?

watch_mount(), watch_sb(), and fsinfo() (again)

geert — Tue, 25 Feb 2020 08:00:43 +0000

And the explicit padding between an odd number of 32-bit fields, and the 64-bit fields.
Fortunately the latter is present in the real declaration:

__u32 __reserved32[1]; /* Reserved params; all must be 0 */

Perhaps that can be traded for an explicit structure length?

watch_mount(), watch_sb(), and fsinfo() (again)

smurf — Tue, 25 Feb 2020 07:00:19 +0000

Regardless of whether netlink would be a good way to do it, this is yet another way to get a string of notifications from the kernel. I do wonder why nobody is complaining loudly that this should use, or maybe improve upon, one of the existing mechanisms.

watch_mount(), watch_sb(), and fsinfo() (again)

anguslees — Tue, 25 Feb 2020 04:32:07 +0000

Is netlink an acceptable alternative to the proposed notification pipe, or are the required semantics different?

watch_mount(), watch_sb(), and fsinfo() (again)

Cyberax — Mon, 24 Feb 2020 23:09:36 +0000

> struct fsinfo_params {
> __u32 at_flags;
Sigh.

You know what's missing here? Yes, it's the explicit structure length.