2023-06-15 16:43:40

by Marcel Holtmann

[permalink] [raw]
Subject: [PATCH] cachefiles: allocate static minor for /dev/cachefiles

The cachefiles misc character device uses MISC_DYNAMIC_MINOR and thus
doesn't support module auto-loading. Assign a static minor number for it
and provide appropriate module aliases for it. This is enough for kmod to
create the /dev/cachefiles device node on startup and facility module
auto-loading.

Signed-off-by: Marcel Holtmann <[email protected]>
---
Documentation/admin-guide/devices.txt | 3 ++-
fs/cachefiles/main.c | 4 +++-
include/linux/miscdevice.h | 1 +
3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/devices.txt b/Documentation/admin-guide/devices.txt
index 06c525e01ea5..21b2dda10006 100644
--- a/Documentation/admin-guide/devices.txt
+++ b/Documentation/admin-guide/devices.txt
@@ -376,8 +376,9 @@
240 = /dev/userio Serio driver testing device
241 = /dev/vhost-vsock Host kernel driver for virtio vsock
242 = /dev/rfkill Turning off radio transmissions (rfkill)
+ 243 = /dev/cachefiles Filesystem caching on files

- 243-254 Reserved for local use
+ 244-254 Reserved for local use
255 Reserved for MISC_DYNAMIC_MINOR

11 char Raw keyboard device (Linux/SPARC only)
diff --git a/fs/cachefiles/main.c b/fs/cachefiles/main.c
index 3f369c6f816d..eead7b5016a7 100644
--- a/fs/cachefiles/main.c
+++ b/fs/cachefiles/main.c
@@ -30,11 +30,13 @@ MODULE_PARM_DESC(cachefiles_debug, "CacheFiles debugging mask");
MODULE_DESCRIPTION("Mounted-filesystem based cache");
MODULE_AUTHOR("Red Hat, Inc.");
MODULE_LICENSE("GPL");
+MODULE_ALIAS("devname:cachefiles");
+MODULE_ALIAS_MISCDEV(CACHEFILES_MINOR);

struct kmem_cache *cachefiles_object_jar;

static struct miscdevice cachefiles_dev = {
- .minor = MISC_DYNAMIC_MINOR,
+ .minor = CACHEFILES_MINOR,
.name = "cachefiles",
.fops = &cachefiles_daemon_fops,
};
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index c0fea6ca5076..d7f989f593b0 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -71,6 +71,7 @@
#define USERIO_MINOR 240
#define VHOST_VSOCK_MINOR 241
#define RFKILL_MINOR 242
+#define CACHEFILES_MINOR 243
#define MISC_DYNAMIC_MINOR 255

struct device;
--
2.40.1



2023-06-22 13:56:25

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] cachefiles: allocate static minor for /dev/cachefiles

Marcel Holtmann <[email protected]> wrote:

> The cachefiles misc character device uses MISC_DYNAMIC_MINOR and thus
> doesn't support module auto-loading. Assign a static minor number for it
> and provide appropriate module aliases for it. This is enough for kmod to
> create the /dev/cachefiles device node on startup and facility module
> auto-loading.

Why? The systemd unit file for it just modprobe's the module first. It's a
specialist device file only for use by the appropriate daemon.

David


2023-06-22 14:59:11

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] cachefiles: allocate static minor for /dev/cachefiles

On Thu, Jun 22, 2023 at 02:42:54PM +0100, David Howells wrote:
> Marcel Holtmann <[email protected]> wrote:
>
> > The cachefiles misc character device uses MISC_DYNAMIC_MINOR and thus
> > doesn't support module auto-loading. Assign a static minor number for it
> > and provide appropriate module aliases for it. This is enough for kmod to
> > create the /dev/cachefiles device node on startup and facility module
> > auto-loading.
>
> Why? The systemd unit file for it just modprobe's the module first. It's a
> specialist device file only for use by the appropriate daemon.

And you really don't want to have auto-module-loading when trying to
open a /dev/foo file, that way lies madness in the past, please let's
learn from our mistakes :)

thanks,

greg k-h

2023-06-25 09:22:44

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] cachefiles: allocate static minor for /dev/cachefiles

On Thu, Jun 15, 2023 at 06:08:06PM +0200, Marcel Holtmann wrote:
> The cachefiles misc character device uses MISC_DYNAMIC_MINOR and thus
> doesn't support module auto-loading. Assign a static minor number for it
> and provide appropriate module aliases for it. This is enough for kmod to
> create the /dev/cachefiles device node on startup and facility module
> auto-loading.
>
> Signed-off-by: Marcel Holtmann <[email protected]>
> ---
> Documentation/admin-guide/devices.txt | 3 ++-
> fs/cachefiles/main.c | 4 +++-
> include/linux/miscdevice.h | 1 +
> 3 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/admin-guide/devices.txt b/Documentation/admin-guide/devices.txt
> index 06c525e01ea5..21b2dda10006 100644
> --- a/Documentation/admin-guide/devices.txt
> +++ b/Documentation/admin-guide/devices.txt
> @@ -376,8 +376,9 @@
> 240 = /dev/userio Serio driver testing device
> 241 = /dev/vhost-vsock Host kernel driver for virtio vsock
> 242 = /dev/rfkill Turning off radio transmissions (rfkill)
> + 243 = /dev/cachefiles Filesystem caching on files
>
> - 243-254 Reserved for local use
> + 244-254 Reserved for local use
> 255 Reserved for MISC_DYNAMIC_MINOR
>
> 11 char Raw keyboard device (Linux/SPARC only)
> diff --git a/fs/cachefiles/main.c b/fs/cachefiles/main.c
> index 3f369c6f816d..eead7b5016a7 100644
> --- a/fs/cachefiles/main.c
> +++ b/fs/cachefiles/main.c
> @@ -30,11 +30,13 @@ MODULE_PARM_DESC(cachefiles_debug, "CacheFiles debugging mask");
> MODULE_DESCRIPTION("Mounted-filesystem based cache");
> MODULE_AUTHOR("Red Hat, Inc.");
> MODULE_LICENSE("GPL");
> +MODULE_ALIAS("devname:cachefiles");
> +MODULE_ALIAS_MISCDEV(CACHEFILES_MINOR);
>
> struct kmem_cache *cachefiles_object_jar;
>
> static struct miscdevice cachefiles_dev = {
> - .minor = MISC_DYNAMIC_MINOR,
> + .minor = CACHEFILES_MINOR,
> .name = "cachefiles",
> .fops = &cachefiles_daemon_fops,
> };
> diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
> index c0fea6ca5076..d7f989f593b0 100644
> --- a/include/linux/miscdevice.h
> +++ b/include/linux/miscdevice.h
> @@ -71,6 +71,7 @@
> #define USERIO_MINOR 240
> #define VHOST_VSOCK_MINOR 241
> #define RFKILL_MINOR 242
> +#define CACHEFILES_MINOR 243
> #define MISC_DYNAMIC_MINOR 255
>
> struct device;
> --
> 2.40.1
>

Ah, the original was in my spam filter as your email does not show up as
being authenticated, you might want to fix your mail server :(

Anyway, as Christoph said, no, this isn't a good idea, keep it dynamic
and only load it if you need to load it please. Or, please explain why
it needs to be autoloaded, what dependancy or userspace program is not
working properly because of this?

thanks,

greg k-h