Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1217087

Re: [PATCH 15/16] mtd: mtdcore: fix initcall level

Show all headers | View raw

Hi Alexander,

No judgment here for the rest of this series, but for this patch:

On Wed, Aug 26, 2015 at 02:28:27PM +0200, Alexander Holler wrote:
> The mtd-core has to be initialized before other dependent mtd-drivers,
> otherwise a crash might occur.
> 
> Currently mtd_init() is called in the initcall-level device, which is the
> same level where most mtd-drivers will end up. By luck this seemed to have
> been called most of the time before other mtd-drivers without having been
> explicitly enforced.

I can't really speak for the original authors, but it does not appear to
be entirely "by luck." Link order was one of the de facto ways to get
this ordering (though it's not really a great one), and mtdcore was
always linked first within the drivers/mtd/ directory structure.

But that's just background, I think this is worth fixing anyway. It
could, for instance, become a problem if drivers are located outside
drivers/mtd/; I see random board files in arch/ that register with MTD,
and I'm actually not sure how they have never tripped on this.

> But if mtd_init() is not called before a dependent
> driver, a null-pointer exception might occur (e.g. because the mtd device
> class isn't registered).
> 
> To fix this, mtd-init() is moved to the initcall-level fs (right before
> the standard initcall level device).

Is there a good reason we shouldn't just make this a subsys_initcall()?
That would match the naming better, and it mirrors what, e.g.,
block/genhd uses. It would also allow flexibility if there are other
current/future use cases that might need to sit between the subsystem
and the drivers.

> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
> ---
>  drivers/mtd/mtdcore.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 8bbbb75..fa8e6452 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -1303,7 +1303,7 @@ static void __exit cleanup_mtd(void)
>  	bdi_destroy(&mtd_bdi);
>  }
>  
> -module_init(init_mtd);
> +fs_initcall_sync(init_mtd);

Why the *_sync() version? init_mtd() is very simple and doesn't have any
multithreading issues to handle.

>  module_exit(cleanup_mtd);
>  
>  MODULE_LICENSE("GPL");
> -- 
> 2.1.0
> 

Regards,
Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 00/16] deps: deterministic driver initialization order Alexander Holler <holler@ahsoftware.de> - 2015-08-26 14:30 +0200
  [PATCH 01/16] deps: dtc: Automatically add new property 'dependencies' which contains a list of referenced phandles Alexander Holler <holler@ahsoftware.de> - 2015-08-26 14:30 +0200
  [PATCH 02/16] deps: dtc: Add option to print initialization order Alexander Holler <holler@ahsoftware.de> - 2015-08-26 14:30 +0200
  [PATCH 16/16] phy: phy-core: fix initcall level Alexander Holler <holler@ahsoftware.de> - 2015-08-26 14:40 +0200
  [PATCH 11/16] deps: WIP: generic: annotate some initcalls Alexander Holler <holler@ahsoftware.de> - 2015-08-26 14:40 +0200
  [PATCH 09/16] deps: dts: imx6q: make some remote-endpoints non-dependencies Alexander Holler <holler@ahsoftware.de> - 2015-08-26 14:40 +0200
  [PATCH 07/16] deps: add debug configuration options Alexander Holler <holler@ahsoftware.de> - 2015-08-26 14:40 +0200
  [PATCH 05/16] deps: introduce initcalls annotated with a struct device_driver Alexander Holler <holler@ahsoftware.de> - 2015-08-26 14:40 +0200
  [PATCH 04/16] deps: dtc: introduce new (virtual) property no-dependencies Alexander Holler <holler@ahsoftware.de> - 2015-08-26 14:40 +0200
  [PATCH 12/16] deps: WIP: imx: annotate some initcalls Alexander Holler <holler@ahsoftware.de> - 2015-08-26 14:40 +0200
  [PATCH 06/16] deps: deterministic driver initialization sequence based on dependencies from the DT Alexander Holler <holler@ahsoftware.de> - 2015-08-26 14:40 +0200
  [PATCH 08/16] deps: dts: kirkwood: dockstar: add dependency ehci -> usb power regulator Alexander Holler <holler@ahsoftware.de> - 2015-08-26 14:40 +0200
  [PATCH 13/16] deps: WIP: omap: annotate some initcalls Alexander Holler <holler@ahsoftware.de> - 2015-08-26 14:40 +0200
  [PATCH 03/16] deps: dtc: Add option to print dependency graph as dot (Graphviz) Alexander Holler <holler@ahsoftware.de> - 2015-08-26 14:40 +0200
  [PATCH 14/16] deps: WIP: kirkwood: annotate some initcalls Alexander Holler <holler@ahsoftware.de> - 2015-08-26 14:40 +0200
  [PATCH 10/16] deps: dts: omap: beagle: make some remote-endpoints non-dependencies Alexander Holler <holler@ahsoftware.de> - 2015-08-26 14:40 +0200
  [PATCH 15/16] mtd: mtdcore: fix initcall level Alexander Holler <holler@ahsoftware.de> - 2015-08-26 14:40 +0200
    Re: [PATCH 15/16] mtd: mtdcore: fix initcall level Brian Norris <computersforpeace@gmail.com> - 2015-09-01 23:20 +0200
      Re: [PATCH 15/16] mtd: mtdcore: fix initcall level Alexander Holler <holler@ahsoftware.de> - 2015-09-02 07:40 +0200
        Re: [PATCH 15/16] mtd: mtdcore: fix initcall level Alexander Holler <holler@ahsoftware.de> - 2015-09-04 06:10 +0200
  Re: [PATCH 00/16] deps: deterministic driver initialization order Alexander Holler <holler@ahsoftware.de> - 2015-08-27 21:10 +0200
    Re: [PATCH 00/16] deps: deterministic driver initialization order Alexander Holler <holler@ahsoftware.de> - 2015-08-27 21:20 +0200
  deps: update in regard to parallel initialization of static linked  drivers Alexander Holler <holler@ahsoftware.de> - 2015-09-04 11:20 +0200

csiph-web