* Re: [PATCH] Re: 2.6.21-rc7-mm2 -- hvsi console driver registration failure
2007-05-04 11:38 ` [PATCH] Re: 2.6.21-rc7-mm2 -- hvsi console driver registration failure Andy Whitcroft
@ 2007-05-04 19:04 ` Linas Vepstas
2007-05-04 20:09 ` Andrew Morton
1 sibling, 0 replies; 4+ messages in thread
From: Linas Vepstas @ 2007-05-04 19:04 UTC (permalink / raw)
To: Andy Whitcroft; +Cc: Andrew Morton, linuxppc-dev, gregkh, paulus, linux-kernel
On Fri, May 04, 2007 at 12:38:58PM +0100, Andy Whitcroft wrote:
>
> Trying to get 2.6.21-rc7-mm2 to boot on large PPC64 seems to be a
> bit of a challenge. We have been seeing panics on boot from the
> hvsi driver:
>
> Couldn't register hvsi console driver
>
> Tracking this back, this seems to come from hvsi driver trying to
> register itself via tty_register_driver() with a zero units.
>
> The failure is triggered by a change in semantics for kmalloc()
> between SLAB and SLUB; kmalloc(0) now returns NULL rather than an
> allocation at the smallest size. Looking at the code in question
> even when the allocation succeeds we will not actually use the
> memory when device->num is zero.
>
> It is not clear to me if this is a bug in the hvsi driver in that
> it should specify some units. It seems we will try and reserve zero
> devices in this case, which seems pointless.
Yes, it seems pointless to me ...
> I have tested with the patch below which seems safe to me and stops
> the errors and even seems to make the console work. But perhaps
> someone with more driver fu, could verify if driver->num of zero
> has any meaning and kick this to the hvsi people if not.
Hollis nominated me to be "hvsi people", although I'm near-totally
ignorant of the thing.
If hvsi_count is zero, then the device tree did not have any
"serial" nodes that speak "hvterm-protocol". The hvsi should not
have even tried to register anything. The attached patch seems more
to the point.
--linas
The hvsi driver is used whenever the device-tree contains
nodes for serial ports, and those serial ports speak the hvterm
protocol. However, if no such nodes are found, then the hvsi
driver should not even register.
This patch avoids a kernel panic with "Couldn't register hvsi
console driver".
In addition, this patch makes tty_register_driver refuse
to do anything, if there are no actual tty ports to be
registered.
Utterly & completely untested.
Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
----
drivers/char/hvsi.c | 4 ++++
drivers/char/tty_io.c | 3 +++
2 files changed, 7 insertions(+)
Index: linux-2.6.21-rc7-mm2/drivers/char/hvsi.c
===================================================================
--- linux-2.6.21-rc7-mm2.orig/drivers/char/hvsi.c 2007-04-26 15:37:33.000000000 -0500
+++ linux-2.6.21-rc7-mm2/drivers/char/hvsi.c 2007-05-04 13:55:56.000000000 -0500
@@ -1148,6 +1148,10 @@ static int __init hvsi_init(void)
{
int i;
+ /* No serial hvterm-protocol device-tree nodes found. */
+ if (hvsi_count == 0)
+ return 0;
+
hvsi_driver = alloc_tty_driver(hvsi_count);
if (!hvsi_driver)
return -ENOMEM;
Index: linux-2.6.21-rc7-mm2/drivers/char/tty_io.c
===================================================================
--- linux-2.6.21-rc7-mm2.orig/drivers/char/tty_io.c 2007-04-26 15:37:33.000000000 -0500
+++ linux-2.6.21-rc7-mm2/drivers/char/tty_io.c 2007-05-04 13:54:14.000000000 -0500
@@ -3724,6 +3724,9 @@ int tty_register_driver(struct tty_drive
if (driver->flags & TTY_DRIVER_INSTALLED)
return 0;
+ if (driver->num == 0)
+ return -ENODEV;
+
if (!(driver->flags & TTY_DRIVER_DEVPTS_MEM)) {
p = kmalloc(driver->num * 3 * sizeof(void *), GFP_KERNEL);
if (!p)
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] Re: 2.6.21-rc7-mm2 -- hvsi console driver registration failure
2007-05-04 11:38 ` [PATCH] Re: 2.6.21-rc7-mm2 -- hvsi console driver registration failure Andy Whitcroft
2007-05-04 19:04 ` Linas Vepstas
@ 2007-05-04 20:09 ` Andrew Morton
2007-05-04 21:21 ` Christoph Lameter
1 sibling, 1 reply; 4+ messages in thread
From: Andrew Morton @ 2007-05-04 20:09 UTC (permalink / raw)
To: Andy Whitcroft
Cc: linuxppc-dev, gregkh, paulus, linux-kernel, Christoph Lameter
On Fri, 04 May 2007 12:38:58 +0100
Andy Whitcroft <apw@shadowen.org> wrote:
>
> Trying to get 2.6.21-rc7-mm2 to boot on large PPC64 seems to be a
> bit of a challenge. We have been seeing panics on boot from the
> hvsi driver:
>
> Couldn't register hvsi console driver
>
> Tracking this back, this seems to come from hvsi driver trying to
> register itself via tty_register_driver() with a zero units.
>
> The failure is triggered by a change in semantics for kmalloc()
> between SLAB and SLUB; kmalloc(0) now returns NULL rather than an
> allocation at the smallest size. Looking at the code in question
> even when the allocation succeeds we will not actually use the
> memory when device->num is zero.
OK, thanks for working that out.
Christoph, we should be emitting loud warnings so that this problem is easy
to debug.
Better, we should be emitting loud warnigns which then disable themselves
and then succeeding the allocation so that people can proceed with their
kernel testing.
When all the loud-warning sites have been fixed, we can take that code out
again.
The present situation is maximally tester-hostile.
> It is not clear to me if this is a bug in the hvsi driver in that
> it should specify some units. It seems we will try and reserve zero
> devices in this case, which seems pointless.
>
> I have tested with the patch below which seems safe to me and stops
> the errors and even seems to make the console work. But perhaps
> someone with more driver fu, could verify if driver->num of zero
> has any meaning and kick this to the hvsi people if not.
>
> -apw
>
> === 8< ===
> tty_register_driver: only allocate tty instances when defined
>
> If device->num is zero we attempt to kmalloc() zero bytes.
> When SLUB is enabled this returns a null pointer and take that as
> an allocation failure and fail the device register. Check for no
> devices and avoid the allocation.
>
> Signed-off-by: Andy Whitcroft <apw@shadowen.org>
> ---
> diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
> index 959a616..71c4579 100644
> --- a/drivers/char/tty_io.c
> +++ b/drivers/char/tty_io.c
> @@ -3724,7 +3724,7 @@ int tty_register_driver(struct tty_driver *driver)
> if (driver->flags & TTY_DRIVER_INSTALLED)
> return 0;
>
> - if (!(driver->flags & TTY_DRIVER_DEVPTS_MEM)) {
> + if (!(driver->flags & TTY_DRIVER_DEVPTS_MEM) && driver->num) {
> p = kmalloc(driver->num * 3 * sizeof(void *), GFP_KERNEL);
> if (!p)
> return -ENOMEM;
^ permalink raw reply [flat|nested] 4+ messages in thread