| Message ID | 164237084692.24166.3761469608708322913@noble.neil.brown.name (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [-,resend] devtmpfs regression fix: reconfigure on each mount | expand |
On Mon, Jan 17, 2022 at 12:07 AM NeilBrown <neilb@suse.de> wrote: > > Since that was changed, the mount options used for devtmpfs are ignored. > This is a regression which affect systemd - which mounts devtmpfs > with "-o mode=755,size=4m,nr_inodes=1m". Hmm, I've applied this, but I'd have loved to see what the actual symptoms of the problem were. Particularly since it's apparently been broken for 18 months with this being the first I hear of it. Yes, yes, I could (and did) search for this on the mailing lists, and found the discussion and more information, but I think that info should have been in the commit message rather than me having to go look for it just to see the clarifications.. Linus
On Mon, Jan 17, 2022 at 09:07:26AM +1100, NeilBrown wrote: > > Prior to Linux v5.4 devtmpfs used mount_single() which treats the given > mount options as "remount" options, so it updates the configuration of the > single super_block on each mount. > Since that was changed, the mount options used for devtmpfs are ignored. > This is a regression which affect systemd - which mounts devtmpfs > with "-o mode=755,size=4m,nr_inodes=1m". > > This patch restores the "remount" effect by calling reconfigure_single() > > Fixes: d401727ea0d7 ("devtmpfs: don't mix {ramfs,shmem}_fill_super() with mount_single()") > Acked-by: Christian Brauner <christian.brauner@ubuntu.com> > Signed-off-by: NeilBrown <neilb@suse.de> > --- > drivers/base/devtmpfs.c | 7 +++++++ > fs/super.c | 4 ++-- > include/linux/fs_context.h | 2 ++ > 3 files changed, 11 insertions(+), 2 deletions(-) Sorry, I thought Al was going to take this as the regression came from his tree, I should have picked it up earlier. I'll queue it up after 5.17-rc1 is out. thanks, greg k-h
On Mon, Jan 17, 2022 at 09:03:58AM +0100, Greg Kroah-Hartman wrote: > On Mon, Jan 17, 2022 at 09:07:26AM +1100, NeilBrown wrote: > > > > Prior to Linux v5.4 devtmpfs used mount_single() which treats the given > > mount options as "remount" options, so it updates the configuration of the > > single super_block on each mount. > > Since that was changed, the mount options used for devtmpfs are ignored. > > This is a regression which affect systemd - which mounts devtmpfs > > with "-o mode=755,size=4m,nr_inodes=1m". > > > > This patch restores the "remount" effect by calling reconfigure_single() > > > > Fixes: d401727ea0d7 ("devtmpfs: don't mix {ramfs,shmem}_fill_super() with mount_single()") > > Acked-by: Christian Brauner <christian.brauner@ubuntu.com> > > Signed-off-by: NeilBrown <neilb@suse.de> > > --- > > drivers/base/devtmpfs.c | 7 +++++++ > > fs/super.c | 4 ++-- > > include/linux/fs_context.h | 2 ++ > > 3 files changed, 11 insertions(+), 2 deletions(-) > > Sorry, I thought Al was going to take this as the regression came from > his tree, I should have picked it up earlier. I'll queue it up after > 5.17-rc1 is out. Ah, nevermind, Linus just took it. thanks, greg k-h
On Mon, 17 Jan 2022, Linus Torvalds wrote: > On Mon, Jan 17, 2022 at 12:07 AM NeilBrown <neilb@suse.de> wrote: > > > > Since that was changed, the mount options used for devtmpfs are ignored. > > This is a regression which affect systemd - which mounts devtmpfs > > with "-o mode=755,size=4m,nr_inodes=1m". > > Hmm, I've applied this, but I'd have loved to see what the actual > symptoms of the problem were. Particularly since it's apparently been > broken for 18 months with this being the first I hear of it. > > Yes, yes, I could (and did) search for this on the mailing lists, and > found the discussion and more information, but I think that info > should have been in the commit message rather than me having to go > look for it just to see the clarifications.. Sorry about that. The trail was a bit convoluted and I hadn't bothered to straighten it out as the behavioural change was so easy to demonstrate. I've had a better look now. A customer with a 5.3 based kernel reported that udev was having problems creating all the symlinks for lots of LUNs for some storage array (With dm devices over the LUNs etc). It ran out of inodes because systemd always mounts devtmpfs with size=4m,nr_inodes=64k This was bumped to 128k and then to 1m in systemd-v250. We provided our customer with a systemd which used a larger limit, but when we tested this fix on Tumbleweed (with a newer kernel), we noticed that it had no effect. Now admittedly the default provided by the kernel is a lot bigger than even the current 1m setting from systemd so maybe this doesn't matter. Had the commit which changed behaviour said "systemd sets nr_inodes to a stupidly low number, let's just ignore the mount options", then I probably would have accept it. But it looked like behaviour change without justification and that suggests a regression. So I patched it. Thanks, NeilBrown
diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c index 1e2c2d3882e2..f41063ac1aee 100644 --- a/drivers/base/devtmpfs.c +++ b/drivers/base/devtmpfs.c @@ -65,8 +65,15 @@ static struct dentry *public_dev_mount(struct file_system_type *fs_type, int fla const char *dev_name, void *data) { struct super_block *s = mnt->mnt_sb; + int err; + atomic_inc(&s->s_active); down_write(&s->s_umount); + err = reconfigure_single(s, flags, data); + if (err < 0) { + deactivate_locked_super(s); + return ERR_PTR(err); + } return dget(s->s_root); } diff --git a/fs/super.c b/fs/super.c index 3bfc0f8fbd5b..a6405d44d4ca 100644 --- a/fs/super.c +++ b/fs/super.c @@ -1423,8 +1423,8 @@ struct dentry *mount_nodev(struct file_system_type *fs_type, } EXPORT_SYMBOL(mount_nodev); -static int reconfigure_single(struct super_block *s, - int flags, void *data) +int reconfigure_single(struct super_block *s, + int flags, void *data) { struct fs_context *fc; int ret; diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h index 6b54982fc5f3..13fa6f3df8e4 100644 --- a/include/linux/fs_context.h +++ b/include/linux/fs_context.h @@ -142,6 +142,8 @@ extern void put_fs_context(struct fs_context *fc); extern int vfs_parse_fs_param_source(struct fs_context *fc, struct fs_parameter *param); extern void fc_drop_locked(struct fs_context *fc); +int reconfigure_single(struct super_block *s, + int flags, void *data); /* * sget() wrappers to be called from the ->get_tree() op.
Prior to Linux v5.4 devtmpfs used mount_single() which treats the given mount options as "remount" options, so it updates the configuration of the single super_block on each mount. Since that was changed, the mount options used for devtmpfs are ignored. This is a regression which affect systemd - which mounts devtmpfs with "-o mode=755,size=4m,nr_inodes=1m". This patch restores the "remount" effect by calling reconfigure_single() Fixes: d401727ea0d7 ("devtmpfs: don't mix {ramfs,shmem}_fill_super() with mount_single()") Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: NeilBrown <neilb@suse.de> --- drivers/base/devtmpfs.c | 7 +++++++ fs/super.c | 4 ++-- include/linux/fs_context.h | 2 ++ 3 files changed, 11 insertions(+), 2 deletions(-)