[PATCH] perf/x86/intel: fix 2 typos

2019-03-15 Thread Alexander Holler
I don't know if fix is functional correct, but at least it compiles now.
It failed before with a config for an AMD CPU.

Bug was introduced with commit d01b1f96a82e5dd7841a1d39db3abfdaf95f70ab
"perf/x86/intel: Make cpuc allocations consistent" which was merged
with 004cc08675b761fd82288bab1b5ba5e1ca746eca.

Signed-off-by: Alexander Holler 
Cc: Peter Zijlstra (Intel) 
Cc: Thomas Gleixner 
Cc: 
---
 arch/x86/events/perf_event.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index b04ae6c..a759557 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -1033,12 +1033,12 @@ static inline int intel_pmu_init(void)
return 0;
 }
 
-static inline int intel_cpuc_prepare(struct cpu_hw_event *cpuc, int cpu)
+static inline int intel_cpuc_prepare(struct cpu_hw_events *cpuc, int cpu)
 {
return 0;
 }
 
-static inline void intel_cpuc_finish(struct cpu_hw_event *cpuc)
+static inline void intel_cpuc_finish(struct cpu_hw_events *cpuc)
 {
 }
 
-- 
2.9.5



Re: [PATCH 4.3 0/2] 4.3.2-stable review

2015-12-10 Thread Alexander Holler

Am 10.12.2015 um 19:03 schrieb Greg Kroah-Hartman:

This is the start of the stable review cycle for the 4.3.2 release.
There are 2 patches in this series, all will be posted as a response
to this one.  If anyone has any issues with these being applied, please
let me know.

Responses should be made by Fri Dec 11 18:02:18 UTC 2015.
Anything received after that time might be too late.

The whole patch series can be found in one patch at:
kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.3.2-rc1.gz
and the diffstat can be found below.


Tested successfully by running a kernel with those two patches. Thanks a 
lot for the quick reaction.


Regards,

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


Re: [PATCH] X.509: Fix the time validation [ver #3]

2015-12-10 Thread Alexander Holler

Am 10.12.2015 um 19:09 schrieb Greg Kroah-Hartman:


We already have one other report of this problem hitting them.  I've now
released 4.3.2-rc1, with a "quick" review period of 24 hours before I
release 4.3.2 with just this fix.  If you could verify I didn't mess
anything up I would appreciate it.


After applying those two patches from 4.3.2-rc1 you've posted instead of 
the one I've cherry-picked, git diff ended up with no difference to the 
source of the kernel I'm currently running. Reading those two patches 
also looks good.


Thanks a lot.

Alexander Holler

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


Re: [PATCH] X.509: Fix the time validation [ver #3]

2015-12-10 Thread Alexander Holler

Am 10.12.2015 um 16:34 schrieb Alexander Holler:

Am 10.12.2015 um 16:26 schrieb Greg Kroah-Hartman:

On Thu, Dec 10, 2015 at 04:15:22PM +0100, Alexander Holler wrote:



Just in case of, I would suggest to quickly push out 4.3.2 (only 4.3
seems
to be affected) which contains at least the patch mentioned in the
subject
(58585c1fc301a36625db41ac7078c4dd0a218d84 in mainline).


58585c1fc301a36625db41ac7078c4dd0a218d84 doesn't reference anything in
Linus's tree, where did you get that git commit id?


Uh, hmm, maybe I've picked the wrong commit number when I've used git
gui blame to find the original commit. Might have been one from my own
trees which are based on mainline. Sorry, having had a second look, the
one I've cherry-picked from mainline was
cc25b994acfbc901429da682d0f73c190e960206


To give my motivation for that mail (and that "quickly"): it's highly 
annoying to end up with a box which does not have network and, as in my 
case, even without working input devices because modules weren't loaded. 
And other people don't might be able to find the problem (and existing 
patch) as quick as I did and might end up even more annoyed than I was 
for a short period of time. ;)



Regards,

Alexander Holler


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


Re: [PATCH] X.509: Fix the time validation [ver #3]

2015-12-10 Thread Alexander Holler

Am 10.12.2015 um 16:26 schrieb Greg Kroah-Hartman:

On Thu, Dec 10, 2015 at 04:15:22PM +0100, Alexander Holler wrote:



Just in case of, I would suggest to quickly push out 4.3.2 (only 4.3 seems
to be affected) which contains at least the patch mentioned in the subject
(58585c1fc301a36625db41ac7078c4dd0a218d84 in mainline).


58585c1fc301a36625db41ac7078c4dd0a218d84 doesn't reference anything in
Linus's tree, where did you get that git commit id?


Uh, hmm, maybe I've picked the wrong commit number when I've used git 
gui blame to find the original commit. Might have been one from my own 
trees which are based on mainline. Sorry, having had a second look, the 
one I've cherry-picked from mainline was 
cc25b994acfbc901429da682d0f73c190e960206


Regards,

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


Re: [PATCH] X.509: Fix the time validation [ver #3]

2015-12-10 Thread Alexander Holler

Am 10.12.2015 um 10:23 schrieb Alexander Holler:

Am 12.11.2015 um 12:38 schrieb David Howells:

This fixes CVE-2015-5327.  It affects kernels from 4.3-rc1 onwards.

Fix the X.509 time validation to use month number-1 when looking up the
number of days in that month.  Also put the month number validation
before
doing the lookup so as not to risk overrunning the array.


I've just run into this with 4.3.1 (mon_len ended up with 0 because of
the wrong index). Which means currently build stable kernels with
signature verification might not load modules (depending on which value
the invalid index mon_len (12) ends up with.


Just in case of, I would suggest to quickly push out 4.3.2 (only 4.3 
seems to be affected) which contains at least the patch mentioned in the 
subject (58585c1fc301a36625db41ac7078c4dd0a218d84 in mainline).


Regards,

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


Re: [PATCH] X.509: Fix the time validation [ver #3]

2015-12-10 Thread Alexander Holler

Am 12.11.2015 um 12:38 schrieb David Howells:

This fixes CVE-2015-5327.  It affects kernels from 4.3-rc1 onwards.

Fix the X.509 time validation to use month number-1 when looking up the
number of days in that month.  Also put the month number validation before
doing the lookup so as not to risk overrunning the array.


I've just run into this with 4.3.1 (mon_len ended up with 0 because of 
the wrong index). Which means currently build stable kernels with 
signature verification might not load modules (depending on which value 
the invalid index mon_len (12) ends up with.


Regards,

Alexander Holler

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


Re: [PATCH] X.509: Fix the time validation [ver #3]

2015-12-10 Thread Alexander Holler

Am 12.11.2015 um 12:38 schrieb David Howells:

This fixes CVE-2015-5327.  It affects kernels from 4.3-rc1 onwards.

Fix the X.509 time validation to use month number-1 when looking up the
number of days in that month.  Also put the month number validation before
doing the lookup so as not to risk overrunning the array.


I've just run into this with 4.3.1 (mon_len ended up with 0 because of 
the wrong index). Which means currently build stable kernels with 
signature verification might not load modules (depending on which value 
the invalid index mon_len (12) ends up with.


Regards,

Alexander Holler

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


Re: [PATCH] X.509: Fix the time validation [ver #3]

2015-12-10 Thread Alexander Holler

Am 10.12.2015 um 16:34 schrieb Alexander Holler:

Am 10.12.2015 um 16:26 schrieb Greg Kroah-Hartman:

On Thu, Dec 10, 2015 at 04:15:22PM +0100, Alexander Holler wrote:



Just in case of, I would suggest to quickly push out 4.3.2 (only 4.3
seems
to be affected) which contains at least the patch mentioned in the
subject
(58585c1fc301a36625db41ac7078c4dd0a218d84 in mainline).


58585c1fc301a36625db41ac7078c4dd0a218d84 doesn't reference anything in
Linus's tree, where did you get that git commit id?


Uh, hmm, maybe I've picked the wrong commit number when I've used git
gui blame to find the original commit. Might have been one from my own
trees which are based on mainline. Sorry, having had a second look, the
one I've cherry-picked from mainline was
cc25b994acfbc901429da682d0f73c190e960206


To give my motivation for that mail (and that "quickly"): it's highly 
annoying to end up with a box which does not have network and, as in my 
case, even without working input devices because modules weren't loaded. 
And other people don't might be able to find the problem (and existing 
patch) as quick as I did and might end up even more annoyed than I was 
for a short period of time. ;)



Regards,

Alexander Holler


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


Re: [PATCH] X.509: Fix the time validation [ver #3]

2015-12-10 Thread Alexander Holler

Am 10.12.2015 um 19:09 schrieb Greg Kroah-Hartman:


We already have one other report of this problem hitting them.  I've now
released 4.3.2-rc1, with a "quick" review period of 24 hours before I
release 4.3.2 with just this fix.  If you could verify I didn't mess
anything up I would appreciate it.


After applying those two patches from 4.3.2-rc1 you've posted instead of 
the one I've cherry-picked, git diff ended up with no difference to the 
source of the kernel I'm currently running. Reading those two patches 
also looks good.


Thanks a lot.

Alexander Holler

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


Re: [PATCH 4.3 0/2] 4.3.2-stable review

2015-12-10 Thread Alexander Holler

Am 10.12.2015 um 19:03 schrieb Greg Kroah-Hartman:

This is the start of the stable review cycle for the 4.3.2 release.
There are 2 patches in this series, all will be posted as a response
to this one.  If anyone has any issues with these being applied, please
let me know.

Responses should be made by Fri Dec 11 18:02:18 UTC 2015.
Anything received after that time might be too late.

The whole patch series can be found in one patch at:
kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.3.2-rc1.gz
and the diffstat can be found below.


Tested successfully by running a kernel with those two patches. Thanks a 
lot for the quick reaction.


Regards,

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


Re: [PATCH] X.509: Fix the time validation [ver #3]

2015-12-10 Thread Alexander Holler

Am 10.12.2015 um 16:26 schrieb Greg Kroah-Hartman:

On Thu, Dec 10, 2015 at 04:15:22PM +0100, Alexander Holler wrote:



Just in case of, I would suggest to quickly push out 4.3.2 (only 4.3 seems
to be affected) which contains at least the patch mentioned in the subject
(58585c1fc301a36625db41ac7078c4dd0a218d84 in mainline).


58585c1fc301a36625db41ac7078c4dd0a218d84 doesn't reference anything in
Linus's tree, where did you get that git commit id?


Uh, hmm, maybe I've picked the wrong commit number when I've used git 
gui blame to find the original commit. Might have been one from my own 
trees which are based on mainline. Sorry, having had a second look, the 
one I've cherry-picked from mainline was 
cc25b994acfbc901429da682d0f73c190e960206


Regards,

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


Re: [PATCH] X.509: Fix the time validation [ver #3]

2015-12-10 Thread Alexander Holler

Am 10.12.2015 um 10:23 schrieb Alexander Holler:

Am 12.11.2015 um 12:38 schrieb David Howells:

This fixes CVE-2015-5327.  It affects kernels from 4.3-rc1 onwards.

Fix the X.509 time validation to use month number-1 when looking up the
number of days in that month.  Also put the month number validation
before
doing the lookup so as not to risk overrunning the array.


I've just run into this with 4.3.1 (mon_len ended up with 0 because of
the wrong index). Which means currently build stable kernels with
signature verification might not load modules (depending on which value
the invalid index mon_len (12) ends up with.


Just in case of, I would suggest to quickly push out 4.3.2 (only 4.3 
seems to be affected) which contains at least the patch mentioned in the 
subject (58585c1fc301a36625db41ac7078c4dd0a218d84 in mainline).


Regards,

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


Re: [PATCH 0/14] init: deps: dependency based (parallelized) init

2015-11-06 Thread Alexander Holler

Hello,

in order to "conserve" the patches, I've setup a repository named 
parallelized_kernel_init at github.


I've just put patches for 4.3 into that repository, also I don't know if 
and how long I will use and maintain this series.


Regards,

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


Re: [PATCH 0/14] init: deps: dependency based (parallelized) init

2015-11-06 Thread Alexander Holler

Hello,

in order to "conserve" the patches, I've setup a repository named 
parallelized_kernel_init at github.


I've just put patches for 4.3 into that repository, also I don't know if 
and how long I will use and maintain this series.


Regards,

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-22 Thread Alexander Holler

Am 19.10.2015 um 13:31 schrieb Alexander Holler:

Am 19.10.2015 um 12:57 schrieb Alexander Holler:

Am 18.10.2015 um 12:11 schrieb Alexander Holler:

Am 18.10.2015 um 07:59 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 07:20:34AM +0200, Alexander Holler wrote:

Am 18.10.2015 um 07:14 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 06:59:22AM +0200, Alexander Holler wrote:

Am 17.10.2015 um 21:36 schrieb Greg Kroah-Hartman:


Again, parallelizing does not solve anything, and causes more
problems
_and_ makes things take longer.  Try it, we have done it in the
past and
proven this, it's pretty easy to test :)


Just because I'm curious, may I ask how I would test that in the
easy way
you have in mind? I've just posted the results of my tests (the
patch
series) but I wonder what you do have in mind.


Use the tool, scripts/bootgraph.pl to create a boot graph of your
boot
sequence.  That should show you the drivers, or other areas, that are
causing your boot to be "slow".


So I've misunderstood you. I've read your paragraph as that it's
easy to
test parallelizing.


Ah, ok, if you want to parallelize everything, add some logic in the
driver core where the probe() callback is made to spin that off into a
new thread for every call, and when it's done, clean up the thread.
That's what I did many years ago to try this all out, if you dig in the
lkml archives there's probably a patch somewhere that you can base the
work off of to test it yourself.


Hmm, I don't think I will do that because that means to setup a new
thread for every call. And it doesn't need much imagination (or
experience) that this introduces quite some overhead.

But maybe it makes sense to try out what I'm doing in my patches,
starting multiple threads once and then just giving them some work. Will


After a having second thought on your simple approach to parallelize
stuff, I have to say that it just can't work because just starting a
thread for every probe() totally ignores possible dependencies.
Regardless if using one thread per probe() call or if feeding probe()
calls to just a few threads.

Maybe that's why previous attempts to parallelize stuff failed. But
that's just an assumption as I'm unaware of these previous attempts.


Or to describe it more verbose, if DEBUG is turned on in
init/dependencies.c (using my patches), it spits out a summary of groups
with initcalls (probe() calls) which are independent from each other and
therfore can be called in parallel. E.g. one of my systems this looks so:

[0.288229] init: vertices: 429 edges 204 count 170
[0.288295] init: group 0 length 66 (start 0)
[0.288329] init: group 1 length 33 (start 66)
[0.288364] init: group 2 length 13 (start 99)
[0.288398] init: group 3 length 7 (start 112)
[0.288432] init: group 4 length 9 (start 119)
[0.288466] init: group 5 length 8 (start 128)
[0.288500] init: group 6 length 11 (start 136)
[0.288534] init: group 7 length 6 (start 147)
[0.288569] init: group 8 length 4 (start 153)
[0.288603] init: group 9 length 8 (start 157)
[0.288637] init: group 10 length 3 (start 165)
[0.288671] init: group 11 length 2 (start 168)
[0.288705] init: using 4 threads to call annotated initcalls

That means the first group contains 66 initcalls which are called using
4 threads, and, after those have finished, the second group with 33
initcalls will be called in parallel (using the same 4 threads).


BTW. That also means that, for the above example, the worst case would 
mean an error rate of 61% if those 170 (annotated) initcalls would be 
started in parallel while ignoring dependencies.


But that's just meant as an (hopefully) interesting number when looking 
at the above numbers a bit different.


(I've understood that the patches aren't wanted.)


Regards,

Alexander Holler


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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-22 Thread Alexander Holler

Am 19.10.2015 um 13:31 schrieb Alexander Holler:

Am 19.10.2015 um 12:57 schrieb Alexander Holler:

Am 18.10.2015 um 12:11 schrieb Alexander Holler:

Am 18.10.2015 um 07:59 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 07:20:34AM +0200, Alexander Holler wrote:

Am 18.10.2015 um 07:14 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 06:59:22AM +0200, Alexander Holler wrote:

Am 17.10.2015 um 21:36 schrieb Greg Kroah-Hartman:


Again, parallelizing does not solve anything, and causes more
problems
_and_ makes things take longer.  Try it, we have done it in the
past and
proven this, it's pretty easy to test :)


Just because I'm curious, may I ask how I would test that in the
easy way
you have in mind? I've just posted the results of my tests (the
patch
series) but I wonder what you do have in mind.


Use the tool, scripts/bootgraph.pl to create a boot graph of your
boot
sequence.  That should show you the drivers, or other areas, that are
causing your boot to be "slow".


So I've misunderstood you. I've read your paragraph as that it's
easy to
test parallelizing.


Ah, ok, if you want to parallelize everything, add some logic in the
driver core where the probe() callback is made to spin that off into a
new thread for every call, and when it's done, clean up the thread.
That's what I did many years ago to try this all out, if you dig in the
lkml archives there's probably a patch somewhere that you can base the
work off of to test it yourself.


Hmm, I don't think I will do that because that means to setup a new
thread for every call. And it doesn't need much imagination (or
experience) that this introduces quite some overhead.

But maybe it makes sense to try out what I'm doing in my patches,
starting multiple threads once and then just giving them some work. Will


After a having second thought on your simple approach to parallelize
stuff, I have to say that it just can't work because just starting a
thread for every probe() totally ignores possible dependencies.
Regardless if using one thread per probe() call or if feeding probe()
calls to just a few threads.

Maybe that's why previous attempts to parallelize stuff failed. But
that's just an assumption as I'm unaware of these previous attempts.


Or to describe it more verbose, if DEBUG is turned on in
init/dependencies.c (using my patches), it spits out a summary of groups
with initcalls (probe() calls) which are independent from each other and
therfore can be called in parallel. E.g. one of my systems this looks so:

[0.288229] init: vertices: 429 edges 204 count 170
[0.288295] init: group 0 length 66 (start 0)
[0.288329] init: group 1 length 33 (start 66)
[0.288364] init: group 2 length 13 (start 99)
[0.288398] init: group 3 length 7 (start 112)
[0.288432] init: group 4 length 9 (start 119)
[0.288466] init: group 5 length 8 (start 128)
[0.288500] init: group 6 length 11 (start 136)
[0.288534] init: group 7 length 6 (start 147)
[0.288569] init: group 8 length 4 (start 153)
[0.288603] init: group 9 length 8 (start 157)
[0.288637] init: group 10 length 3 (start 165)
[0.288671] init: group 11 length 2 (start 168)
[0.288705] init: using 4 threads to call annotated initcalls

That means the first group contains 66 initcalls which are called using
4 threads, and, after those have finished, the second group with 33
initcalls will be called in parallel (using the same 4 threads).


BTW. That also means that, for the above example, the worst case would 
mean an error rate of 61% if those 170 (annotated) initcalls would be 
started in parallel while ignoring dependencies.


But that's just meant as an (hopefully) interesting number when looking 
at the above numbers a bit different.


(I've understood that the patches aren't wanted.)


Regards,

Alexander Holler


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


Re: [PATCH 10/14] init: deps: IDs for annotated initcalls

2015-10-20 Thread Alexander Holler

Am 20.10.2015 um 12:42 schrieb Alexander Holler:


Another idea to split this one file into multiple ones would be to
reserve blocks of IDs. E.g. use 1-2 for networking stuff,
1000-1200 for I2C and so on.


In detail it could look like

driver_ids_base.h:

enum {
 drvid_i2c_base = 1000,
 drvid_networking_base = 1200,
 drvid_usb_base = 3000,
};

driver_ids_i2c.h:

# include "driver_ids_base.h"

enum {
 drvid_i2c_start = drvid_i2c_base,

 /* drivers/i2c */
 drvid_i2c,
 drvid_i2c_dev,
 drvid_i2c_busses_start,
 /* drivers/i2c/busses */
 drvid_i2c_gpio,
 (...)

 drvid_i2c_end
};


Which, of course, should be enhanced with a compile time error if 
drvid_i2c_end >= drvid_networking_base.



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


Re: [PATCH 10/14] init: deps: IDs for annotated initcalls

2015-10-20 Thread Alexander Holler

Am 20.10.2015 um 12:42 schrieb Alexander Holler:

Am 20.10.2015 um 12:30 schrieb Alexander Holler:

Am 19.10.2015 um 15:12 schrieb Mark Brown:

On Sat, Oct 17, 2015 at 08:46:44PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 20:29 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 07:55:17PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 19:45 schrieb Greg Kroah-Hartman:



A file like this is going to be a nightmare to maintain and ensure
that
it actually is correct, I don't see it as a viable solution, sorry.



How often will drivers be added? The only changes on this file will
happen
if a driver will be added and then just one ID will be added.



Look at how many drivers we add every kernel release, it's a
non-trivial
amount.



I still don't see your problem. As long as the IDs in the enum are
ordered
according to the directories, there won't be more merge conflicts
than in
the Makefile or Kconfig for that directory. And as mentioned, it's e.g.
possible to split the one file into multiple ones, e.g.

enum driver_ids {

#include "foo"
#include "bar"

};

Of cource, the content of foo and bar might look a bit unusual.


If it's a purely mechanical thing we really ought to be able to arrange
for it to be generated during the build rather than have to have more
typing.  If the values matter then people have to think about what they
are which is more effort and rather indirect.


It's just that I didn't thought much about another solution, and the
time
I've spend to think about something else which provides a usable ID,
didn't
end in a solution. So I would be happy if someone else would offer an
idea.


A checksum of the driver name?


That requires the driver name, which is only available if the struct
device_driver is available (which isn't always the case). And it would
require time to build the checksums.

Another idea to split this one file into multiple ones would be to
reserve blocks of IDs. E.g. use 1-2 for networking stuff,
1000-1200 for I2C and so on.


In detail it could look like

driver_ids_base.h:

enum {
 drvid_i2c_base = 1000,
 drvid_networking_base = 1200,
 drvid_usb_base = 3000,
};

driver_ids_i2c.h:

# include "driver_ids_base.h"

enum {
 drvid_i2c_start = drvid_i2c_base,

 /* drivers/i2c */
 drvid_i2c,
 drvid_i2c_dev,
 drvid_i2c_busses_start,
 /* drivers/i2c/busses */
 drvid_i2c_gpio,
 (...)

 drvid_i2c_end
};


Maybe I should mention that these IDs don't have to be consistent across 
kernel version.


They are only used to identify drivers at runtime. Of course, it might 
make sense to keep them consistent, but that only would make it easier 
to speak about drivers/initcalls while mentioning only their ID (which 
imho isn't that good). For debugging purposes my patches are already 
automatically generating names out of this enum.


Regards,

Alexander Holler

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


Re: [PATCH 10/14] init: deps: IDs for annotated initcalls

2015-10-20 Thread Alexander Holler

Am 20.10.2015 um 12:30 schrieb Alexander Holler:

Am 19.10.2015 um 15:12 schrieb Mark Brown:

On Sat, Oct 17, 2015 at 08:46:44PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 20:29 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 07:55:17PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 19:45 schrieb Greg Kroah-Hartman:



A file like this is going to be a nightmare to maintain and ensure
that
it actually is correct, I don't see it as a viable solution, sorry.



How often will drivers be added? The only changes on this file will
happen
if a driver will be added and then just one ID will be added.



Look at how many drivers we add every kernel release, it's a
non-trivial
amount.



I still don't see your problem. As long as the IDs in the enum are
ordered
according to the directories, there won't be more merge conflicts
than in
the Makefile or Kconfig for that directory. And as mentioned, it's e.g.
possible to split the one file into multiple ones, e.g.

enum driver_ids {

#include "foo"
#include "bar"

};

Of cource, the content of foo and bar might look a bit unusual.


If it's a purely mechanical thing we really ought to be able to arrange
for it to be generated during the build rather than have to have more
typing.  If the values matter then people have to think about what they
are which is more effort and rather indirect.


It's just that I didn't thought much about another solution, and the
time
I've spend to think about something else which provides a usable ID,
didn't
end in a solution. So I would be happy if someone else would offer an
idea.


A checksum of the driver name?


That requires the driver name, which is only available if the struct
device_driver is available (which isn't always the case). And it would
require time to build the checksums.

Another idea to split this one file into multiple ones would be to
reserve blocks of IDs. E.g. use 1-2 for networking stuff,
1000-1200 for I2C and so on.


In detail it could look like

driver_ids_base.h:

enum {
drvid_i2c_base = 1000,
drvid_networking_base = 1200,
drvid_usb_base = 3000,
};

driver_ids_i2c.h:

# include "driver_ids_base.h"

enum {
drvid_i2c_start = drvid_i2c_base,

/* drivers/i2c */
drvid_i2c,
drvid_i2c_dev,
drvid_i2c_busses_start,
/* drivers/i2c/busses */
drvid_i2c_gpio,
(...)

    drvid_i2c_end
};


Regards,

Alexander Holler


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


Re: [PATCH 10/14] init: deps: IDs for annotated initcalls

2015-10-20 Thread Alexander Holler

Am 19.10.2015 um 15:12 schrieb Mark Brown:

On Sat, Oct 17, 2015 at 08:46:44PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 20:29 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 07:55:17PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 19:45 schrieb Greg Kroah-Hartman:



A file like this is going to be a nightmare to maintain and ensure that
it actually is correct, I don't see it as a viable solution, sorry.



How often will drivers be added? The only changes on this file will happen
if a driver will be added and then just one ID will be added.



Look at how many drivers we add every kernel release, it's a non-trivial
amount.



I still don't see your problem. As long as the IDs in the enum are ordered
according to the directories, there won't be more merge conflicts than in
the Makefile or Kconfig for that directory. And as mentioned, it's e.g.
possible to split the one file into multiple ones, e.g.

enum driver_ids {

#include "foo"
#include "bar"

};

Of cource, the content of foo and bar might look a bit unusual.


If it's a purely mechanical thing we really ought to be able to arrange
for it to be generated during the build rather than have to have more
typing.  If the values matter then people have to think about what they
are which is more effort and rather indirect.


It's just that I didn't thought much about another solution, and the time
I've spend to think about something else which provides a usable ID, didn't
end in a solution. So I would be happy if someone else would offer an idea.


A checksum of the driver name?


That requires the driver name, which is only available if the struct 
device_driver is available (which isn't always the case). And it would 
require time to build the checksums.


Another idea to split this one file into multiple ones would be to 
reserve blocks of IDs. E.g. use 1-2 for networking stuff, 
1000-1200 for I2C and so on.


Regards,

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


Re: [PATCH 10/14] init: deps: IDs for annotated initcalls

2015-10-20 Thread Alexander Holler

Am 19.10.2015 um 15:12 schrieb Mark Brown:

On Sat, Oct 17, 2015 at 08:46:44PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 20:29 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 07:55:17PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 19:45 schrieb Greg Kroah-Hartman:



A file like this is going to be a nightmare to maintain and ensure that
it actually is correct, I don't see it as a viable solution, sorry.



How often will drivers be added? The only changes on this file will happen
if a driver will be added and then just one ID will be added.



Look at how many drivers we add every kernel release, it's a non-trivial
amount.



I still don't see your problem. As long as the IDs in the enum are ordered
according to the directories, there won't be more merge conflicts than in
the Makefile or Kconfig for that directory. And as mentioned, it's e.g.
possible to split the one file into multiple ones, e.g.

enum driver_ids {

#include "foo"
#include "bar"

};

Of cource, the content of foo and bar might look a bit unusual.


If it's a purely mechanical thing we really ought to be able to arrange
for it to be generated during the build rather than have to have more
typing.  If the values matter then people have to think about what they
are which is more effort and rather indirect.


It's just that I didn't thought much about another solution, and the time
I've spend to think about something else which provides a usable ID, didn't
end in a solution. So I would be happy if someone else would offer an idea.


A checksum of the driver name?


That requires the driver name, which is only available if the struct 
device_driver is available (which isn't always the case). And it would 
require time to build the checksums.


Another idea to split this one file into multiple ones would be to 
reserve blocks of IDs. E.g. use 1-2 for networking stuff, 
1000-1200 for I2C and so on.


Regards,

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


Re: [PATCH 10/14] init: deps: IDs for annotated initcalls

2015-10-20 Thread Alexander Holler

Am 20.10.2015 um 12:30 schrieb Alexander Holler:

Am 19.10.2015 um 15:12 schrieb Mark Brown:

On Sat, Oct 17, 2015 at 08:46:44PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 20:29 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 07:55:17PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 19:45 schrieb Greg Kroah-Hartman:



A file like this is going to be a nightmare to maintain and ensure
that
it actually is correct, I don't see it as a viable solution, sorry.



How often will drivers be added? The only changes on this file will
happen
if a driver will be added and then just one ID will be added.



Look at how many drivers we add every kernel release, it's a
non-trivial
amount.



I still don't see your problem. As long as the IDs in the enum are
ordered
according to the directories, there won't be more merge conflicts
than in
the Makefile or Kconfig for that directory. And as mentioned, it's e.g.
possible to split the one file into multiple ones, e.g.

enum driver_ids {

#include "foo"
#include "bar"

};

Of cource, the content of foo and bar might look a bit unusual.


If it's a purely mechanical thing we really ought to be able to arrange
for it to be generated during the build rather than have to have more
typing.  If the values matter then people have to think about what they
are which is more effort and rather indirect.


It's just that I didn't thought much about another solution, and the
time
I've spend to think about something else which provides a usable ID,
didn't
end in a solution. So I would be happy if someone else would offer an
idea.


A checksum of the driver name?


That requires the driver name, which is only available if the struct
device_driver is available (which isn't always the case). And it would
require time to build the checksums.

Another idea to split this one file into multiple ones would be to
reserve blocks of IDs. E.g. use 1-2 for networking stuff,
1000-1200 for I2C and so on.


In detail it could look like

driver_ids_base.h:

enum {
drvid_i2c_base = 1000,
drvid_networking_base = 1200,
drvid_usb_base = 3000,
};

driver_ids_i2c.h:

# include "driver_ids_base.h"

enum {
drvid_i2c_start = drvid_i2c_base,

/* drivers/i2c */
drvid_i2c,
drvid_i2c_dev,
drvid_i2c_busses_start,
/* drivers/i2c/busses */
drvid_i2c_gpio,
(...)

    drvid_i2c_end
};


Regards,

Alexander Holler


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


Re: [PATCH 10/14] init: deps: IDs for annotated initcalls

2015-10-20 Thread Alexander Holler

Am 20.10.2015 um 12:42 schrieb Alexander Holler:

Am 20.10.2015 um 12:30 schrieb Alexander Holler:

Am 19.10.2015 um 15:12 schrieb Mark Brown:

On Sat, Oct 17, 2015 at 08:46:44PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 20:29 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 07:55:17PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 19:45 schrieb Greg Kroah-Hartman:



A file like this is going to be a nightmare to maintain and ensure
that
it actually is correct, I don't see it as a viable solution, sorry.



How often will drivers be added? The only changes on this file will
happen
if a driver will be added and then just one ID will be added.



Look at how many drivers we add every kernel release, it's a
non-trivial
amount.



I still don't see your problem. As long as the IDs in the enum are
ordered
according to the directories, there won't be more merge conflicts
than in
the Makefile or Kconfig for that directory. And as mentioned, it's e.g.
possible to split the one file into multiple ones, e.g.

enum driver_ids {

#include "foo"
#include "bar"

};

Of cource, the content of foo and bar might look a bit unusual.


If it's a purely mechanical thing we really ought to be able to arrange
for it to be generated during the build rather than have to have more
typing.  If the values matter then people have to think about what they
are which is more effort and rather indirect.


It's just that I didn't thought much about another solution, and the
time
I've spend to think about something else which provides a usable ID,
didn't
end in a solution. So I would be happy if someone else would offer an
idea.


A checksum of the driver name?


That requires the driver name, which is only available if the struct
device_driver is available (which isn't always the case). And it would
require time to build the checksums.

Another idea to split this one file into multiple ones would be to
reserve blocks of IDs. E.g. use 1-2 for networking stuff,
1000-1200 for I2C and so on.


In detail it could look like

driver_ids_base.h:

enum {
 drvid_i2c_base = 1000,
 drvid_networking_base = 1200,
 drvid_usb_base = 3000,
};

driver_ids_i2c.h:

# include "driver_ids_base.h"

enum {
 drvid_i2c_start = drvid_i2c_base,

 /* drivers/i2c */
 drvid_i2c,
 drvid_i2c_dev,
 drvid_i2c_busses_start,
 /* drivers/i2c/busses */
 drvid_i2c_gpio,
 (...)

 drvid_i2c_end
};


Maybe I should mention that these IDs don't have to be consistent across 
kernel version.


They are only used to identify drivers at runtime. Of course, it might 
make sense to keep them consistent, but that only would make it easier 
to speak about drivers/initcalls while mentioning only their ID (which 
imho isn't that good). For debugging purposes my patches are already 
automatically generating names out of this enum.


Regards,

Alexander Holler

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


Re: [PATCH 10/14] init: deps: IDs for annotated initcalls

2015-10-20 Thread Alexander Holler

Am 20.10.2015 um 12:42 schrieb Alexander Holler:


Another idea to split this one file into multiple ones would be to
reserve blocks of IDs. E.g. use 1-2 for networking stuff,
1000-1200 for I2C and so on.


In detail it could look like

driver_ids_base.h:

enum {
 drvid_i2c_base = 1000,
 drvid_networking_base = 1200,
 drvid_usb_base = 3000,
};

driver_ids_i2c.h:

# include "driver_ids_base.h"

enum {
 drvid_i2c_start = drvid_i2c_base,

 /* drivers/i2c */
 drvid_i2c,
 drvid_i2c_dev,
 drvid_i2c_busses_start,
 /* drivers/i2c/busses */
 drvid_i2c_gpio,
 (...)

 drvid_i2c_end
};


Which, of course, should be enhanced with a compile time error if 
drvid_i2c_end >= drvid_networking_base.



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


Re: [PATCH 03/14] init: deps: dt: use (HW-specific) dependencies provided by the DT too

2015-10-19 Thread Alexander Holler

Am 19.10.2015 um 18:27 schrieb Rob Herring:


There is a need to annotate DTB's with type information, but that
needs to be looked at in context of all types, not just one specific
type.


With which I disagree. You will need type information if you don't have 
knowledge about the context, and the context is usually only known by 
the drivers.


Regards,

Alexander Holler

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


Re: [PATCH 03/14] init: deps: dt: use (HW-specific) dependencies provided by the DT too

2015-10-19 Thread Alexander Holler

Am 19.10.2015 um 14:37 schrieb Mark Brown:

On Sat, Oct 17, 2015 at 07:14:16PM +0200, Alexander Holler wrote:

This patch adds dependencies provided by the hardware description in
the used DT. This avoids the use of the deferred probe mechanism
on most (if not all) DT based kernels.

Drawback is that the binary DT blob has to be enhanced with type
information for phandles (which are used as dependencies) which
needs a modified dtc.


You probably want to loop the DT and DTC maintainers in on this - adding
Frank, Rob and David and leaving context for their reference.  It would
probably help if you could explicitly say why the DTB needs to be
annotated and why this annotiation is best done via a DTC modification


I've had them on the cc-list on the previous two evolutions of these 
patches, when the whole stuff was for DT only. The annotation is not for 
DTB but for initcalls. But maybe you mean with annotation the missing 
type information in DTBs, which is why I had to add a new property.



(rather than doing something like add new properties, or just guessing
that any phandle reference is a dependency).


Besides the remote-endpoints, which have been introduced after my first 
patch to use phandles as dependencies (1.5 years ago or so), every 
phandle also was a dependency.


But anyway, the stuff was ignored before and the current evolution of 
the patches will never see mainline (too).


So, just see the whole approach as failed. I don't have a problem with 
that. At least I do that and almost did that before, I've just posted 
the newest version of the approach because I see it as the final 
evolution and don't will work further on that stuff anymore.


Regards,

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-19 Thread Alexander Holler

Am 19.10.2015 um 12:57 schrieb Alexander Holler:

Am 18.10.2015 um 12:11 schrieb Alexander Holler:

Am 18.10.2015 um 07:59 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 07:20:34AM +0200, Alexander Holler wrote:

Am 18.10.2015 um 07:14 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 06:59:22AM +0200, Alexander Holler wrote:

Am 17.10.2015 um 21:36 schrieb Greg Kroah-Hartman:


Again, parallelizing does not solve anything, and causes more
problems
_and_ makes things take longer.  Try it, we have done it in the
past and
proven this, it's pretty easy to test :)


Just because I'm curious, may I ask how I would test that in the
easy way
you have in mind? I've just posted the results of my tests (the patch
series) but I wonder what you do have in mind.


Use the tool, scripts/bootgraph.pl to create a boot graph of your boot
sequence.  That should show you the drivers, or other areas, that are
causing your boot to be "slow".


So I've misunderstood you. I've read your paragraph as that it's
easy to
test parallelizing.


Ah, ok, if you want to parallelize everything, add some logic in the
driver core where the probe() callback is made to spin that off into a
new thread for every call, and when it's done, clean up the thread.
That's what I did many years ago to try this all out, if you dig in the
lkml archives there's probably a patch somewhere that you can base the
work off of to test it yourself.


Hmm, I don't think I will do that because that means to setup a new
thread for every call. And it doesn't need much imagination (or
experience) that this introduces quite some overhead.

But maybe it makes sense to try out what I'm doing in my patches,
starting multiple threads once and then just giving them some work. Will


After a having second thought on your simple approach to parallelize
stuff, I have to say that it just can't work because just starting a
thread for every probe() totally ignores possible dependencies.
Regardless if using one thread per probe() call or if feeding probe()
calls to just a few threads.

Maybe that's why previous attempts to parallelize stuff failed. But
that's just an assumption as I'm unaware of these previous attempts.


Or to describe it more verbose, if DEBUG is turned on in 
init/dependencies.c (using my patches), it spits out a summary of groups 
with initcalls (probe() calls) which are independent from each other and 
therfore can be called in parallel. E.g. one of my systems this looks so:


[0.288229] init: vertices: 429 edges 204 count 170
[0.288295] init: group 0 length 66 (start 0)
[0.288329] init: group 1 length 33 (start 66)
[0.288364] init: group 2 length 13 (start 99)
[0.288398] init: group 3 length 7 (start 112)
[0.288432] init: group 4 length 9 (start 119)
[0.288466] init: group 5 length 8 (start 128)
[0.288500] init: group 6 length 11 (start 136)
[0.288534] init: group 7 length 6 (start 147)
[0.288569] init: group 8 length 4 (start 153)
[0.288603] init: group 9 length 8 (start 157)
[0.288637] init: group 10 length 3 (start 165)
[0.288671] init: group 11 length 2 (start 168)
[0.288705] init: using 4 threads to call annotated initcalls

That means the first group contains 66 initcalls which are called using 
4 threads, and, after those have finished, the second group with 33 
initcalls will be called in parallel (using the same 4 threads).



Regards,

Alexander Holler


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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-19 Thread Alexander Holler

Am 18.10.2015 um 12:11 schrieb Alexander Holler:

Am 18.10.2015 um 07:59 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 07:20:34AM +0200, Alexander Holler wrote:

Am 18.10.2015 um 07:14 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 06:59:22AM +0200, Alexander Holler wrote:

Am 17.10.2015 um 21:36 schrieb Greg Kroah-Hartman:


Again, parallelizing does not solve anything, and causes more
problems
_and_ makes things take longer.  Try it, we have done it in the
past and
proven this, it's pretty easy to test :)


Just because I'm curious, may I ask how I would test that in the
easy way
you have in mind? I've just posted the results of my tests (the patch
series) but I wonder what you do have in mind.


Use the tool, scripts/bootgraph.pl to create a boot graph of your boot
sequence.  That should show you the drivers, or other areas, that are
causing your boot to be "slow".


So I've misunderstood you. I've read your paragraph as that it's easy to
test parallelizing.


Ah, ok, if you want to parallelize everything, add some logic in the
driver core where the probe() callback is made to spin that off into a
new thread for every call, and when it's done, clean up the thread.
That's what I did many years ago to try this all out, if you dig in the
lkml archives there's probably a patch somewhere that you can base the
work off of to test it yourself.


Hmm, I don't think I will do that because that means to setup a new
thread for every call. And it doesn't need much imagination (or
experience) that this introduces quite some overhead.

But maybe it makes sense to try out what I'm doing in my patches,
starting multiple threads once and then just giving them some work. Will


After a having second thought on your simple approach to parallelize 
stuff, I have to say that it just can't work because just starting a 
thread for every probe() totally ignores possible dependencies. 
Regardless if using one thread per probe() call or if feeding probe() 
calls to just a few threads.


Maybe that's why previous attempts to parallelize stuff failed. But 
that's just an assumption as I'm unaware of these previous attempts.


Regards,

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-19 Thread Alexander Holler

Am 19.10.2015 um 12:57 schrieb Alexander Holler:

Am 18.10.2015 um 12:11 schrieb Alexander Holler:

Am 18.10.2015 um 07:59 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 07:20:34AM +0200, Alexander Holler wrote:

Am 18.10.2015 um 07:14 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 06:59:22AM +0200, Alexander Holler wrote:

Am 17.10.2015 um 21:36 schrieb Greg Kroah-Hartman:


Again, parallelizing does not solve anything, and causes more
problems
_and_ makes things take longer.  Try it, we have done it in the
past and
proven this, it's pretty easy to test :)


Just because I'm curious, may I ask how I would test that in the
easy way
you have in mind? I've just posted the results of my tests (the patch
series) but I wonder what you do have in mind.


Use the tool, scripts/bootgraph.pl to create a boot graph of your boot
sequence.  That should show you the drivers, or other areas, that are
causing your boot to be "slow".


So I've misunderstood you. I've read your paragraph as that it's
easy to
test parallelizing.


Ah, ok, if you want to parallelize everything, add some logic in the
driver core where the probe() callback is made to spin that off into a
new thread for every call, and when it's done, clean up the thread.
That's what I did many years ago to try this all out, if you dig in the
lkml archives there's probably a patch somewhere that you can base the
work off of to test it yourself.


Hmm, I don't think I will do that because that means to setup a new
thread for every call. And it doesn't need much imagination (or
experience) that this introduces quite some overhead.

But maybe it makes sense to try out what I'm doing in my patches,
starting multiple threads once and then just giving them some work. Will


After a having second thought on your simple approach to parallelize
stuff, I have to say that it just can't work because just starting a
thread for every probe() totally ignores possible dependencies.
Regardless if using one thread per probe() call or if feeding probe()
calls to just a few threads.

Maybe that's why previous attempts to parallelize stuff failed. But
that's just an assumption as I'm unaware of these previous attempts.


Or to describe it more verbose, if DEBUG is turned on in 
init/dependencies.c (using my patches), it spits out a summary of groups 
with initcalls (probe() calls) which are independent from each other and 
therfore can be called in parallel. E.g. one of my systems this looks so:


[0.288229] init: vertices: 429 edges 204 count 170
[0.288295] init: group 0 length 66 (start 0)
[0.288329] init: group 1 length 33 (start 66)
[0.288364] init: group 2 length 13 (start 99)
[0.288398] init: group 3 length 7 (start 112)
[0.288432] init: group 4 length 9 (start 119)
[0.288466] init: group 5 length 8 (start 128)
[0.288500] init: group 6 length 11 (start 136)
[0.288534] init: group 7 length 6 (start 147)
[0.288569] init: group 8 length 4 (start 153)
[0.288603] init: group 9 length 8 (start 157)
[0.288637] init: group 10 length 3 (start 165)
[0.288671] init: group 11 length 2 (start 168)
[0.288705] init: using 4 threads to call annotated initcalls

That means the first group contains 66 initcalls which are called using 
4 threads, and, after those have finished, the second group with 33 
initcalls will be called in parallel (using the same 4 threads).



Regards,

Alexander Holler


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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-19 Thread Alexander Holler

Am 18.10.2015 um 12:11 schrieb Alexander Holler:

Am 18.10.2015 um 07:59 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 07:20:34AM +0200, Alexander Holler wrote:

Am 18.10.2015 um 07:14 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 06:59:22AM +0200, Alexander Holler wrote:

Am 17.10.2015 um 21:36 schrieb Greg Kroah-Hartman:


Again, parallelizing does not solve anything, and causes more
problems
_and_ makes things take longer.  Try it, we have done it in the
past and
proven this, it's pretty easy to test :)


Just because I'm curious, may I ask how I would test that in the
easy way
you have in mind? I've just posted the results of my tests (the patch
series) but I wonder what you do have in mind.


Use the tool, scripts/bootgraph.pl to create a boot graph of your boot
sequence.  That should show you the drivers, or other areas, that are
causing your boot to be "slow".


So I've misunderstood you. I've read your paragraph as that it's easy to
test parallelizing.


Ah, ok, if you want to parallelize everything, add some logic in the
driver core where the probe() callback is made to spin that off into a
new thread for every call, and when it's done, clean up the thread.
That's what I did many years ago to try this all out, if you dig in the
lkml archives there's probably a patch somewhere that you can base the
work off of to test it yourself.


Hmm, I don't think I will do that because that means to setup a new
thread for every call. And it doesn't need much imagination (or
experience) that this introduces quite some overhead.

But maybe it makes sense to try out what I'm doing in my patches,
starting multiple threads once and then just giving them some work. Will


After a having second thought on your simple approach to parallelize 
stuff, I have to say that it just can't work because just starting a 
thread for every probe() totally ignores possible dependencies. 
Regardless if using one thread per probe() call or if feeding probe() 
calls to just a few threads.


Maybe that's why previous attempts to parallelize stuff failed. But 
that's just an assumption as I'm unaware of these previous attempts.


Regards,

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


Re: [PATCH 03/14] init: deps: dt: use (HW-specific) dependencies provided by the DT too

2015-10-19 Thread Alexander Holler

Am 19.10.2015 um 18:27 schrieb Rob Herring:


There is a need to annotate DTB's with type information, but that
needs to be looked at in context of all types, not just one specific
type.


With which I disagree. You will need type information if you don't have 
knowledge about the context, and the context is usually only known by 
the drivers.


Regards,

Alexander Holler

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


Re: [PATCH 03/14] init: deps: dt: use (HW-specific) dependencies provided by the DT too

2015-10-19 Thread Alexander Holler

Am 19.10.2015 um 14:37 schrieb Mark Brown:

On Sat, Oct 17, 2015 at 07:14:16PM +0200, Alexander Holler wrote:

This patch adds dependencies provided by the hardware description in
the used DT. This avoids the use of the deferred probe mechanism
on most (if not all) DT based kernels.

Drawback is that the binary DT blob has to be enhanced with type
information for phandles (which are used as dependencies) which
needs a modified dtc.


You probably want to loop the DT and DTC maintainers in on this - adding
Frank, Rob and David and leaving context for their reference.  It would
probably help if you could explicitly say why the DTB needs to be
annotated and why this annotiation is best done via a DTC modification


I've had them on the cc-list on the previous two evolutions of these 
patches, when the whole stuff was for DT only. The annotation is not for 
DTB but for initcalls. But maybe you mean with annotation the missing 
type information in DTBs, which is why I had to add a new property.



(rather than doing something like add new properties, or just guessing
that any phandle reference is a dependency).


Besides the remote-endpoints, which have been introduced after my first 
patch to use phandles as dependencies (1.5 years ago or so), every 
phandle also was a dependency.


But anyway, the stuff was ignored before and the current evolution of 
the patches will never see mainline (too).


So, just see the whole approach as failed. I don't have a problem with 
that. At least I do that and almost did that before, I've just posted 
the newest version of the approach because I see it as the final 
evolution and don't will work further on that stuff anymore.


Regards,

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-18 Thread Alexander Holler

Am 18.10.2015 um 07:59 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 07:20:34AM +0200, Alexander Holler wrote:

Am 18.10.2015 um 07:14 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 06:59:22AM +0200, Alexander Holler wrote:

Am 17.10.2015 um 21:36 schrieb Greg Kroah-Hartman:


Again, parallelizing does not solve anything, and causes more problems
_and_ makes things take longer.  Try it, we have done it in the past and
proven this, it's pretty easy to test :)


Just because I'm curious, may I ask how I would test that in the easy way
you have in mind? I've just posted the results of my tests (the patch
series) but I wonder what you do have in mind.


Use the tool, scripts/bootgraph.pl to create a boot graph of your boot
sequence.  That should show you the drivers, or other areas, that are
causing your boot to be "slow".


So I've misunderstood you. I've read your paragraph as that it's easy to
test parallelizing.


Ah, ok, if you want to parallelize everything, add some logic in the
driver core where the probe() callback is made to spin that off into a
new thread for every call, and when it's done, clean up the thread.
That's what I did many years ago to try this all out, if you dig in the
lkml archives there's probably a patch somewhere that you can base the
work off of to test it yourself.


Hmm, I don't think I will do that because that means to setup a new 
thread for every call. And it doesn't need much imagination (or 
experience) that this introduces quite some overhead.


