[PATCH] perf/x86/intel: fix 2 typos
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
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]
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]
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]
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]
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]
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]
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]
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]
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
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]
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]
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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)?
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)?
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)?
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)?
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)?
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)?
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)?
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/