metag test failure in -next due to commit 'module: relocate module_init from init.h to module.h'

metag test failure in -next due to commit 'module: relocate module_init from init.h to module.h'

[Guenter Roeck]

Hi,

My qemu metag runtime test fails in -next due to commit 5ebbb3badaa8 ("module:
relocate module_init from init.h to module.h"). Bisect log is as follows.
A log of a failed test is available at
http://server.roeck-us.net:8010/builders/qemu-metag-next/builds/173/steps/qemubuildcommand/logs/stdio

Essentially the test just hangs in boot.

Reverting the commit fixes the problem.

Guenter
---

# bad: [94feb25d6fcd34b0d45c18cf0185d83d0c13c60a] Add linux-next specific files for 20150604
# good: [c65b99f046843d2455aa231747b5a07a999a9f3d] Linux 4.1-rc6
git bisect start 'HEAD' 'v4.1-rc6'
# good: [46cb2717b4b827977bccd276096791d0efdb4119] Merge remote-tracking branch 'drm/drm-next'
git bisect good 46cb2717b4b827977bccd276096791d0efdb4119
# good: [37104644ae9384d6fc6574f16dc9782259ab89b1] Merge remote-tracking branch 'kvms390/next'
git bisect good 37104644ae9384d6fc6574f16dc9782259ab89b1
# good: [311942c925377a65157e4b14778a22bb05808314] Merge remote-tracking branch 'staging/staging-next'
git bisect good 311942c925377a65157e4b14778a22bb05808314
# good: [4b40a87efb321750a527aea556337f24b5268783] Merge remote-tracking branch 'clk/clk-next'
git bisect good 4b40a87efb321750a527aea556337f24b5268783
# good: [0cd6ba7205ab368b324d5a5f601aa3f5f89c246f] MAINTAINERS: update sound soc intel patterns
git bisect good 0cd6ba7205ab368b324d5a5f601aa3f5f89c246f
# good: [076edbd2f016f2fb4a80b95625cd65a9aed4e06c] Merge remote-tracking branch 'rtc/rtc-next'
git bisect good 076edbd2f016f2fb4a80b95625cd65a9aed4e06c
# good: [8c169d94c31fced71d907e26b19b4f5f0bd9d7bf] scsi-resolve-sg-buffer-const-ness-issue-fix
git bisect good 8c169d94c31fced71d907e26b19b4f5f0bd9d7bf
# good: [baaf163cd06a517d85dc39a6fa00107e3620b779] Merge branch 'module_init-device_initcall-4.1-rc6' into init-v4.1-rc6
git bisect good baaf163cd06a517d85dc39a6fa00107e3620b779
# good: [8f0b9591c464a9fd783b481b2c9cddf008043ba4] Merge branch 'module-builtin_driver-4.1-rc6' into init-v4.1-rc6
git bisect good 8f0b9591c464a9fd783b481b2c9cddf008043ba4
# bad: [fc3af372b8c176c08e8921d772ca2bff5620f15b] kernel/relay.c: use kvfree() in relay_free_page_array()
git bisect bad fc3af372b8c176c08e8921d772ca2bff5620f15b
# bad: [cdcdbb925bb730624d1312620263c0cf3666da5c] Merge remote-tracking branch 'init/init-v4.1-rc6'
git bisect bad cdcdbb925bb730624d1312620263c0cf3666da5c
# good: [3a283d1794f58a054f247152fb4aa15088aad561] mips: make loongsoon serial driver explicitly modular
git bisect good 3a283d1794f58a054f247152fb4aa15088aad561
# bad: [3491bfd125cd3d2b7654e2244fb1c262c7212049] Merge branch 'module-misc-4.1-rc6' into init-v4.1-rc6
git bisect bad 3491bfd125cd3d2b7654e2244fb1c262c7212049
# bad: [5ebbb3badaa8ffc50558af88cdea27497953356a] module: relocate module_init from init.h to module.h
git bisect bad 5ebbb3badaa8ffc50558af88cdea27497953356a
# first bad commit: [5ebbb3badaa8ffc50558af88cdea27497953356a] module: relocate module_init from init.h to module.h

Re: metag test failure in -next due to commit 'module: relocate module_init from init.h to module.h'

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 3814 bytes --]