But maybe it makes sense to try out what I'm doing in my patches, 
starting multiple threads once and then just giving them some work. Will 
keep that in mind, also I don't think I will post any patch in the next 
few years. ;)


Regards,

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-18 Thread Alexander Holler

Am 18.10.2015 um 07:59 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 07:20:34AM +0200, Alexander Holler wrote:

Am 18.10.2015 um 07:14 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 06:59:22AM +0200, Alexander Holler wrote:

Am 17.10.2015 um 21:36 schrieb Greg Kroah-Hartman:


Again, parallelizing does not solve anything, and causes more problems
_and_ makes things take longer.  Try it, we have done it in the past and
proven this, it's pretty easy to test :)


Just because I'm curious, may I ask how I would test that in the easy way
you have in mind? I've just posted the results of my tests (the patch
series) but I wonder what you do have in mind.


Use the tool, scripts/bootgraph.pl to create a boot graph of your boot
sequence.  That should show you the drivers, or other areas, that are
causing your boot to be "slow".


So I've misunderstood you. I've read your paragraph as that it's easy to
test parallelizing.


Ah, ok, if you want to parallelize everything, add some logic in the
driver core where the probe() callback is made to spin that off into a
new thread for every call, and when it's done, clean up the thread.
That's what I did many years ago to try this all out, if you dig in the
lkml archives there's probably a patch somewhere that you can base the
work off of to test it yourself.


Hmm, I don't think I will do that because that means to setup a new 
thread for every call. And it doesn't need much imagination (or 
experience) that this introduces quite some overhead.


But maybe it makes sense to try out what I'm doing in my patches, 
starting multiple threads once and then just giving them some work. Will 
keep that in mind, also I don't think I will post any patch in the next 
few years. ;)


Regards,

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 18.10.2015 um 07:14 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 06:59:22AM +0200, Alexander Holler wrote:

Am 17.10.2015 um 21:36 schrieb Greg Kroah-Hartman:


Again, parallelizing does not solve anything, and causes more problems
_and_ makes things take longer.  Try it, we have done it in the past and
proven this, it's pretty easy to test :)


Just because I'm curious, may I ask how I would test that in the easy way
you have in mind? I've just posted the results of my tests (the patch
series) but I wonder what you do have in mind.


Use the tool, scripts/bootgraph.pl to create a boot graph of your boot
sequence.  That should show you the drivers, or other areas, that are
causing your boot to be "slow".


So I've misunderstood you. I've read your paragraph as that it's easy to 
test parallelizing.



Hope this helps,


Unfortunately no, but thanks anyway. ;)

Alexander Holler

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 21:36 schrieb Greg Kroah-Hartman:


Again, parallelizing does not solve anything, and causes more problems
_and_ makes things take longer.  Try it, we have done it in the past and
proven this, it's pretty easy to test :)


Just because I'm curious, may I ask how I would test that in the easy 
way you have in mind? I've just posted the results of my tests (the 
patch series) but I wonder what you do have in mind.


Regards,

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 21:08 schrieb Linus Torvalds:

On Sat, Oct 17, 2015 at 12:01 PM, Alexander Holler  wrote:


That isn't a flag day thing. It's a config option everyone can turn on and
off whenever he wants.


That's a flag-day thing. We've done it (drm comes to mind - several times).

I'm disappointed, because I _know_ I pointed you in the direction of
stable sorting about a month ago. I really had hoped you'd have taken
that into account.


Sorry, but I have to add another comment about that stable topological 
sort which can be easily found by searching the web.


As the author mentioned, he has done some search on that topic too and 
has found nothing. So, either I would have to trust his word (and his 
non-public proof) that it works or I would have to prove it myself.


Besides that he says it's based on bubble sort, which is known for its 
speed. So I would have had to write in C in order to see if the speed 
would be acceptable and would still be without any proof that it always 
works correctly.


Regards,

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 21:58 schrieb Alexander Holler:


Unfortunately it's quiet a lot of work to add dependencies for everything.


