[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 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 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-20000 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 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 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(&edma_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
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&p 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 wo
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 = <ðphy0>; }; }; + +&usb0 { + dependencies = <&usb_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 = <&hdmi_mux_2>; + no-dependencies = <&hdmi_mux_2>; }; ipu2_di0_mipi: endpoint@2 { @@ -205,6 +206,7 @@ ipu2_di1_hdmi: endpoint@1 { remote-endpoint = <&hdmi_mux_3>; + no-dependencies = <&hdmi_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 = <&hdmi_mux_0>; + no-dependencies = <&hdmi_mux_0>; }; ipu1_di0_mipi: endpoint@2 { remote-endpoint = <&mipi_mux_0>; + no-dependencies = <&mipi_mux_0>; }; ipu1_di0_lvds0: endpoint@3 { @@ -1175,10 +1177,12 @@ ipu1_di1_hdmi: endpoint@1 { remote-endpoint = <&hdmi_mux_1>; + no-dependencies = <&hdmi_mux_1>; }; ipu1_di1_mipi: endpoint@2 { remote-endpoint = <&mipi_mux_1>; + no-dependencies = <&mipi_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 = <&dpi_out>; + no-dependencies = <&dpi_out>; }; }; @@ -109,6 +110,7 @@ tfp410_out: endpoint@0 { remote-endpoint = <&dvi_connector_in>; + no-dependencies = <&dvi_connector_in>; }; }; }; @@ -150,6 +152,7 @@ etb_in: endpoint { slave-mode; remote-endpoint = <&etm_out>; + no-dependencies = <&etm_out>; }; }; }; @@ -373,6 +376,7 @@ port { venc_out: endpoint { remote-endpoint = <&tv_connector_in>; + no-dependencies = <&tv_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", &size); + 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 --- 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) { +
[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 = ad
[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 = &graph.edge_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.nverti
[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/
[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), &compare_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>; }; bar { remote-endpoint = <&foo>; no-dependencies = <&foo>; }; 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 +#inclu
[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
nd that these patches are an offer, I wasn't paid for them and I don't 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: 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 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
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/
Re: First kernel patch (optimization)
Am 20.09.2015 um 12:41 schrieb Alexander Holler: Am 20.09.2015 um 04:21 schrieb Theodore Ts'o: As far as what you want to do next, you have a personal "proof of concept" patch that seems to work well enough for you. Great! I'm sure you can keep using it for your own purposes. If you can convince someone with the skills to get the patch to an upstreamable state, it is my judgement that this is doable, so this puts your feature in a much better state than the FALLOC_FL_NO_HIDE_STALE flag. However, there is still a non-trivial amount of work left to do to turn your "proof of concept" patch into something that is upstremable, including changing the interface to using the FS_SECRM_FL flag. And your whining that other people should change *their* priorities to match *yours* is not likely to help. Besides that I have absolutely no knowledge about FALLOC_FL_NO_HIDE_STALE or the FS_SECRM_FL flag, I've never whined. I've complained about the tone very often used on this list. And it doesn't Just to explain why I'm still quiet happy with my very simple approach to use a simple modified discard mechanism to wipe files: My main use case (an that of several other people I know) is to have a simple way to wipe photos on sd-cards or to wipe other files I've copied once onto (vfat-formatted) usb-sticks while never having modified these files afterwards. And that works like a charm and doesn't need complicated patches which might be very different for every filesystem. And the check (and returning an error) if a file is already in use when trying to wipe it, makes it unnecessary to introduce something more complicated to schedule wiping. I also don't care if something is left in swap or similiar. So instead of trying to perfectly solve a big problem (which already was unsuccessful tried in case of the Linux kernel), I've just reduced the problem to fit the main use cases most people have and solved that with a very simple, but working approach. 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: First kernel patch (optimization)
Am 21.09.2015 um 17:47 schrieb Austin S Hemmelgarn: On 2015-09-20 06:41, Alexander Holler wrote: Am 20.09.2015 um 04:21 schrieb Theodore Ts'o: On Sat, Sep 19, 2015 at 07:47:22PM +0200, Alexander Holler wrote: Again, I don't think that encryption is an alternative. Besides that there is always the thread that strong encrytion will become regulated, there is also the very real thread that someone might end up in jail when using encryption and throwing away the key to delete stuff. E.g., as to my knowledge, in the UK you might end up in jail if you don't hand out a password. So what happens if you've deleted the key and are really unable to hand it out and the people which have an interest in what you've once stored don't believe you? First off, this is why I will never live in the UK. Secondly, this is (First, it should, of course, read threat, not thread, my English becomes worse when I'm getting angry, besides that I was working on a problem with threads just before.) Just in case of, I have not used the UK as an example because I might hate it or similar (nothing of that is the case). I've used the UK as an example to make it clear that such can happen in every country (besides that I know that some kernel maintainers live there).. And e.g. the US has had a time with regulated encryption (and I think there recently was another attempt to regulate it again). And besides states, there might be other people which might getting some unwanted ideas if they might believe that there is something of value for them encrypted somewhere by you. So, in my humble opinion, it's always better to get rid of something clearly. That also helps against the problem that the encryption used today, might be worthless tomorrow. 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: First kernel patch (optimization)
Am 21.09.2015 um 17:47 schrieb Austin S Hemmelgarn: The problem I see with this argument is: 1. There's a lot of code in the kernel that wouldn't be merged today in the state it's in, this creates a false sense of what quality is expected for new code (BTRFS in particular comes to mind here). Just to say it a last time, THE CODE I'VE POSTED WAS NEVER MEANT FOR MERGING in that state. Regardless how many people will still come by and repeat that it was ugly, bad and broken to just use that as an additional argument against me. I've absolutely no idea how you all start to test an idea and demonstrate it others, but I don't waste time on such tasks with looking for style, (premature) optimization or even races. I know how write production ready code, doing such since a long time and I know how much time that costs, and I know that only fools (or students which have to show that they can write good code) spend this time for code which might end up in the waste bin 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: First kernel patch (optimization)
Am 20.09.2015 um 04:21 schrieb Theodore Ts'o: On Sat, Sep 19, 2015 at 07:47:22PM +0200, Alexander Holler wrote: Perhaps not so surprisingly, over a decade later, it is not currently at the top of the priority list of any of the current file system or VFS developers, as far as I know. One of the reasons for that is that there are a number of other ways of achieving the same functionality. These include using tmpfs, or using file system level encryption. They require a bit more system administrator setup than just being able to set the FS_SECR_FL flag, true, but just because it's more convenient doesn't mean that it's worth doing. Again, I don't think that encryption is an alternative. Besides that there is always the thread that strong encrytion will become regulated, there is also the very real thread that someone might end up in jail when using encryption and throwing away the key to delete stuff. E.g., as to my knowledge, in the UK you might end up in jail if you don't hand out a password. So what happens if you've deleted the key and are really unable to hand it out and the people which have an interest in what you've once stored don't believe you? So this is a feature request. It's a reasonable feature request, in that if someone would like to pay $$$ for some consultant to implement it in a way that is bug-free, I suspect it could go upstream. Someone who was very motivated and with the sufficient skills could also invest their own effort to make a patch that can go upstream too. You've elected not, to because you believe it would take you months of "unpaid time". That's purely within your rights to do. But you don't have the right to try to tell other people what work to do on their behalf --- not unless you are paying their salary. First I haven't request that someone implements it for me. Besides that what you're describing is what maintainers do all the time. Of course, it's their job to request quality, but, in my humble opinion, very often they are requesting stuff just to request something. And that "month of unpaid time" was for sure a cynical exaggeration I've done while having been angry. In fact I believe the way I've outlined with the ugly code (proof of concept) could be implemented by someone like you in a weekend. For me it needs quiet some more time because I had and still have almost zero knowledge about all locks and whatever else is used in the filesystem code. But nevertheless I was able to fix up a lot of stuff during another afternoon. E.g. I've added checks if a file is in use or if AT_WIPE was called on a directory and then returned errors in those cases. Unfortunately the code changed in 4.2 and that patch doesn't apply anymore and now, because I don't really need those implementation details (I'm aware of the problems of my patch), I've thrown the patch into the waste bin. Besides that my concept doesn't work on BTRFS what I'm currently using for various reasons (mainly compression) on most of my systems. And I have no idea if it ever will (because I don't know why discard on BTRFS doesn't really discard what I think it should discard. ;) ). Seeing that the weight of the other file system developers are against the patch, it's never gone into the mainline Linux kernel, even though I could have forced the feature into ext4. However, this patch is in active use in practically every single data center kernel for Google, and it's in use in at least one other very large publically traded company that uses cluster file systems such as Hadoopfs. And if someone wants a copy of the FALLOC_FL_NO_HIDE_STALE patch for ext4, I'm happy to give it to them. But it hasn't gone upstream, and I'm OK with that. Sure, but please don't forget its quiet some difference if someone does stuff without being paid and all he earns are unfriendly comments. In fact I still don't care much about if any code from me ends up in mainline, but I dislike quiet a lot the tone used by many maintainers to refuse things someone offered in a good believe. E.g. recently I've read that a maintainer requested that patch posters should be aware of his calendar (like conferences he visits, merge windows he has to care for and similar stuff. ?!? As far as what you want to do next, you have a personal "proof of concept" patch that seems to work well enough for you. Great! I'm sure you can keep using it for your own purposes. If you can convince someone with the skills to get the patch to an upstreamable state, it is my judgement that this is doable, so this puts your feature in a much better state than the FALLOC_FL_NO_HIDE_STALE flag. However, there is still a non-trivial amount of work left to do to turn your "proof of concept" patc
Re: First kernel patch (optimization)
Am 19.09.2015 um 16:22 schrieb Theodore Ts'o: On Sat, Sep 19, 2015 at 02:52:06PM +0200, Alexander Holler wrote: I've recently posted a proof of concept for wiping files, or in other words to really delete files, And it was a disaster because if someone posts imperfect pathhes on this list, people have fun trying to eat you (because they seem bored or whatever). People gave you feedback on how what would be necessary to make the patch acceptable, and you rejected the advice complaining that it would take you months of "unpaid time". You were then complaining that the people who gave you feedback wouldn't fix the patch for you, and that you threatened that if we didn't integrate your racy patch into the core VFS, you would go switch to FreeBSD. Sorry, but I can happily live without feedback like "charming". I've complained about the requirement to post perfect patches without even the promise that they will ever have a chance to end up in the kernel. But where should someone get such an OK for an idea without the possibility to post preliminary patches without earning insulting comments? Anyway, I've accepted that I'm the error here which is why I almost don't post any patches here anymore. Even posting perfect patches is a game, because there exists always a space, newline or variable name which might be used to start annoying discussions. Trust me, the issues with your patch went *way* beyond extra spaces or newlines. It was (and is, imho) a working proof of concept. I've posted the patch to describe the idea, and NOT as a perfect patch ready for inclusion. I'm fully aware that there are million reasons why some parts of a file might not be deleted, but that is a matter how you describe the functionality. And the concept works imho for many use cases. At least much, much, much better than just nothing. But obviously, one of biggest failures one can do, is to post imperfect patches on this list. Regardless how you describe or mark them (mine had a RFC and WIP in front of), people will use every small detail against you, in order to have something to criticise or to make some fun of. So, there is no reason to wonder about the lack of such tasks. Greg was specifically looking for patches that could be done in a weekend. Sorry, I've missed that detail. So maybe create a tracker for such weekend-tasks. If you want to raise the argument that we should lower the standards for accepting patches so that more patches can accomplished within a weekend's worth of work, we can have that discussion --- but I'm not sure it will have the end result that you are hoping for. No. I don't want to lower the standards. Maybe in regard to silly style stuff, but not in regard to code quality (and I mean real bugs like races, deadlocks or such, and not if a line has more than 80 characters). I would have liked some comments like "good or bad idea" but this list is imho the wrong place to search for such useful comments. I haven't searched for comments on the code, as I was FULLY aware that the code is ugly and NOT ready for inclusion), 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: First kernel patch (optimization)
Am 19.09.2015 um 14:52 schrieb Alexander Holler: Am 19.09.2015 um 07:18 schrieb Greg KH: I have been saying for years that we have a lack of real projects / tasks / ideas for people who are skilled, yet have no idea what to do. I know of well over a hundred people I have email addresses of that have asked me for these types of things, and have patches in the kernel that are non-trivial to prove that they have the skill to do real things. It's a real problem, and one that I don't have an answer for. We need these types of tasks, and I don't have them, and every maintainer I ask about it also doesn't have them. What we are asking for is people to somehow come up with tasks on their own, as if they know what needs to be done. I've recently posted a proof of concept for wiping files, or in other words to really delete files, And it was a disaster because if someone posts imperfect pathhes on this list, people have fun trying to eat you (because they seem bored or whatever). Even posting perfect patches is a game, because there exists always a space, newline or variable name which might be used to start annoying discussions. So, there is no reason to wonder about the lack of such tasks. So even if you don't agree, here as task: wipe files in real. ;) By the way, in that discussion (about wiping files) I was blamed for writing bugs in bugzilla without offering a patch, for something they believe it is a feature (whereas I still believe it is a bug to not really delete files). That leaded me to the question if there is somewhere a feature request tracker, for which I've got, of course, no answer. So, if there is a lack of real projects / tasks / ideas, maybe it might make sense to setup such a feature request tracker (or idea pool), maybe with the possibility to let people up/down vote ideas. 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: First kernel patch (optimization)
Am 19.09.2015 um 07:18 schrieb Greg KH: I have been saying for years that we have a lack of real projects / tasks / ideas for people who are skilled, yet have no idea what to do. I know of well over a hundred people I have email addresses of that have asked me for these types of things, and have patches in the kernel that are non-trivial to prove that they have the skill to do real things. It's a real problem, and one that I don't have an answer for. We need these types of tasks, and I don't have them, and every maintainer I ask about it also doesn't have them. What we are asking for is people to somehow come up with tasks on their own, as if they know what needs to be done. I've recently posted a proof of concept for wiping files, or in other words to really delete files, And it was a disaster because if someone posts imperfect pathhes on this list, people have fun trying to eat you (because they seem bored or whatever). Even posting perfect patches is a game, because there exists always a space, newline or variable name which might be used to start annoying discussions. So, there is no reason to wonder about the lack of such tasks. So even if you don't agree, here as task: wipe files in real. ;) 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 16/16] phy: phy-core: fix initcall level
Am 18.09.2015 um 08:16 schrieb Kishon Vijay Abraham I: Hi, On Wednesday 26 August 2015 05:58 PM, Alexander Holler wrote: The phy-core has to be initialized before other dependent usb-drivers, otherwise a crash might occur. Currently phy_core_init() is called in the initcall-level device, which is the same level where most usb-drivers will end up. By luck this seemed to have been called most of the time before other usb-drivers without having been explicitly enforced. But if phy_core_init() is not called before a dependent driver, a null-pointer exception might occur (e.g. because the phy device class isn't registered). Did you actually face a problem? IIUC the modules get loaded based on the drivers/Makefile order (unless the other modules are in a different initcall table). I had a problem while playing with a modified init-system (based on dependencies). So not an actual problem. IMHO the fix should be in the module that caused the crash. Change it to use module_init? The problem arises if the init-system ignores the link order and assumes all drivers in the same initcall level can be called without any special ordering. The problem might also appear if a driver changes its name, directory or position in file system. E.g. how to you make sure that a driver in staging will be linked after the phy-core? Actually this happens, but I would assume its by luck. I assume if staging would be renamed to 'beta-quality' a lot of stuff would actually fail because of the problem with the implicit link order. Anyway, nothing which really has to be fixed. It's just a notice that maybe another initcall level of 'subsys' or something else before 'device' might be a better place for phy-core. I've chosen fs_sync instead of subsys because otherwise I would have had to look up if phy-core depends on another subsystem and therefore has to be initialized after subsys. Regards, Alexander Holler Thanks Kishon To fix this, phy_core_init() is moved to the initcall-level fs (right before the standard initcall level device). Signed-off-by: Alexander Holler --- drivers/phy/phy-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c index fc48fac..4945029 100644 --- a/drivers/phy/phy-core.c +++ b/drivers/phy/phy-core.c @@ -930,7 +930,7 @@ static int __init phy_core_init(void) return 0; } -module_init(phy_core_init); +fs_initcall_sync(phy_core_init); static void __exit phy_core_exit(void) { -- 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/ -- 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/2] deps: parallel initialization of (device-)drivers
Am 09.09.2015 um 20:35 schrieb Alexander Holler: Hello, as already mentioned, I've implemented the stuff to initialize drivers in parallel. What follows are two patches to be used on top of my already posted series (for 4.2) which implements annotated initcalls and DT based dependencies. But be warned: many drivers which are in the same initcall level still depend on the link order given by the Makefile and directoy (-name) and therefor will fail. That means without moving them to other initcall levels or explicit dependencies (which are a TODO) for these drivers, the whole stuff currently works only for some configurations and you likely will need to add several patches for your board. Another update: I've now did what I've described as TODO above. That means I have everything working to parallelize the (whole) init-system regardless the arch or DT/ACPI or whatever. Cleaning up the new stuff to post it here will need some time. And collecting the _mandatory_ dependencies to parallelize all static linked drivers (from all initcall levels) will need much more time. Even on systems where most stuff is build as a module, the list of drivers initialized through initcalls is usually several dozens or even hundreds. You might use 'grep initcall_ System.map | wc -l' to get an idea. Therefor I don't know when I will post cleaned up patches and/or some benchmark times. The interest seems rather low. 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 2/2] deps: avoid multiple calls to memmove by just setting duplicates to 0
Besides make the code (almost unmeasurable) faster, this makes the ugly looking loop I've used to remove duplicates cleaner. Disadvantage is that the ordered array now contains 'holes' and the number of elements in the array doesn't really match the number of ordered elements. But this only makes a difference for debugging. This patch also adds an of_node_put() for duplicate dt nodes, something I previously had forgotten. Signed-off-by: Alexander Holler --- drivers/of/of_dependencies.c | 34 -- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/drivers/of/of_dependencies.c b/drivers/of/of_dependencies.c index 85cef84..ac0c0f5 100644 --- a/drivers/of/of_dependencies.c +++ b/drivers/of/of_dependencies.c @@ -323,21 +323,20 @@ static bool __init all_compatibles_same(struct device_node *node1, /* * The order is based on devices but we are calling drivers. * Therefor the order contains some drivers more than once. - * Remove the duplicates. + * Disable the duplicates by setting them to 0. */ -static void __init of_init_remove_duplicates(void) +static void __init of_init_disable_duplicates(void) { unsigned i, j; for (i = 1; i < order.count; ++i) for (j = 0; j < i; ++j) { + if (!order.order[j]) + continue; if (all_compatibles_same(order.order[j], order.order[i])) { - --order.count; - memmove(&order.order[i], &order.order[i+1], - (order.count - i) * - sizeof(order.order[0])); - --i; + of_node_put(order.order[i]); + order.order[i] = 0; break; } } @@ -416,7 +415,8 @@ static void __init build_tgroups(void) unsigned dist = 0; for (i = 0; i < order.count; ++i) { - if (distance[order.order[i]->phandle] != dist) { + if (order.order[i] && + distance[order.order[i]->phandle] != dist) { dist = distance[order.order[i]->phandle]; count_groups++; tgroup[count_groups].start = i; @@ -436,6 +436,8 @@ static void __init of_init_print_order(void) pr_info("Initialization order:\n"); for (i = 0; i < order.count; ++i) { + if (!order.order[i]) + continue; #ifdef CONFIG_OF_DEPENDENCIES_PARALLEL pr_info("init %u 0x%x (group %u)", i, order.order[i]->phandle, @@ -496,10 +498,10 @@ static int __init of_init_build_order(void) #ifdef CONFIG_OF_DEPENDENCIES_PARALLEL build_order_by_distance(); - of_init_remove_duplicates(); + of_init_disable_duplicates(); build_tgroups(); #else - of_init_remove_duplicates(); + of_init_disable_duplicates(); #endif #ifdef CONFIG_OF_DEPENDENCIES_PRINT_INIT_ORDER @@ -516,7 +518,8 @@ static void __init of_init_free_order(void) unsigned i; for (i = 0; i < order.count; ++i) - of_node_put(order.order[i]); + if (order.order[i]) + of_node_put(order.order[i]); order.count = 0; /* remove_new_phandles(); */ } @@ -593,8 +596,10 @@ static int __init initcall_thread(void *thread_nr) start = atomic_read(&ostart); count = atomic_read(&ocount); while ((i = atomic_dec_return(&shared_counter)) >= 0) - init_if_matched(order.order[start + count - 1 - i], - (unsigned)thread_nr); + if (order.order[start + count - 1 - i]) + init_if_matched( + order.order[start + count - 1 - i], +(unsigned)thread_nr); prepare_to_wait(&group_waitqueue, &wait, TASK_UNINTERRUPTIBLE); if (!atomic_dec_and_test(&count_initcall_threads)) { schedule(); @@ -629,7 +634,8 @@ static void __init of_init_drivers_non_threaded(void) if (!of_init_build_order()) { for (i = 0; i < order.count; ++i) - init_if_matched(order.order[i], 0); + if (order.order[i]) + init_if_matched(order.order[i], 0); of_init_free_order(); } ac = __annotated_initcall_start; -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the bo
[PATCH 0/2] deps: parallel initialization of (device-)drivers
Hello, as already mentioned, I've implemented the stuff to initialize drivers in parallel. What follows are two patches to be used on top of my already posted series (for 4.2) which implements annotated initcalls and DT based dependencies. But be warned: many drivers which are in the same initcall level still depend on the link order given by the Makefile and directoy (-name) and therefor will fail. That means without moving them to other initcall levels or explicit dependencies (which are a TODO) for these drivers, the whole stuff currently works only for some configurations and you likely will need to add several patches for your board. I've already posted two patches to move two drivers into another initcall level. While playing with the stuff, I've found several more drivers which are suffering under the same problem and need the same modification: block/noop-iosched.c crypto/hmac.c cryoto/sha_generic.c drivers/mtd/ofpart.c drivers/tty/serial/8250/8250_core.c To offer an impression how this patch series might work in action, below is the relevant part from the kernel log for a configuration I'm using successfully on an imx6q. Maybe someone has interest in that stuff. Regards, Alexander Holler --- (...) [2.628325] Thread 0 calling (ordered) initcall for driver reg-fixed-voltage (regulator-fixed) [2.629196] Thread 3 calling (ordered) initcall for driver imx6q-pinctrl (fsl,imx6q-iomuxc) [2.629331] Thread 0 calling (ordered) initcall for driver gpio-mxc (fsl,imx1-gpio) [2.630276] imx6q-pinctrl 20e.iomuxc: initialized IMX pinctrl driver [2.632543] Thread 0 calling (ordered) initcall for driver anatop_regulator (fsl,anatop-regulator) [2.632556] Thread 1 calling (ordered) initcall for driver imx-sdma (fsl,imx6q-sdma) [2.632563] Thread 2 calling (ordered) initcall for driver mxs-dma (fsl,imx23-dma-apbh) [2.632598] Thread 3 calling (ordered) initcall for driver sram (mmio-sram) [2.634502] mxs-dma 11.dma-apbh: initialized [2.634848] Thread 3 calling (ordered) initcall for driver mxs_phy (fsl,imx6sx-usbphy) [2.635165] Thread 0 calling (ordered) initcall for driver imx2-wdt (fsl,imx21-wdt) [2.635181] Thread 2 calling (ordered) initcall for driver snvs_rtc (fsl,sec-v4.0-mon-rtc-lp) [2.635493] snvs_rtc 20cc034.snvs-rtc-lp: rtc core: setting system clock to 2015-09-09 16:37:09 UTC (1441816629) [2.635813] snvs_rtc 20cc034.snvs-rtc-lp: rtc core: registered 20cc034.snvs-rtc-lp as rtc0 [2.635978] Thread 2 calling (ordered) initcall for driver ahci-imx (fsl,imx53-ahci) [2.636317] imx-sdma 20ec000.sdma: initialized [2.636322] ahci-imx 220.sata: fsl,transmit-level-mV not specified, using 0024 [2.636332] ahci-imx 220.sata: fsl,transmit-boost-mdB not specified, using 0480 [2.636338] ahci-imx 220.sata: fsl,transmit-atten-16ths not specified, using 2000 [2.636347] ahci-imx 220.sata: fsl,receive-eq-mdB not specified, using 0500 [2.636690] Thread 1 calling (ordered) initcall for driver wandboard-rfkill (wand,imx6q-wandboard-rfkill) [2.637160] imx-sdma 20ec000.sdma: loaded firmware 1.1 [2.637166] imx2-wdt 20bc000.wdog: timeout 60 sec (nowayout=0) [2.637283] wandboard-rfkill rfkill: Wandboard rfkill initialization [2.637402] Thread 0 calling (ordered) initcall for driver leds-gpio (gpio-leds) [2.637422] wandboard-rfkill rfkill: Turning of power [2.639193] ahci-imx 220.sata: SSS flag set, parallel bus scan disabled [2.639253] ahci-imx 220.sata: AHCI 0001.0300 32 slots 1 ports 3 Gbps 0x1 impl platform mode [2.639299] ahci-imx 220.sata: flags: ncq sntf stag pm led clo only pmp pio slum part ccc apst [2.640579] scsi host0: ahci-imx [2.640902] ata1: SATA max UDMA/133 mmio [mem 0x0220-0x02203fff] port 0x100 irq 67 [2.663463] wandboard-rfkill rfkill: initialize wifi chip [2.663642] wandboard-rfkill rfkill: wifi-rfkill registered. [2.663720] wandboard-rfkill rfkill: initialize bluetooth chip [2.663919] wandboard-rfkill rfkill: bluetooth-rfkill registered. [2.664289] Thread 1 calling (ordered) initcall for driver imx-gpc (fsl,imx6q-gpc) [2.664335] Thread 3 calling (ordered) initcall for driver imx-uart (fsl,imx6q-uart) [2.664769] 202.serial: ttymxc0 at MMIO 0x202 (irq = 23, base_baud = 500) is a IMX [2.983471] ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300) [2.984610] ata1.00: ATA-8: Hitachi HTS542525K9SA00, BBFOC31P, max UDMA/133 [2.984620] ata1.00: 488397168 sectors, multi 0: LBA48 NCQ (depth 31/32) [2.985793] ata1.00: configured for UDMA/133 [2.985912] Default I/O scheduler not found. Using noop. [2.986213] scsi 0:0:0:0: Direct-Access ATA Hitachi HTS54252 C31P PQ: 0 ANSI: 5 [2.986776] scsi 0:0:0:0: Failed to register bsg queue, errno=-19 [3.734832] console [ttymxc0] enabled [3.739241] 21ec000.serial: ttymx
[PATCH 1/2] deps: parallel initialization of (device-)drivers
This initializes drivers (annotated or in the initcall level device) in parallel. Which drivers can be initialized in parallel is calculated by using the dependencies. That means, currently, only annotated drivers which are are referenced in the used DT will be in order. For all others it is assumed that, as long as they belong to the same initcall level (device), they can be called in any order. Unfortunately this isn't allways true and several drivers are depending on the link-order (based on the Makefile and the directory). This is, imho, a bug or at least a very fragile way to do such and should be, again imho, fixed. Otherwise problems might arise if e.g. a driver is moved from staging to its final position (which changes its place in the list of initcalls too). But this isn't really the topic of this patch and I'm mentioning this here just as a warning or as hint in case someone experiences problems when enabling the feature this patch provides. Signed-off-by: Alexander Holler --- drivers/of/Kconfig | 20 drivers/of/of_dependencies.c | 245 ++- 2 files changed, 261 insertions(+), 4 deletions(-) diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index 26c4b1a..7e6e910 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -132,4 +132,24 @@ config OF_DEPENDENCIES_DEBUG_CALLS_OF_ANNOTATED_INITCALLS help Used for debugging purposes. +config OF_DEPENDENCIES_PARALLEL + bool "Initialize annotated initcalls in parallel" + depends on OF_DEPENDENCIES + help + Calculates which (annotated) initcalls can be called in parallel + and calls them using multiple threads. Be warned, this doesn't + work always as it should because of missing dependencies and + because it assumes that drivers belonging to the same initcall + level can be called in an order different than the order they + are linked. + +config OF_DEPENDENCIES_THREADS + int "Number of threads to use for parallel initialization" + depends on OF_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. + endif # OF diff --git a/drivers/of/of_dependencies.c b/drivers/of/of_dependencies.c index 06435d5..85cef84 100644 --- a/drivers/of/of_dependencies.c +++ b/drivers/of/of_dependencies.c @@ -11,12 +11,16 @@ */ #include +#include #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 */ +#ifdef CONFIG_OF_DEPENDENCIES_PARALLEL + uint32_t x; +#endif struct edgenode *next; /* next edge in list */ }; @@ -120,6 +124,9 @@ static int __init insert_edge(uint32_t x, uint32_t y) graph.include_node[x] = 1; graph.include_node[y] = 1; p->y = y; +#ifdef CONFIG_OF_DEPENDENCIES_PARALLEL + p->x = x; +#endif p->next = graph.edges[x]; graph.edges[x] = p; /* insert at head of list */ @@ -336,6 +343,90 @@ static void __init of_init_remove_duplicates(void) } } +#ifdef CONFIG_OF_DEPENDENCIES_PARALLEL +/* + * The algorithm I've used below to calculate the max. distance for + * nodes to the root node likely isn't the fasted. But based on the + * already done implementation of the topological sort, this is an + * easy way to achieve this. Instead of first doing an topological + * sort and then using the stuff below to calculate the distances, + * using an algorithm which does spit out distances directly would + * be likely faster. + * If you want to spend the time, you could have a look e.g. at the + * topic 'layered graph drawing'. + */ +/* max. distance from a node to root */ +static unsigned distance[MAX_DT_NODES+1] __initdata; +static struct device_node *order_by_distance[MAX_DT_NODES+1] __initdata; + +static void __init calc_max_distance(uint32_t v) +{ + unsigned i; + unsigned max_dist = 0; + + for (i = 0; i < graph.nedges; ++i) + if (graph.edge_slots[i].x == v) + max_dist = max(max_dist, + distance[graph.edge_slots[i].y] + 1); + distance[v] = max_dist; +} + +static void __init calc_distances(void) +{ + unsigned i; + + for (i = 0; i < order.count; ++i) + calc_max_distance(order.order[i]->phandle); +} + +static void __init build_order_by_distance(void) +{ + unsigned i, j; + unsigned max_distance = 0; + unsigned new_order_count = 0; + + calc_distances(); + order_by_distance[new_order_count++] = order.order[0]; + for (i = 1; i < order.count; ++i) { + if (distance[order.o
Re: [PATCH 15/16] mtd: mtdcore: fix initcall level
Am 04.09.2015 um 06:00 schrieb Alexander Holler: Am 02.09.2015 um 07:34 schrieb Alexander Holler: Am 01.09.2015 um 23:19 schrieb Brian Norris: Hi Alexander, No judgment here for the rest of this series, but for this patch: On Wed, Aug 26, 2015 at 02:28:27PM +0200, Alexander Holler wrote: The mtd-core has to be initialized before other dependent mtd-drivers, otherwise a crash might occur. Currently mtd_init() is called in the initcall-level device, which is the same level where most mtd-drivers will end up. By luck this seemed to have been called most of the time before other mtd-drivers without having been explicitly enforced. I can't really speak for the original authors, but it does not appear to be entirely "by luck." Link order was one of the de facto ways to get this ordering (though it's not really a great one), and mtdcore was always linked first within the drivers/mtd/ directory structure. But that's just background, I think this is worth fixing anyway. It could, for instance, become a problem if drivers are located outside drivers/mtd/; I see random board files in arch/ that register with MTD, and I'm actually not sure how they have never tripped on this. As I've just had a look at my patches in order to clean up the patch for parallel initialization (to post it here too): drivers/mtd/ofparts.c has the same problem. In order to let the NAND-driver see the partitions defined in the DT I had to move this into another initcall level (fs sync) too. 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/
deps: update in regard to parallel initialization of static linked drivers
Am 26.08.2015 um 14:28 schrieb Alexander Holler: Hello, (...) Some numbers (5 boots on each board, without and with ordering drivers), all times are seconds. (...) imx6q (armv7): unordered: 3.451998 3.418864 3.446952 3.429974 3.440996 (3.4377568) ordered: 3.538312 3.549019 3.538105 3.515916 3.555715 (3.5394134) (...) Further improvements could be to initialize drivers in parallel (using multiple cores) and/or to use this stuff on x86 (ACPI) too (e.g. by using a minimal DT which just includes dependencies). I've now made a quick implementation which uses multiple threads to initialize annotated drivers in parallel. It worked out of the box. Here are the times (dmesg | grep Freeing) I've now measured on the imx6q (4 cores): 3.388273 3.399892 3.411615 3.410523 3.388802 (3.399821) So it's now at least as fast as without ordering the drivers. Even on the one system where ordering didn't help without parallel initialization (likely because the unordered sequence of initcalls is already quiet good on that system, in comparison to the other systems I've tested). And my current implementation for the parallel stuff is far from being perfect and can be optimized much more (enough to not post a patch here). In addition I've added another driver to my config since I measure the old times, so the new times are including one more initialization of a driver (it now calls 30 annotated (static linked) drivers at boot, most stuff is still in modules (and some not annotated static linked drivers) on that system). Maybe this helps raising interest enough that someone else will test and maybe give some (reasonable, not about style) feedback on my patches. 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 15/16] mtd: mtdcore: fix initcall level
Am 02.09.2015 um 07:34 schrieb Alexander Holler: Am 01.09.2015 um 23:19 schrieb Brian Norris: Hi Alexander, No judgment here for the rest of this series, but for this patch: On Wed, Aug 26, 2015 at 02:28:27PM +0200, Alexander Holler wrote: The mtd-core has to be initialized before other dependent mtd-drivers, otherwise a crash might occur. Currently mtd_init() is called in the initcall-level device, which is the same level where most mtd-drivers will end up. By luck this seemed to have been called most of the time before other mtd-drivers without having been explicitly enforced. I can't really speak for the original authors, but it does not appear to be entirely "by luck." Link order was one of the de facto ways to get this ordering (though it's not really a great one), and mtdcore was always linked first within the drivers/mtd/ directory structure. But that's just background, I think this is worth fixing anyway. It could, for instance, become a problem if drivers are located outside drivers/mtd/; I see random board files in arch/ that register with MTD, and I'm actually not sure how they have never tripped on this. I've already found at least a half a dozen other drivers with the same problem through my shuffling of the drivers which all end up in the standard initcall level device. I'm aware that this is based on the link order, which itself is based on linker behaviour (and maybe other things like make or other build tools). I'm just calling it luck, because this is nowhere explicitly stated, at least I've never seen such a statement, neither in any source, nor somewhere in Documentation. So I've choosen the term "by luck" in order to provoke a bit (or to stimulate a discussion about that widespread problem). A good example why "luck" might not be far away from the truth is what happens when a drivers moves e.g. from staging to it's real position. Also the position will stay inside the same initcall level, the move of the driver into another directory (maybe together with a rename) will likely change its position in the initcall-sequence, without anyone really expecting this. But if mtd_init() is not called before a dependent driver, a null-pointer exception might occur (e.g. because the mtd device class isn't registered). To fix this, mtd-init() is moved to the initcall-level fs (right before the standard initcall level device). Is there a good reason we shouldn't just make this a subsys_initcall()? That would match the naming better, and it mirrors what, e.g., block/genhd uses. It would also allow flexibility if there are other current/future use cases that might need to sit between the subsystem and the drivers. No real reason here. The names for the initcall levels seem to be outdated since a long time anyway, and I think just speaking about the numbers 1-7 (or 0-14) would be better anyways. The only reason why I've used the fs (sync) level is because I was too lazy to check out if mtdcore might depend on something else in any other level. Therefor I've used the one most close to were it was before. Lazy was the wrong term. It is time consuming, cumbersome and therefor error-prone to check on what other stuff a driver depends. One reason why choosing the right place in the initcall sequence isn't that easy and why some automation make sense. Also I've an idea about how to fix that in general for all drivers (by using the same algorithm I've now introduced just for DT-based drivers with a device description). Wouldn't be that hard to use that for all drivers, but as I've written in a follow up to the introductory mail, one step after another. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to 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 15/16] mtd: mtdcore: fix initcall level
Am 01.09.2015 um 23:19 schrieb Brian Norris: Hi Alexander, No judgment here for the rest of this series, but for this patch: On Wed, Aug 26, 2015 at 02:28:27PM +0200, Alexander Holler wrote: The mtd-core has to be initialized before other dependent mtd-drivers, otherwise a crash might occur. Currently mtd_init() is called in the initcall-level device, which is the same level where most mtd-drivers will end up. By luck this seemed to have been called most of the time before other mtd-drivers without having been explicitly enforced. I can't really speak for the original authors, but it does not appear to be entirely "by luck." Link order was one of the de facto ways to get this ordering (though it's not really a great one), and mtdcore was always linked first within the drivers/mtd/ directory structure. But that's just background, I think this is worth fixing anyway. It could, for instance, become a problem if drivers are located outside drivers/mtd/; I see random board files in arch/ that register with MTD, and I'm actually not sure how they have never tripped on this. I've already found at least a half a dozen other drivers with the same problem through my shuffling of the drivers which all end up in the standard initcall level device. I'm aware that this is based on the link order, which itself is based on linker behaviour (and maybe other things like make or other build tools). I'm just calling it luck, because this is nowhere explicitly stated, at least I've never seen such a statement, neither in any source, nor somewhere in Documentation. So I've choosen the term "by luck" in order to provoke a bit (or to stimulate a discussion about that widespread problem). But if mtd_init() is not called before a dependent driver, a null-pointer exception might occur (e.g. because the mtd device class isn't registered). To fix this, mtd-init() is moved to the initcall-level fs (right before the standard initcall level device). Is there a good reason we shouldn't just make this a subsys_initcall()? That would match the naming better, and it mirrors what, e.g., block/genhd uses. It would also allow flexibility if there are other current/future use cases that might need to sit between the subsystem and the drivers. No real reason here. The names for the initcall levels seem to be outdated since a long time anyway, and I think just speaking about the numbers 1-7 (or 0-14) would be better anyways. The only reason why I've used the fs (sync) level is because I was too lazy to check out if mtdcore might depend on something else in any other level. Therefor I've used the one most close to were it was before. Also I've an idea about how to fix that in general for all drivers (by using the same algorithm I've now introduced just for DT-based drivers with a device description). Wouldn't be that hard to use that for all drivers, but as I've written in a follow up to the introductory mail, one step after another. Regards, Alexander Holler -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to 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 net-next v5 00/11] ipv6: Only create RTF_CACHE route after encountering pmtu exception
Am 28.08.2015 um 11:27 schrieb Alexander Holler: Am 28.08.2015 um 09:36 schrieb Martin KaFai Lau: On Mon, Aug 17, 2015 at 11:43:20AM +0200, Alexander Holler wrote: That's why I vote to check out if it's possible/reasonable to backport this series to the stable kernels. I have backported to 4.0.y without major issue, so possible. Sure, as this was likely one of the versions they've used to create the patch. I did try on 3.1x and gave up. It is a lot of changes, so I don't think it is a good idea for -stable. Depends on what you're expecting from a (stable) kernel. The patch description mentions what happens when a system deals with a lot of other ipv6-systems and that problem is easy to exercise and to value. Rating the information leak is harder, some people even won't understand that this might be a problem. And now look at which kernel-versions are now used in new devices (likely something <= 3.10, which is more than two years old), how long they will be used, and make a guess about IPv6 usage in 5 years. Anyway, I've no insights about all the politics happening in the background (e.g. stuff like the LTSI tree) and I've just wanted raise awareness about that (imho important) patch series. Not to speak about phones, but those are most likely a problem of one specific company ;) 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 net-next v5 00/11] ipv6: Only create RTF_CACHE route after encountering pmtu exception
Am 28.08.2015 um 09:36 schrieb Martin KaFai Lau: On Mon, Aug 17, 2015 at 11:43:20AM +0200, Alexander Holler wrote: That's why I vote to check out if it's possible/reasonable to backport this series to the stable kernels. I have backported to 4.0.y without major issue, so possible. Sure, as this was likely one of the versions they've used to create the patch. I did try on 3.1x and gave up. It is a lot of changes, so I don't think it is a good idea for -stable. Depends on what you're expecting from a (stable) kernel. The patch description mentions what happens when a system deals with a lot of other ipv6-systems and that problem is easy to exercise and to value. Rating the information leak is harder, some people even won't understand that this might be a problem. And now look at which kernel-versions are now used in new devices (likely something <= 3.10, which is more than two years old), how long they will be used, and make a guess about IPv6 usage in 5 years. Anyway, I've no insights about all the politics happening in the background (e.g. stuff like the LTSI tree) and I've just wanted raise awareness about that (imho important) patch 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 00/16] deps: deterministic driver initialization order
Am 27.08.2015 um 21:01 schrieb Alexander Holler: Am 26.08.2015 um 14:28 schrieb Alexander Holler: The final 2 patches are fixes which should end up in mainline, regardless if this feature is merged or not. Just in case your serial or MTD-NAND partitions don't work, the drivers drivers/tty/serial/8250/8250_core.c and drivers/mtd/ofpart.c need the same trivial fix for their initcall level as found in the last two patches of my series. I'm not going to post these patches now until I've got some feedback for the other stuff. And just in case, I also have an idea how to fix such dependencies for drivers without DT (by adding hard-wired dependencies directly to drivers). That would also be usable for ACPI and x86. But one step after another. Based on my past experiences, I don't even think the stuff I've just posted will ever end up in the kernel. 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 00/16] deps: deterministic driver initialization order
Am 26.08.2015 um 14:28 schrieb Alexander Holler: The final 2 patches are fixes which should end up in mainline, regardless if this feature is merged or not. Just in case your serial or MTD-NAND partitions don't work, the drivers drivers/tty/serial/8250/8250_core.c and drivers/mtd/ofpart.c need the same trivial fix for their initcall level as found in the last two patches of my series. I'm not going to post these patches now until I've got some feedback for the other stuff. 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 05/16] deps: introduce initcalls annotated with a struct device_driver
Make it possible to identify initcalls before calling them by adding a pointer to a struct device_driver to the stored pointer to an initcall. 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, dev_drv) core_initcall(fn) annotated_initcall(core, fn, dev_drv) core_initcall_sync(fn) annotated_initcall_sync(core, fn, dev_drv) ... late_initcall(fn) annotated_initcall(late, fn, dev_drv) module_init(fn) annotated_module_init(fn, dev_drv) module_platform_driver(drv) no changes necessary, done automatically module_platform_driver_probe(drv, probe)no changes necessary module_i2c_driver(i2c_drv) no changes necessary, done automatically E.g. to make the driver sram offering an annotated initcall the following patch is necessary: -postcore_initcall(sram_init); +annotated_initcall(postcore, sram_init, sram_driver.driver); 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 + include/asm-generic/vmlinux.lds.h | 6 ++ include/linux/device.h| 12 include/linux/i2c.h | 2 +- include/linux/init.h | 33 + include/linux/module.h| 2 ++ include/linux/platform_device.h | 16 init/Kconfig | 3 +++ 8 files changed, 70 insertions(+), 5 deletions(-) diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S index 8b60fde..10a328f 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_INITCALL CON_INITCALL SECURITY_INITCALL INIT_RAM_FS diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index 8bd374d..7318ba7 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_INITCALL \ + 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_INITCALL \ CON_INITCALL\ SECURITY_INITCALL \ INIT_RAM_FS \ diff --git a/include/linux/device.h b/include/linux/device.h index a2b4ea7..128fddd 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -1321,4 +1321,16 @@ static int __init __driver##_init(void) \ } \ device_initcall(__driver##_init); +#define annotated_module_driver(__driver, __register, __unregister, ...) \ +static int __init __driver##_init(void) \ +{ \ + return __register(&(__driver), ##__VA_ARGS__); \ +} \ +annotated_module_init(__driver##_init, __driver.driver); \ +static void __exit __driver##_exit(void) \ +{ \ + __unregister(&(__driver), ##__VA_ARGS__); \ +} \ +module_exit(__driver##_exit) + #endif /* _DEVICE_H_ */ diff --git a/include/linux/i2c.h b/include/linux/i2c.h index e83a738..fa63ec1 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -626,7 +626,7 @@ static inline int i2c_adapter_id(struct i2c_adapter *adap) * use this macro once, and calling it replaces module_init() and module_exit() */ #define module_i2c_driver(__i2c_driver) \ - module_driver(__i2c_driver, i2c_add_driver, \ + annotated_module_driver(__i2c_driver, i2c_add_driver, \ i2c_del_driver) #endif /* I2C */ diff --git a/include/linux/init.h b/include/linux/init.h index b449f37..52ea986 100644 --- a/include/linux/init.h +++ b
[PATCH 10/16] deps: dts: 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 = <&dpi_out>; + no-dependencies = <&dpi_out>; }; }; @@ -109,6 +110,7 @@ tfp410_out: endpoint@0 { remote-endpoint = <&dvi_connector_in>; + no-dependencies = <&dvi_connector_in>; }; }; }; @@ -150,6 +152,7 @@ etb_in: endpoint { slave-mode; remote-endpoint = <&etm_out>; + no-dependencies = <&etm_out>; }; }; }; @@ -373,6 +376,7 @@ port { venc_out: endpoint { remote-endpoint = <&tv_connector_in>; + no-dependencies = <&tv_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 09/16] deps: dts: 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 = <&hdmi_mux_2>; + no-dependencies = <&hdmi_mux_2>; }; ipu2_di0_mipi: endpoint@2 { @@ -205,6 +206,7 @@ ipu2_di1_hdmi: endpoint@1 { remote-endpoint = <&hdmi_mux_3>; + no-dependencies = <&hdmi_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 = <&hdmi_mux_0>; + no-dependencies = <&hdmi_mux_0>; }; ipu1_di0_mipi: endpoint@2 { remote-endpoint = <&mipi_mux_0>; + no-dependencies = <&mipi_mux_0>; }; ipu1_di0_lvds0: endpoint@3 { @@ -1175,10 +1177,12 @@ ipu1_di1_hdmi: endpoint@1 { remote-endpoint = <&hdmi_mux_1>; + no-dependencies = <&hdmi_mux_1>; }; ipu1_di1_mipi: endpoint@2 { remote-endpoint = <&mipi_mux_1>; + no-dependencies = <&mipi_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/16] deps: WIP: kirkwood: annotate some initcalls
WIP means Work In Progress. Change some kirkwood drivers to offer annotated initcalls. Signed-off-by: Alexander Holler --- drivers/dma/mv_xor.c | 2 +- drivers/net/ethernet/marvell/mv643xx_eth.c | 3 ++- drivers/usb/host/ehci-orion.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c index f1325f6..e766ab2 100644 --- a/drivers/dma/mv_xor.c +++ b/drivers/dma/mv_xor.c @@ -1295,7 +1295,7 @@ static int __init mv_xor_init(void) { return platform_driver_register(&mv_xor_driver); } -module_init(mv_xor_init); +annotated_module_init(mv_xor_init, mv_xor_driver.driver); /* it's currently unsafe to unload this module */ #if 0 diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c index d52639b..5eec4d8 100644 --- a/drivers/net/ethernet/marvell/mv643xx_eth.c +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c @@ -3239,7 +3239,8 @@ static int __init mv643xx_eth_init_module(void) return rc; } -module_init(mv643xx_eth_init_module); +annotated_module_init(mv643xx_eth_init_module, + mv643xx_eth_shared_driver.driver); static void __exit mv643xx_eth_cleanup_module(void) { diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c index bfcbb9a..28f8793 100644 --- a/drivers/usb/host/ehci-orion.c +++ b/drivers/usb/host/ehci-orion.c @@ -333,7 +333,7 @@ static int __init ehci_orion_init(void) ehci_init_driver(&ehci_orion_hc_driver, &orion_overrides); return platform_driver_register(&ehci_orion_driver); } -module_init(ehci_orion_init); +annotated_module_init(ehci_orion_init, ehci_orion_driver.driver); static void __exit ehci_orion_cleanup(void) { -- 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 16/16] phy: phy-core: fix initcall level
The phy-core has to be initialized before other dependent usb-drivers, otherwise a crash might occur. Currently phy_core_init() is called in the initcall-level device, which is the same level where most usb-drivers will end up. By luck this seemed to have been called most of the time before other usb-drivers without having been explicitly enforced. But if phy_core_init() is not called before a dependent driver, a null-pointer exception might occur (e.g. because the phy device class isn't registered). To fix this, phy_core_init() is moved to the initcall-level fs (right before the standard initcall level device). Signed-off-by: Alexander Holler --- drivers/phy/phy-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c index fc48fac..4945029 100644 --- a/drivers/phy/phy-core.c +++ b/drivers/phy/phy-core.c @@ -930,7 +930,7 @@ static int __init phy_core_init(void) return 0; } -module_init(phy_core_init); +fs_initcall_sync(phy_core_init); static void __exit phy_core_exit(void) { -- 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 15/16] mtd: mtdcore: fix initcall level
The mtd-core has to be initialized before other dependent mtd-drivers, otherwise a crash might occur. Currently mtd_init() is called in the initcall-level device, which is the same level where most mtd-drivers will end up. By luck this seemed to have been called most of the time before other mtd-drivers without having been explicitly enforced. But if mtd_init() is not called before a dependent driver, a null-pointer exception might occur (e.g. because the mtd device class isn't registered). To fix this, mtd-init() is moved to the initcall-level fs (right before the standard initcall level device). Signed-off-by: Alexander Holler --- drivers/mtd/mtdcore.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index 875..fa8e6452 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -1303,7 +1303,7 @@ static void __exit cleanup_mtd(void) bdi_destroy(&mtd_bdi); } -module_init(init_mtd); +fs_initcall_sync(init_mtd); module_exit(cleanup_mtd); MODULE_LICENSE("GPL"); -- 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/16] deps: WIP: imx: annotate some initcalls
WIP means Work In Progress. Change some imx drivers to offer annotated initcalls. Signed-off-by: Alexander Holler --- arch/arm/mach-imx/gpc.c | 2 +- arch/arm/mach-imx/mmdc.c | 2 +- drivers/dma/mxs-dma.c | 2 +- drivers/gpio/gpio-mxc.c | 2 +- drivers/i2c/busses/i2c-imx.c | 2 +- drivers/pinctrl/freescale/pinctrl-imx6q.c | 2 +- drivers/power/reset/imx-snvs-poweroff.c | 2 +- drivers/regulator/anatop-regulator.c | 3 ++- drivers/tty/serial/imx.c | 2 +- drivers/usb/phy/phy-mxs-usb.c | 2 +- sound/soc/fsl/imx-audmux.c| 2 +- 11 files changed, 12 insertions(+), 11 deletions(-) diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c index 8c4467f..3802614 100644 --- a/arch/arm/mach-imx/gpc.c +++ b/arch/arm/mach-imx/gpc.c @@ -468,4 +468,4 @@ static int __init imx_pgc_init(void) { return platform_driver_register(&imx_gpc_driver); } -subsys_initcall(imx_pgc_init); +annotated_initcall(subsys, imx_pgc_init, imx_gpc_driver.driver); diff --git a/arch/arm/mach-imx/mmdc.c b/arch/arm/mach-imx/mmdc.c index db9621c..cf84c03 100644 --- a/arch/arm/mach-imx/mmdc.c +++ b/arch/arm/mach-imx/mmdc.c @@ -87,4 +87,4 @@ static int __init imx_mmdc_init(void) { return platform_driver_register(&imx_mmdc_driver); } -postcore_initcall(imx_mmdc_init); +annotated_initcall(postcore, imx_mmdc_init, imx_mmdc_driver.driver); diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c index 60de352..5bfa232 100644 --- a/drivers/dma/mxs-dma.c +++ b/drivers/dma/mxs-dma.c @@ -884,4 +884,4 @@ static int __init mxs_dma_module_init(void) { return platform_driver_probe(&mxs_dma_driver, mxs_dma_probe); } -subsys_initcall(mxs_dma_module_init); +annotated_initcall(subsys, mxs_dma_module_init, mxs_dma_driver.driver); diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c index ec1eb1b..0a22a62 100644 --- a/drivers/gpio/gpio-mxc.c +++ b/drivers/gpio/gpio-mxc.c @@ -506,7 +506,7 @@ static int __init gpio_mxc_init(void) { return platform_driver_register(&mxc_gpio_driver); } -postcore_initcall(gpio_mxc_init); +annotated_initcall(postcore, gpio_mxc_init, mxc_gpio_driver.driver); MODULE_AUTHOR("Freescale Semiconductor, " "Daniel Mack , " diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index 785aa67..c20d03c 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -1110,7 +1110,7 @@ static int __init i2c_adap_imx_init(void) { return platform_driver_register(&i2c_imx_driver); } -subsys_initcall(i2c_adap_imx_init); +annotated_initcall(subsys, i2c_adap_imx_init, i2c_imx_driver.driver); static void __exit i2c_adap_imx_exit(void) { diff --git a/drivers/pinctrl/freescale/pinctrl-imx6q.c b/drivers/pinctrl/freescale/pinctrl-imx6q.c index 4d1fcb8..77a1ffd 100644 --- a/drivers/pinctrl/freescale/pinctrl-imx6q.c +++ b/drivers/pinctrl/freescale/pinctrl-imx6q.c @@ -489,7 +489,7 @@ static int __init imx6q_pinctrl_init(void) { return platform_driver_register(&imx6q_pinctrl_driver); } -arch_initcall(imx6q_pinctrl_init); +annotated_initcall(arch, imx6q_pinctrl_init, imx6q_pinctrl_driver.driver); static void __exit imx6q_pinctrl_exit(void) { diff --git a/drivers/power/reset/imx-snvs-poweroff.c b/drivers/power/reset/imx-snvs-poweroff.c index ad6ce50..57a06dc 100644 --- a/drivers/power/reset/imx-snvs-poweroff.c +++ b/drivers/power/reset/imx-snvs-poweroff.c @@ -63,4 +63,4 @@ static int __init imx_poweroff_init(void) { return platform_driver_register(&imx_poweroff_driver); } -device_initcall(imx_poweroff_init); +annotated_initcall(device, imx_poweroff_init, imx_poweroff_driver.driver); diff --git a/drivers/regulator/anatop-regulator.c b/drivers/regulator/anatop-regulator.c index 738adfa..97808da 100644 --- a/drivers/regulator/anatop-regulator.c +++ b/drivers/regulator/anatop-regulator.c @@ -331,7 +331,8 @@ static int __init anatop_regulator_init(void) { return platform_driver_register(&anatop_regulator_driver); } -postcore_initcall(anatop_regulator_init); +annotated_initcall(postcore, anatop_regulator_init, + anatop_regulator_driver.driver); static void __exit anatop_regulator_exit(void) { diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index 54fdc78..c79e19b 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -2024,7 +2024,7 @@ static void __exit imx_serial_exit(void) uart_unregister_driver(&imx_reg); } -module_init(imx_serial_init); +annotated_module_init(imx_serial_init, serial_imx_driver.driver); module_exit(imx_serial_exit); MODULE_AUTHOR("Sascha Hauer"); diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c index 3fcc048..dd2768d 100644 --- a/drivers/usb/phy/phy-mxs-usb.c +++ b/d
[PATCH 13/16] deps: WIP: omap: annotate some initcalls
WIP means Work In Progress. Change some omap drivers to offer annotated initcalls. Signed-off-by: Alexander Holler --- arch/arm/common/edma.c | 2 +- drivers/bus/omap_l3_smx.c | 2 +- drivers/dma/omap-dma.c | 2 +- drivers/gpio/gpio-twl4030.c| 2 +- drivers/gpu/drm/tilcdc/tilcdc_drv.c| 2 +- drivers/i2c/busses/i2c-omap.c | 2 +- drivers/iommu/omap-iommu.c | 2 +- drivers/mailbox/omap-mailbox.c | 2 +- drivers/memory/omap-gpmc.c | 2 +- drivers/mfd/omap-usb-host.c| 2 +- drivers/mfd/omap-usb-tll.c | 2 +- drivers/mfd/tps65217.c | 2 +- drivers/net/ethernet/ti/cpsw.c | 2 +- drivers/net/ethernet/ti/davinci_mdio.c | 2 +- drivers/phy/phy-twl4030-usb.c | 2 +- drivers/regulator/twl-regulator.c | 2 +- drivers/tty/serial/omap-serial.c | 2 +- drivers/usb/host/ehci-omap.c | 2 +- drivers/usb/host/ohci-omap3.c | 2 +- drivers/usb/musb/omap2430.c| 2 +- 20 files changed, 20 insertions(+), 20 deletions(-) diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c index 873dbfc..29f363c 100644 --- a/arch/arm/common/edma.c +++ b/arch/arm/common/edma.c @@ -1872,5 +1872,5 @@ static int __init edma_init(void) { return platform_driver_probe(&edma_driver, edma_probe); } -arch_initcall(edma_init); +annotated_initcall(arch, edma_init, edma_driver.driver); diff --git a/drivers/bus/omap_l3_smx.c b/drivers/bus/omap_l3_smx.c index 360a5c0..e2172b8 100644 --- a/drivers/bus/omap_l3_smx.c +++ b/drivers/bus/omap_l3_smx.c @@ -302,7 +302,7 @@ static int __init omap3_l3_init(void) { return platform_driver_register(&omap3_l3_driver); } -postcore_initcall_sync(omap3_l3_init); +annotated_initcall_sync(postcore, omap3_l3_init, omap3_l3_driver.driver); static void __exit omap3_l3_exit(void) { diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c index 249445c..0866ae9 100644 --- a/drivers/dma/omap-dma.c +++ b/drivers/dma/omap-dma.c @@ -1285,7 +1285,7 @@ static int omap_dma_init(void) { return platform_driver_register(&omap_dma_driver); } -subsys_initcall(omap_dma_init); +annotated_initcall(subsys, omap_dma_init, omap_dma_driver.driver); static void __exit omap_dma_exit(void) { diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c index 9e1dbb9..60c0d1a 100644 --- a/drivers/gpio/gpio-twl4030.c +++ b/drivers/gpio/gpio-twl4030.c @@ -615,7 +615,7 @@ static int __init gpio_twl4030_init(void) { return platform_driver_register(&gpio_twl4030_driver); } -subsys_initcall(gpio_twl4030_init); +annotated_initcall(subsys, gpio_twl4030_init, gpio_twl4030_driver.driver); static void __exit gpio_twl4030_exit(void) { diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 0f283a3..8c50f23 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -715,7 +715,7 @@ static void __exit tilcdc_drm_fini(void) tilcdc_tfp410_fini(); } -module_init(tilcdc_drm_init); +annotated_module_init(tilcdc_drm_init, tilcdc_platform_driver.driver); module_exit(tilcdc_drm_fini); MODULE_AUTHOR("Rob Clark http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 11/16] deps: WIP: generic: annotate some initcalls
WIP means Work In Progress. Change some generic drivers to offer annotated initcalls. Signed-off-by: Alexander Holler --- drivers/input/keyboard/gpio_keys.c | 2 +- drivers/misc/sram.c| 2 +- drivers/regulator/fixed.c | 3 ++- drivers/usb/phy/phy-generic.c | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c index ddf4045..319bcf7 100644 --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c @@ -855,7 +855,7 @@ static void __exit gpio_keys_exit(void) platform_driver_unregister(&gpio_keys_device_driver); } -late_initcall(gpio_keys_init); +annotated_initcall(late, gpio_keys_init, gpio_keys_device_driver.driver); module_exit(gpio_keys_exit); MODULE_LICENSE("GPL"); diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c index 15c33cc..eb662bd 100644 --- a/drivers/misc/sram.c +++ b/drivers/misc/sram.c @@ -243,4 +243,4 @@ static int __init sram_init(void) return platform_driver_register(&sram_driver); } -postcore_initcall(sram_init); +annotated_initcall(postcore, sram_init, sram_driver.driver); diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c index ff62d69..ec35d10 100644 --- a/drivers/regulator/fixed.c +++ b/drivers/regulator/fixed.c @@ -221,7 +221,8 @@ static int __init regulator_fixed_voltage_init(void) { return platform_driver_register(®ulator_fixed_voltage_driver); } -subsys_initcall(regulator_fixed_voltage_init); +annotated_initcall(subsys, regulator_fixed_voltage_init, + regulator_fixed_voltage_driver.driver); static void __exit regulator_fixed_voltage_exit(void) { diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c index deee68e..baf95d0 100644 --- a/drivers/usb/phy/phy-generic.c +++ b/drivers/usb/phy/phy-generic.c @@ -360,7 +360,7 @@ static int __init usb_phy_generic_init(void) { return platform_driver_register(&usb_phy_generic_driver); } -subsys_initcall(usb_phy_generic_init); +annotated_initcall(subsys, usb_phy_generic_init, usb_phy_generic_driver.driver); static void __exit usb_phy_generic_exit(void) { -- 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/16] deps: dtc: 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 dd4658c..3fb5cef 100644 --- a/scripts/dtc/dependencies.c +++ b/scripts/dtc/dependencies.c @@ -106,3 +106,347 @@ void add_dependencies(struct boot_info *bi) { process_nodes_props(bi->dt, 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 = &graph.edge_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; + graph.nve
[PATCH 08/16] deps: dts: 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 = <ðphy0>; }; }; + +&usb0 { + dependencies = <&usb_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 06/16] deps: deterministic driver initialization sequence based on dependencies from the DT
Use annotated initcalls along with dependencies found in the DT to sort the initialization of drivers. This makes stuff like in-driver hacks (e.g. omap), bruteforce trial-and-error (deferred probe calls which are killing error messages) and similar workarounds to circumvent the limited level based driver initialization sequence unnecessary. If enabled, this changes the driver initialization sequence as described in the following table. old (level based) new (ordered, in parts) - early early pure (0)pure (0) core (1, 1s)core (1, 1s) postcore (2, 2s)postcore (2, 2s) arch (3, 3s)arch (3, 3s) subsys (4. 4s) subsys (4, 4s) fs (5, 5s) fs (5, 5s) rootfs rootfs device (6. 6s) annotated initcalls (ordered by DT) annotated initcalls (not found in DT) device (6. 6s) late (7, 7s)late (7, 7s) To use this feature new binary DT blobs which are including dependencies (type information for phandles) have to be used and most drivers found in the DT should have been annotated. Signed-off-by: Alexander Holler --- drivers/of/Kconfig | 12 ++ drivers/of/Makefile | 1 + drivers/of/of_dependencies.c| 410 include/linux/of_dependencies.h | 20 ++ init/main.c | 11 +- 5 files changed, 453 insertions(+), 1 deletion(-) create mode 100644 drivers/of/of_dependencies.c create mode 100644 include/linux/of_dependencies.h diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index 59bb855..9bf6c73 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -102,4 +102,16 @@ config OF_OVERLAY While this option is selected automatically when needed, you can enable it manually to improve device tree unit test coverage. +config OF_DEPENDENCIES + bool "Device Tree dependency based initialization order (EXPERIMENTAL)" + select ANNOTATED_INITCALLS + help + Enables dependency based initialization order of drivers. + + For correct operation the binary DT blob should have been + populated with properties of type "dependencies" and the + drivers referenced in the DT should have been annotated. + + If unsure, say N here. + endif # OF diff --git a/drivers/of/Makefile b/drivers/of/Makefile index 156c072..05ced0d 100644 --- a/drivers/of/Makefile +++ b/drivers/of/Makefile @@ -14,5 +14,6 @@ obj-$(CONFIG_OF_MTD) += of_mtd.o obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o obj-$(CONFIG_OF_RESOLVE) += resolver.o obj-$(CONFIG_OF_OVERLAY) += overlay.o +obj-$(CONFIG_OF_DEPENDENCIES) += of_dependencies.o obj-$(CONFIG_OF_UNITTEST) += unittest-data/ diff --git a/drivers/of/of_dependencies.c b/drivers/of/of_dependencies.c new file mode 100644 index 000..64ed049 --- /dev/null +++ b/drivers/of/of_dependencies.c @@ -0,0 +1,410 @@ +/* + * Code for building a deterministic initialization order based on dependencies + * defined in the device tree. + * + * 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 + +#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; + uint32_t ph_root; /* the phandle of the root */ +}; +static struct init_order order __i
[PATCH 07/16] deps: add debug configuration options
Add some debug options to print annotated initcalls, the ordered list of annotated initcalls and to print a message right before an annotated initcall is called. Signed-off-by: Alexander Holler --- drivers/of/Kconfig | 18 ++ drivers/of/of_dependencies.c | 57 ++-- 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig index 9bf6c73..26c4b1a 100644 --- a/drivers/of/Kconfig +++ b/drivers/of/Kconfig @@ -114,4 +114,22 @@ config OF_DEPENDENCIES If unsure, say N here. +config OF_DEPENDENCIES_PRINT_INIT_ORDER + bool "Print dependency based initialization order" + depends on OF_DEPENDENCIES + help + Used for debugging purposes. + +config OF_DEPENDENCIES_PRINT_ANNOTATED_INITCALLS + bool "Print names of annotated drivers" + depends on OF_DEPENDENCIES + help + Used for debugging purposes. + +config OF_DEPENDENCIES_DEBUG_CALLS_OF_ANNOTATED_INITCALLS + bool "Show when annotated initcalls are actually called" + depends on OF_DEPENDENCIES + help + Used for debugging purposes. + endif # OF diff --git a/drivers/of/of_dependencies.c b/drivers/of/of_dependencies.c index 64ed049..06435d5 100644 --- a/drivers/of/of_dependencies.c +++ b/drivers/of/of_dependencies.c @@ -336,6 +336,41 @@ static void __init of_init_remove_duplicates(void) } } +#ifdef CONFIG_OF_DEPENDENCIES_PRINT_INIT_ORDER +static void __init of_init_print_order(void) +{ + unsigned i; + struct property *prop; + const char *cp; + + pr_info("Initialization order:\n"); + for (i = 0; i < order.count; ++i) { + pr_info("init %u 0x%x", i, order.order[i]->phandle); + if (order.order[i]->name) + pr_cont(" %s", order.order[i]->name); + if (order.order[i]->full_name) + pr_cont(" (%s)", order.order[i]->full_name); + prop = __of_find_property(order.order[i], "compatible", NULL); + for (cp = of_prop_next_string(prop, NULL); cp; +cp = of_prop_next_string(prop, cp)) + pr_cont(" %s", cp); + } +} +#endif + +#ifdef CONFIG_OF_DEPENDENCIES_PRINT_ANNOTATED_INITCALLS +static void __init of_init_print_annotated(void) +{ + struct _annotated_initcall *ac; + + pr_info("Annotated drivers:\n"); + ac = __annotated_initcall_start; + for (; ac < __annotated_initcall_end; ++ac) + pr_info("Driver '%s' (%s)\n", ac->driver->name, + ac->driver->of_match_table->compatible); +} +#endif + static int __init of_init_build_order(void) { int rc = 0; @@ -363,7 +398,12 @@ static int __init of_init_build_order(void) return -EINVAL; /* cycle found */ of_init_remove_duplicates(); - +#ifdef CONFIG_OF_DEPENDENCIES_PRINT_INIT_ORDER + of_init_print_order(); +#endif +#ifdef CONFIG_OF_DEPENDENCIES_PRINT_ANNOTATED_INITCALLS + of_init_print_annotated(); +#endif return rc; } @@ -386,6 +426,12 @@ static void __init init_if_matched(struct device_node *node) if (ac->initcall && ac->driver->of_match_table) if (of_match_node(ac->driver->of_match_table, node)) { +#ifdef CONFIG_OF_DEPENDENCIES_DEBUG_CALLS_OF_ANNOTATED_INITCALLS + pr_info("Calling (ordered) initcall for driver %s (%s)\n", + ac->driver->name, + ac->driver->of_match_table ? + ac->driver->of_match_table->compatible : ""); +#endif do_one_initcall(*ac->initcall); ac->initcall = 0; } @@ -404,7 +450,14 @@ void __init of_init_drivers(void) } ac = __annotated_initcall_start; for (; ac < __annotated_initcall_end; ++ac) { - if (ac->initcall) + if (ac->initcall) { +#ifdef CONFIG_OF_DEPENDENCIES_DEBUG_CALLS_OF_ANNOTATED_INITCALLS + pr_info("Calling (unordered) initcall for driver %s (%s)\n", + ac->driver->name, + ac->driver->of_match_table ? + ac->driver->of_match_table->compatible : ""); +#endif do_one_initcall(*ac->initcall); + } } } -- 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/16] deps: dtc: 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>; }; bar { remote-endpoint = <&foo>; no-dependencies = <&foo>; }; 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 2c1e0d4..76e4d91 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,9 +123,21 @@ 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 03/16] deps: dtc: 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 3fb5cef..2c1e0d4 100644 --- a/scripts/dtc/dependencies.c +++ b/scripts/dtc/dependencies.c @@ -298,7 +298,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; @@ -328,6 +328,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; @@ -352,9 +355,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)) @@ -375,13 +379,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; } @@ -424,7 +449,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; @@ -436,12 +461,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 = ad
[PATCH 00/16] deps: deterministic driver initialization order
Hello, I've already written a lot on that topic, therefor this introductory mail doesn't explain everything. Please have a look at the threads "dt: dependencies (for deterministic driver initialization order based on the DT)" and "On-demand device registration" on LKML for previous discussions. In short, the current init system is quiet old and doesn't really match todays HW which needs much more drivers than 20 years ago. As I've mentioned in those threads, initcalls should be identifiable in order to sort them. My previous attempt intercepted the driver registrations in order to do that, which unfortunately ended up with having to deal how drivers are creating devices. I've now spend some more time and this partly new approach now annotates the initcalls directly, by storing a pointer to a struct device_driver in addition to the pointer to the initcall. This makes my previous patches smaller, cleaner and better to understand, the result which you can see in this little patch series. The whole patch series is made up of several parts. The first 4 patches are modifying dtc to include dependencies (type information for phandles). The next 3 patches are the ones which are implementing those annotated initcalls and the driver sort algorithm. Then there are 3 patches with small changes on some dts. The following 4 patches with the WIP (Work In Progress) are changing some initcalls to annotated initcalls. I would split them up in a bunch of patches (one for each driver), if I get the promise that the other patches will be merged into mainline (but not without that promise). The final 2 patches are fixes which should end up in mainline, regardless if this feature is merged or not. Some numbers (5 boots on each board, without and with ordering drivers), all times are seconds. Kirkwood (dockstar, armv5): Boot to "Freeing unused kernel memory" (includes mounting the rootfs), unordered: 4.456016 3.937801 4.114788 4.114526 3.949480 (average 4.1145222) ordered: 3.173054 3.164045 3.141418 3.480679 3.459298 (3.2836988) Time needed to sort (of_init_build_order()): 0.003024 Time needed to match drivers to the order (without calling them): 0.002884 Beagleboard (rev C4, armv7): unordered: 6.706024 6.821746 6.696014 6.673675 6.769866 (6.733465) ordered: 5.544860 5.514160 5.505859 5.527374 5.496795 (5.5178096) sorting: 0.021209 matching: 0.006165 Beaglebone Black (rev A5, armv7): unordered: 3.826531 3.825662 3.826648 3.825434 3.825263 (3.8259076) ordered: 2.838554 2.838322 2.839459 2.838467 2.838421 (2.8386446) sorting: 0.004769 matching: 0.004860 imx6q (armv7): unordered: 3.451998 3.418864 3.446952 3.429974 3.440996 (3.4377568) ordered: 3.538312 3.549019 3.538105 3.515916 3.555715 (3.5394134) sorting: 0.004622 matching: 0.003868 Because I'm using different configuration options on these boards, the absolute times aren't comparable. But in conclusion, most boards are booting faster when the initcalls have been ordered. In addition, ordering the initcalls makes the initialization sequence more deterministic (it can even be seen without booting a machine). But I still think the most benefit of these little series is by not having to use in-driver hacks or that ugly brute force trial-and-error deferred probing in order to fix a wrong default call order of initcalls. Further improvements could be to initialize drivers in parallel (using multiple cores) and/or to use this stuff on x86 (ACPI) too (e.g. by using a minimal DT which just includes dependencies). To test this series with any other DT enabled board, look at which drivers are referenced in the DT (see patch 2 or 3), annotate these drivers (trivial, see patch 5 or the examples with WIP in the subject), turn on CONFIG_OF_DEPENDENCIES, compile and boot. Just to mention it, unless CONFIG_OF_DEPENDENCIES is turned on (which is marked as experimental), nothing will change, even if you've annotated some drivers (the new macros will just map to the old ones). Regards, Alexander Holler PS: Please keep in mind that these patches are an offer. I'm already 100+ patches above mainline and don't really care to maintain some more for myself. -- 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/16] deps: dtc: 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) { +
Re: [PATCH net-next v5 00/11] ipv6: Only create RTF_CACHE route after encountering pmtu exception
Am 15.08.2015 um 09:48 schrieb Alexander Holler: Am 30.07.2015 um 13:57 schrieb Alexander Holler: Am 29.07.2015 um 11:25 schrieb Alexander Holler: Am 23.05.2015 um 05:55 schrieb Martin KaFai Lau: To complete the discussion, that "annoying behaviour" is also a big information leak. Because routes aren't considered confidential and aren't subject to privacy, that broken behaviour enabled *everyone* on the same system to see *all* the remote IPv6 systems to which there have been connection establishment tries. Just in case I haven't described the problem I see clearly enough: "Everyone" means everything (other SW) too, and if "Happy_Eyeballs" algorithms are used (see RFC 6555), this also affects systems which only have an IPv4 connection to the world, as long as IPv6 is enabled. That means it does not only affect multiuser systems and the current behaviour of kernels < 4.2 renders e.g. the private mode of most browsers somewhat useless too (in regard to protection against other SW and/or users running on the same system). That's why I vote to check out if it's possible/reasonable to backport this series to the stable kernels. 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 net-next v5 00/11] ipv6: Only create RTF_CACHE route after encountering pmtu exception
Am 30.07.2015 um 13:57 schrieb Alexander Holler: Am 29.07.2015 um 11:25 schrieb Alexander Holler: Am 23.05.2015 um 05:55 schrieb Martin KaFai Lau: This series is to avoid creating a RTF_CACHE route whenever we are consulting the fib6 tree with a new destination. Instead, only create RTF_CACHE route when we see a pmtu exception. That even helps on systems without an IPv6-connection to world because it avoids the IPv6 route add/delete pairs which happened before whenever an IPv6-connection was tried (e.g. by Happy Eyeballs algorithms). I think that's worse a laud. thanks. Of course, I meant worth. Sorry, but the left part of my brain seems to be sometimes in a (maybe forced) power save mode. ;) Also I wonder how the previous algorithm went into the kernel at all or why it wasn't fixed earlier. Anyway, it's great that someone took the time to fix that annoying behaviour (I've had on my radar since quiet some time). To complete the discussion, that "annoying behaviour" is also a big information leak. Because routes aren't considered confidential and aren't subject to privacy, that broken behaviour enabled *everyone* on the same system to see *all* the remote IPv6 systems to which there have been connection establishment tries. E.g. I can see the following on a system when browsing to facebook.com and google.com: [aholler@krabat snetmanmon.git]$ ./snetmanmon snetmanmon.conf.simple_example snetmanmon V1.3-5-g9f06 (C) 2015 Alexander Holler (...) New route 2a00:1450:4001:80c::100a (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' New route 2a03:2880:2130:cf05:face:b00c:0:1 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' New route 2a00:1450:4001:80c::1007 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' New route 2a00:1450:400f:803::101f (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' New route 2a00:1450:4001:80c::1008 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' New route 2a00:1450:4001:80c::1017 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' New route 2a00:1450:4016:804::200d (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' New route 2a00:1450:4001:80c::1000 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' New route 2a00:1450:4001:80c::1016 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' New route 2a00:1450:400f:803::1013 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' New route 2a00:1450:4001:80c::1006 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' New route 2a00:1450:4001:80c::1018 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' New route 2a00:1450:4016:804::2009 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' New route 2a00:1450:4001:80c::1005 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' Route 2a00:1450:4001:80c::100a (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' was deleted Route 2a03:2880:2130:cf05:face:b00c:0:1 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' was deleted Route 2a00:1450:4001:80c::1000 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' was deleted Route 2a00:1450:4001:80c::1005 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' was deleted Route 2a00:1450:4001:80c::1006 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' was deleted Route 2a00:1450:4001:80c::1007 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' was deleted Route 2a00:1450:4001:80c::1008 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' was deleted Route 2a00:1450:4001:80c::1016 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' was deleted Route 2a00:1450:4001:80c::1017 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' was deleted Route 2a00:1450:4001:80c::1018 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' was deleted Route 2a00:1450:400f:803::1013 (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' was deleted Route 2a00:1450:400f:803::101f (gateway fe80::21f:7bff:feb4:d13, type v6, scope universe) on interface 'virbr0' was deleted Route 2a00:1450:4016:804::2009 (gatewa
Re: [PATCH 4.1 099/267] phy: twl4030-usb: remove incorrect pm_runtime_get_sync() in probe function.
Am 11.08.2015 um 10:29 schrieb NeilBrown: With the current mainline code (plus my twl4030 charger enhancements, which are not deeply relevant), the refcount does go to zero when nothing is plugged in, and goes to 2 when a regular USB cable is plugged in. Also I think it's just a little miswording (or extended typo), I'm not using a regular USB cable, but an OTG cable when using the host mode of the musb on the Beagleboard. Just to avoid confusion. Or do you talk about the client mode when it goes to 2? Besides that, I'm sorry to not be of further help. My deeper knowledge about the musb sources in the kernel and u-boot (and the HW in question), has gone lost and I've currently no real reason to refresh 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 4.1 099/267] phy: twl4030-usb: remove incorrect pm_runtime_get_sync() in probe function.
Am 09.08.2015 um 11:00 schrieb NeilBrown: On Sat, 8 Aug 2015 12:18:55 +0530 Kishon Vijay Abraham I wrote: On Saturday 08 August 2015 11:23 AM, Alexander Holler wrote: Hello, this patch killed the musb-host functionality on my classic Beagleboard (rev c4). Symptom was that it there was a message I don't remember and the attached device didn't enumerate anymore (likely because of missing power, but I'm not sure). A simple revert has fixed it, I haven't looked further into the problem. Neil Brown, how was this tested? Well, I have a board with an OMAP3 connected to a twl4030 for USB and I noted that it wasn't power-managed properly and when I made that change, it was. I don't recall the exact details This is probably related to Commit: 56301df6bcaa ("phy: twl4030-usb: make runtime pm more reliable.") I certainly only tested with that patch in place. Cherry-Picking 56301df6bcaa instead of reverting d1221a608bd did the trick too. So it looks like 56301df6bcaa is indeed a prerequisit for d1221a608bd. Therefor I suggest to feed 56301df6bcaa to the stable series (e.g. 4.1.6) too. 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: [Bugfix] x86, irq: Fix a regression caused by commit b5dc8e6c21e7
Am 09.08.2015 um 10:15 schrieb Jiang Liu: Alex Deucher, Mark Rustad and Alexander Holler reported a regression with the latest v4.2-rc4 kernel, which breaks some SATA controllers. With multi-MSI capable SATA controllers, only the first port works, all other ports times out when executing SATA commands. This regression bisects to 52f518a3a7c2 ("x86/MSI: Use hierarchical irqdomains to manage MSI interrupts"), but it's not the root cause, it just triggers a bug caused by b5dc8e6c21e7 ("x86/irq: Use hierarchical irqdomain to manage CPU interrupt vectors"). With this patch applied, the affected SATA controllers work as expected. Signed-off-by: Jiang Liu Reported-by: Alex Deucher Reported-by: Mark Rustad Reported-by: Alexander Holler --- Hi Alex, Mark and Alexandler, Sorry for the long delay to root cause this regression, it's really annoying. Could you please help test this patch against the latest v4.2-rcx? Works. Thanks. Tested-by: 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.1 099/267] phy: twl4030-usb: remove incorrect pm_runtime_get_sync() in probe function.
Hello, this patch killed the musb-host functionality on my classic Beagleboard (rev c4). Symptom was that it there was a message I don't remember and the attached device didn't enumerate anymore (likely because of missing power, but I'm not sure). A simple revert has fixed it, I haven't looked further into the problem. Regards, Alexander Holler Am 31.07.2015 um 21:39 schrieb Greg Kroah-Hartman: 4.1-stable review patch. If anyone has any objections, please let me know. -- From: NeilBrown commit 4724e27114c4a7ec07db227a17fcab6f165c upstream. The USB phy should initialize with power-off, and will be powered on by the USB system when a cable connection is detected. Having this pm_runtime_get_sync() during probe causes the phy to *always* be powered on. Removing it returns to sensible power management. Fixes: 96be39ab34b77c6f6f5cd6ae03aac6c6449ee5c4 Signed-off-by: NeilBrown Signed-off-by: Kishon Vijay Abraham I Signed-off-by: Greg Kroah-Hartman --- drivers/phy/phy-twl4030-usb.c |1 - 1 file changed, 1 deletion(-) --- a/drivers/phy/phy-twl4030-usb.c +++ b/drivers/phy/phy-twl4030-usb.c @@ -711,7 +711,6 @@ static int twl4030_usb_probe(struct plat pm_runtime_use_autosuspend(&pdev->dev); pm_runtime_set_autosuspend_delay(&pdev->dev, 2000); pm_runtime_enable(&pdev->dev); - pm_runtime_get_sync(&pdev->dev); /* Our job is to use irqs and status from the power module * to keep the transceiver disabled when nothing's connected. -- 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/ -- 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: ATA failure regression in kernel 4.2
Am 31.07.2015 um 12:42 schrieb Jiang Liu: On 2015/7/31 18:38, Alexander Holler wrote: Am 28.07.2015 um 20:37 schrieb Alexander Holler: Same problem here with the AMD SATA controller (1022:7801). It failed to identify the second disk when using 4.2-rc4. nointremap helped, nothing else tested A quick look at the change history and two tests are suggesting that the right one already is in cc. commit f7fa7ae x86/irq: Avoid memory allocation in __assign_irq_vector() fails while af87bae x86/htirq: Use new irqdomain interfaces to allocate/free IRQ works. Inbetween almost all changes are from Jiang Liu. Haven't bisected it up to now but might do it later. Hi Alexander, Really thanks for you help. I have debugged this regression for a whole day without any progress:( Everything seems correct to me, and I have no hardware to reproduce and debug it. Still need more time:( > Thanks! > Gerry > There's nothing to excuse. It wasn't a flame, just a determination. I'm sorry if it might have sounded like a flame. Only people which are doing nothing don't make failures, and doing nothing often is a failure itself. But I'm drifting aways. I've now bisected those commits I've had already identified by other means down to the commit 52f518a3a7c2f80551a38d38be28bc9f335e713c "x86/MSI: Use hierarchical irqdomains to manage MSI interrupts". That is the one where 4.2-rc* fails here first. 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: ATA failure regression in kernel 4.2
Am 28.07.2015 um 20:37 schrieb Alexander Holler: Same problem here with the AMD SATA controller (1022:7801). It failed to identify the second disk when using 4.2-rc4. nointremap helped, nothing else tested A quick look at the change history and two tests are suggesting that the right one already is in cc. commit f7fa7ae x86/irq: Avoid memory allocation in __assign_irq_vector() fails while af87bae x86/htirq: Use new irqdomain interfaces to allocate/free IRQ works. Inbetween almost all changes are from Jiang Liu. Haven't bisected it up to now but might do it later. 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: ATA failure regression in kernel 4.2
Am 28.07.2015 um 20:19 schrieb Alex Deucher: On Mon, Jul 27, 2015 at 12:30 PM, Jiang Liu wrote: On 2015/7/27 23:21, Alex Deucher wrote: On Sun, Jul 26, 2015 at 11:01 PM, Jiang Liu wrote: On 2015/7/25 1:38, Alex Deucher wrote: On Thu, Jul 23, 2015 at 2:44 PM, Alex Deucher wrote: On Thu, Jul 23, 2015 at 2:35 PM, Tejun Heo wrote: Hello, On Thu, Jul 23, 2015 at 01:48:24PM -0400, Alex Deucher wrote: Something new in kernel 4.2 seems to have broken one of my hard drives (ssd) in kernel 4.2. 4.1 and older kernels work fine. Here are the relevant logs. ... [6.547628] ata2.00: qc timeout (cmd 0xec) [6.547721] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4) [7.007213] ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300) [ 16.997819] ata2.00: qc timeout (cmd 0xec) [ 16.997910] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4) [ 16.997995] ata2: limiting SATA link speed to 3.0 Gbps [ 17.457400] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320) [ 47.429257] ata2.00: qc timeout (cmd 0xec) [ 47.429349] ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4) [ 47.22] ata2: SATA link up 3.0 Gbps (SStatus 123 SControl 320) Nothing really rings a bell. Timeouts on IDENTIFY. Could be IRQ related. Which controller is it (lspci -nn)? Also, can you try to bisect the issue? 00:11.0 SATA controller [0106]: Advanced Micro Devices, Inc. [AMD] FCH SATA Controller [AHCI mode] [1022:7801] (rev 40) 00:14.1 IDE interface [0101]: Advanced Micro Devices, Inc. [AMD] FCH IDE Controller [1022:780c] I can take a look at bisecting later this week. You were right about the interrupts. This is an AMD Kaveri APU system. Hi Alex, Could you please help to provide more information about the system so we could identify the issue? Dmesg and /proc/interrupts from good and bad kernels are welcomed. Thanks! Gerry See attached. Thanks! Hi Alex, Thanks for the info. Seems something is wrong with multiple-MSI support. To narrow down the scope, could you please help to: I'm also not getting interrupts in my gpu driver. I haven't bisected this specifically, but I suspect it is related since it to used to work in 4.1. Whether I enable MSIs or not in my driver, I get a huge numbers of interrupts on all CPUs as soon as the driver is loaded, but the driver isr never gets called. E.g., 49: 117757835 117763227 117787837 117868913 PCI-MSI 524288-edge amdgpu nointremap doesn't seem to help. Same problem here with the AMD SATA controller (1022:7801). It failed to identify the second disk when using 4.2-rc4. nointremap helped, nothing else tested 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] modsign: provide option to automatically delete the key after modules were installed
Hello, besides that calling rm on Linux is just snake oil, the patch below still automatically hides the key used to sign modules and offers a statistically good chance that the key sometimes might physically disappear from the storage used to compile a kernel. So many people still might consider it useful. As mentioned below, at least one well known person seems to might find it useful too. Should I rewrite the commit message (e.g. to nothing as the subject already describes it perfectly) or is the chance to get such stuff from someone outside the holy circles included already zero? Sorry for asking (that way), but I'm curious if there is really zero interest in it. Regards, Alexander Holler Am 23.01.2015 um 02:20 schrieb Alexander Holler: I usually throw away (delete) the key used to sign modules after having called make -jN (b)zImage modules && make -jN modules_install. Because I've got bored to always have to type rm signing_key.* afterwards, I've build this patch some time ago. As I'm not eager anymore to publish kernel patches, it rested in my private chest of patches until I've seen the keynote of Linux.conf.au 2015. It made me aware that this patch might have a chance to become included. ;) Signed-off-by: Alexander Holler --- Makefile | 7 +++ init/Kconfig | 12 2 files changed, 19 insertions(+) diff --git a/Makefile b/Makefile index fb93350..95e07ca 100644 --- a/Makefile +++ b/Makefile @@ -1129,6 +1129,13 @@ _modinst_: @cp -f $(objtree)/modules.order $(MODLIB)/ @cp -f $(objtree)/modules.builtin $(MODLIB)/ $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst +ifeq ($(CONFIG_MODULE_SIG_THROW_AWAY), y) + @echo "###" + @echo "### Deleting key used to sign modules." + @echo "###" + @rm ./signing_key.priv + @rm ./signing_key.x509 +endif # This depmod is only for convenience to give the initial # boot a modules.dep even before / is mounted read-write. However the diff --git a/init/Kconfig b/init/Kconfig index 9afb971..f29304e 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -1884,6 +1884,18 @@ config MODULE_SIG_ALL Sign all modules during make modules_install. Without this option, modules must be signed manually, using the scripts/sign-file tool. +config MODULE_SIG_THROW_AWAY + bool "Automatically delete the key after modules were installed" + default n + depends on MODULE_SIG_ALL + help + Delete the key used to sign modules after modules were installed. + Be aware of the consequences. The downside is that you won't be + able to build any module for a (maybe running) kernel, but will + have to rebuild the kernel and all modules in order to add or modify + a module. The upside is that you don't have to secure the key in + order to keep a running kernel safe from unwanted modules. + comment "Do not forget to sign required modules with scripts/sign-file" depends on MODULE_SIG_FORCE && !MODULE_SIG_ALL -- 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 00/13] Discover and probe dependencies
Am 18.06.2015 um 16:49 schrieb Russell King - ARM Linux: I think you may need to pick a better example to illustrate your point. :) It's easy to describe: Sort driver probe calls and forget about the whole device-stuff. The real target is to sort the driver probe calls and ideally the whole device stuff could be ignored, leaving it as however it may work. Unfortunately, that's not as easy to implement as it sounds. ;) I've too hit the device-stuff very late in my driver-probe-ordering experiment and just did it somehow, because I already was at the limit of the time I wanted to spend. ;) So to repeat myself, I think the first target has to be to identify drivers either without having to call their initcalls at all, or by making sure their initcalls just register and do nothing else (what I've called "well-done"). 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/