On Fri, Jun 05, 2015 at 01:40:55PM -0700, Guenter Roeck wrote:
> My qemu metag runtime test fails in -next due to commit 5ebbb3badaa8 ("module:
> relocate module_init from init.h to module.h"). Bisect log is as follows.
> A log of a failed test is available at
> http://server.roeck-us.net:8010/builders/qemu-metag-next/builds/173/steps/qemubuildcommand/logs/stdio
> 
> Essentially the test just hangs in boot.
> 
> Reverting the commit fixes the problem.

Thanks for finding and bisecting Guenter!

Some build warnings from the same commit caught my eye, which made it a
quick one to figure out. Paul, please consider applying the patch below
before the offending commit "module: relocate module_init from init.h to
module.h", so as not to break bisection.

Cheers
James

From e12856c559d7dff2ad4f6497996610e12e7c7e2d Mon Sep 17 00:00:00 2001
From: James Hogan <james.hogan@imgtec.com>
Date: Fri, 5 Jun 2015 22:17:18 +0100
Subject: [PATCH] tty/metag_da: Avoid module_init/module_exit
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The metag_da TTY driver can't get built as a module at the moment, but
it still uses module_init() and module_exit(). Those macros are moving
to module.h which isn't included by metag_da.c, which will result in the
following build warnings (remarkably no build errors) and an apparent
failure to boot as the TTY driver won't be loaded.

drivers/tty/metag_da.c:660: warning: data definition has no type or storage class
drivers/tty/metag_da.c:660: warning: type defaults to ‘int’ in declaration of ‘module_init’
drivers/tty/metag_da.c:660: warning: parameter names (without types) in function declaration
drivers/tty/metag_da.c:661: warning: data definition has no type or storage class
drivers/tty/metag_da.c:661: warning: type defaults to ‘int’ in declaration of ‘module_exit’
drivers/tty/metag_da.c:661: warning: parameter names (without types) in function declaration
drivers/tty/metag_da.c:572: warning: ‘dashtty_init’ defined but not used
drivers/tty/metag_da.c:645: warning: ‘dashtty_exit’ defined but not used
drivers/tty/metag_da.c In function ‘dash_console_write’:
drivers/tty/metag_da.c:670 : warning: passing argument 4 of ‘chancall’ discards qualifiers from pointer target type

Instead of just adding the module.h include, now would be a good time to
remove the use of these macros, replacing the module_init with
device_initcall, and removing the exit function altogether since it
isn't needed. If module support is added later the code can always be
resurrected.

Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.cz>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: linux-metag@vger.kernel.org
---
 drivers/tty/metag_da.c | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/tty/metag_da.c b/drivers/tty/metag_da.c
index 3774600741d8..9325262289f9 100644
--- a/drivers/tty/metag_da.c
+++ b/drivers/tty/metag_da.c
@@ -640,25 +640,7 @@ err_destroy_ports:
 	put_tty_driver(channel_driver);
 	return ret;
 }
-
-static void dashtty_exit(void)
-{
-	int nport;
-	struct dashtty_port *dport;
-
-	del_timer_sync(&put_timer);
-	kthread_stop(dashtty_thread);
-	del_timer_sync(&poll_timer);
-	tty_unregister_driver(channel_driver);
-	for (nport = 0; nport < NUM_TTY_CHANNELS; nport++) {
-		dport = &dashtty_ports[nport];
-		tty_port_destroy(&dport->port);
-	}
-	put_tty_driver(channel_driver);
-}
-
-module_init(dashtty_init);
-module_exit(dashtty_exit);
+device_initcall(dashtty_init);
 
 #ifdef CONFIG_DA_CONSOLE
 
-- 
2.3.6


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: metag test failure in -next due to commit 'module: relocate module_init from init.h to module.h'

[Guenter Roeck]