And (just in case of), also I'm a non-native English writer, I know the 
difference between quiet and quite. But, unfortunately, that doesn't 
prevent me to type it wrong. It's like "teh" instead of "the". 
Unfortunately I'm not very good in writing English fast without errors, 
but that doesn't mean I'm dumb (also many native English writers prefer 
to assume that).


Regards,

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


Re: [PATCH 0/14] init: deps: dependency based (parallelized) init

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 22:20 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 09:43:17PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 20:38 schrieb Greg Kroah-Hartman:


So how long does that really take to call all probe functions in all
possible order?  Real numbers please.  We have the tools to determine
where at boot time delays are happening, please use them to find the
problem drivers.


No idea. You might ask Tomeu Vizoso (I've added him to cc) for details (or
search for the thread "On-demand device registration" where he complained
that his chromebook boots slow). I've just measured, that most my ARM boxes
booted faster when I've used the ordering, instead of slower through the
introduce overhead to order initcalls (without having parallelized the
initcalls).


I've already asked him, I don't like his patch series to try to resolve
this issue either :)


Posting times doesn't make much sense, as they heavily depend on the
configuration. Instead I've posted patches so you can test it yourself.

But if you want a real time, my Netbook with a single core but HT Atom N270
boots in one second instead of two to "dmesg | grep Freeing".


Try running the boot time graphic tool to determine where that time is
spent, odds are you just need to enable a single driver to async it's
probe function and you should be fine.

Again, fix up the broken driver(s), don't paper over the issues with
core changes that are not necessary.


I assume you are aware of the 80/20 rule. So you might see the 
parallelize feature of topological sort as an attempt to do some of the 
work in the last 20 percent left.


Sorry, but I've no idea why you are now trying to pin the stuff I'm 
talking about to a specific issue.


Or to talk in more clear words:

The current initcall ordering is, in my humble opinion, a whole and 
mostly undocumented mess. And I'm pretty sure it will become worse, I 
just have to wait and see. And if every attempt to fix that will be 
killed as fast as my one ...


Anyway, thanks for comments.

Regards,

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 21:36 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 09:14:34PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 21:08 schrieb Linus Torvalds:

On Sat, Oct 17, 2015 at 12:01 PM, Alexander Holler  wrote:


That isn't a flag day thing. It's a config option everyone can turn on and
off whenever he wants.


That's a flag-day thing. We've done it (drm comes to mind - several times).

I'm disappointed, because I _know_ I pointed you in the direction of
stable sorting about a month ago. I really had hoped you'd have taken
that into account.


It's impossible to take it into account because I don't want to miss the
parallelize functionality. And without that, all the stuff doesn't offer
enough benefits to be worse the effort but just adds some time necessary to
do the sorting. It might solve the deferred probe problems, but without much
benefit.


Again, parallelizing does not solve anything, and causes more problems
_and_ makes things take longer.  Try it, we have done it in the past and
proven this, it's pretty easy to test :)


I've tested it, otherwise I wouldn't have posted the patches.

Unfortunately it's quiet a lot of work to add dependencies for everything.

So maybe I'm able to offer some better numbers in a year or such, when I 
was bored often enough to add more dependencies for initcalls.


Not to mention that small changes in the order can have quiet big 
differences in the boot time, so it's quiet hard to parallelize stuff 
(add dependencies) correctly like e.g. the pci/acpi/processor stuff. 
Especially because many reasons for the current order aren't mentioned 
in the source and are hard to see without specific knowledge about the HW.


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


Re: [PATCH 0/14] init: deps: dependency based (parallelized) init

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 20:38 schrieb Greg Kroah-Hartman:


So how long does that really take to call all probe functions in all
possible order?  Real numbers please.  We have the tools to determine
where at boot time delays are happening, please use them to find the
problem drivers.


No idea. You might ask Tomeu Vizoso (I've added him to cc) for details 
(or search for the thread "On-demand device registration" where he 
complained that his chromebook boots slow). I've just measured, that 
most my ARM boxes booted faster when I've used the ordering, instead of 
slower through the introduce overhead to order initcalls (without having 
parallelized the initcalls).


Posting times doesn't make much sense, as they heavily depend on the 
configuration. Instead I've posted patches so you can test it yourself.


But if you want a real time, my Netbook with a single core but HT Atom 
N270 boots in one second instead of two to "dmesg | grep Freeing".


Regards,

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 21:08 schrieb Linus Torvalds:

On Sat, Oct 17, 2015 at 12:01 PM, Alexander Holler  wrote:


That isn't a flag day thing. It's a config option everyone can turn on and
off whenever he wants.


That's a flag-day thing. We've done it (drm comes to mind - several times).

I'm disappointed, because I _know_ I pointed you in the direction of
stable sorting about a month ago. I really had hoped you'd have taken
that into account.


It's impossible to take it into account because I don't want to miss the 
parallelize functionality. And without that, all the stuff doesn't offer 
enough benefits to be worse the effort but just adds some time necessary 
to do the sorting. It might solve the deferred probe problems, but 
without much benefit.


Regards,

Alexander Holler

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 21:03 schrieb Linus Torvalds:

On Sat, Oct 17, 2015 at 11:37 AM, Alexander Holler  wrote:


Otherwise it's impossible to call initcalls in parallel. I've seen a stable
topological sort somewhere, but whenever you want to parallelize the
initcalls, the stable ordering would be gone anyway. So I've decided not to
look further at a stable topological sort.


So five seconds of googling gave me freely usable source code for a
stable topological sort, that also has a nice reported added
advantage:

  "An interesting property of a stable topological sort is that cyclic
dependencies are tolerated and resolved according to original order of
elements in sequence. This is a desirable feature for many
applications because it allows to sort any sequence with any
imaginable dependencies between the elements"

which seems to be *exactly* what you'd want, especially considering
that right now your patches add extra "no-dependency" markers exactly
because of the cyclical problem.


That's the stable topological sort I've mentioned the link to in the 
discussion with you.




I think it was the #2 hit on google for "stable topological sort". I
didn't look closely at the source code, but it was not big.

And no, since we don't actually want to parallelize the initcalls
anyway (I had this discussion with you just a month ago), your
objections seem even more questionable. We have separate machinery for
"do this asynchronously", and we want to _keep_ that separate.


I've understood that now.

Sorry for wasting your time.

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 20:52 schrieb Linus Torvalds:

On Sat, Oct 17, 2015 at 11:37 AM, Alexander Holler  wrote:


I'm making dependencies the only ordering for annotated initcalls.


Yeah, and quite frankly, that just means that I'm not going to merge it.

We do not do "flag-day" things. We've done them in the past, and it
has always been a major and unacceptable pain.


That isn't a flag day thing. It's a config option everyone can turn on 
and off whenever he wants.




And if the dependency ordering is "outside" of the traditional
link-time ordering, then it is by definition a flag-day event. Every
time you add a dependency to even just *one* driver, it magically
changes ordering wrt all the other drivers, because now it's in a
different ordering domain.

So please reconsider. Or stop cc'ing me. Because "flag day" changes
really are not acceptable.


I'm choosing the second option. I will answer further mails on that 
topic, but will remove you from cc. Besides that I consider these 
patches as a total failure and will not post any further version.


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


Re: [PATCH 11/14] init: deps: annotate various initcalls

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 20:47 schrieb Linus Torvalds:

On Sat, Oct 17, 2015 at 10:14 AM, Alexander Holler  wrote:


diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index 873dbfc..d5d2459 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -1872,5 +1872,4 @@ static int __init edma_init(void)
  {
 return platform_driver_probe(_driver, edma_probe);
  }
-arch_initcall(edma_init);
-
+annotated_initcall_drv(arch, edma_init, drvid_edma, NULL, edma_driver.driver);
diff --git a/arch/arm/crypto/aes-ce-glue.c b/arch/arm/crypto/aes-ce-glue.c
index b445a5d..d9bcb89 100644
--- a/arch/arm/crypto/aes-ce-glue.c
+++ b/arch/arm/crypto/aes-ce-glue.c
@@ -520,5 +520,10 @@ static void __exit aes_exit(void)
 crypto_unregister_algs(aes_algs, ARRAY_SIZE(aes_algs));
  }

-module_init(aes_init);
+static const unsigned dependencies[] __initconst __maybe_unused = {
+   drvid_cryptomgr,
+   0
+};
+
+annotated_module_init(aes_init, drvid_aes_ce_arm, dependencies);
  module_exit(aes_exit);


So I think this is kind of a sign of the same disease I mentioned
earlier: making dependencies "separate" from the init levels, now
means that you do the initialization of the dependencies *instead* of
the init level. And that smells bad and wrong, and causes this kind of
patch that is not only huge, but si unreadable and the end result
looks like crap too.

We've actually been quite good at having the module attributes all be
*separate* things that work together. So the code had

   module_init(aes_init);
   module_exit(aes_exit);

but also things like

   MODULE_DESCRIPTION("AES-ECB/CBC/CTR/XTS using ARMv8 Crypto Extensions");
   MODULE_AUTHOR("Ard Biesheuvel ");
   MODULE_LICENSE("GPL v2");

and that all helps improve readablity and keep things sane.

In contrast, turds like these are just pure and utter crap:

static const unsigned dependencies[] __initconst __maybe_unused = {
   drvid_cryptomgr,
   0
};
annotated_module_init(aes_init, drvid_aes_ce_arm, dependencies);

and yes, I know that we have things like this for the driver ID lists
etc, but that doesn't make it better.

No, I think any dependency model should strive to make this really
really easy and separate, and do things like

module_depends(cryptomgr);

and then just use that to fill in a link section or something like
that. And no, there's no way we will ever maintain a "list of
dependency identifiers". This is stuff that should be all about
scripting, or - better yet - just make the link section contain
strings so that you don't *need* any C level identifiers.

That would be trivial to do by just making the "module_depends()"
macro be something like

   #define _dependency(x,y) \
  static const  struct module_dependency_attribute \
  __used __attribute__ ((__section__ ("__dependencies")))  \
 * __dependency_attr =  { x,y }

   #define module_depends(x) \
 _dependency(#x, KBUILD_NAME)

   #define module_provides(x) \
 _dependency(KBUILD_NAME, #x)

And if a module depends on multiple other things, then you just have
multiple of those "module_depends()" things. There's some gcc trick to
generating numbered (per compilation unit) C identifiers (so that you
can have multiple of those "__dependency_attr" variables in the same
file), but I forget it right now.

And this is also where I think those "module_init()" vs
"subsys_init()" things come in. "module_init()" means that it's a
driver level thing, which would mean that module_init() implies

   module_depends(level7);
   module_provides(level7_end);

so that the module would automatically be sorted wrt the "driver" level.

Another advantage (apart from legibility of the source, and
integrating with the *existing* level-based dependencies) is that
using something like "module_depends()" and "module_provides()" means
that it should be easy to parse even outside of a C compiler, so you
could - if you want to - make all the dependencies be done not as part
of compiling the source, but as a separate scripting thing. That could
be useful for things like statistics and visualization tools that
don't want to actually build the kernel, but want to just show the
dependencies between different modules.

So no. I do *not* think big patches like this are acceptable. This
kind of patch - along with the patch that just adds the random
dependency identifier C enums - is exactly what we do *not* want. If
we do dependencies, they should all be small and local things, and
they should not *replace* the existing "module_init()" vs
"arch_init()" system, they should add on top of it.


Thanks for the detailed answer and you are right.

But I had to start somehow and, unfortunately, I don't have the 
resources to fulfill your requirements.


Regards,

Alexander Holler
--
To

Re: [PATCH 10/14] init: deps: IDs for annotated initcalls

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 20:29 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 07:55:17PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 19:45 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 07:14:23PM +0200, Alexander Holler wrote:

These patch contains the IDs for initcalls I've annotated.

This patch is NOT meant for merging into mainline in its current form.

It should be discussed about how to add these IDs and in which form, if
the feature ends up in mainline at all.

E.g. it could make sense to split this file into several files in order
to avoid merge conflicts.

It also might make sense to prefill this file with IDs for many drivers.

E.g. the following script will use a modules.dep file to produce IDs for
modules. It's meant to be used on a very complete modules.dep build through
make allmodconfig && make -jN modules && make modules_install.


A file like this is going to be a nightmare to maintain and ensure that
it actually is correct, I don't see it as a viable solution, sorry.


How often will drivers be added? The only changes on this file will happen
if a driver will be added and then just one ID will be added.


Look at how many drivers we add every kernel release, it's a non-trivial
amount.


I still don't see your problem. As long as the IDs in the enum are 
ordered according to the directories, there won't be more merge 
conflicts than in the Makefile or Kconfig for that directory. And as 
mentioned, it's e.g. possible to split the one file into multiple ones, e.g.


enum driver_ids {

#include "foo"
#include "bar"

};

Of cource, the content of foo and bar might look a bit unusual.




As said above, the file could be filled with IDs for all existing modules,
regardless if they are already annotated.

If that's a nightmare, I wonder what how you name the necessary stuff to
maintain the link order through Makefiles.


There usually isn't a problem and the issue is that we link by
subsystem, so somehow it's all working fairly well.


So why should that be worse with the one file?

But, like you, I don't like such a big enum.

It's just that I didn't thought much about another solution, and the 
time I've spend to think about something else which provides a usable 
ID, didn't end in a solution. So I would be happy if someone else would 
offer an idea.


Regards,

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 20:23 schrieb Linus Torvalds:

On Sat, Oct 17, 2015 at 10:14 AM, Alexander Holler  wrote:


Assuming three different ethernet-drivers, without any special code,
the dependency graph would not require any special order inbetween them
and would look like that:


This seems *fundamentally* wrong.

This is in no way specific to network drivers (or to disk drivers, or
to anything else).

So requiring extra logic for this implies that something is seriously wrong.

If two drivers aren't ordered by dependencies, they should always be
in link order, regardless of any hacks like these. If they're not,
things are wrong.

I think your problem is that you make that dependency thing a separate
ordering, so now it matters whether a driver has a dependency or not.


I'm making dependencies the only ordering for annotated initcalls.

Otherwise it's impossible to call initcalls in parallel. I've seen a 
stable topological sort somewhere, but whenever you want to parallelize 
the initcalls, the stable ordering would be gone anyway. So I've decided 
not to look further at a stable topological sort.



If something like this is to work, it has to work *with* the normal
ordering, not outside of it and then have these kinds of broken
special cases.

The normal init orderings (ie core -> postcore -> arch -> subsys -> fs
-> rootfs -> device -> late) should just be an extra dependency, I
think.

The way that you just insert the annotated dependencies in between
levels 6 and 7 ("device" and "late") can't be right. It means - for
example - that you can't have subsystems that have dependencies.


Sorry, but that's wrong.

I've choosen to place initcalls between 5 and 6 to make it easier to 
move both, subsystems as well as normal drivers to the (new) level with 
annotated initcalls. If you look at what I've already "annotated", you 
will see there are quiet a lot initcalls I've moved from below 6 to the 
new level.



So I really think that if we do dependencies, then the current levels
have to be added as dependencies, so that "subsys_initcall(xyz)"
basically means "xyz depends on the 'subsys' event, and 'subsys_end'
depends on xyz". Then within that, you might have another bus driver
that in turn depends on 'xyz'.


It would be absolutely no problem to introduce "virtual" initcalls for 
any level, e.g. just


depencies = {
everything_below
}

initcall foo()
{
    return 0;
}

annotated_initcall(foo, id, dependencies),

Regards,

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


Re: [PATCH 0/14] init: deps: dependency based (parallelized) init

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 19:44 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 07:14:13PM +0200, Alexander Holler wrote:

Hello,

here is the newest version of my patches to use a dependency based
initialization order. It now works without DT too.

Background:

Currently initcalls are ordered by some levels and the link order. This
means whenever a file is renamed, changes directory or a Makefile is
modified the order with which initcalls are called might change. This
might result in problems. Furthermore, the required dependencies are
often not documented, sometimes there are comments in the source or in a
commit message, but most often the knowledge why a specific initcall
belongs to a specific initcall level isn't obvious without carefully
examing he source. And initcalls are used by drivers and subsystems, and
the count of both have grown quiet a lot in the last years. So it's
rather difficult to maintain a proper link order.


Files move around very rarely, is this really an issue?


No idea. You are maintaining the staging area. ;)




Another problem is that the link order can't be modified dynamically at
boot time to include dependencies dictated by the hardware. To circumvent
this, a brute-force trial-and-error mechanism called deferred probes has
been introduced, but this approach, while beeing KISS, has its own
problems.


What problems does deferred probing have?  Why not just fix that if
there is issues with it, as it was supposed to solve this issue without
needing to annotate anything.


I've not looked why deferred probes are sometimes causing such a large 
delay. But giving that it's brutforce and non-deterministic, some 
drivers might be probed a dozen times, or important drivers might be 
probed very late, forcing all previous probed drivers to be probed again 
(late). Just think at the case the link order is a, b, c but drivers 
have to be called in the order c, b, a.



To solve these problems I've written patches to use a topological sort at
boot time which uses dependencies to calculate the order with which
initcalls are called.

Why? What are the benefits (assuming correct dependencies are available)?

- It offers a clear in-source documentation for dependencies between
   initcalls.
- It is robust in regard to file or directory name changes and changes in
   a Makefile.
- If enabled, the order with which drivers for interfaces are called
   (e.g. network interfaces, hard disks), can be defined independent of
   the link order. These might result in more stable interface names or
   numbers.
- If enabled, it makes the the deferred probes obsolete, which might
   result in faster boot times.
- If enabled, it is possible to call initcalls in parallel. E.g. the
   shipped kernel for Fedora 21 (4.1.7-100.fc21.x86_64) contains around
   560 initcalls. These are all called in series. Also some of them use
   asynchronous stuff by themself, most don't do.


But that shipped kernel boots to X in less than 2 seconds, so there
isn't really a speed issue here, right?


It's noticeable if your phone, or any other thing you want to use (like 
your route planner, clock or whatever boots in one second instead of two 
when you turn it on.



Drawbacks:

- It requires a small amount of time to calculate the order a boot time.
   But this time is most often smaller than the time saved by using
   multiple cores to call initcalls or by not needing deferred probes.


How much time is needed?


I've measured 3ms on a slow ARM box.




- Dependencies are required. For everything which can be build as a
   module, looking at modules.dep might give some pointers. Looking at
   the help from menuconfig also might give some pointers. But in the
   end, the most preferable way would be if maintainers or other people
   which have a deeper knowledge about the source and functionality
   would add the dependencies.


How will a "normal" driver author figure out what those dependancies are
in order to be able to write them down?  That's my biggest objection


Most drivers are done c from an existing driver. And if someone adds 
new code, he should know what these new code is for and on what it depends.



here, I have no idea how to add these, nor how to properly review such a
submission.  What about systems that have different ordering/dependancy
requirements for the same drivers due to different ways the hardware is
hooked up?  That is not going to work well here, unless I'm missing
something.


Hmm, how is that solved now?

If you have dependencies dictated by a special HW, this dependencies 
should come in by the HW description.


The deferred probe mechanism exists just since 1 or 2 years. And once 
you had to search the source of non-displayed error in a set of several 
dozens possible sources (because many errors are now non-errors but 
-517) you might learn to hate deferred probes as much as I do. I'm sorry 
for the harsh words in regard to deferred probes.


The way to use dependencies doesn

Re: [PATCH 10/14] init: deps: IDs for annotated initcalls

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 19:45 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 07:14:23PM +0200, Alexander Holler wrote:

These patch contains the IDs for initcalls I've annotated.

This patch is NOT meant for merging into mainline in its current form.

It should be discussed about how to add these IDs and in which form, if
the feature ends up in mainline at all.

E.g. it could make sense to split this file into several files in order
to avoid merge conflicts.

It also might make sense to prefill this file with IDs for many drivers.

E.g. the following script will use a modules.dep file to produce IDs for
modules. It's meant to be used on a very complete modules.dep build through
make allmodconfig && make -jN modules && make modules_install.


A file like this is going to be a nightmare to maintain and ensure that
it actually is correct, I don't see it as a viable solution, sorry.


How often will drivers be added? The only changes on this file will 
happen if a driver will be added and then just one ID will be added.


As said above, the file could be filled with IDs for all existing 
modules, regardless if they are already annotated.


If that's a nightmare, I wonder what how you name the necessary stuff to 
maintain the link order through Makefiles.


But besides that, I'm totally open to any other idea how to identify a 
driver. For me, just using an enum looks like the most trivial way.


Alexander Holler

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


[PATCH 12/14] dt: dts: deps: kirkwood: dockstar: add dependency ehci -> usb power regulator

2015-10-17 Thread Alexander Holler
This serves as an example how easy it is to fix an initialization order
if the order depends on the DT. No source code changes will be necessary.

If you look at the dependency graph for the dockstar, you will see that
there is no dependency between ehci and the usb power regulator. This
ends up with the fact that the regulator will be initialized after ehci.

Fix this by adding one dependency to the .dts.

Signed-off-by: Alexander Holler 
---
 arch/arm/boot/dts/kirkwood-dockstar.dts | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/kirkwood-dockstar.dts 
b/arch/arm/boot/dts/kirkwood-dockstar.dts
index 8497363..426d8840 100644
--- a/arch/arm/boot/dts/kirkwood-dockstar.dts
+++ b/arch/arm/boot/dts/kirkwood-dockstar.dts
@@ -107,3 +107,7 @@
phy-handle = <>;
};
 };
+
+ {
+   dependencies = <_power>;
+};
-- 
2.1.0

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


[PATCH 13/14] dt: dts: deps: imx6q: make some remote-endpoints non-dependencies

2015-10-17 Thread Alexander Holler
This is necessary to remove dependency cycles introduced by
'remote-endpoints'.

Signed-off-by: Alexander Holler 
---
 arch/arm/boot/dts/imx6q.dtsi   | 2 ++
 arch/arm/boot/dts/imx6qdl.dtsi | 4 
 2 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 399103b..8db7f25 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -184,6 +184,7 @@
 
ipu2_di0_hdmi: endpoint@1 {
remote-endpoint = <_mux_2>;
+   no-dependencies = <_mux_2>;
};
 
ipu2_di0_mipi: endpoint@2 {
@@ -205,6 +206,7 @@
 
ipu2_di1_hdmi: endpoint@1 {
remote-endpoint = <_mux_3>;
+   no-dependencies = <_mux_3>;
};
 
ipu2_di1_mipi: endpoint@2 {
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index b57033e..db3d0d0 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -1150,10 +1150,12 @@
 
ipu1_di0_hdmi: endpoint@1 {
remote-endpoint = <_mux_0>;
+   no-dependencies = <_mux_0>;
};
 
ipu1_di0_mipi: endpoint@2 {
remote-endpoint = <_mux_0>;
+   no-dependencies = <_mux_0>;
};
 
ipu1_di0_lvds0: endpoint@3 {
@@ -1175,10 +1177,12 @@
 
ipu1_di1_hdmi: endpoint@1 {
remote-endpoint = <_mux_1>;
+   no-dependencies = <_mux_1>;
};
 
ipu1_di1_mipi: endpoint@2 {
remote-endpoint = <_mux_1>;
+   no-dependencies = <_mux_1>;
};
 
ipu1_di1_lvds0: endpoint@3 {
-- 
2.1.0

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


[PATCH 14/14] dt: dts: deps: omap: beagle: make some remote-endpoints non-dependencies

2015-10-17 Thread Alexander Holler
This is necessary to remove dependency cycles introduced by
'remote-endpoints'.

Signed-off-by: Alexander Holler 
---
 arch/arm/boot/dts/omap3-beagle.dts | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-beagle.dts 
b/arch/arm/boot/dts/omap3-beagle.dts
index a547411..78ba39e 100644
--- a/arch/arm/boot/dts/omap3-beagle.dts
+++ b/arch/arm/boot/dts/omap3-beagle.dts
@@ -101,6 +101,7 @@
 
tfp410_in: endpoint@0 {
remote-endpoint = <_out>;
+   no-dependencies = <_out>;
};
};
 
@@ -109,6 +110,7 @@
 
tfp410_out: endpoint@0 {
remote-endpoint = <_connector_in>;
+   no-dependencies = <_connector_in>;
};
};
};
@@ -150,6 +152,7 @@
etb_in: endpoint {
slave-mode;
remote-endpoint = <_out>;
+   no-dependencies = <_out>;
};
};
};
@@ -373,6 +376,7 @@
port {
venc_out: endpoint {
remote-endpoint = <_connector_in>;
+   no-dependencies = <_connector_in>;
ti,channels = <2>;
};
};
-- 
2.1.0

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


[PATCH 03/14] init: deps: dt: use (HW-specific) dependencies provided by the DT too

2015-10-17 Thread Alexander Holler
This patch adds dependencies provided by the hardware description in
the used DT. This avoids the use of the deferred probe mechanism
on most (if not all) DT based kernels.

Drawback is that the binary DT blob has to be enhanced with type
information for phandles (which are used as dependencies) which
needs a modified dtc.

Signed-off-by: Alexander Holler 
---
 drivers/of/base.c   | 114 
 include/linux/of.h  |   3 ++
 init/dependencies.c |   4 ++
 3 files changed, 121 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 8b5a187..423ddff 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -12,6 +12,8 @@
  *  Reconsolidated from arch/x/kernel/prom.c by Stephen Rothwell and
  *  Grant Likely.
  *
+ *  The dependency related stuff was done by Alexander Holler.
+ *
  *  This program is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU General Public License
  *  as published by the Free Software Foundation; either version
@@ -2308,3 +2310,115 @@ struct device_node *of_graph_get_remote_port(const 
struct device_node *node)
return of_get_next_parent(np);
 }
 EXPORT_SYMBOL(of_graph_get_remote_port);
