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

Groups > linux.kernel > #1218671

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

Show all headers | View raw

Am 02.09.2015 um 07:34 schrieb Alexander Holler:
> Am 01.09.2015 um 23:19 schrieb Brian Norris:
>> 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.
>
> I've already found at least a half a dozen other drivers with the same
> problem through my shuffling of the drivers which all end up in the
> standard initcall level device. I'm aware that this is based on the link
> order, which itself is based on linker behaviour (and maybe other things
> like make or other build tools). I'm just calling it luck, because this
> is nowhere explicitly stated, at least I've never seen such a statement,
> neither in any source, nor somewhere in Documentation. So I've choosen
> the term "by luck" in order to provoke a bit (or to stimulate a
> discussion about that widespread problem).

A good example why "luck" might not be far away from the truth is what 
happens when a drivers moves e.g. from staging to it's real position. 
Also the position will stay inside the same initcall level, the move of 
the driver into another directory (maybe together with a rename) will 
likely change its position in the initcall-sequence, without anyone 
really expecting 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.
>
> No real reason here. The names for the initcall levels seem to be
> outdated since a long time anyway, and I think just speaking about the
> numbers 1-7 (or 0-14) would be better anyways. The only reason why I've
> used the fs (sync) level is because I was too lazy to check out if
> mtdcore might depend on something else in any other level. Therefor I've
> used the one most close to were it was before.

Lazy was the wrong term. It is time consuming, cumbersome and therefor 
error-prone to check on what other stuff a driver depends. One reason 
why choosing the right place in the initcall sequence isn't that easy 
and why some automation make sense.

> Also I've an idea about how to fix that in general for all drivers (by
> using the same algorithm I've now introduced just for DT-based drivers
> with a device description). Wouldn't be that hard to use that for all
> drivers, but as I've written in a follow up to the introductory mail,
> one step after another.
>
> Regards,
>
> Alexander Holler

--
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