On 06/05/2015 02:39 PM, James Hogan wrote:
[ ... ]
>
>  From e12856c559d7dff2ad4f6497996610e12e7c7e2d Mon Sep 17 00:00:00 2001
> From: James Hogan <james.hogan@imgtec.com>
> Date: Fri, 5 Jun 2015 22:17:18 +0100
> Subject: [PATCH] tty/metag_da: Avoid module_init/module_exit
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> The metag_da TTY driver can't get built as a module at the moment, but
> it still uses module_init() and module_exit(). Those macros are moving
> to module.h which isn't included by metag_da.c, which will result in the
> following build warnings (remarkably no build errors) and an apparent
> failure to boot as the TTY driver won't be loaded.
>
> drivers/tty/metag_da.c:660: warning: data definition has no type or storage class
> drivers/tty/metag_da.c:660: warning: type defaults to ‘int’ in declaration of ‘module_init’
> drivers/tty/metag_da.c:660: warning: parameter names (without types) in function declaration
> drivers/tty/metag_da.c:661: warning: data definition has no type or storage class
> drivers/tty/metag_da.c:661: warning: type defaults to ‘int’ in declaration of ‘module_exit’
> drivers/tty/metag_da.c:661: warning: parameter names (without types) in function declaration
> drivers/tty/metag_da.c:572: warning: ‘dashtty_init’ defined but not used
> drivers/tty/metag_da.c:645: warning: ‘dashtty_exit’ defined but not used
> drivers/tty/metag_da.c In function ‘dash_console_write’:
> drivers/tty/metag_da.c:670 : warning: passing argument 4 of ‘chancall’ discards qualifiers from pointer target type
>
> Instead of just adding the module.h include, now would be a good time to
> remove the use of these macros, replacing the module_init with
> device_initcall, and removing the exit function altogether since it
> isn't needed. If module support is added later the code can always be
> resurrected.
>
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jslaby@suse.cz>
> Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> Cc: linux-metag@vger.kernel.org

Yes, that does the trick.

Tested-by: Guenter Roeck <linux@roeck-us.net>

Thanks,
Guenter

Re: metag test failure in -next due to commit 'module: relocate module_init from init.h to module.h'

[Paul Gortmaker]

[Re: metag test failure in -next due to commit 'module: relocate module_init from init.h to module.h'] On 05/06/2015 (Fri 17:39) Guenter Roeck wrote:

> On 06/05/2015 02:39 PM, James Hogan wrote:
> [ ... ]
> >
> > From e12856c559d7dff2ad4f6497996610e12e7c7e2d Mon Sep 17 00:00:00 2001
> >From: James Hogan <james.hogan@imgtec.com>
> >Date: Fri, 5 Jun 2015 22:17:18 +0100
> >Subject: [PATCH] tty/metag_da: Avoid module_init/module_exit
> >MIME-Version: 1.0
> >Content-Type: text/plain; charset=UTF-8
> >Content-Transfer-Encoding: 8bit
> >
> >The metag_da TTY driver can't get built as a module at the moment, but
> >it still uses module_init() and module_exit(). Those macros are moving
> >to module.h which isn't included by metag_da.c, which will result in the
> >following build warnings (remarkably no build errors) and an apparent
> >failure to boot as the TTY driver won't be loaded.
> >
> >drivers/tty/metag_da.c:660: warning: data definition has no type or storage class
> >drivers/tty/metag_da.c:660: warning: type defaults to ‘int’ in declaration of ‘module_init’
> >drivers/tty/metag_da.c:660: warning: parameter names (without types) in function declaration
> >drivers/tty/metag_da.c:661: warning: data definition has no type or storage class
> >drivers/tty/metag_da.c:661: warning: type defaults to ‘int’ in declaration of ‘module_exit’
> >drivers/tty/metag_da.c:661: warning: parameter names (without types) in function declaration
> >drivers/tty/metag_da.c:572: warning: ‘dashtty_init’ defined but not used
> >drivers/tty/metag_da.c:645: warning: ‘dashtty_exit’ defined but not used
> >drivers/tty/metag_da.c In function ‘dash_console_write’:
> >drivers/tty/metag_da.c:670 : warning: passing argument 4 of ‘chancall’ discards qualifiers from pointer target type
> >
> >Instead of just adding the module.h include, now would be a good time to
> >remove the use of these macros, replacing the module_init with
> >device_initcall, and removing the exit function altogether since it
> >isn't needed. If module support is added later the code can always be
> >resurrected.
> >
> >Reported-by: Guenter Roeck <linux@roeck-us.net>
> >Signed-off-by: James Hogan <james.hogan@imgtec.com>
> >Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >Cc: Jiri Slaby <jslaby@suse.cz>
> >Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> >Cc: linux-metag@vger.kernel.org
> 
> Yes, that does the trick.
> 
> Tested-by: Guenter Roeck <linux@roeck-us.net>

Thanks guys, I'll add that to the module_init-device_initcall conversion
branch, which will put it ahead of the code relocation commit.

I think most other arch run with -Werror-implicit (or whatever it is) so
that these kinds of things would not "leak" past into a silent boot
fail like this.

Paul.
--

> 
> Thanks,
> Guenter
>