+
+#ifdef CONFIG_DEPENDENCIES
+
+static const struct _annotated_initcall * __init find_matching_driver(
+   const struct _annotated_initcall *from, const struct device_node *node)
+{
+   while (++from != __annotated_initcall_end)
+   if (from->driver &&
+   __of_match_node(from->driver->of_match_table,
+   node))
+   return from;
+   return NULL;
+}
+
+static int __init add_dep_list(const struct device_node *node, unsigned drvid)
+{
+   const __be32 *list, *list_end;
+   uint32_t ph;
+   int size = 0;
+   int rc = 0;
+   const struct device_node *dep;
+   const struct _annotated_initcall *ac;
+
+   list = __of_get_property(node, "dependencies", );
+   if (!list || !size || size % sizeof(*list))
+   return 0;
+   list_end = list + size / sizeof(*list);
+   while (list < list_end) {
+   ph = be32_to_cpup(list++);
+   if (unlikely(!ph)) {
+   /* Should never happen */
+   if (node->name)
+   pr_warn("phandle == 0 for %s\n", node->name);
+   continue;
+   }
+   dep = of_find_node_by_phandle(ph);
+   if (unlikely(!dep)) {
+   pr_err("No DT node for dependency with phandle 0x%x 
found\n",
+   ph);
+   continue;
+   }
+   ac = __annotated_initcall_start - 1;
+   while ((ac = find_matching_driver(ac, dep))) {
+   if (!ac->id)
+   continue;
+   rc = add_initcall_dependency(drvid, ac->id);
+   if (rc)
+   return rc;
+   }
+   }
+
+   return rc;
+}
+
+static int __init add_deps(unsigned parent, const struct device_node *node)
+{
+   struct device_node *child;
+   const struct _annotated_initcall *ac;
+   int rc = 0;
+   bool found_one_driver = false;
+
+   if (!__of_device_is_available(node))
+   return 0;
+   if (__of_get_property(node, "compatible", NULL)) {
+   ac = __annotated_initcall_start - 1;
+   while ((ac = find_matching_driver(ac, node))) {
+   if (!ac->id)
+   continue;
+   found_one_driver = true;
+   rc = add_initcall_dependency(ac->id, parent);
+   if (unlikely(rc))
+   return rc;
+   rc = add_dep_list(node, ac->id);
+   if (unlikely(rc))
+   return rc;
+   for_each_child_of_node(node, child) {
+   rc = add_deps(ac->id, child);
+   if (unlikely(rc))
+   return rc;
+   }
+   }
+   if (found_one_driver)
+   return rc;
+   }
+   for_each_child_of_node(node, child) {
+   rc = add_deps(parent, child);
+   if (unlikely(rc))
+   break;
+   }
+
+   return rc;
+}
+
+int __init of_add_dependencies(void)
+{
+   int rc = 0;
+   struct device_node *child;
+   struct device_node *root =  of_find_node_by_path("/");
+
+   if (unlikely(!root))
+   return -EINVAL;
+
+   for_each_child_of_node(root, child) {
+   rc = add_deps(0, child);

[PATCH 06/14] dtc: deps: Automatically add new property 'dependencies' which contains a list of referenced phandles

2015-10-17 Thread Alexander Holler
During the step from .dts to .dtb the information about dependcies
contained in the .dts through phandle references is lost. This makes it
impossible to use the binary blob to create a dependency graph without
knowing the semantic of all cell arrays.

Therefor automatically add a new property called 'dependencies' to all
nodes which have phandle references in one of their properties.

This new property will contain an array of phandles with one value for
every phandle referenced by other properties in the node.

If such a property already exists (e.g. to manually add dependencies
through the .dts), the existing list will be expanded.

Added phandles will be the phandle of either the referenced node itself
(if it has a property named 'compatible', or of the next parent of the
referenced node which as property named 'compatible'. This ensures only
dependencies to drivers will be added.

References to phandles of parent or child nodes will not be added to this
property, because this information is already contained in the blob (in
the form of the tree itself).

No dependencies to disabled nodes will be added.

Signed-off-by: Alexander Holler 
---
 scripts/dtc/Makefile   |   3 +-
 scripts/dtc/Makefile.dtc   |   1 +
 scripts/dtc/dependencies.c | 108 +
 scripts/dtc/dtc.c  |  12 -
 scripts/dtc/dtc.h  |   3 ++
 5 files changed, 125 insertions(+), 2 deletions(-)
 create mode 100644 scripts/dtc/dependencies.c

diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
index 2a48022..1174cf9 100644
--- a/scripts/dtc/Makefile
+++ b/scripts/dtc/Makefile
@@ -4,7 +4,7 @@ hostprogs-y := dtc
 always := $(hostprogs-y)
 
 dtc-objs   := dtc.o flattree.o fstree.o data.o livetree.o treesource.o \
-  srcpos.o checks.o util.o
+  srcpos.o checks.o util.o dependencies.o
 dtc-objs   += dtc-lexer.lex.o dtc-parser.tab.o
 
 # Source files need to get at the userspace version of libfdt_env.h to compile
@@ -13,6 +13,7 @@ HOSTCFLAGS_DTC := -I$(src) -I$(src)/libfdt
 
 HOSTCFLAGS_checks.o := $(HOSTCFLAGS_DTC)
 HOSTCFLAGS_data.o := $(HOSTCFLAGS_DTC)
+HOSTCFLAGS_dependencies.o := $(HOSTCFLAGS_DTC)
 HOSTCFLAGS_dtc.o := $(HOSTCFLAGS_DTC)
 HOSTCFLAGS_flattree.o := $(HOSTCFLAGS_DTC)
 HOSTCFLAGS_fstree.o := $(HOSTCFLAGS_DTC)
diff --git a/scripts/dtc/Makefile.dtc b/scripts/dtc/Makefile.dtc
index bece49b..5fb5343 100644
--- a/scripts/dtc/Makefile.dtc
+++ b/scripts/dtc/Makefile.dtc
@@ -6,6 +6,7 @@
 DTC_SRCS = \
checks.c \
data.c \
+   dependencies.c \
dtc.c \
flattree.c \
fstree.c \
diff --git a/scripts/dtc/dependencies.c b/scripts/dtc/dependencies.c
new file mode 100644
index 000..dd4658c
--- /dev/null
+++ b/scripts/dtc/dependencies.c
@@ -0,0 +1,108 @@
+/*
+ * Code to add a property which contains dependencies (used phandle references)
+ * to all (driver) nodes which are having phandle references.
+ *
+ * Copyright (C) 2014 Alexander Holler 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include 
+
+/* Searches upwards for a node with a property 'compatible' */
+static struct node *find_compatible_not_disabled(struct node *node)
+{
+   struct property *prop;
+
+   while (node) {
+   prop = get_property(node, "compatible");
+   if (prop) {
+   prop = get_property(node, "status");
+   if (prop)
+   if (!prop->val.len ||
+   (strcmp(prop->val.val, "okay") &&
+   strcmp(prop->val.val, "ok")))
+   return NULL; /* disabled */
+   return node;
+   }
+   node = node->parent;
+   }
+   return NULL;
+}
+
+static bool is_parent_of(struct node *node1, struct node *node2)
+{
+   while (node2) {
+   if (node2->parent == node1)
+   return true;
+   node2 = node2->parent;
+   }
+   return false;
+
+}
+
+static void add_deps(struct node *dt, struct node *node, struct property *prop)
+{
+   struct marker *m = prop->val.markers;
+   struct node *refnode;
+   cell_t phandle;
+   struct property *prop_deps;
+   unsigned i;
+   cell_t *cell;
+   struct node *source;
+   struct node *target;
+
+   for_each_marker_of_type(m, REF_PHANDLE) {
+   assert(m->offset + sizeof(cell_t) <= prop->val.len);
+
+   refnode = get_node_by_ref(dt, m->ref);
+   if (!refnode) {
+   fprintf(stderr,
+   

[PATCH 09/14] dtc: deps: Add option to print dependency graph as dot (Graphviz)

2015-10-17 Thread Alexander Holler
Add option -T do print a dependency graph in dot format for
generating a picture with Graphviz.

E.g.

dtc -T foo.dts | dot -T svg -o foo.svg

would generate the picture foo.png with the dependency graph.

Convential dependencies (those based on the tree structure) are having
black arrows, dependencies based on the property 'dependencies' are
having cyan arrows.

Option -D to not automatically add dependencies does still work, so
you could build a classic dependency graph with

dtc -D -T foo.dts | dot -T png -o foo_no_auto_deps.png

This works with binary blobs as input too. E.g.

CROSS_COMPILE=gcc-foo ARCH=arm make foo.dtb
scripts/dtc/dtc -I dtb -T arch/arm/boot/dts/foo.dtb

would print the dot file.

Signed-off-by: Alexander Holler 
---
 scripts/dtc/dependencies.c | 49 --
 scripts/dtc/dtc.c  | 19 +++---
 scripts/dtc/dtc.h  |  2 +-
 3 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/scripts/dtc/dependencies.c b/scripts/dtc/dependencies.c
index 360d8f5..76e4d91 100644
--- a/scripts/dtc/dependencies.c
+++ b/scripts/dtc/dependencies.c
@@ -331,7 +331,7 @@ static void __init topological_sort(void)
depth_first_search(i);
 }
 
-static int __init add_dep_list(struct device_node *node)
+static int __init add_dep_list(struct device_node *node, bool print_dot)
 {
const __be32 *list, *list_end;
uint32_t ph;
@@ -361,6 +361,9 @@ static int __init add_dep_list(struct device_node *node)
rc = insert_edge(node->phandle, ph);
if (rc)
break;
+   if (print_dot)
+   printf("  node0x%x -> node0x%x [color=cyan]\n",
+   node->phandle, ph);
}
 
return rc;
@@ -385,9 +388,10 @@ static const char *of_prop_next_string(struct property 
*prop, const char *cur)
 }
 
 static int __init add_deps_lnx(struct device_node *parent,
-   struct device_node *node)
+   struct device_node *node, bool print_dot)
 {
struct device_node *child;
+   const char *cp;
int rc = 0;
 
if (!__of_device_is_available(node))
@@ -408,13 +412,34 @@ static int __init add_deps_lnx(struct device_node *parent,
return -EINVAL;
}
order.parent_by_phandle[node->phandle] = parent->phandle;
-   rc = add_dep_list(node);
+   if (print_dot) {
+   struct property *prop;
+
+   printf("node0x%x [label=\"0x%x %s", node->phandle,
+   node->phandle, node->name);
+   if (node->full_name)
+   printf(" (%s)", node->full_name);
+   prop = get_property(node, "compatible");
+   if (prop) {
+   printf("\\n");
+   for (cp = of_prop_next_string(prop, NULL); cp;
+cp = of_prop_next_string(prop, cp)) {
+   if (cp != prop->val.val)
+   putchar(' ');
+   printf("%s", cp);
+   }
+   }
+   printf("\"];\n");
+   printf("  node0x%x -> node0x%x\n", node->phandle,
+   parent->phandle);
+   }
+   rc = add_dep_list(node, print_dot);
if (unlikely(rc))
return rc;
parent = node; /* change the parent only if node is a driver */
}
for_each_child_of_node(node, child) {
-   rc = add_deps_lnx(parent, child);
+   rc = add_deps_lnx(parent, child, print_dot);
if (unlikely(rc))
break;
}
@@ -457,7 +482,7 @@ void __init of_init_print_order(const char *name)
}
 }
 
-int __init of_init_build_order(struct device_node *root)
+int __init of_init_build_order(struct device_node *root, const char *print_dot)
 {
struct device_node *child;
int rc = 0;
@@ -469,12 +494,24 @@ int __init of_init_build_order(struct device_node *root)
calc_max_phandle(root);
order.old_max_phandle = order.max_phandle;
 
+   if (print_dot) {
+   printf("digraph G {\n");
+   printf("node0x%x [label=\"0x%x root (/)\"];\n",
+   order.max_phandle+1, order.max_phandle+1);
+   }
+
for_each_child_of_node(root, child) {
-   rc = add_deps_lnx(root, chi

[PATCH 08/14] dtc: deps: Add option to print initialization order

2015-10-17 Thread Alexander Holler
Add option -t to print the default initialization order.
No other output will be generated.

To print the order, just use something like this:

CROSS_COMPILE=gcc-foo ARCH=arm make foo.dtb
scripts/dtc/dtc -I dtb -t arch/arm/boot/dts/foo.dtb

Since it's now possible to check to for cycles in the dependency graph,
this is now done too.

Signed-off-by: Alexander Holler 
---
 scripts/dtc/dependencies.c | 344 +
 scripts/dtc/dtc.c  |  24 +++-
 scripts/dtc/dtc.h  |   2 +
 3 files changed, 369 insertions(+), 1 deletion(-)

diff --git a/scripts/dtc/dependencies.c b/scripts/dtc/dependencies.c
index 77d5c54..360d8f5 100644
--- a/scripts/dtc/dependencies.c
+++ b/scripts/dtc/dependencies.c
@@ -139,3 +139,347 @@ void add_dependencies(struct boot_info *bi)
process_nodes_props(bi->dt, bi->dt);
del_prop_no_dependencies(bi->dt);
 }
+
+/*
+ * The code below is in large parts a copy of drivers/of/of_dependencies.c
+ * in the Linux kernel. So both files do share the same bugs.
+ * The next few ugly defines do exist to keep the differences at a minimum.
+ */
+static struct node *tree;
+#define pr_cont(format, ...) printf(format, ##__VA_ARGS__)
+#define pr_info(format, ...) printf(format, ##__VA_ARGS__)
+#define pr_warn(format, ...) printf(format, ##__VA_ARGS__)
+#define pr_err(format, ...) fprintf(stderr, format, ##__VA_ARGS__)
+typedef cell_t __be32;
+#define device_node node
+#define full_name fullpath
+#define __initdata
+#define __init
+#define unlikely(a) (a)
+#define of_node_put(a)
+#define of_find_node_by_phandle(v) get_node_by_phandle(tree, v)
+#define __of_get_property(a, b, c) get_property(a, b)
+#define for_each_child_of_node(a, b) for_each_child(a, b)
+
+
+#define MAX_DT_NODES 1000 /* maximum number of vertices */
+#define MAX_EDGES (MAX_DT_NODES*2) /* maximum number of edges (dependencies) */
+
+struct edgenode {
+   uint32_t y; /* phandle */
+   struct edgenode *next; /* next edge in list */
+};
+
+/*
+ * Vertex numbers do correspond to phandle numbers. That means the graph
+ * does contain as much vertices as the maximum of all phandles.
+ * Or in other words, we assume that for all phandles in the device tree
+ * 0 < phandle < MAX_DT_NODES+1 is true.
+ */
+struct dep_graph {
+   struct edgenode edge_slots[MAX_EDGES]; /* used to avoid kmalloc */
+   struct edgenode *edges[MAX_DT_NODES+1]; /* adjacency info */
+   unsigned nvertices; /* number of vertices in graph */
+   unsigned nedges; /* number of edges in graph */
+   bool processed[MAX_DT_NODES+1]; /* which vertices have been processed */
+   bool include_node[MAX_DT_NODES+1]; /* which nodes to consider */
+   bool discovered[MAX_DT_NODES+1]; /* which vertices have been found */
+   bool finished; /* if true, cut off search immediately */
+};
+static struct dep_graph graph __initdata;
+
+struct init_order {
+   uint32_t max_phandle; /* the max used phandle */
+   uint32_t old_max_phandle; /* used to keep track of added phandles */
+   struct device_node *order[MAX_DT_NODES+1];
+   unsigned count;
+   /* Used to keep track of parent devices in regard to the DT */
+   uint32_t parent_by_phandle[MAX_DT_NODES+1];
+   struct device *device_by_phandle[MAX_DT_NODES+1];
+};
+static struct init_order order __initdata;
+
+
+/* Copied from drivers/of/base.c (because it's lockless). */
+static int __init __of_device_is_available(struct device_node *device)
+{
+   struct property *status;
+
+   if (!device)
+   return 0;
+
+   status = get_property(device, "status");
+   if (status == NULL)
+   return 1;
+
+   if (status->val.len > 0) {
+   if (!strcmp(status->val.val, "okay") ||
+   !strcmp(status->val.val, "ok"))
+   return 1;
+   }
+
+   return 0;
+}
+
+/*
+ * x is a dependant of y or in other words
+ * y will be initialized before x.
+ */
+static int __init insert_edge(uint32_t x, uint32_t y)
+{
+   struct edgenode *p; /* temporary pointer */
+
+   if (unlikely(x > MAX_DT_NODES || y > MAX_DT_NODES)) {
+   pr_err("Node found with phandle 0x%x > MAX_DT_NODES (%d)!\n",
+   x > MAX_DT_NODES ? x : y, MAX_DT_NODES);
+   return -EINVAL;
+   }
+   if (unlikely(!x || !y))
+   return 0;
+   if (unlikely(graph.nedges >= MAX_EDGES)) {
+   pr_err("Maximum number of edges (%d) reached!\n", MAX_EDGES);
+   return -EINVAL;
+   }
+   p = _slots[graph.nedges++];
+   graph.include_node[x] = 1;
+   graph.include_node[y] = 1;
+   p->y = y;
+   p->next = graph.edges[x];
+   graph.edges[x] = p; /* insert at head of list */
+
+   graph.nvertices = (x > graph.nvertices) ? x : graph.nvertices;

[PATCH 10/14] init: deps: IDs for annotated initcalls

2015-10-17 Thread Alexander Holler
These patch contains the IDs for initcalls I've annotated.

This patch is NOT meant for merging into mainline in its current form.

It should be discussed about how to add these IDs and in which form, if
the feature ends up in mainline at all.

E.g. it could make sense to split this file into several files in order
to avoid merge conflicts.

It also might make sense to prefill this file with IDs for many drivers.

E.g. the following script will use a modules.dep file to produce IDs for
modules. It's meant to be used on a very complete modules.dep build through
make allmodconfig && make -jN modules && make modules_install.


if [ $# -ne 2 -o ! -f "$1" -o -f "$2" ]; then
  echo "Builds drvids"
  echo "usage: $(basename $0) modules.dep headerfile"
  echo "headerfile must not exist"
  exit 1
fi

echo -e "#ifndef _LINUX_DRIVER_IDS_H\n#define _LINUX_DRIVER_IDS_H\n\nenum 
{\n\tdrvid_unused," > "$2"
sed -e 's%.*/\(.*\).ko.gz:.*%\tdrvid_\1,%' "$1" | sed -e 's/-/_/g' >> "$2"
echo -e "\tdrvid_max\n};\n\n#endif /* _LINUX_DRIVER_IDS_H */\n" >> "$2"


Signed-off-by: Alexander Holler 
---
 include/linux/driver_ids.h | 581 -
 1 file changed, 580 insertions(+), 1 deletion(-)

diff --git a/include/linux/driver_ids.h b/include/linux/driver_ids.h
index 330080e..f029eaf 100644
--- a/include/linux/driver_ids.h
+++ b/include/linux/driver_ids.h
@@ -14,7 +14,214 @@
 
 enum {
drvid_unused,
-   /* To be filled */
+
+   /* arch/arm/common */
+   drvid_edma,
+   /* arch/arm/crypto */
+   drvid_aes_arm,
+   drvid_aes_ce_arm,
+   drvid_aesbs,
+   drvid_sha1_mod,
+   drvid_sha1_neon,
+   /* arch/arm/kernel */
+   drvid_customize_machine,
+   drvid_proc_cpu,
+   drvid_proc_dma_arm,
+   drvid_swp_emulation,
+   /* arch/arm/mach-imx */
+   drvid_imx_gpc,
+   drvid_imx_mmdc,
+   /* arch/arm/mach-omap2 */
+   drvid_omap_i2c_cmdline,
+   /* arch/arm/mm */
+   drvid_alignment,
+   drvid_atomic_pool,
+   drvid_dma_debug,
+   /* arch/arm/plat-omap */
+   drvid_omap_dm_timer,
+   /* arch/x86/crypto */
+   drvid_aes_x86,
+   /* arch/x86/kernel */
+   drvid_apm,
+   drvid_cpufreq_tsc,
+   drvid_hpet_late,
+   drvid_kernel_offset_dumper,
+   drvid_pci_iommu,
+   drvid_pmc_atom,
+   drvid_reboot,
+   drvid_rtc_cmos_x86,
+   drvid_tsc_clocksource,
+   /* arch/x86/kernel/acpi */
+   drvid_ffh_cstate,
+   drvid_hpet_insert_resource,
+   /* arch/x86/kernel/cpu/mcheck */
+   drvid_mcheck,
+   drvid_mcheck_debugfs,
+   drvid_thermal_throttle,
+   /* arch/x86/kernel/cpu/microcode */
+   drvid_microcode,
+   /* arch/x86/pci */
+   drvid_pci_arch,
+   drvid_pci_subsys,
+   drvid_pcibios_assign_resources,
+   /* block */
+   drvid_bio,
+   drvid_blk_dev_integrity,
+   drvid_blk_ioc,
+   drvid_blk_iopoll,
+   drvid_blk_mq,
+   drvid_blk_scsi_ioctl,
+   drvid_blk_settings,
+   drvid_blk_softirq,
+   drvid_bsg,
+   drvid_cfq_iosched,
+   drvid_deadline_iosched,
+   drvid_genhd,
+   drvid_noop,
+   drvid_proc_genhd,
+   /* crypto */
+   drvid_aes_generic,
+   drvid_authenc,
+   drvid_authencesn,
+   drvid_cbc,
+   drvid_chainiv,
+   drvid_crc32c,
+   drvid_crct10dif,
+   drvid_cryptd,
+   drvid_crypto_algapi,
+   drvid_crypto_wq,
+   drvid_cryptomgr,
+   drvid_deflate,
+   drvid_des_generic,
+   drvid_ecb,
+   drvid_eseqiv,
+   drvid_hmac,
+   drvid_lzo,
+   drvid_mcryptd,
+   drvid_md5,
+   drvid_michael_mic,
+   drvid_sha1_generic,
+   drvid_sha256_generic,
+   drvid_sha512_generic,
+   drvid_xor,
+   /* crypto/asymmetric_keys */
+   drvid_asymmetric_key,
+   drvid_x509,
+   /* drivers/acpi */
+   drvid_acpi,
+   drvid_acpi_ac,
+   drvid_acpi_battery,
+   drvid_acpi_button,
+   drvid_acpi_event,
+   drvid_acpi_fan,
+   drvid_acpi_processor_driver,
+   drvid_acpi_reserve_resources,
+   drvid_acpi_sbs,
+   drvid_acpi_smb_hc,
+   drvid_acpi_thermal,
+   /* drivers/ata */
+   drvid_ahci,
+   drvid_ahci_imx,
+   drvid_ata,
+   drvid_ata_generic,
+   drvid_ata_piix,
+   drvid_pata_acpi,
+   drvid_pata_amd,
+   drvid_pata_jmicron,
+   drvid_pata_mpiix,
+   drvid_pata_oldpiix,
+   drvid_pata_sch,
+   drvid_pata_sis,
+   /* drivers/base */
+   drvid_firmware_class,
+   /* drivers/block */
+   drvid_loop,
+   /* drivers/bus */
+   drvid_omap_l3_noc,
+   drvid_omap_l3_smx,
+   drvid_omap_ocp2scp,
+   drvid_imx_weim,
+   /* drivers/char */
+   drvid_agpgart,

[PATCH 05/14] init: deps: order I2C bus drivers by their ID

2015-10-17 Thread Alexander Holler
In order to provide a stable I2C bus numbering, we order I2C bus drivers
according to their driver ID.

If that is better or worse than ordering by link order might have to be
discussed, but for now, it's good to provide an example about how to order
something based on driver IDs.

Signed-off-by: Alexander Holler 
---
 include/linux/driver_ids.h |  9 +
 init/dependencies.c| 25 +
 2 files changed, 34 insertions(+)

diff --git a/include/linux/driver_ids.h b/include/linux/driver_ids.h
index 1133a68..330080e 100644
--- a/include/linux/driver_ids.h
+++ b/include/linux/driver_ids.h
@@ -17,6 +17,15 @@ enum {
/* To be filled */
 
/*
+* I2C bus drivers will be ordered according to their ID (which
+* means according to their appearance here).
+* This provides stable I2C bus numbers.
+* Therefor their IDs have to be in the following block.
+*/
+   drvid_i2c_busses_start,
+   drvid_i2c_busses_end,
+
+   /*
 * Network drivers will be ordered according to the link order
 * (which means not necessarily according to their appearance
 * here).
diff --git a/init/dependencies.c b/init/dependencies.c
index 027fc4b..d20d6fc 100644
--- a/init/dependencies.c
+++ b/init/dependencies.c
@@ -298,10 +298,22 @@ static int __init add_dependencies(void)
return 0;
 }
 
+static int __init compare_unsigned(const void *lhs, const void *rhs)
+{
+   if (*(unsigned *)lhs < *(unsigned *)rhs)
+   return -1;
+   if (*(unsigned *)lhs > *(unsigned *)rhs)
+   return 1;
+   return 0;
+}
+
 static void __init build_inventory(void)
 {
const struct _annotated_initcall *ac;
unsigned id_last_network_driver = 0;
+   unsigned id_i2c_bus_driver[drvid_i2c_busses_end
+  - drvid_i2c_busses_start];
+   unsigned count_i2c_bus_drivers = 0;
 
ac = __annotated_initcall_start;
for (; ac < __annotated_initcall_end; ++ac) {
@@ -316,6 +328,19 @@ static void __init build_inventory(void)
id_last_network_driver);
id_last_network_driver = ac->id;
}
+   /* order I2C bus drivers by their ID */
+   if (ac->id > drvid_i2c_busses_start &&
+   ac->id < drvid_i2c_busses_end)
+   id_i2c_bus_driver[count_i2c_bus_drivers++] = ac->id;
+   }
+   if (count_i2c_bus_drivers > 1) {
+   unsigned i;
+
+   sort(id_i2c_bus_driver, count_i2c_bus_drivers,
+   sizeof(unsigned), _unsigned, NULL);
+   for (i = 1; i < count_i2c_bus_drivers; ++i)
+   add_initcall_dependency(id_i2c_bus_driver[i],
+   id_i2c_bus_driver[i-1]);
}
 }
 
-- 
2.1.0

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


[PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler
In order to provide stable interface numbers, network interface drivers
will be ordered by the link order. This is easy to accomplish by adding
dependencies.

Assuming three different ethernet-drivers, without any special code,
the dependency graph would not require any special order inbetween them
and would look like that:

eth-driver-base
   /  |   \
eth-x   eth-yeth-z

Now we just add dependencies. With the additional dependencies the graph
looks like:

 eth-driver-base
  | | |
eth-x   | |
  | | |
eth-y  -| |
  |   |
eth-z  ---|

Signed-off-by: Alexander Holler 
---
 include/linux/driver_ids.h | 11 +++
 init/dependencies.c|  9 +
 2 files changed, 20 insertions(+)

diff --git a/include/linux/driver_ids.h b/include/linux/driver_ids.h
index 60964fe..1133a68 100644
--- a/include/linux/driver_ids.h
+++ b/include/linux/driver_ids.h
@@ -15,6 +15,17 @@
 enum {
drvid_unused,
/* To be filled */
+
+   /*
+* Network drivers will be ordered according to the link order
+* (which means not necessarily according to their appearance
+* here).
+* This provides stable interface numbers.
+* Therefor their IDs have to be in the following block.
+*/
+   drvid_network_drivers_start,
+   drvid_network_drivers_end,
+
drvid_max
 };
 
diff --git a/init/dependencies.c b/init/dependencies.c
index b484f67..027fc4b 100644
--- a/init/dependencies.c
+++ b/init/dependencies.c
@@ -301,12 +301,21 @@ static int __init add_dependencies(void)
 static void __init build_inventory(void)
 {
const struct _annotated_initcall *ac;
+   unsigned id_last_network_driver = 0;
 
ac = __annotated_initcall_start;
for (; ac < __annotated_initcall_end; ++ac) {
include_node[ac->id] = true;
annotated_initcall_by_drvid[ac->id] = ac;
nvertices = max(nvertices, ac->id);
+   /* order network drivers by link order*/
+   if (ac->id > drvid_network_drivers_start &&
+   ac->id < drvid_network_drivers_end) {
+   if (id_last_network_driver)
+   add_initcall_dependency(ac->id,
+   id_last_network_driver);
+   id_last_network_driver = ac->id;
+   }
}
 }
 
-- 
2.1.0

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


[PATCH 07/14] dtc: deps: introduce new (virtual) property no-dependencies

2015-10-17 Thread Alexander Holler
In some cases it makes sense to handle some phandles not as dependencies.

This is escpecially true for 'remote-endpoint' properties, because these
otherwise introducing dependency cycles into the graph. To avoid these,
one end of each remote-endpoint pairs has not to be handled as a
dependency.

The syntax is like

foo {
remote-endpoint = <>;
};
bar {
remote-endpoint = <>;
no-dependencies = <>;
};

Without that 'no-dependencies' property dtc would automatically add a
dependency to foo to the property 'dependencies' of the node bar.
But with that 'no-dependencies' it will not automatically add the
listed dependencies.

The property 'no-dependencies' is virtual property and will not be added
to any output file.

Signed-off-by: Alexander Holler 
---
 scripts/dtc/dependencies.c | 33 +
 1 file changed, 33 insertions(+)

diff --git a/scripts/dtc/dependencies.c b/scripts/dtc/dependencies.c
index dd4658c..77d5c54 100644
--- a/scripts/dtc/dependencies.c
+++ b/scripts/dtc/dependencies.c
@@ -44,6 +44,23 @@ static bool is_parent_of(struct node *node1, struct node 
*node2)
 
 }
 
+static bool is_no_dependency(struct node *dt, struct property *prop, cell_t ph)
+{
+   struct node *node;
+   unsigned i;
+   cell_t *cell = (cell_t *)(prop->val.val);
+
+   for (i = 0; i < prop->val.len/4; ++i) {
+   node = get_node_by_phandle(dt, cpu_to_fdt32(*cell++));
+   if (node) {
+   node = find_compatible_not_disabled(node);
+   if (node && get_node_phandle(dt, node) == ph)
+   return true;
+   }
+   }
+   return false;
+}
+
 static void add_deps(struct node *dt, struct node *node, struct property *prop)
 {
struct marker *m = prop->val.markers;
@@ -73,6 +90,10 @@ static void add_deps(struct node *dt, struct node *node, 
struct property *prop)
is_parent_of(target, source))
continue;
phandle = get_node_phandle(dt, target);
+   prop_deps = get_property(node, "no-dependencies");
+   if (prop_deps && is_no_dependency(dt, prop_deps, phandle))
+   /* avoid adding non-dependencies */
+   continue;
prop_deps = get_property(source, "dependencies");
if (!prop_deps) {
add_property(source,
@@ -102,7 +123,19 @@ static void process_nodes_props(struct node *dt, struct 
node *node)
process_nodes_props(dt, child);
 }
 
+static void del_prop_no_dependencies(struct node *node)
+{
+   struct node *child;
+
+   if (!node)
+   return;
+   delete_property_by_name(node, "no-dependencies");
+   for_each_child(node, child)
+   del_prop_no_dependencies(child);
+}
+
 void add_dependencies(struct boot_info *bi)
 {
process_nodes_props(bi->dt, bi->dt);
+   del_prop_no_dependencies(bi->dt);
 }
-- 
2.1.0

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


[PATCH 02/14] init: deps: use annotated initcalls for a dependency based (optionally parallelized) init

2015-10-17 Thread Alexander Holler
Based on the dependencies provided by annotated initcalls, this patch
introduces a topological sort to sort initcalls and (optionally) uses
multiple threads to call initcalls.

If the feature is disabled, nothing changes.

Signed-off-by: Alexander Holler 
---
 include/linux/init.h |   6 +
 init/.gitignore  |   1 +
 init/Kconfig.deps|  38 +
 init/Makefile|  15 ++
 init/dependencies.c  | 394 +++
 init/main.c  |  10 +-
 lib/Kconfig.debug|   1 +
 7 files changed, 464 insertions(+), 1 deletion(-)
 create mode 100644 init/.gitignore
 create mode 100644 init/Kconfig.deps
 create mode 100644 init/dependencies.c

diff --git a/include/linux/init.h b/include/linux/init.h
index 758fd18..264f83f 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -160,6 +160,12 @@ extern void (*late_time_init)(void);
 
 extern bool initcall_debug;
 
+/* Defined in init/dependencies.c */
+void __init do_annotated_initcalls(void);
+
+/* id_dependency will be initialized before id */
+int __init add_initcall_dependency(unsigned id, unsigned id_dependency);
+
 #endif
   
 #ifndef MODULE
diff --git a/init/.gitignore b/init/.gitignore
new file mode 100644
index 000..38e6d06
--- /dev/null
+++ b/init/.gitignore
@@ -0,0 +1 @@
+driver_names.c
diff --git a/init/Kconfig.deps b/init/Kconfig.deps
new file mode 100644
index 000..9ced0d4
--- /dev/null
+++ b/init/Kconfig.deps
@@ -0,0 +1,38 @@
+config DEPENDENCIES
+   bool "Use dependency based initialization sequence (DO NOT USE)"
+   select ANNOTATED_INITCALLS
+   help
+ This will likely crash your kernel at startup. You have been warned.
+ That means you should make sure you have a working backup kernel
+ you can boot from in case the kernel with this feature turned on
+ crashes.
+ In order to benefit from this feature, statically linked drivers
+ have to provide dependencies.
+
+config DEPENDENCIES_PRINT_INIT_ORDER
+   bool "Print dependency based initialization order"
+   depends on DEPENDENCIES
+   help
+ Used for debugging purposes.
+
+config DEPENDENCIES_PRINT_CALLS
+   bool "Show when annotated initcalls are actually called"
+   depends on DEPENDENCIES
+   help
+ Used for debugging purposes.
+
+config DEPENDENCIES_PARALLEL
+   bool "Call annotated initcalls in parallel"
+   depends on DEPENDENCIES
+   help
+ Calculates which (annotated) initcalls can be called in parallel
+ and calls them using multiple threads.
+
+config DEPENDENCIES_THREADS
+   int "Number of threads to use for parallel initialization"
+   depends on DEPENDENCIES_PARALLEL
+   default 0
+   help
+ 0 means the number of threads used for parallel initialization
+ of drivers equals the number of online CPUs.
+ 1 means the threaded initialization is disabled.
diff --git a/init/Makefile b/init/Makefile
index 7bc47ee..6a8c22c 100644
--- a/init/Makefile
+++ b/init/Makefile
@@ -9,6 +9,7 @@ else
 obj-$(CONFIG_BLK_DEV_INITRD)   += initramfs.o
 endif
 obj-$(CONFIG_GENERIC_CALIBRATE_DELAY) += calibrate.o
+obj-$(CONFIG_DEPENDENCIES) += dependencies.o
 
 ifneq ($(CONFIG_ARCH_INIT_TASK),y)
 obj-y  += init_task.o
@@ -19,6 +20,20 @@ mounts-$(CONFIG_BLK_DEV_RAM) += do_mounts_rd.o
 mounts-$(CONFIG_BLK_DEV_INITRD)+= do_mounts_initrd.o
 mounts-$(CONFIG_BLK_DEV_MD)+= do_mounts_md.o
 
+quiet_cmd_make-driver_names = GEN $@
+  cmd_make-driver_names = sed $< > $@ \
+   -e 's/^\tdrvid_\(.*\),/\t"\1",/' \
+   -e 's/^\tdrvid_max$$/\t"max"/' \
+   -e 's/^enum {/static const char *driver_names[] __initdata = 
{/' \
+   -e '/^\#ifndef _LINUX_DRIVER_IDS_H$$/d' \
+   -e '/^\#define _LINUX_DRIVER_IDS_H$$/d' \
+   -e '/^\#endif \/\* _LINUX_DRIVER_IDS_H \*\/$$/d'
+
+$(obj)/driver_names.c: $(srctree)/include/linux/driver_ids.h
+   $(call cmd,make-driver_names)
+
+$(obj)/dependencies.o: $(obj)/driver_names.c
+
 # dependencies on generated files need to be listed explicitly
 $(obj)/version.o: include/generated/compile.h
 
diff --git a/init/dependencies.c b/init/dependencies.c
new file mode 100644
index 000..c47817c
--- /dev/null
+++ b/init/dependencies.c
@@ -0,0 +1,394 @@
+/*
+ * Code for building a deterministic initialization order
+ * based on dependencies.
+ *
+ * Copyright (C) 2014 Alexander Holler 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+/* #define DEBUG */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#if defined(CONFIG_DEPENDENCIES_PRINT_INIT_ORDER) \
+

[PATCH 01/14] init: deps: introduce annotated initcalls

2015-10-17 Thread Alexander Holler
Make it possible to identify initcalls before calling them by adding an
ID, an optional pointer to a list of IDs the initcalls depends on and
an optional pointer to a struct device_driver.

This is e.g. necessary in order to sort initcalls by whatever means
before calling them.

To annotate an initcall, the following changes are necessary on
drivers which want to offer that feature:

now annotated

pure_initcall(fn)
annotated_initcall(pure, fn, id, dependencies) or
annotated_initcall_drv(pure, fn, id, dependencies, drv)
core_initcall(fn)
annotated_initcall(core, fn, id, dependencies) or
annotated_initcall_drv(core, fn, id, dependencies, drv)
core_initcall_sync(fn)
annotated_initcall_sync(core, fn, id, dependencies) or
annotated_initcall_sync_drv(core, fn, id, dependencies,
drv)
(...)
late_initcall(fn)
annotated_initcall(late, fn, id, dependencies)
module_init(fn)
annotated_module_init(fn, id, dependencies)

module_platform_driver(drv)
annotated_module_platform_driver(drv, id, dependencies)
module_platform_driver_probe(drv, probe)
annotated:module_platform_driver_probe(drv, probe, id,
 dependencies)
module_i2c_driver(i2c_drv)
annotated_module_i2c_driver(i2c_drv, id, dependencies)
module_usb_driver(usb_drv)
annotated_module_usb_driver(usb_drv, id, dependencies)
module_phy_driver(__phy_drivers)
annotated_module_phy_driver(__phy_drivers, id,
dependencies)
module_pci_driver(pci_driver)
annotated_module_pci_driver(pci_driver, id,
dependencies)
module_serio_driver(serio_driver)
annotated_module_serio_driver(serio_driver, id,
dependencies)
module_acpi_driver(acpi_driver)
annotated_module_acpi_driver(acpi_driver, id,
dependencies)

E.g. to make the driver sram offering an annotated initcall the
following patch is necessary:


-postcore_initcall(sram_init);
+annotated_initcall_drv(postcore, sram_init, drvid_sram, NULL,
+   sram_driver.driver);


The change for a module with dependencies might look like:


-module_platform_driver(gpio_led_driver);
+static const unsigned dependencies[] __initdata __maybe_unused = {
+   drvid_leds,
+   0
+};
+
+annotated_module_platform_driver(gpio_led_driver, drvid_gpio_led,
+   dependencies);


These changes can be done without any fear. If the feature is disabled,
which is the default, the new macros will just map to the old ones and
nothing is changed at all.

Signed-off-by: Alexander Holler 
---
 arch/arm/kernel/vmlinux.lds.S |  1 +
 arch/arm/mach-omap2/soc.h | 10 ++-
 drivers/usb/storage/usb.h | 14 ++
 include/acpi/acpi_bus.h   | 13 +
 include/asm-generic/vmlinux.lds.h |  7 +
 include/linux/device.h| 14 ++
 include/linux/driver_ids.h| 21 ++
 include/linux/i2c.h   |  4 +++
 include/linux/init.h  | 58 +++
 include/linux/module.h|  7 +
 include/linux/pci.h   |  4 +++
 include/linux/phy.h   | 16 +++
 include/linux/platform_device.h   | 41 +--
 include/linux/serio.h |  5 
 include/linux/usb.h   | 12 
 init/Kconfig  |  3 ++
 16 files changed, 226 insertions(+), 4 deletions(-)
 create mode 100644 include/linux/driver_ids.h

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 8b60fde..c22784e 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -213,6 +213,7 @@ SECTIONS
 #endif
INIT_SETUP(16)
INIT_CALLS
+   ANNOTATED_INITCALLS
CON_INITCALL
SECURITY_INITCALL
INIT_RAM_FS
diff --git a/arch/arm/mach-omap2/soc.h b/arch/arm/mach-omap2/soc.h
index f97654d..75f7012 100644
--- a/arch/arm/mach-omap2/soc.h
+++ b/arch/arm/mach-omap2/soc.h
@@ -554,5 +554,13 @@ level(__##fn);
 #define omap_late_initcall(fn) omap_initcall(late_initcall, fn)
 #define omap_late_initcall_sync(fn)omap_initcall(late_initcall_sync, fn)
 
-#endif /* __ASSEMBLY__ */
+#define annotated_omap_initcall(level, fn, id, deps)   \
+static int __init __used __##fn(void)  \
+{  \
+   if (!soc_is_omap

[PATCH 0/14] init: deps: dependency based (parallelized) init

2015-10-17 Thread Alexander Holler
 need them in the mainline kernel.
As long as you are able to use a polite language, you can send me any
comment, regardless if it's good or bad. But, please, keep away with
Baby-Speak or contumelious comments.

And, just in case, I'm aware that adding the necessary dependencies means
some effort and a lot of (trivial) commits and therefor having all
possible initcalls annotated would be a long term goal. But, besides that
this could be done smoothly without any need to hurry, I think it makes
sense. Otherwise, in my humble opinion, the problems to keep an overview
and ordering initcalls will just become worse.


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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 18.10.2015 um 07:14 schrieb Greg Kroah-Hartman:

On Sun, Oct 18, 2015 at 06:59:22AM +0200, Alexander Holler wrote:

Am 17.10.2015 um 21:36 schrieb Greg Kroah-Hartman:


Again, parallelizing does not solve anything, and causes more problems
_and_ makes things take longer.  Try it, we have done it in the past and
proven this, it's pretty easy to test :)


Just because I'm curious, may I ask how I would test that in the easy way
you have in mind? I've just posted the results of my tests (the patch
series) but I wonder what you do have in mind.


Use the tool, scripts/bootgraph.pl to create a boot graph of your boot
sequence.  That should show you the drivers, or other areas, that are
causing your boot to be "slow".


So I've misunderstood you. I've read your paragraph as that it's easy to 
test parallelizing.



Hope this helps,


Unfortunately no, but thanks anyway. ;)

Alexander Holler

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 21:36 schrieb Greg Kroah-Hartman:


Again, parallelizing does not solve anything, and causes more problems
_and_ makes things take longer.  Try it, we have done it in the past and
proven this, it's pretty easy to test :)


Just because I'm curious, may I ask how I would test that in the easy 
way you have in mind? I've just posted the results of my tests (the 
patch series) but I wonder what you do have in mind.


Regards,

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


[PATCH 01/14] init: deps: introduce annotated initcalls

2015-10-17 Thread Alexander Holler
Make it possible to identify initcalls before calling them by adding an
ID, an optional pointer to a list of IDs the initcalls depends on and
an optional pointer to a struct device_driver.

This is e.g. necessary in order to sort initcalls by whatever means
before calling them.

To annotate an initcall, the following changes are necessary on
drivers which want to offer that feature:

now annotated

pure_initcall(fn)
annotated_initcall(pure, fn, id, dependencies) or
annotated_initcall_drv(pure, fn, id, dependencies, drv)
core_initcall(fn)
annotated_initcall(core, fn, id, dependencies) or
annotated_initcall_drv(core, fn, id, dependencies, drv)
core_initcall_sync(fn)
annotated_initcall_sync(core, fn, id, dependencies) or
annotated_initcall_sync_drv(core, fn, id, dependencies,
drv)
(...)
late_initcall(fn)
annotated_initcall(late, fn, id, dependencies)
module_init(fn)
annotated_module_init(fn, id, dependencies)

module_platform_driver(drv)
annotated_module_platform_driver(drv, id, dependencies)
module_platform_driver_probe(drv, probe)
annotated:module_platform_driver_probe(drv, probe, id,
 dependencies)
module_i2c_driver(i2c_drv)
annotated_module_i2c_driver(i2c_drv, id, dependencies)
module_usb_driver(usb_drv)
annotated_module_usb_driver(usb_drv, id, dependencies)
module_phy_driver(__phy_drivers)
annotated_module_phy_driver(__phy_drivers, id,
dependencies)
module_pci_driver(pci_driver)
annotated_module_pci_driver(pci_driver, id,
dependencies)
module_serio_driver(serio_driver)
annotated_module_serio_driver(serio_driver, id,
dependencies)
module_acpi_driver(acpi_driver)
annotated_module_acpi_driver(acpi_driver, id,
dependencies)

E.g. to make the driver sram offering an annotated initcall the
following patch is necessary:


-postcore_initcall(sram_init);
+annotated_initcall_drv(postcore, sram_init, drvid_sram, NULL,
+   sram_driver.driver);


The change for a module with dependencies might look like:


-module_platform_driver(gpio_led_driver);
+static const unsigned dependencies[] __initdata __maybe_unused = {
+   drvid_leds,
+   0
+};
+
+annotated_module_platform_driver(gpio_led_driver, drvid_gpio_led,
+   dependencies);


These changes can be done without any fear. If the feature is disabled,
which is the default, the new macros will just map to the old ones and
nothing is changed at all.

Signed-off-by: Alexander Holler <hol...@ahsoftware.de>
---
 arch/arm/kernel/vmlinux.lds.S |  1 +
 arch/arm/mach-omap2/soc.h | 10 ++-
 drivers/usb/storage/usb.h | 14 ++
 include/acpi/acpi_bus.h   | 13 +
 include/asm-generic/vmlinux.lds.h |  7 +
 include/linux/device.h| 14 ++
 include/linux/driver_ids.h| 21 ++
 include/linux/i2c.h   |  4 +++
 include/linux/init.h  | 58 +++
 include/linux/module.h|  7 +
 include/linux/pci.h   |  4 +++
 include/linux/phy.h   | 16 +++
 include/linux/platform_device.h   | 41 +--
 include/linux/serio.h |  5 
 include/linux/usb.h   | 12 
 init/Kconfig  |  3 ++
 16 files changed, 226 insertions(+), 4 deletions(-)
 create mode 100644 include/linux/driver_ids.h

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 8b60fde..c22784e 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -213,6 +213,7 @@ SECTIONS
 #endif
INIT_SETUP(16)
INIT_CALLS
+   ANNOTATED_INITCALLS
CON_INITCALL
SECURITY_INITCALL
INIT_RAM_FS
diff --git a/arch/arm/mach-omap2/soc.h b/arch/arm/mach-omap2/soc.h
index f97654d..75f7012 100644
--- a/arch/arm/mach-omap2/soc.h
+++ b/arch/arm/mach-omap2/soc.h
@@ -554,5 +554,13 @@ level(__##fn);
 #define omap_late_initcall(fn) omap_initcall(late_initcall, fn)
 #define omap_late_initcall_sync(fn)omap_initcall(late_initcall_sync, fn)
 
-#endif /* __ASSEMBLY__ */
+#define annotated_omap_initcall(level, fn, id, deps)   \
+static int __init __used __##fn(void)  \
+{  \
+   if (!soc_i

[PATCH 0/14] init: deps: dependency based (parallelized) init

2015-10-17 Thread Alexander Holler
 need them in the mainline kernel.
As long as you are able to use a polite language, you can send me any
comment, regardless if it's good or bad. But, please, keep away with
Baby-Speak or contumelious comments.

And, just in case, I'm aware that adding the necessary dependencies means
some effort and a lot of (trivial) commits and therefor having all
possible initcalls annotated would be a long term goal. But, besides that
this could be done smoothly without any need to hurry, I think it makes
sense. Otherwise, in my humble opinion, the problems to keep an overview
and ordering initcalls will just become worse.


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


[PATCH 02/14] init: deps: use annotated initcalls for a dependency based (optionally parallelized) init

2015-10-17 Thread Alexander Holler
Based on the dependencies provided by annotated initcalls, this patch
introduces a topological sort to sort initcalls and (optionally) uses
multiple threads to call initcalls.

If the feature is disabled, nothing changes.

Signed-off-by: Alexander Holler <hol...@ahsoftware.de>
---
 include/linux/init.h |   6 +
 init/.gitignore  |   1 +
 init/Kconfig.deps|  38 +
 init/Makefile|  15 ++
 init/dependencies.c  | 394 +++
 init/main.c  |  10 +-
 lib/Kconfig.debug|   1 +
 7 files changed, 464 insertions(+), 1 deletion(-)
 create mode 100644 init/.gitignore
 create mode 100644 init/Kconfig.deps
 create mode 100644 init/dependencies.c

diff --git a/include/linux/init.h b/include/linux/init.h
index 758fd18..264f83f 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -160,6 +160,12 @@ extern void (*late_time_init)(void);
 
 extern bool initcall_debug;
 
+/* Defined in init/dependencies.c */
+void __init do_annotated_initcalls(void);
+
+/* id_dependency will be initialized before id */
+int __init add_initcall_dependency(unsigned id, unsigned id_dependency);
+
 #endif
   
 #ifndef MODULE
diff --git a/init/.gitignore b/init/.gitignore
new file mode 100644
index 000..38e6d06
--- /dev/null
+++ b/init/.gitignore
@@ -0,0 +1 @@
+driver_names.c
diff --git a/init/Kconfig.deps b/init/Kconfig.deps
new file mode 100644
index 000..9ced0d4
--- /dev/null
+++ b/init/Kconfig.deps
@@ -0,0 +1,38 @@
+config DEPENDENCIES
+   bool "Use dependency based initialization sequence (DO NOT USE)"
+   select ANNOTATED_INITCALLS
+   help
+ This will likely crash your kernel at startup. You have been warned.
+ That means you should make sure you have a working backup kernel
+ you can boot from in case the kernel with this feature turned on
+ crashes.
+ In order to benefit from this feature, statically linked drivers
+ have to provide dependencies.
+
+config DEPENDENCIES_PRINT_INIT_ORDER
+   bool "Print dependency based initialization order"
+   depends on DEPENDENCIES
+   help
+ Used for debugging purposes.
+
+config DEPENDENCIES_PRINT_CALLS
+   bool "Show when annotated initcalls are actually called"
+   depends on DEPENDENCIES
+   help
+ Used for debugging purposes.
+
+config DEPENDENCIES_PARALLEL
+   bool "Call annotated initcalls in parallel"
+   depends on DEPENDENCIES
+   help
+ Calculates which (annotated) initcalls can be called in parallel
+ and calls them using multiple threads.
+
+config DEPENDENCIES_THREADS
+   int "Number of threads to use for parallel initialization"
+   depends on DEPENDENCIES_PARALLEL
+   default 0
+   help
+ 0 means the number of threads used for parallel initialization
+ of drivers equals the number of online CPUs.
+ 1 means the threaded initialization is disabled.
diff --git a/init/Makefile b/init/Makefile
index 7bc47ee..6a8c22c 100644
--- a/init/Makefile
+++ b/init/Makefile
@@ -9,6 +9,7 @@ else
 obj-$(CONFIG_BLK_DEV_INITRD)   += initramfs.o
 endif
 obj-$(CONFIG_GENERIC_CALIBRATE_DELAY) += calibrate.o
+obj-$(CONFIG_DEPENDENCIES) += dependencies.o
 
 ifneq ($(CONFIG_ARCH_INIT_TASK),y)
 obj-y  += init_task.o
@@ -19,6 +20,20 @@ mounts-$(CONFIG_BLK_DEV_RAM) += do_mounts_rd.o
 mounts-$(CONFIG_BLK_DEV_INITRD)+= do_mounts_initrd.o
 mounts-$(CONFIG_BLK_DEV_MD)+= do_mounts_md.o
 
+quiet_cmd_make-driver_names = GEN $@
+  cmd_make-driver_names = sed $< > $@ \
+   -e 's/^\tdrvid_\(.*\),/\t"\1",/' \
+   -e 's/^\tdrvid_max$$/\t"max"/' \
+   -e 's/^enum {/static const char *driver_names[] __initdata = 
{/' \
+   -e '/^\#ifndef _LINUX_DRIVER_IDS_H$$/d' \
+   -e '/^\#define _LINUX_DRIVER_IDS_H$$/d' \
+   -e '/^\#endif \/\* _LINUX_DRIVER_IDS_H \*\/$$/d'
+
+$(obj)/driver_names.c: $(srctree)/include/linux/driver_ids.h
+   $(call cmd,make-driver_names)
+
+$(obj)/dependencies.o: $(obj)/driver_names.c
+
 # dependencies on generated files need to be listed explicitly
 $(obj)/version.o: include/generated/compile.h
 
diff --git a/init/dependencies.c b/init/dependencies.c
new file mode 100644
index 000..c47817c
--- /dev/null
+++ b/init/dependencies.c
@@ -0,0 +1,394 @@
+/*
+ * Code for building a deterministic initialization order
+ * based on dependencies.
+ *
+ * Copyright (C) 2014 Alexander Holler <hol...@ahsoftware.de>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+/* #define DEBUG */
+
+#include 
+#include 
+#include 
+#inclu

[PATCH 05/14] init: deps: order I2C bus drivers by their ID

2015-10-17 Thread Alexander Holler
In order to provide a stable I2C bus numbering, we order I2C bus drivers
according to their driver ID.

If that is better or worse than ordering by link order might have to be
discussed, but for now, it's good to provide an example about how to order
something based on driver IDs.

Signed-off-by: Alexander Holler <hol...@ahsoftware.de>
---
 include/linux/driver_ids.h |  9 +
 init/dependencies.c| 25 +
 2 files changed, 34 insertions(+)

diff --git a/include/linux/driver_ids.h b/include/linux/driver_ids.h
index 1133a68..330080e 100644
--- a/include/linux/driver_ids.h
+++ b/include/linux/driver_ids.h
@@ -17,6 +17,15 @@ enum {
/* To be filled */
 
/*
+* I2C bus drivers will be ordered according to their ID (which
+* means according to their appearance here).
+* This provides stable I2C bus numbers.
+* Therefor their IDs have to be in the following block.
+*/
+   drvid_i2c_busses_start,
+   drvid_i2c_busses_end,
+
+   /*
 * Network drivers will be ordered according to the link order
 * (which means not necessarily according to their appearance
 * here).
diff --git a/init/dependencies.c b/init/dependencies.c
index 027fc4b..d20d6fc 100644
--- a/init/dependencies.c
+++ b/init/dependencies.c
@@ -298,10 +298,22 @@ static int __init add_dependencies(void)
return 0;
 }
 
+static int __init compare_unsigned(const void *lhs, const void *rhs)
+{
+   if (*(unsigned *)lhs < *(unsigned *)rhs)
+   return -1;
+   if (*(unsigned *)lhs > *(unsigned *)rhs)
+   return 1;
+   return 0;
+}
+
 static void __init build_inventory(void)
 {
const struct _annotated_initcall *ac;
unsigned id_last_network_driver = 0;
+   unsigned id_i2c_bus_driver[drvid_i2c_busses_end
+  - drvid_i2c_busses_start];
+   unsigned count_i2c_bus_drivers = 0;
 
ac = __annotated_initcall_start;
for (; ac < __annotated_initcall_end; ++ac) {
@@ -316,6 +328,19 @@ static void __init build_inventory(void)
id_last_network_driver);
id_last_network_driver = ac->id;
}
+   /* order I2C bus drivers by their ID */
+   if (ac->id > drvid_i2c_busses_start &&
+   ac->id < drvid_i2c_busses_end)
+   id_i2c_bus_driver[count_i2c_bus_drivers++] = ac->id;
+   }
+   if (count_i2c_bus_drivers > 1) {
+   unsigned i;
+
+   sort(id_i2c_bus_driver, count_i2c_bus_drivers,
+   sizeof(unsigned), _unsigned, NULL);
+   for (i = 1; i < count_i2c_bus_drivers; ++i)
+   add_initcall_dependency(id_i2c_bus_driver[i],
+   id_i2c_bus_driver[i-1]);
}
 }
 
-- 
2.1.0

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


[PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler
In order to provide stable interface numbers, network interface drivers
will be ordered by the link order. This is easy to accomplish by adding
dependencies.

Assuming three different ethernet-drivers, without any special code,
the dependency graph would not require any special order inbetween them
and would look like that:

eth-driver-base
   /  |   \
eth-x   eth-yeth-z

Now we just add dependencies. With the additional dependencies the graph
looks like:

 eth-driver-base
  | | |
eth-x   | |
  | | |
eth-y  -| |
  |   |
eth-z  ---|

Signed-off-by: Alexander Holler <hol...@ahsoftware.de>
---
 include/linux/driver_ids.h | 11 +++
 init/dependencies.c|  9 +
 2 files changed, 20 insertions(+)

diff --git a/include/linux/driver_ids.h b/include/linux/driver_ids.h
index 60964fe..1133a68 100644
--- a/include/linux/driver_ids.h
+++ b/include/linux/driver_ids.h
@@ -15,6 +15,17 @@
 enum {
drvid_unused,
/* To be filled */
+
+   /*
+* Network drivers will be ordered according to the link order
+* (which means not necessarily according to their appearance
+* here).
+* This provides stable interface numbers.
+* Therefor their IDs have to be in the following block.
+*/
+   drvid_network_drivers_start,
+   drvid_network_drivers_end,
+
drvid_max
 };
 
diff --git a/init/dependencies.c b/init/dependencies.c
index b484f67..027fc4b 100644
--- a/init/dependencies.c
+++ b/init/dependencies.c
@@ -301,12 +301,21 @@ static int __init add_dependencies(void)
 static void __init build_inventory(void)
 {
const struct _annotated_initcall *ac;
+   unsigned id_last_network_driver = 0;
 
ac = __annotated_initcall_start;
for (; ac < __annotated_initcall_end; ++ac) {
include_node[ac->id] = true;
annotated_initcall_by_drvid[ac->id] = ac;
nvertices = max(nvertices, ac->id);
+   /* order network drivers by link order*/
+   if (ac->id > drvid_network_drivers_start &&
+   ac->id < drvid_network_drivers_end) {
+   if (id_last_network_driver)
+   add_initcall_dependency(ac->id,
+   id_last_network_driver);
+   id_last_network_driver = ac->id;
+   }
}
 }
 
-- 
2.1.0

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


[PATCH 07/14] dtc: deps: introduce new (virtual) property no-dependencies

2015-10-17 Thread Alexander Holler
In some cases it makes sense to handle some phandles not as dependencies.

This is escpecially true for 'remote-endpoint' properties, because these
otherwise introducing dependency cycles into the graph. To avoid these,
one end of each remote-endpoint pairs has not to be handled as a
dependency.

The syntax is like

foo {
remote-endpoint = <>;
};
bar {
remote-endpoint = <>;
no-dependencies = <>;
};

Without that 'no-dependencies' property dtc would automatically add a
dependency to foo to the property 'dependencies' of the node bar.
But with that 'no-dependencies' it will not automatically add the
listed dependencies.

The property 'no-dependencies' is virtual property and will not be added
to any output file.

Signed-off-by: Alexander Holler <hol...@ahsoftware.de>
---
 scripts/dtc/dependencies.c | 33 +
 1 file changed, 33 insertions(+)

diff --git a/scripts/dtc/dependencies.c b/scripts/dtc/dependencies.c
index dd4658c..77d5c54 100644
--- a/scripts/dtc/dependencies.c
+++ b/scripts/dtc/dependencies.c
@@ -44,6 +44,23 @@ static bool is_parent_of(struct node *node1, struct node 
*node2)
 
 }
 
+static bool is_no_dependency(struct node *dt, struct property *prop, cell_t ph)
+{
+   struct node *node;
+   unsigned i;
+   cell_t *cell = (cell_t *)(prop->val.val);
+
+   for (i = 0; i < prop->val.len/4; ++i) {
+   node = get_node_by_phandle(dt, cpu_to_fdt32(*cell++));
+   if (node) {
+   node = find_compatible_not_disabled(node);
+   if (node && get_node_phandle(dt, node) == ph)
+   return true;
+   }
+   }
+   return false;
+}
+
 static void add_deps(struct node *dt, struct node *node, struct property *prop)
 {
struct marker *m = prop->val.markers;
@@ -73,6 +90,10 @@ static void add_deps(struct node *dt, struct node *node, 
struct property *prop)
is_parent_of(target, source))
continue;
phandle = get_node_phandle(dt, target);
+   prop_deps = get_property(node, "no-dependencies");
+   if (prop_deps && is_no_dependency(dt, prop_deps, phandle))
+   /* avoid adding non-dependencies */
+   continue;
prop_deps = get_property(source, "dependencies");
if (!prop_deps) {
add_property(source,
@@ -102,7 +123,19 @@ static void process_nodes_props(struct node *dt, struct 
node *node)
process_nodes_props(dt, child);
 }
 
+static void del_prop_no_dependencies(struct node *node)
+{
+   struct node *child;
+
+   if (!node)
+   return;
+   delete_property_by_name(node, "no-dependencies");
+   for_each_child(node, child)
+   del_prop_no_dependencies(child);
+}
+
 void add_dependencies(struct boot_info *bi)
 {
process_nodes_props(bi->dt, bi->dt);
+   del_prop_no_dependencies(bi->dt);
 }
-- 
2.1.0

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


[PATCH 10/14] init: deps: IDs for annotated initcalls

2015-10-17 Thread Alexander Holler
These patch contains the IDs for initcalls I've annotated.

This patch is NOT meant for merging into mainline in its current form.

It should be discussed about how to add these IDs and in which form, if
the feature ends up in mainline at all.

E.g. it could make sense to split this file into several files in order
to avoid merge conflicts.

It also might make sense to prefill this file with IDs for many drivers.

E.g. the following script will use a modules.dep file to produce IDs for
modules. It's meant to be used on a very complete modules.dep build through
make allmodconfig && make -jN modules && make modules_install.


if [ $# -ne 2 -o ! -f "$1" -o -f "$2" ]; then
  echo "Builds drvids"
  echo "usage: $(basename $0) modules.dep headerfile"
  echo "headerfile must not exist"
  exit 1
fi

echo -e "#ifndef _LINUX_DRIVER_IDS_H\n#define _LINUX_DRIVER_IDS_H\n\nenum 
{\n\tdrvid_unused," > "$2"
sed -e 's%.*/\(.*\).ko.gz:.*%\tdrvid_\1,%' "$1" | sed -e 's/-/_/g' >> "$2"
echo -e "\tdrvid_max\n};\n\n#endif /* _LINUX_DRIVER_IDS_H */\n" >> "$2"


Signed-off-by: Alexander Holler <hol...@ahsoftware.de>
---
 include/linux/driver_ids.h | 581 -
 1 file changed, 580 insertions(+), 1 deletion(-)

diff --git a/include/linux/driver_ids.h b/include/linux/driver_ids.h
index 330080e..f029eaf 100644
--- a/include/linux/driver_ids.h
+++ b/include/linux/driver_ids.h
@@ -14,7 +14,214 @@
 
 enum {
drvid_unused,
-   /* To be filled */
+
+   /* arch/arm/common */
+   drvid_edma,
+   /* arch/arm/crypto */
+   drvid_aes_arm,
+   drvid_aes_ce_arm,
+   drvid_aesbs,
+   drvid_sha1_mod,
+   drvid_sha1_neon,
+   /* arch/arm/kernel */
+   drvid_customize_machine,
+   drvid_proc_cpu,
+   drvid_proc_dma_arm,
+   drvid_swp_emulation,
+   /* arch/arm/mach-imx */
+   drvid_imx_gpc,
+   drvid_imx_mmdc,
+   /* arch/arm/mach-omap2 */
+   drvid_omap_i2c_cmdline,
+   /* arch/arm/mm */
+   drvid_alignment,
+   drvid_atomic_pool,
+   drvid_dma_debug,
+   /* arch/arm/plat-omap */
+   drvid_omap_dm_timer,
+   /* arch/x86/crypto */
+   drvid_aes_x86,
+   /* arch/x86/kernel */
+   drvid_apm,
+   drvid_cpufreq_tsc,
+   drvid_hpet_late,
+   drvid_kernel_offset_dumper,
+   drvid_pci_iommu,
+   drvid_pmc_atom,
+   drvid_reboot,
+   drvid_rtc_cmos_x86,
+   drvid_tsc_clocksource,
+   /* arch/x86/kernel/acpi */
+   drvid_ffh_cstate,
+   drvid_hpet_insert_resource,
+   /* arch/x86/kernel/cpu/mcheck */
+   drvid_mcheck,
+   drvid_mcheck_debugfs,
+   drvid_thermal_throttle,
+   /* arch/x86/kernel/cpu/microcode */
+   drvid_microcode,
+   /* arch/x86/pci */
+   drvid_pci_arch,
+   drvid_pci_subsys,
+   drvid_pcibios_assign_resources,
+   /* block */
+   drvid_bio,
+   drvid_blk_dev_integrity,
+   drvid_blk_ioc,
+   drvid_blk_iopoll,
+   drvid_blk_mq,
+   drvid_blk_scsi_ioctl,
+   drvid_blk_settings,
+   drvid_blk_softirq,
+   drvid_bsg,
+   drvid_cfq_iosched,
+   drvid_deadline_iosched,
+   drvid_genhd,
+   drvid_noop,
+   drvid_proc_genhd,
+   /* crypto */
+   drvid_aes_generic,
+   drvid_authenc,
+   drvid_authencesn,
+   drvid_cbc,
+   drvid_chainiv,
+   drvid_crc32c,
+   drvid_crct10dif,
+   drvid_cryptd,
+   drvid_crypto_algapi,
+   drvid_crypto_wq,
+   drvid_cryptomgr,
+   drvid_deflate,
+   drvid_des_generic,
+   drvid_ecb,
+   drvid_eseqiv,
+   drvid_hmac,
+   drvid_lzo,
+   drvid_mcryptd,
+   drvid_md5,
+   drvid_michael_mic,
+   drvid_sha1_generic,
+   drvid_sha256_generic,
+   drvid_sha512_generic,
+   drvid_xor,
+   /* crypto/asymmetric_keys */
+   drvid_asymmetric_key,
+   drvid_x509,
+   /* drivers/acpi */
+   drvid_acpi,
+   drvid_acpi_ac,
+   drvid_acpi_battery,
+   drvid_acpi_button,
+   drvid_acpi_event,
+   drvid_acpi_fan,
+   drvid_acpi_processor_driver,
+   drvid_acpi_reserve_resources,
+   drvid_acpi_sbs,
+   drvid_acpi_smb_hc,
+   drvid_acpi_thermal,
+   /* drivers/ata */
+   drvid_ahci,
+   drvid_ahci_imx,
+   drvid_ata,
+   drvid_ata_generic,
+   drvid_ata_piix,
+   drvid_pata_acpi,
+   drvid_pata_amd,
+   drvid_pata_jmicron,
+   drvid_pata_mpiix,
+   drvid_pata_oldpiix,
+   drvid_pata_sch,
+   drvid_pata_sis,
+   /* drivers/base */
+   drvid_firmware_class,
+   /* drivers/block */
+   drvid_loop,
+   /* drivers/bus */
+   drvid_omap_l3_noc,
+   drvid_omap_l3_smx,
+   drvid_omap_ocp2scp,
+   drvid_imx_weim,
+   /* drivers/char */
+ 

[PATCH 03/14] init: deps: dt: use (HW-specific) dependencies provided by the DT too

2015-10-17 Thread Alexander Holler
This patch adds dependencies provided by the hardware description in
the used DT. This avoids the use of the deferred probe mechanism
on most (if not all) DT based kernels.

Drawback is that the binary DT blob has to be enhanced with type
information for phandles (which are used as dependencies) which
needs a modified dtc.

Signed-off-by: Alexander Holler <hol...@ahsoftware.de>
---
 drivers/of/base.c   | 114 
 include/linux/of.h  |   3 ++
 init/dependencies.c |   4 ++
 3 files changed, 121 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 8b5a187..423ddff 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -12,6 +12,8 @@
  *  Reconsolidated from arch/x/kernel/prom.c by Stephen Rothwell and
  *  Grant Likely.
  *
+ *  The dependency related stuff was done by Alexander Holler.
+ *
  *  This program is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU General Public License
  *  as published by the Free Software Foundation; either version
@@ -2308,3 +2310,115 @@ struct device_node *of_graph_get_remote_port(const 
struct device_node *node)
return of_get_next_parent(np);
 }
 EXPORT_SYMBOL(of_graph_get_remote_port);
+
+#ifdef CONFIG_DEPENDENCIES
+
+static const struct _annotated_initcall * __init find_matching_driver(
+   const struct _annotated_initcall *from, const struct device_node *node)
+{
+   while (++from != __annotated_initcall_end)
+   if (from->driver &&
+   __of_match_node(from->driver->of_match_table,
+   node))
+   return from;
+   return NULL;
+}
+
+static int __init add_dep_list(const struct device_node *node, unsigned drvid)
+{
+   const __be32 *list, *list_end;
+   uint32_t ph;
+   int size = 0;
+   int rc = 0;
+   const struct device_node *dep;
+   const struct _annotated_initcall *ac;
+
+   list = __of_get_property(node, "dependencies", );
+   if (!list || !size || size % sizeof(*list))
+   return 0;
+   list_end = list + size / sizeof(*list);
+   while (list < list_end) {
+   ph = be32_to_cpup(list++);
+   if (unlikely(!ph)) {
+   /* Should never happen */
+   if (node->name)
+   pr_warn("phandle == 0 for %s\n", node->name);
+   continue;
+   }
+   dep = of_find_node_by_phandle(ph);
+   if (unlikely(!dep)) {
+   pr_err("No DT node for dependency with phandle 0x%x 
found\n",
+   ph);
+   continue;
+   }
+   ac = __annotated_initcall_start - 1;
+   while ((ac = find_matching_driver(ac, dep))) {
+   if (!ac->id)
+   continue;
+   rc = add_initcall_dependency(drvid, ac->id);
+   if (rc)
+   return rc;
+   }
+   }
+
+   return rc;
+}
+
+static int __init add_deps(unsigned parent, const struct device_node *node)
+{
+   struct device_node *child;
+   const struct _annotated_initcall *ac;
+   int rc = 0;
+   bool found_one_driver = false;
+
+   if (!__of_device_is_available(node))
+   return 0;
+   if (__of_get_property(node, "compatible", NULL)) {
+   ac = __annotated_initcall_start - 1;
+   while ((ac = find_matching_driver(ac, node))) {
+   if (!ac->id)
+   continue;
+   found_one_driver = true;
+   rc = add_initcall_dependency(ac->id, parent);
+   if (unlikely(rc))
+   return rc;
+   rc = add_dep_list(node, ac->id);
+   if (unlikely(rc))
+   return rc;
+   for_each_child_of_node(node, child) {
+   rc = add_deps(ac->id, child);
+   if (unlikely(rc))
+   return rc;
+   }
+   }
+   if (found_one_driver)
+   return rc;
+   }
+   for_each_child_of_node(node, child) {
+   rc = add_deps(parent, child);
+   if (unlikely(rc))
+   break;
+   }
+
+   return rc;
+}
+
+int __init of_add_dependencies(void)
+{
+   int rc = 0;
+   struct device_node *child;
+   struct device_node *root =  of_find_node_by_path("/");
+
+   if (unlikely(!root))
+   return -EINVAL;
+
+   for_each_child_of_node(root, child) {
+   

[PATCH 06/14] dtc: deps: Automatically add new property 'dependencies' which contains a list of referenced phandles

2015-10-17 Thread Alexander Holler
During the step from .dts to .dtb the information about dependcies
contained in the .dts through phandle references is lost. This makes it
impossible to use the binary blob to create a dependency graph without
knowing the semantic of all cell arrays.

Therefor automatically add a new property called 'dependencies' to all
nodes which have phandle references in one of their properties.

This new property will contain an array of phandles with one value for
every phandle referenced by other properties in the node.

If such a property already exists (e.g. to manually add dependencies
through the .dts), the existing list will be expanded.

Added phandles will be the phandle of either the referenced node itself
(if it has a property named 'compatible', or of the next parent of the
referenced node which as property named 'compatible'. This ensures only
dependencies to drivers will be added.

References to phandles of parent or child nodes will not be added to this
property, because this information is already contained in the blob (in
the form of the tree itself).

No dependencies to disabled nodes will be added.

Signed-off-by: Alexander Holler <hol...@ahsoftware.de>
---
 scripts/dtc/Makefile   |   3 +-
 scripts/dtc/Makefile.dtc   |   1 +
 scripts/dtc/dependencies.c | 108 +
 scripts/dtc/dtc.c  |  12 -
 scripts/dtc/dtc.h  |   3 ++
 5 files changed, 125 insertions(+), 2 deletions(-)
 create mode 100644 scripts/dtc/dependencies.c

diff --git a/scripts/dtc/Makefile b/scripts/dtc/Makefile
index 2a48022..1174cf9 100644
--- a/scripts/dtc/Makefile
+++ b/scripts/dtc/Makefile
@@ -4,7 +4,7 @@ hostprogs-y := dtc
 always := $(hostprogs-y)
 
 dtc-objs   := dtc.o flattree.o fstree.o data.o livetree.o treesource.o \
-  srcpos.o checks.o util.o
+  srcpos.o checks.o util.o dependencies.o
 dtc-objs   += dtc-lexer.lex.o dtc-parser.tab.o
 
 # Source files need to get at the userspace version of libfdt_env.h to compile
@@ -13,6 +13,7 @@ HOSTCFLAGS_DTC := -I$(src) -I$(src)/libfdt
 
 HOSTCFLAGS_checks.o := $(HOSTCFLAGS_DTC)
 HOSTCFLAGS_data.o := $(HOSTCFLAGS_DTC)
+HOSTCFLAGS_dependencies.o := $(HOSTCFLAGS_DTC)
 HOSTCFLAGS_dtc.o := $(HOSTCFLAGS_DTC)
 HOSTCFLAGS_flattree.o := $(HOSTCFLAGS_DTC)
 HOSTCFLAGS_fstree.o := $(HOSTCFLAGS_DTC)
diff --git a/scripts/dtc/Makefile.dtc b/scripts/dtc/Makefile.dtc
index bece49b..5fb5343 100644
--- a/scripts/dtc/Makefile.dtc
+++ b/scripts/dtc/Makefile.dtc
@@ -6,6 +6,7 @@
 DTC_SRCS = \
checks.c \
data.c \
+   dependencies.c \
dtc.c \
flattree.c \
fstree.c \
diff --git a/scripts/dtc/dependencies.c b/scripts/dtc/dependencies.c
new file mode 100644
index 000..dd4658c
--- /dev/null
+++ b/scripts/dtc/dependencies.c
@@ -0,0 +1,108 @@
+/*
+ * Code to add a property which contains dependencies (used phandle references)
+ * to all (driver) nodes which are having phandle references.
+ *
+ * Copyright (C) 2014 Alexander Holler <hol...@ahsoftware.de>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include 
+
+/* Searches upwards for a node with a property 'compatible' */
+static struct node *find_compatible_not_disabled(struct node *node)
+{
+   struct property *prop;
+
+   while (node) {
+   prop = get_property(node, "compatible");
+   if (prop) {
+   prop = get_property(node, "status");
+   if (prop)
+   if (!prop->val.len ||
+   (strcmp(prop->val.val, "okay") &&
+   strcmp(prop->val.val, "ok")))
+   return NULL; /* disabled */
+   return node;
+   }
+   node = node->parent;
+   }
+   return NULL;
+}
+
+static bool is_parent_of(struct node *node1, struct node *node2)
+{
+   while (node2) {
+   if (node2->parent == node1)
+   return true;
+   node2 = node2->parent;
+   }
+   return false;
+
+}
+
+static void add_deps(struct node *dt, struct node *node, struct property *prop)
+{
+   struct marker *m = prop->val.markers;
+   struct node *refnode;
+   cell_t phandle;
+   struct property *prop_deps;
+   unsigned i;
+   cell_t *cell;
+   struct node *source;
+   struct node *target;
+
+   for_each_marker_of_type(m, REF_PHANDLE) {
+   assert(m->offset + sizeof(cell_t) <= prop->val.len);
+
+   refnode = get_node_by_ref(dt, m->ref);
+   if (!refnode) {
+  

[PATCH 09/14] dtc: deps: Add option to print dependency graph as dot (Graphviz)

2015-10-17 Thread Alexander Holler
Add option -T do print a dependency graph in dot format for
generating a picture with Graphviz.

E.g.

dtc -T foo.dts | dot -T svg -o foo.svg

would generate the picture foo.png with the dependency graph.

Convential dependencies (those based on the tree structure) are having
black arrows, dependencies based on the property 'dependencies' are
having cyan arrows.

Option -D to not automatically add dependencies does still work, so
you could build a classic dependency graph with

dtc -D -T foo.dts | dot -T png -o foo_no_auto_deps.png

This works with binary blobs as input too. E.g.

CROSS_COMPILE=gcc-foo ARCH=arm make foo.dtb
scripts/dtc/dtc -I dtb -T arch/arm/boot/dts/foo.dtb

would print the dot file.

Signed-off-by: Alexander Holler <hol...@ahsoftware.de>
---
 scripts/dtc/dependencies.c | 49 --
 scripts/dtc/dtc.c  | 19 +++---
 scripts/dtc/dtc.h  |  2 +-
 3 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/scripts/dtc/dependencies.c b/scripts/dtc/dependencies.c
index 360d8f5..76e4d91 100644
--- a/scripts/dtc/dependencies.c
+++ b/scripts/dtc/dependencies.c
@@ -331,7 +331,7 @@ static void __init topological_sort(void)
depth_first_search(i);
 }
 
-static int __init add_dep_list(struct device_node *node)
+static int __init add_dep_list(struct device_node *node, bool print_dot)
 {
const __be32 *list, *list_end;
uint32_t ph;
@@ -361,6 +361,9 @@ static int __init add_dep_list(struct device_node *node)
rc = insert_edge(node->phandle, ph);
if (rc)
break;
+   if (print_dot)
+   printf("  node0x%x -> node0x%x [color=cyan]\n",
+   node->phandle, ph);
}
 
return rc;
@@ -385,9 +388,10 @@ static const char *of_prop_next_string(struct property 
*prop, const char *cur)
 }
 
 static int __init add_deps_lnx(struct device_node *parent,
-   struct device_node *node)
+   struct device_node *node, bool print_dot)
 {
struct device_node *child;
+   const char *cp;
int rc = 0;
 
if (!__of_device_is_available(node))
@@ -408,13 +412,34 @@ static int __init add_deps_lnx(struct device_node *parent,
return -EINVAL;
}
order.parent_by_phandle[node->phandle] = parent->phandle;
-   rc = add_dep_list(node);
+   if (print_dot) {
+   struct property *prop;
+
+   printf("node0x%x [label=\"0x%x %s", node->phandle,
+   node->phandle, node->name);
+   if (node->full_name)
+   printf(" (%s)", node->full_name);
+   prop = get_property(node, "compatible");
+   if (prop) {
+   printf("\\n");
+   for (cp = of_prop_next_string(prop, NULL); cp;
+cp = of_prop_next_string(prop, cp)) {
+   if (cp != prop->val.val)
+   putchar(' ');
+   printf("%s", cp);
+   }
+   }
+   printf("\"];\n");
+   printf("  node0x%x -> node0x%x\n", node->phandle,
+   parent->phandle);
+   }
+   rc = add_dep_list(node, print_dot);
if (unlikely(rc))
return rc;
parent = node; /* change the parent only if node is a driver */
}
for_each_child_of_node(node, child) {
-   rc = add_deps_lnx(parent, child);
+   rc = add_deps_lnx(parent, child, print_dot);
if (unlikely(rc))
break;
}
@@ -457,7 +482,7 @@ void __init of_init_print_order(const char *name)
}
 }
 
-int __init of_init_build_order(struct device_node *root)
+int __init of_init_build_order(struct device_node *root, const char *print_dot)
 {
struct device_node *child;
int rc = 0;
@@ -469,12 +494,24 @@ int __init of_init_build_order(struct device_node *root)
calc_max_phandle(root);
order.old_max_phandle = order.max_phandle;
 
+   if (print_dot) {
+   printf("digraph G {\n");
+   printf("node0x%x [label=\"0x%x root (/)\"];\n",
+   order.max_phandle+1, order.max_phandle+1);
+   }
+
for_each_child_of_node(root, child) {
- 

[PATCH 08/14] dtc: deps: Add option to print initialization order

2015-10-17 Thread Alexander Holler
Add option -t to print the default initialization order.
No other output will be generated.

To print the order, just use something like this:

CROSS_COMPILE=gcc-foo ARCH=arm make foo.dtb
scripts/dtc/dtc -I dtb -t arch/arm/boot/dts/foo.dtb

Since it's now possible to check to for cycles in the dependency graph,
this is now done too.

Signed-off-by: Alexander Holler <hol...@ahsoftware.de>
---
 scripts/dtc/dependencies.c | 344 +
 scripts/dtc/dtc.c  |  24 +++-
 scripts/dtc/dtc.h  |   2 +
 3 files changed, 369 insertions(+), 1 deletion(-)

diff --git a/scripts/dtc/dependencies.c b/scripts/dtc/dependencies.c
index 77d5c54..360d8f5 100644
--- a/scripts/dtc/dependencies.c
+++ b/scripts/dtc/dependencies.c
@@ -139,3 +139,347 @@ void add_dependencies(struct boot_info *bi)
process_nodes_props(bi->dt, bi->dt);
del_prop_no_dependencies(bi->dt);
 }
+
+/*
+ * The code below is in large parts a copy of drivers/of/of_dependencies.c
+ * in the Linux kernel. So both files do share the same bugs.
+ * The next few ugly defines do exist to keep the differences at a minimum.
+ */
+static struct node *tree;
+#define pr_cont(format, ...) printf(format, ##__VA_ARGS__)
+#define pr_info(format, ...) printf(format, ##__VA_ARGS__)
+#define pr_warn(format, ...) printf(format, ##__VA_ARGS__)
+#define pr_err(format, ...) fprintf(stderr, format, ##__VA_ARGS__)
+typedef cell_t __be32;
+#define device_node node
+#define full_name fullpath
+#define __initdata
+#define __init
+#define unlikely(a) (a)
+#define of_node_put(a)
+#define of_find_node_by_phandle(v) get_node_by_phandle(tree, v)
+#define __of_get_property(a, b, c) get_property(a, b)
+#define for_each_child_of_node(a, b) for_each_child(a, b)
+
+
+#define MAX_DT_NODES 1000 /* maximum number of vertices */
+#define MAX_EDGES (MAX_DT_NODES*2) /* maximum number of edges (dependencies) */
+
+struct edgenode {
+   uint32_t y; /* phandle */
+   struct edgenode *next; /* next edge in list */
+};
+
+/*
+ * Vertex numbers do correspond to phandle numbers. That means the graph
+ * does contain as much vertices as the maximum of all phandles.
+ * Or in other words, we assume that for all phandles in the device tree
+ * 0 < phandle < MAX_DT_NODES+1 is true.
+ */
+struct dep_graph {
+   struct edgenode edge_slots[MAX_EDGES]; /* used to avoid kmalloc */
+   struct edgenode *edges[MAX_DT_NODES+1]; /* adjacency info */
+   unsigned nvertices; /* number of vertices in graph */
+   unsigned nedges; /* number of edges in graph */
+   bool processed[MAX_DT_NODES+1]; /* which vertices have been processed */
+   bool include_node[MAX_DT_NODES+1]; /* which nodes to consider */
+   bool discovered[MAX_DT_NODES+1]; /* which vertices have been found */
+   bool finished; /* if true, cut off search immediately */
+};
+static struct dep_graph graph __initdata;
+
+struct init_order {
+   uint32_t max_phandle; /* the max used phandle */
+   uint32_t old_max_phandle; /* used to keep track of added phandles */
+   struct device_node *order[MAX_DT_NODES+1];
+   unsigned count;
+   /* Used to keep track of parent devices in regard to the DT */
+   uint32_t parent_by_phandle[MAX_DT_NODES+1];
+   struct device *device_by_phandle[MAX_DT_NODES+1];
+};
+static struct init_order order __initdata;
+
+
+/* Copied from drivers/of/base.c (because it's lockless). */
+static int __init __of_device_is_available(struct device_node *device)
+{
+   struct property *status;
+
+   if (!device)
+   return 0;
+
+   status = get_property(device, "status");
+   if (status == NULL)
+   return 1;
+
+   if (status->val.len > 0) {
+   if (!strcmp(status->val.val, "okay") ||
+   !strcmp(status->val.val, "ok"))
+   return 1;
+   }
+
+   return 0;
+}
+
+/*
+ * x is a dependant of y or in other words
+ * y will be initialized before x.
+ */
+static int __init insert_edge(uint32_t x, uint32_t y)
+{
+   struct edgenode *p; /* temporary pointer */
+
+   if (unlikely(x > MAX_DT_NODES || y > MAX_DT_NODES)) {
+   pr_err("Node found with phandle 0x%x > MAX_DT_NODES (%d)!\n",
+   x > MAX_DT_NODES ? x : y, MAX_DT_NODES);
+   return -EINVAL;
+   }
+   if (unlikely(!x || !y))
+   return 0;
+   if (unlikely(graph.nedges >= MAX_EDGES)) {
+   pr_err("Maximum number of edges (%d) reached!\n", MAX_EDGES);
+   return -EINVAL;
+   }
+   p = _slots[graph.nedges++];
+   graph.include_node[x] = 1;
+   graph.include_node[y] = 1;
+   p->y = y;
+   p->next = graph.edges[x];
+   graph.edges[x] = p; /* insert at head of list */
+
+   graph.nvertices = (x > gr

[PATCH 13/14] dt: dts: deps: imx6q: make some remote-endpoints non-dependencies

2015-10-17 Thread Alexander Holler
This is necessary to remove dependency cycles introduced by
'remote-endpoints'.

Signed-off-by: Alexander Holler <hol...@ahsoftware.de>
---
 arch/arm/boot/dts/imx6q.dtsi   | 2 ++
 arch/arm/boot/dts/imx6qdl.dtsi | 4 
 2 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 399103b..8db7f25 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -184,6 +184,7 @@
 
ipu2_di0_hdmi: endpoint@1 {
remote-endpoint = <_mux_2>;
+   no-dependencies = <_mux_2>;
};
 
ipu2_di0_mipi: endpoint@2 {
@@ -205,6 +206,7 @@
 
ipu2_di1_hdmi: endpoint@1 {
remote-endpoint = <_mux_3>;
+   no-dependencies = <_mux_3>;
};
 
ipu2_di1_mipi: endpoint@2 {
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index b57033e..db3d0d0 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -1150,10 +1150,12 @@
 
ipu1_di0_hdmi: endpoint@1 {
remote-endpoint = <_mux_0>;
+   no-dependencies = <_mux_0>;
};
 
ipu1_di0_mipi: endpoint@2 {
remote-endpoint = <_mux_0>;
+   no-dependencies = <_mux_0>;
};
 
ipu1_di0_lvds0: endpoint@3 {
@@ -1175,10 +1177,12 @@
 
ipu1_di1_hdmi: endpoint@1 {
remote-endpoint = <_mux_1>;
+   no-dependencies = <_mux_1>;
};
 
ipu1_di1_mipi: endpoint@2 {
remote-endpoint = <_mux_1>;
+   no-dependencies = <_mux_1>;
};
 
ipu1_di1_lvds0: endpoint@3 {
-- 
2.1.0

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


[PATCH 14/14] dt: dts: deps: omap: beagle: make some remote-endpoints non-dependencies

2015-10-17 Thread Alexander Holler
This is necessary to remove dependency cycles introduced by
'remote-endpoints'.

Signed-off-by: Alexander Holler <hol...@ahsoftware.de>
---
 arch/arm/boot/dts/omap3-beagle.dts | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-beagle.dts 
b/arch/arm/boot/dts/omap3-beagle.dts
index a547411..78ba39e 100644
--- a/arch/arm/boot/dts/omap3-beagle.dts
+++ b/arch/arm/boot/dts/omap3-beagle.dts
@@ -101,6 +101,7 @@
 
tfp410_in: endpoint@0 {
remote-endpoint = <_out>;
+   no-dependencies = <_out>;
};
};
 
@@ -109,6 +110,7 @@
 
tfp410_out: endpoint@0 {
remote-endpoint = <_connector_in>;
+   no-dependencies = <_connector_in>;
};
};
};
@@ -150,6 +152,7 @@
etb_in: endpoint {
slave-mode;
remote-endpoint = <_out>;
+   no-dependencies = <_out>;
};
};
};
@@ -373,6 +376,7 @@
port {
venc_out: endpoint {
remote-endpoint = <_connector_in>;
+   no-dependencies = <_connector_in>;
ti,channels = <2>;
};
};
-- 
2.1.0

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


[PATCH 12/14] dt: dts: deps: kirkwood: dockstar: add dependency ehci -> usb power regulator

2015-10-17 Thread Alexander Holler
This serves as an example how easy it is to fix an initialization order
if the order depends on the DT. No source code changes will be necessary.

If you look at the dependency graph for the dockstar, you will see that
there is no dependency between ehci and the usb power regulator. This
ends up with the fact that the regulator will be initialized after ehci.

Fix this by adding one dependency to the .dts.

Signed-off-by: Alexander Holler <hol...@ahsoftware.de>
---
 arch/arm/boot/dts/kirkwood-dockstar.dts | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/kirkwood-dockstar.dts 
b/arch/arm/boot/dts/kirkwood-dockstar.dts
index 8497363..426d8840 100644
--- a/arch/arm/boot/dts/kirkwood-dockstar.dts
+++ b/arch/arm/boot/dts/kirkwood-dockstar.dts
@@ -107,3 +107,7 @@
phy-handle = <>;
};
 };
+
+ {
+   dependencies = <_power>;
+};
-- 
2.1.0

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


Re: [PATCH 10/14] init: deps: IDs for annotated initcalls

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 19:45 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 07:14:23PM +0200, Alexander Holler wrote:

These patch contains the IDs for initcalls I've annotated.

This patch is NOT meant for merging into mainline in its current form.

It should be discussed about how to add these IDs and in which form, if
the feature ends up in mainline at all.

E.g. it could make sense to split this file into several files in order
to avoid merge conflicts.

It also might make sense to prefill this file with IDs for many drivers.

E.g. the following script will use a modules.dep file to produce IDs for
modules. It's meant to be used on a very complete modules.dep build through
make allmodconfig && make -jN modules && make modules_install.


A file like this is going to be a nightmare to maintain and ensure that
it actually is correct, I don't see it as a viable solution, sorry.


How often will drivers be added? The only changes on this file will 
happen if a driver will be added and then just one ID will be added.


As said above, the file could be filled with IDs for all existing 
modules, regardless if they are already annotated.


If that's a nightmare, I wonder what how you name the necessary stuff to 
maintain the link order through Makefiles.


But besides that, I'm totally open to any other idea how to identify a 
driver. For me, just using an enum looks like the most trivial way.


Alexander Holler

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


Re: [PATCH 10/14] init: deps: IDs for annotated initcalls

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 20:29 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 07:55:17PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 19:45 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 07:14:23PM +0200, Alexander Holler wrote:

These patch contains the IDs for initcalls I've annotated.

This patch is NOT meant for merging into mainline in its current form.

It should be discussed about how to add these IDs and in which form, if
the feature ends up in mainline at all.

E.g. it could make sense to split this file into several files in order
to avoid merge conflicts.

It also might make sense to prefill this file with IDs for many drivers.

E.g. the following script will use a modules.dep file to produce IDs for
modules. It's meant to be used on a very complete modules.dep build through
make allmodconfig && make -jN modules && make modules_install.


A file like this is going to be a nightmare to maintain and ensure that
it actually is correct, I don't see it as a viable solution, sorry.


How often will drivers be added? The only changes on this file will happen
if a driver will be added and then just one ID will be added.


Look at how many drivers we add every kernel release, it's a non-trivial
amount.


I still don't see your problem. As long as the IDs in the enum are 
ordered according to the directories, there won't be more merge 
conflicts than in the Makefile or Kconfig for that directory. And as 
mentioned, it's e.g. possible to split the one file into multiple ones, e.g.


enum driver_ids {

#include "foo"
#include "bar"

};

Of cource, the content of foo and bar might look a bit unusual.




As said above, the file could be filled with IDs for all existing modules,
regardless if they are already annotated.

If that's a nightmare, I wonder what how you name the necessary stuff to
maintain the link order through Makefiles.


There usually isn't a problem and the issue is that we link by
subsystem, so somehow it's all working fairly well.


So why should that be worse with the one file?

But, like you, I don't like such a big enum.

It's just that I didn't thought much about another solution, and the 
time I've spend to think about something else which provides a usable 
ID, didn't end in a solution. So I would be happy if someone else would 
offer an idea.


Regards,

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


Re: [PATCH 11/14] init: deps: annotate various initcalls

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 20:47 schrieb Linus Torvalds:

On Sat, Oct 17, 2015 at 10:14 AM, Alexander Holler <hol...@ahsoftware.de> wrote:


diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index 873dbfc..d5d2459 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -1872,5 +1872,4 @@ static int __init edma_init(void)
  {
 return platform_driver_probe(_driver, edma_probe);
  }
-arch_initcall(edma_init);
-
+annotated_initcall_drv(arch, edma_init, drvid_edma, NULL, edma_driver.driver);
diff --git a/arch/arm/crypto/aes-ce-glue.c b/arch/arm/crypto/aes-ce-glue.c
index b445a5d..d9bcb89 100644
--- a/arch/arm/crypto/aes-ce-glue.c
+++ b/arch/arm/crypto/aes-ce-glue.c
@@ -520,5 +520,10 @@ static void __exit aes_exit(void)
 crypto_unregister_algs(aes_algs, ARRAY_SIZE(aes_algs));
  }

-module_init(aes_init);
+static const unsigned dependencies[] __initconst __maybe_unused = {
+   drvid_cryptomgr,
+   0
+};
+
+annotated_module_init(aes_init, drvid_aes_ce_arm, dependencies);
  module_exit(aes_exit);


So I think this is kind of a sign of the same disease I mentioned
earlier: making dependencies "separate" from the init levels, now
means that you do the initialization of the dependencies *instead* of
the init level. And that smells bad and wrong, and causes this kind of
patch that is not only huge, but si unreadable and the end result
looks like crap too.

We've actually been quite good at having the module attributes all be
*separate* things that work together. So the code had

   module_init(aes_init);
   module_exit(aes_exit);

but also things like

   MODULE_DESCRIPTION("AES-ECB/CBC/CTR/XTS using ARMv8 Crypto Extensions");
   MODULE_AUTHOR("Ard Biesheuvel <ard.biesheu...@linaro.org>");
   MODULE_LICENSE("GPL v2");

and that all helps improve readablity and keep things sane.

In contrast, turds like these are just pure and utter crap:

static const unsigned dependencies[] __initconst __maybe_unused = {
   drvid_cryptomgr,
   0
};
annotated_module_init(aes_init, drvid_aes_ce_arm, dependencies);

and yes, I know that we have things like this for the driver ID lists
etc, but that doesn't make it better.

No, I think any dependency model should strive to make this really
really easy and separate, and do things like

module_depends(cryptomgr);

and then just use that to fill in a link section or something like
that. And no, there's no way we will ever maintain a "list of
dependency identifiers". This is stuff that should be all about
scripting, or - better yet - just make the link section contain
strings so that you don't *need* any C level identifiers.

That would be trivial to do by just making the "module_depends()"
macro be something like

   #define _dependency(x,y) \
  static const  struct module_dependency_attribute \
  __used __attribute__ ((__section__ ("__dependencies")))  \
 * __dependency_attr =  { x,y }

   #define module_depends(x) \
 _dependency(#x, KBUILD_NAME)

   #define module_provides(x) \
 _dependency(KBUILD_NAME, #x)

And if a module depends on multiple other things, then you just have
multiple of those "module_depends()" things. There's some gcc trick to
generating numbered (per compilation unit) C identifiers (so that you
can have multiple of those "__dependency_attr" variables in the same
file), but I forget it right now.

And this is also where I think those "module_init()" vs
"subsys_init()" things come in. "module_init()" means that it's a
driver level thing, which would mean that module_init() implies

   module_depends(level7);
   module_provides(level7_end);

so that the module would automatically be sorted wrt the "driver" level.

Another advantage (apart from legibility of the source, and
integrating with the *existing* level-based dependencies) is that
using something like "module_depends()" and "module_provides()" means
that it should be easy to parse even outside of a C compiler, so you
could - if you want to - make all the dependencies be done not as part
of compiling the source, but as a separate scripting thing. That could
be useful for things like statistics and visualization tools that
don't want to actually build the kernel, but want to just show the
dependencies between different modules.

So no. I do *not* think big patches like this are acceptable. This
kind of patch - along with the patch that just adds the random
dependency identifier C enums - is exactly what we do *not* want. If
we do dependencies, they should all be small and local things, and
they should not *replace* the existing "module_init()" vs
"arch_init()" system, they should add on top of it.


Thanks for the detailed answer and you are right.

But I had to start somehow and, unfortunately, I don't have the 
resources to fu

Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 21:03 schrieb Linus Torvalds:

On Sat, Oct 17, 2015 at 11:37 AM, Alexander Holler <hol...@ahsoftware.de> wrote:


Otherwise it's impossible to call initcalls in parallel. I've seen a stable
topological sort somewhere, but whenever you want to parallelize the
initcalls, the stable ordering would be gone anyway. So I've decided not to
look further at a stable topological sort.


So five seconds of googling gave me freely usable source code for a
stable topological sort, that also has a nice reported added
advantage:

  "An interesting property of a stable topological sort is that cyclic
dependencies are tolerated and resolved according to original order of
elements in sequence. This is a desirable feature for many
applications because it allows to sort any sequence with any
imaginable dependencies between the elements"

which seems to be *exactly* what you'd want, especially considering
that right now your patches add extra "no-dependency" markers exactly
because of the cyclical problem.


That's the stable topological sort I've mentioned the link to in the 
discussion with you.




I think it was the #2 hit on google for "stable topological sort". I
didn't look closely at the source code, but it was not big.

And no, since we don't actually want to parallelize the initcalls
anyway (I had this discussion with you just a month ago), your
objections seem even more questionable. We have separate machinery for
"do this asynchronously", and we want to _keep_ that separate.


I've understood that now.

Sorry for wasting your time.

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


Re: [PATCH 0/14] init: deps: dependency based (parallelized) init

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 19:44 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 07:14:13PM +0200, Alexander Holler wrote:

Hello,

here is the newest version of my patches to use a dependency based
initialization order. It now works without DT too.

Background:

Currently initcalls are ordered by some levels and the link order. This
means whenever a file is renamed, changes directory or a Makefile is
modified the order with which initcalls are called might change. This
might result in problems. Furthermore, the required dependencies are
often not documented, sometimes there are comments in the source or in a
commit message, but most often the knowledge why a specific initcall
belongs to a specific initcall level isn't obvious without carefully
examing he source. And initcalls are used by drivers and subsystems, and
the count of both have grown quiet a lot in the last years. So it's
rather difficult to maintain a proper link order.


Files move around very rarely, is this really an issue?


No idea. You are maintaining the staging area. ;)




Another problem is that the link order can't be modified dynamically at
boot time to include dependencies dictated by the hardware. To circumvent
this, a brute-force trial-and-error mechanism called deferred probes has
been introduced, but this approach, while beeing KISS, has its own
problems.


What problems does deferred probing have?  Why not just fix that if
there is issues with it, as it was supposed to solve this issue without
needing to annotate anything.


I've not looked why deferred probes are sometimes causing such a large 
delay. But giving that it's brutforce and non-deterministic, some 
drivers might be probed a dozen times, or important drivers might be 
probed very late, forcing all previous probed drivers to be probed again 
(late). Just think at the case the link order is a, b, c but drivers 
have to be called in the order c, b, a.



To solve these problems I've written patches to use a topological sort at
boot time which uses dependencies to calculate the order with which
initcalls are called.

Why? What are the benefits (assuming correct dependencies are available)?

- It offers a clear in-source documentation for dependencies between
   initcalls.
- It is robust in regard to file or directory name changes and changes in
   a Makefile.
- If enabled, the order with which drivers for interfaces are called
   (e.g. network interfaces, hard disks), can be defined independent of
   the link order. These might result in more stable interface names or
   numbers.
- If enabled, it makes the the deferred probes obsolete, which might
   result in faster boot times.
- If enabled, it is possible to call initcalls in parallel. E.g. the
   shipped kernel for Fedora 21 (4.1.7-100.fc21.x86_64) contains around
   560 initcalls. These are all called in series. Also some of them use
   asynchronous stuff by themself, most don't do.


But that shipped kernel boots to X in less than 2 seconds, so there
isn't really a speed issue here, right?


It's noticeable if your phone, or any other thing you want to use (like 
your route planner, clock or whatever boots in one second instead of two 
when you turn it on.



Drawbacks:

- It requires a small amount of time to calculate the order a boot time.
   But this time is most often smaller than the time saved by using
   multiple cores to call initcalls or by not needing deferred probes.


How much time is needed?


I've measured 3ms on a slow ARM box.




- Dependencies are required. For everything which can be build as a
   module, looking at modules.dep might give some pointers. Looking at
   the help from menuconfig also might give some pointers. But in the
   end, the most preferable way would be if maintainers or other people
   which have a deeper knowledge about the source and functionality
   would add the dependencies.


How will a "normal" driver author figure out what those dependancies are
in order to be able to write them down?  That's my biggest objection


Most drivers are done c from an existing driver. And if someone adds 
new code, he should know what these new code is for and on what it depends.



here, I have no idea how to add these, nor how to properly review such a
submission.  What about systems that have different ordering/dependancy
requirements for the same drivers due to different ways the hardware is
hooked up?  That is not going to work well here, unless I'm missing
something.


Hmm, how is that solved now?

If you have dependencies dictated by a special HW, this dependencies 
should come in by the HW description.


The deferred probe mechanism exists just since 1 or 2 years. And once 
you had to search the source of non-displayed error in a set of several 
dozens possible sources (because many errors are now non-errors but 
-517) you might learn to hate deferred probes as much as I do. I'm sorry 
for the harsh words in regard to deferred probes.


The way to use dependencies doesn

Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 20:23 schrieb Linus Torvalds:

On Sat, Oct 17, 2015 at 10:14 AM, Alexander Holler <hol...@ahsoftware.de> wrote:


Assuming three different ethernet-drivers, without any special code,
the dependency graph would not require any special order inbetween them
and would look like that:


This seems *fundamentally* wrong.

This is in no way specific to network drivers (or to disk drivers, or
to anything else).

So requiring extra logic for this implies that something is seriously wrong.

If two drivers aren't ordered by dependencies, they should always be
in link order, regardless of any hacks like these. If they're not,
things are wrong.

I think your problem is that you make that dependency thing a separate
ordering, so now it matters whether a driver has a dependency or not.


I'm making dependencies the only ordering for annotated initcalls.

Otherwise it's impossible to call initcalls in parallel. I've seen a 
stable topological sort somewhere, but whenever you want to parallelize 
the initcalls, the stable ordering would be gone anyway. So I've decided 
not to look further at a stable topological sort.



If something like this is to work, it has to work *with* the normal
ordering, not outside of it and then have these kinds of broken
special cases.

The normal init orderings (ie core -> postcore -> arch -> subsys -> fs
-> rootfs -> device -> late) should just be an extra dependency, I
think.

The way that you just insert the annotated dependencies in between
levels 6 and 7 ("device" and "late") can't be right. It means - for
example - that you can't have subsystems that have dependencies.


Sorry, but that's wrong.

I've choosen to place initcalls between 5 and 6 to make it easier to 
move both, subsystems as well as normal drivers to the (new) level with 
annotated initcalls. If you look at what I've already "annotated", you 
will see there are quiet a lot initcalls I've moved from below 6 to the 
new level.



So I really think that if we do dependencies, then the current levels
have to be added as dependencies, so that "subsys_initcall(xyz)"
basically means "xyz depends on the 'subsys' event, and 'subsys_end'
depends on xyz". Then within that, you might have another bus driver
that in turn depends on 'xyz'.


It would be absolutely no problem to introduce "virtual" initcalls for 
any level, e.g. just


depencies = {
everything_below
}

initcall foo()
{
return 0;
}

annotated_initcall(foo, id, dependencies),

Regards,

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 20:52 schrieb Linus Torvalds:

On Sat, Oct 17, 2015 at 11:37 AM, Alexander Holler <hol...@ahsoftware.de> wrote:


I'm making dependencies the only ordering for annotated initcalls.


Yeah, and quite frankly, that just means that I'm not going to merge it.

We do not do "flag-day" things. We've done them in the past, and it
has always been a major and unacceptable pain.


That isn't a flag day thing. It's a config option everyone can turn on 
and off whenever he wants.




And if the dependency ordering is "outside" of the traditional
link-time ordering, then it is by definition a flag-day event. Every
time you add a dependency to even just *one* driver, it magically
changes ordering wrt all the other drivers, because now it's in a
different ordering domain.

So please reconsider. Or stop cc'ing me. Because "flag day" changes
really are not acceptable.


I'm choosing the second option. I will answer further mails on that 
topic, but will remove you from cc. Besides that I consider these 
patches as a total failure and will not post any further version.


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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 21:08 schrieb Linus Torvalds:

On Sat, Oct 17, 2015 at 12:01 PM, Alexander Holler <hol...@ahsoftware.de> wrote:


That isn't a flag day thing. It's a config option everyone can turn on and
off whenever he wants.


That's a flag-day thing. We've done it (drm comes to mind - several times).

I'm disappointed, because I _know_ I pointed you in the direction of
stable sorting about a month ago. I really had hoped you'd have taken
that into account.


It's impossible to take it into account because I don't want to miss the 
parallelize functionality. And without that, all the stuff doesn't offer 
enough benefits to be worse the effort but just adds some time necessary 
to do the sorting. It might solve the deferred probe problems, but 
without much benefit.


Regards,

Alexander Holler

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


Re: [PATCH 0/14] init: deps: dependency based (parallelized) init

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 20:38 schrieb Greg Kroah-Hartman:


So how long does that really take to call all probe functions in all
possible order?  Real numbers please.  We have the tools to determine
where at boot time delays are happening, please use them to find the
problem drivers.


No idea. You might ask Tomeu Vizoso (I've added him to cc) for details 
(or search for the thread "On-demand device registration" where he 
complained that his chromebook boots slow). I've just measured, that 
most my ARM boxes booted faster when I've used the ordering, instead of 
slower through the introduce overhead to order initcalls (without having 
parallelized the initcalls).


Posting times doesn't make much sense, as they heavily depend on the 
configuration. Instead I've posted patches so you can test it yourself.


But if you want a real time, my Netbook with a single core but HT Atom 
N270 boots in one second instead of two to "dmesg | grep Freeing".


Regards,

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 21:36 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 09:14:34PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 21:08 schrieb Linus Torvalds:

On Sat, Oct 17, 2015 at 12:01 PM, Alexander Holler <hol...@ahsoftware.de> wrote:


That isn't a flag day thing. It's a config option everyone can turn on and
off whenever he wants.


That's a flag-day thing. We've done it (drm comes to mind - several times).

I'm disappointed, because I _know_ I pointed you in the direction of
stable sorting about a month ago. I really had hoped you'd have taken
that into account.


It's impossible to take it into account because I don't want to miss the
parallelize functionality. And without that, all the stuff doesn't offer
enough benefits to be worse the effort but just adds some time necessary to
do the sorting. It might solve the deferred probe problems, but without much
benefit.


Again, parallelizing does not solve anything, and causes more problems
_and_ makes things take longer.  Try it, we have done it in the past and
proven this, it's pretty easy to test :)


I've tested it, otherwise I wouldn't have posted the patches.

Unfortunately it's quiet a lot of work to add dependencies for everything.

So maybe I'm able to offer some better numbers in a year or such, when I 
was bored often enough to add more dependencies for initcalls.


Not to mention that small changes in the order can have quiet big 
differences in the boot time, so it's quiet hard to parallelize stuff 
(add dependencies) correctly like e.g. the pci/acpi/processor stuff. 
Especially because many reasons for the current order aren't mentioned 
in the source and are hard to see without specific knowledge about the HW.


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


Re: [PATCH 0/14] init: deps: dependency based (parallelized) init

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 22:20 schrieb Greg Kroah-Hartman:

On Sat, Oct 17, 2015 at 09:43:17PM +0200, Alexander Holler wrote:

Am 17.10.2015 um 20:38 schrieb Greg Kroah-Hartman:


So how long does that really take to call all probe functions in all
possible order?  Real numbers please.  We have the tools to determine
where at boot time delays are happening, please use them to find the
problem drivers.


No idea. You might ask Tomeu Vizoso (I've added him to cc) for details (or
search for the thread "On-demand device registration" where he complained
that his chromebook boots slow). I've just measured, that most my ARM boxes
booted faster when I've used the ordering, instead of slower through the
introduce overhead to order initcalls (without having parallelized the
initcalls).


I've already asked him, I don't like his patch series to try to resolve
this issue either :)


Posting times doesn't make much sense, as they heavily depend on the
configuration. Instead I've posted patches so you can test it yourself.

But if you want a real time, my Netbook with a single core but HT Atom N270
boots in one second instead of two to "dmesg | grep Freeing".


Try running the boot time graphic tool to determine where that time is
spent, odds are you just need to enable a single driver to async it's
probe function and you should be fine.

Again, fix up the broken driver(s), don't paper over the issues with
core changes that are not necessary.


I assume you are aware of the 80/20 rule. So you might see the 
parallelize feature of topological sort as an attempt to do some of the 
work in the last 20 percent left.


Sorry, but I've no idea why you are now trying to pin the stuff I'm 
talking about to a specific issue.


Or to talk in more clear words:

The current initcall ordering is, in my humble opinion, a whole and 
mostly undocumented mess. And I'm pretty sure it will become worse, I 
just have to wait and see. And if every attempt to fix that will be 
killed as fast as my one ...


Anyway, thanks for comments.

Regards,

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 21:58 schrieb Alexander Holler:


Unfortunately it's quiet a lot of work to add dependencies for everything.


And (just in case of), also I'm a non-native English writer, I know the 
difference between quiet and quite. But, unfortunately, that doesn't 
prevent me to type it wrong. It's like "teh" instead of "the". 
Unfortunately I'm not very good in writing English fast without errors, 
but that doesn't mean I'm dumb (also many native English writers prefer 
to assume that).


Regards,

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


Re: [PATCH 04/14] init: deps: order network interfaces by link order

2015-10-17 Thread Alexander Holler

Am 17.10.2015 um 21:08 schrieb Linus Torvalds:

On Sat, Oct 17, 2015 at 12:01 PM, Alexander Holler <hol...@ahsoftware.de> wrote:


That isn't a flag day thing. It's a config option everyone can turn on and
off whenever he wants.


That's a flag-day thing. We've done it (drm comes to mind - several times).

I'm disappointed, because I _know_ I pointed you in the direction of
stable sorting about a month ago. I really had hoped you'd have taken
that into account.


Sorry, but I have to add another comment about that stable topological 
sort which can be easily found by searching the web.


As the author mentioned, he has done some search on that topic too and 
has found nothing. So, either I would have to trust his word (and his 
non-public proof) that it works or I would have to prove it myself.


Besides that he says it's based on bubble sort, which is known for its 
speed. So I would have had to write in C in order to see if the speed 
would be acceptable and would still be without any proof that it always 
works correctly.


Regards,

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


Re: AMD-IOMMU and problem with __init(data)?

2015-09-29 Thread Alexander Holler

Am 29.09.2015 um 17:06 schrieb Joerg Roedel:

As expected it is no bug in the AMD IOMMU driver, but in your code.

On Wed, Sep 23, 2015 at 09:04:31PM +0200, Alexander Holler wrote:

struct _annotated_initcall {
initcall_t initcall;
unsigned driver_id;
unsigned *dependencies;
struct device_driver *driver;
};


This struct gets aligned on a 32 bytes boundary.


+#define ANNOTATED_INITCALLS\
+   VMLINUX_SYMBOL(__annotated_initcall_start) = .; \
+   *(.annotated_initcall.init) \
+   VMLINUX_SYMBOL(__annotated_initcall_end) = .;


But this section does not.


+   ac = __annotated_initcall_start;
+   pr_info("ac %p ID %u\n", ac, ac->driver_id);
+   BUG_ON(ac->driver_id != 23);


So when you access __annotated_initcall_start here, you don't access the
first element of your array, but actually the zero padding before your
struct.

On my system the section was aligned on an 8 bytes boundary, which means
there were 24 bytes of padding before the symbol you try to access.


Hmm. Thanks a lot. Also I've checked the alignment (at least twice) and 
remember it was 32bit. But maybe I've checked something different or 
looked at some file for ARM or x86(_32) or was confused or similar. But 
now, when I look at ARM the initcall section seems to be aligned to 8 
too. So I wonder why the stuff works on ARM (v5 and v7) and on an Intel 
Atom (32bit). I think at least the armv5 box should have trapped (fatal) 
too, but maybe that changed.


Sorry for not having looked at the alignment at least once more. 
Alignment bugs are always hard to see and I've already assumed such, 
especially because any other kernel seems to work, but I was obviously 
unable to see it.


Again, thanks a lot.

Regards,

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


Re: AMD-IOMMU and problem with __init(data)?

2015-09-29 Thread Alexander Holler

Am 29.09.2015 um 17:06 schrieb Joerg Roedel:

As expected it is no bug in the AMD IOMMU driver, but in your code.

On Wed, Sep 23, 2015 at 09:04:31PM +0200, Alexander Holler wrote:

struct _annotated_initcall {
initcall_t initcall;
unsigned driver_id;
unsigned *dependencies;
struct device_driver *driver;
};


This struct gets aligned on a 32 bytes boundary.


+#define ANNOTATED_INITCALLS\
+   VMLINUX_SYMBOL(__annotated_initcall_start) = .; \
+   *(.annotated_initcall.init) \
+   VMLINUX_SYMBOL(__annotated_initcall_end) = .;


But this section does not.


+   ac = __annotated_initcall_start;
+   pr_info("ac %p ID %u\n", ac, ac->driver_id);
+   BUG_ON(ac->driver_id != 23);


So when you access __annotated_initcall_start here, you don't access the
first element of your array, but actually the zero padding before your
struct.

On my system the section was aligned on an 8 bytes boundary, which means
there were 24 bytes of padding before the symbol you try to access.


Hmm. Thanks a lot. Also I've checked the alignment (at least twice) and 
remember it was 32bit. But maybe I've checked something different or 
looked at some file for ARM or x86(_32) or was confused or similar. But 
now, when I look at ARM the initcall section seems to be aligned to 8 
too. So I wonder why the stuff works on ARM (v5 and v7) and on an Intel 
Atom (32bit). I think at least the armv5 box should have trapped (fatal) 
too, but maybe that changed.


Sorry for not having looked at the alignment at least once more. 
Alignment bugs are always hard to see and I've already assumed such, 
especially because any other kernel seems to work, but I was obviously 
unable to see it.


Again, thanks a lot.

Regards,

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


Re: AMD-IOMMU and problem with __init(data)?

2015-09-26 Thread Alexander Holler

Am 23.09.2015 um 21:04 schrieb Alexander Holler:

Am 23.09.2015 um 17:50 schrieb Alexander Holler:

Am 23.09.2015 um 13:43 schrieb Joerg Roedel:

If it's necessary, I could try put together a small patch which
kills a system (reproducible here).


That would help too, please also send me your .config and I'll try to
reproduce the issue here.


Will do. Later.


Done. Patch(es) and config attached. Note, while reducing the patch,
I've noted that when I change


Just in case of, the command line I'm using is

ro root=/dev/sdb2 rootfstype=btrfs rootflags=compress=zlib enforcing=0 
selinux=0 cgroup_disable=memory swapaccount=0 
earlycon=uart8250,io,0x3f8,115200n8 console=ttyS0,115200n8 console=tty0 
memtest=3 LANG=de_DE.UTF-8 video=DVI-D-1:1920x1080-32@60


And memtest never failed (I'm always using that). So it's unlikely it's 
a problem with the memory.


But usually I don't have enabled the serial, that means normally I'm 
booting with


root=/dev/sdb2 rootfstype=btrfs rootflags=compress=zlib enforcing=0 
selinux=0 cgroup_disable=memory swapaccount=0 console=tty0 memtest=3 
LANG=de_DE.UTF-8 video=DVI-D-1:1920x1080-32@60


so maybe enabling the serial does something bad. Also I've enabled it 
before and haven't seen something bad. And because I assume the IOMMU is 
the problem, I don't use any cards, just the HW on mb.


Regards,

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


Re: AMD-IOMMU and problem with __init(data)?

2015-09-26 Thread Alexander Holler

Am 23.09.2015 um 21:04 schrieb Alexander Holler:

Am 23.09.2015 um 17:50 schrieb Alexander Holler:

Am 23.09.2015 um 13:43 schrieb Joerg Roedel:

If it's necessary, I could try put together a small patch which
kills a system (reproducible here).


That would help too, please also send me your .config and I'll try to
reproduce the issue here.


Will do. Later.


Done. Patch(es) and config attached. Note, while reducing the patch,
I've noted that when I change


Just in case of, the command line I'm using is

ro root=/dev/sdb2 rootfstype=btrfs rootflags=compress=zlib enforcing=0 
selinux=0 cgroup_disable=memory swapaccount=0 
earlycon=uart8250,io,0x3f8,115200n8 console=ttyS0,115200n8 console=tty0 
memtest=3 LANG=de_DE.UTF-8 video=DVI-D-1:1920x1080-32@60


And memtest never failed (I'm always using that). So it's unlikely it's 
a problem with the memory.


But usually I don't have enabled the serial, that means normally I'm 
booting with


root=/dev/sdb2 rootfstype=btrfs rootflags=compress=zlib enforcing=0 
selinux=0 cgroup_disable=memory swapaccount=0 console=tty0 memtest=3 
LANG=de_DE.UTF-8 video=DVI-D-1:1920x1080-32@60


so maybe enabling the serial does something bad. Also I've enabled it 
before and haven't seen something bad. And because I assume the IOMMU is 
the problem, I don't use any cards, just the HW on mb.


Regards,

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


Re: AMD-IOMMU and problem with __init(data)?

2015-09-23 Thread Alexander Holler

Am 23.09.2015 um 17:50 schrieb Alexander Holler:

Am 23.09.2015 um 13:43 schrieb Joerg Roedel:

If it's necessary, I could try put together a small patch which
kills a system (reproducible here).


That would help too, please also send me your .config and I'll try to
reproduce the issue here.


Will do. Later.


Done. Patch(es) and config attached. Note, while reducing the patch, 
I've noted that when I change



struct _annotated_initcall {
initcall_t initcall;
unsigned driver_id;
unsigned *dependencies;
struct device_driver *driver;
};

to

struct _annotated_initcall {
initcall_t initcall;
unsigned driver_id;
};

it does not crash. That's what the second patch does. So in order to 
crash a system, just use patch 0001.


Regards,

Alexander Holler
>From d94e5568925d7cb8a3fc628606e8fcf1374ea177 Mon Sep 17 00:00:00 2001
From: Alexander Holler 
Date: Wed, 23 Sep 2015 19:43:51 +0200
Subject: [PATCH 1/2] CRASH on AMD with CONFIG_AMD_IOMMU

---
 include/asm-generic/vmlinux.lds.h |  6 ++
 init/main.c   | 30 +-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 8bd374d..13f9189 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -660,6 +660,11 @@
 		INIT_CALLS_LEVEL(7)	\
 		VMLINUX_SYMBOL(__initcall_end) = .;
 
+#define ANNOTATED_INITCALLS		\
+		VMLINUX_SYMBOL(__annotated_initcall_start) = .;		\
+		*(.annotated_initcall.init)\
+		VMLINUX_SYMBOL(__annotated_initcall_end) = .;
+
 #define CON_INITCALL			\
 		VMLINUX_SYMBOL(__con_initcall_start) = .;		\
 		*(.con_initcall.init)	\
@@ -816,6 +821,7 @@
 		INIT_DATA		\
 		INIT_SETUP(initsetup_align)\
 		INIT_CALLS		\
+		ANNOTATED_INITCALLS	\
 		CON_INITCALL		\
 		SECURITY_INITCALL	\
 		INIT_RAM_FS		\
diff --git a/init/main.c b/init/main.c
index 5650655..1498f9b 100644
--- a/init/main.c
+++ b/init/main.c
@@ -859,12 +859,40 @@ static void __init do_initcall_level(int level)
 		do_one_initcall(*fn);
 }
 
+struct _annotated_initcall {
+	initcall_t initcall;
+	unsigned driver_id;
+	unsigned *dependencies;
+	struct device_driver *driver;
+};
+extern struct _annotated_initcall __annotated_initcall_start[];
+
+#define annotated_initcall(fn, id) \
+	static struct _annotated_initcall __annotated_initcall_##fn __used \
+	__attribute__((__section__(".annotated_initcall.init"))) = \
+			{ .initcall = fn, .driver_id = id }
+
+
+static int __init foo(void)
+{
+	return 0;
+}
+
+annotated_initcall(foo, 23);
+
 static void __init do_initcalls(void)
 {
 	int level;
+	struct _annotated_initcall *ac;
 
-	for (level = 0; level < ARRAY_SIZE(initcall_levels) - 1; level++)
+	for (level = 0; level < ARRAY_SIZE(initcall_levels) - 3; level++)
 		do_initcall_level(level);
+
+	ac = __annotated_initcall_start;
+	pr_info("ac %p ID %u\n", ac, ac->driver_id);
+	BUG_ON(ac->driver_id != 23);
+	do_initcall_level(level++);
+	do_initcall_level(level);
 }
 
 /*
-- 
2.1.0

>From 17c9b286710931947c63ef49bba69f679ecdc400 Mon Sep 17 00:00:00 2001
From: Alexander Holler 
Date: Wed, 23 Sep 2015 20:39:28 +0200
Subject: [PATCH 2/2] NO crash

---
 init/main.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/init/main.c b/init/main.c
index 1498f9b..4c1f041 100644
--- a/init/main.c
+++ b/init/main.c
@@ -862,8 +862,6 @@ static void __init do_initcall_level(int level)
 struct _annotated_initcall {
 	initcall_t initcall;
 	unsigned driver_id;
-	unsigned *dependencies;
-	struct device_driver *driver;
 };
 extern struct _annotated_initcall __annotated_initcall_start[];
 
-- 
2.1.0



crash_amd_iommu.config.bz2
Description: application/bzip


Re: AMD-IOMMU and problem with __init(data)?

2015-09-23 Thread Alexander Holler

Am 23.09.2015 um 13:43 schrieb Joerg Roedel:

Hey Alexander,

On Wed, Sep 23, 2015 at 12:22:24PM +0200, Alexander Holler wrote:

[1.539496] AMD-Vi: Lazy IO/TLB flushing enabled
[1.545741] AHO: count_annotated 25
[1.549259] AHO: build inventory
[1.552517] AHO: ac 81d400d8 ic   (null) ID
2177560225 deps 00b0 drv 81d25090
[1.562801] BUG: unable to handle kernel paging request at
039c2af5
(...)


Do you possibly have the full BUG message including the stacktrace?


The full msg is

-
[1.552517] AHO: ac 81d400d8 ic   (null) ID 
2177560225 deps 00b0 drv 81d25090
[1.562801] BUG: unable to handle kernel paging request at 
039c2af5

[1.569889] IP: [] do_annotated_initcalls+0x6f/0x25b
[1.576490] PGD 0
[1.578587] Oops: 0002 [#1] SMP
[1.581947] Modules linked in:
[1.585085] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
4.2.1-deps-00154-gb5f382c-dirty #768
[1.593374] Hardware name: System manufacturer System Product 
Name/F2A85-M, BIOS 6508 07/11/2014
[1.602184] task: 88042d508000 ti: 88042d51 task.ti: 
88042d51
[1.609693] RIP: 0010:[]  [] 
do_annotated_initcalls+0x6f/0x25b

[1.618718] RSP: 0018:88042d513f08  EFLAGS: 00010296
[1.624056] RAX: 81caeea1 RBX: 81d400d8 RCX: 

[1.631210] RDX: 81caeea1 RSI: 0246 RDI: 
81da7ae8
[1.638365] RBP: 0001 R08:  R09: 

[1.645519] R10: 01f9 R11: 0006 R12: 

[1.652676] R13:  R14:  R15: 

[1.659830] FS:  () GS:88043ec0() 
knlGS:

[1.667940] CS:  0010 DS:  ES:  CR0: 8005003b
[1.673707] CR2: 039c2af5 CR3: 01c0b000 CR4: 
000406f0

[1.680864] Stack:
[1.682908]  0006 0001  
81c9ced8
[1.690531]  8000 8164b3d0  
8164b3d9
[1.698153]  81c25380 81656f5f  


[1.705777] Call Trace:
[1.708255]  [] ? kernel_init_freeable+0xda/0x16a
[1.714544]  [] ? rest_init+0x70/0x70
[1.719793]  [] ? kernel_init+0x9/0xe0
[1.725129]  [] ? ret_from_fork+0x3f/0x70
[1.730724]  [] ? rest_init+0x70/0x70
[1.735974] Code: d4 81 73 4d 8b 4b 08 85 c9 74 40 48 8b 13 4c 8b 4b 
18 48 89 de 4c 8b 43 10 48 c7 c7 e0 4e 9d 81 e8 1c fe 9a ff 8b 53 08 48 
89 d0  82 54 3c d1 81 01 48 89 1c d5 40 f2 d0 81 8b 15 c7 63 07 00

[1.758158] RIP  [] do_annotated_initcalls+0x6f/0x25b
[1.764845]  RSP 
[1.768361] CR2: 039c2af5
[1.771710] ---[ end trace 5a4348fb7eabd051 ]---
[1.776363] [ cut here ]
[1.781010] WARNING: CPU: 0 PID: 1 at kernel/smp.c:292 
smp_call_function_single+0xe7/0x100()

[1.789472] Modules linked in:
[1.792610] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G  D 
4.2.1-deps-00154-gb5f382c-dirty #768
[1.802112] Hardware name: System manufacturer System Product 
Name/F2A85-M, BIOS 6508 07/11/2014
[1.810921]   819d6fba 8164f11e 

[1.818542]  81039647  810c87c0 
81c22040
[1.826166]   039c2af5 8109bc97 
88042d513cf8

[1.833790] Call Trace:
[1.836267]  [] ? dump_stack+0x40/0x50
[1.841603]  [] ? warn_slowpath_common+0x87/0xd0
[1.847806]  [] ? cpu_clock_event_start+0x30/0x30
[1.854095]  [] ? smp_call_function_single+0xe7/0x100
[1.860729]  [] ? task_function_call+0x42/0x50
[1.866760]  [] ? perf_cgroup_switch+0x160/0x160
[1.872963]  [] ? cgroup_exit+0xb0/0x130
[1.878470]  [] ? do_exit+0x347/0x9a0
[1.883720]  [] ? oops_end+0x8c/0xd0
[1.82]  [] ? no_context+0x123/0x370
[1.894392]  [] ? page_fault+0x22/0x30
[1.899728]  [] ? do_annotated_initcalls+0x6f/0x25b
[1.906190]  [] ? do_annotated_initcalls+0x69/0x25b
[1.912653]  [] ? kernel_init_freeable+0xda/0x16a
[1.918941]  [] ? rest_init+0x70/0x70
[1.924190]  [] ? kernel_init+0x9/0xe0
[1.929526]  [] ? ret_from_fork+0x3f/0x70
[1.935123]  [] ? rest_init+0x70/0x70
[1.940371] ---[ end trace 5a4348fb7eabd052 ]---
[1.945023] Kernel panic - not syncing: Attempted to kill init! 
exitcode=0x0009

[1.945023]
[1.954235] ---[ end Kernel panic - not syncing: Attempted to kill 
init! exitcode=0x0009

-

The bug happens because the code tried to uses foo[ID] and with an ID of 
2177560225 it wents clearly out of bounds. ;)





If it's necessary, I could try put together a small patch which
kills a system (reproducible here).


That would help too, please also send me your .config and I'll try to
reproduce the issue here.


Will do. Later

AMD-IOMMU and problem with __init(data)?

2015-09-23 Thread Alexander Holler

Hello,

It looks like I have a problem with the AMD IOMMU and it's handling of 
__init or __initdata.


I'm working on something which stores some structs right after 
INIT_CALLS but before CON_INITCALL (see include/asm-generic/vmlinux.lds.h).


This structures will be accessed right after the initcalls from level fs 
(5, see init/main.c), have been called.


Here is how that structure is defined:
--
struct _annotated_initcall {
initcall_t initcall;
unsigned driver_id;
unsigned *dependencies;
struct device_driver *driver;
};
extern struct _annotated_initcall __annotated_initcall_start[],
  __annotated_initcall_end[];
--

The code which uses that is
--
struct _annotated_initcall *ac;

ac = __annotated_initcall_start;
for (; ac < __annotated_initcall_end; ++ac)
pr_info("AHO: ac %p ic %p ID %u deps %p drv %p\n",
ac, ac->initcall, ac->driver_id,
ac->dependencies, ac->driver);
--

What now happens if I've enabled CONFIG_AMD_IOMMU is the following:

--
(...)
[1.240362] io scheduler noop registered
[1.395764] iommu: Adding device :00:00.0 to group 0
[1.401478] iommu: Adding device :00:01.0 to group 1
[1.406828] iommu: Adding device :00:01.1 to group 1
[1.412487] iommu: Adding device :00:10.0 to group 2
[1.417839] iommu: Adding device :00:10.1 to group 2
[1.423501] iommu: Adding device :00:11.0 to group 3
[1.429157] iommu: Adding device :00:12.0 to group 4
[1.434510] iommu: Adding device :00:12.2 to group 4
[1.440166] iommu: Adding device :00:13.0 to group 5
[1.445520] iommu: Adding device :00:13.2 to group 5
[1.451196] iommu: Adding device :00:14.0 to group 6
[1.456551] iommu: Adding device :00:14.2 to group 6
[1.461904] iommu: Adding device :00:14.3 to group 6
[1.467568] iommu: Adding device :00:14.4 to group 7
[1.473226] iommu: Adding device :00:15.0 to group 8
[1.480807] iommu: Adding device :00:15.1 to group 8
[1.486470] iommu: Adding device :00:18.0 to group 9
[1.491822] iommu: Adding device :00:18.1 to group 9
[1.497176] iommu: Adding device :00:18.2 to group 9
[1.502528] iommu: Adding device :00:18.3 to group 9
[1.507883] iommu: Adding device :00:18.4 to group 9
[1.513237] iommu: Adding device :00:18.5 to group 9
[1.518593] iommu: Adding device :03:00.0 to group 8
[1.523932] AMD-Vi: Found IOMMU at :00:00.2 cap 0x40
[1.529276] AMD-Vi:  Extended features:  PreF PPR GT IA
[1.534776] AMD-Vi: Interrupt remapping enabled
[1.539496] AMD-Vi: Lazy IO/TLB flushing enabled
[1.545741] AHO: count_annotated 25
[1.549259] AHO: build inventory
[1.552517] AHO: ac 81d400d8 ic   (null) ID 
2177560225 deps 00b0 drv 81d25090
[1.562801] BUG: unable to handle kernel paging request at 
039c2af5

(...)
--

The bug happens because the field driver_id of the structure (and likely 
the other stuff) is wrong.


If I disable CONFIG_AMD_IOMMU it looks like it should and how it works 
on ARM and Intel systems:

--
(...)
[1.151906] io scheduler noop registered
[1.307088] PCI-DMA: Using software bounce buffering for IO (SWIOTLB)
[1.313563] software IO TLB [mem 0x894ca000-0x8d4ca000] (64MB) mapped 
at [8800894ca000-88008d4c9fff]

[1.323411] AHO: count_annotated 25
[1.326934] AHO: build inventory
[1.330189] AHO: ac 81d3cea0 ic 81cadcb4 ID 176 deps 
81d22090 drv   (null)

(...)
--

Does anyone have an idea what's going on?

Kernel is 4.2.1 (x86_64) and HW is an A10-5800K.

If it's necessary, I could try put together a small patch which kills a 
system (reproducible here).


Thanks in advance,

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


  1   2   3   4   5   6   7   8   9   10   >