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
