Re: [PATCH] chardev: call tty_init() in real chrdev_init()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/7/22 1:03 AM, Greg KH wrote:
On Mon, Feb 07, 2022 at 12:27:31AM +0800, Xu Yu wrote:
It is confusing that tty_init() in called in the initialization of
memdev, i.e., static chr_dev_init().

Through blame, it is introduced by commit 31d1d48e199e ("Fix init
ordering of /dev/console vs callers of modprobe"), which fixes the
initialization order of /dev/console driver. However, there seems
to be a typo in the patch, i.e., chrdev_init, instead of chr_dev_init.

This fixes the typo, IIUC.

Note that the return value of tty_init() is always 0, and thus no error
handling is provided in chrdev_init().

Fixes: 31d1d48e199e ("Fix init ordering of /dev/console vs callers of modprobe")
Signed-off-by: Xu Yu <xuyu@xxxxxxxxxxxxxxxxx>
---
  drivers/char/mem.c | 2 +-
  fs/char_dev.c      | 1 +
  2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index cc296f0823bd..8c90881f8115 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -775,7 +775,7 @@ static int __init chr_dev_init(void)
  			      NULL, devlist[minor].name);
  	}
- return tty_init();
+	return 0;
  }
fs_initcall(chr_dev_init);
diff --git a/fs/char_dev.c b/fs/char_dev.c
index ba0ded7842a7..fc042a0a098f 100644
--- a/fs/char_dev.c
+++ b/fs/char_dev.c
@@ -667,6 +667,7 @@ static struct kobject *base_probe(dev_t dev, int *part, void *data)
  void __init chrdev_init(void)
  {
  	cdev_map = kobj_map_init(base_probe, &chrdevs_lock);
+	tty_init();
  }

You just changed the ordering sequence here, are you SURE this is
correct?

To be honest, not 100% sure.


How was this tested?  Did you verify that the problem that the original
commit here was fixing is now not happening again?

I tried to reproduce the issue described in the original commit, and
failed. The issue does not appear, or my reproduction is wrong.
  1. revert 31d1d48e199e manually;
  2. request_module("xxx") anywhere before do_initcalls(), since
     tty_init() now is initialized by module_init();
  3. no warning on request_module is shown.


And what real problem is this solving?  How did you hit the issue that
this solves?

No real problem actually. As described in the log, it is confusing that
tty_init() in called in the initialization of memdev. They don't have
strong dependencies. I found the issue when I read through codes of
drivers/char/mem.c.


And finally, yes, it is not good to throw away the return value of
tty_init().  If it really can not return anything but 0, then let us
make it a void function first.

Got it. But I will first try to figure out whether this patch is a real
issue.


thanks,

greg k-h

--
Thanks,
Yu



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux