Messages in this thread | | | Date | Thu, 6 Aug 2020 10:09:41 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH v4 00/14] liblockdep fixes for 5.9-rc1 |
| |
* Sasha Levin <sashal@kernel.org> wrote:
> Hi Linus, > > Please consider applying these patches for liblockdep, or alternatively > pull from: > > git://git.kernel.org/pub/scm/linux/kernel/git/sashal/linux.git tags/liblockdep-fixes-040820 > > The patches fix up compilation and functionality of liblockdep on 5.8, > they were tested using liblockdep's internal testsuite. > > I was unable to get the x86 folks to pull these fixes for the past few > months:
So the primary reason I didn't pull is that liblockdep was permanently build-broken from February 2019 to around February 2020, despite me pinging you multiple times about it.
> - https://lkml.org/lkml/2020/2/17/1089
This pull request still said that if fixes "most of" liblockdep, not "all of", which is the benchmark really after such a long series of breakage.
> - https://lkml.org/lkml/2020/4/18/817
This still said "most of".
> - https://lkml.org/lkml/2020/6/22/1262
Same 'most of' verbiage.
> Which is why this pull request ends up going straight to you.
So at this point I think we need to ask whether it's worth it: are there any actual users of liblockdep, besides the testcases in liblockdep itself? I see there's a 'liblockdep-dev' package for Debian, but not propagated to Ubuntu or other popular variants AFAICS.
Also, could you please specify whether all bugs are fixed or just 'most'?
> Sasha Levin (14): > tools headers: Add kprobes.h header > tools headers: Add rcupdate.h header > tools/kernel.h: extend with dummy RCU functions > tools bitmap: add bitmap_andnot definition > tools/lib/lockdep: add definition required for IRQ flag tracing > tools bitmap: add bitmap_clear definition > tools/lib/lockdep: Hook up vsprintf, find_bit, hweight libraries > tools/lib/lockdep: Enable building with CONFIG_TRACE_IRQFLAGS > tools/lib/lockdep: New stacktrace API > tools/lib/lockdep: call lockdep_init_task on init > tools/lib/lockdep: switch to using lockdep_init_map_waits > tools/kernel.h: hide noinstr > tools/lib/lockdep: explicitly declare lockdep_init_task() > tools/kernel.h: hide task_struct.hardirq_chain_key
Style nits, please use consistent titles for patches:
- First word should be capitalized consistently, instead of mismash of lower case mixed with upper case.
- First word should preferably be a verb, i.e. "Add new stacktrace API stubs", not "New stacktrace API"
Also, please always check linux-next whether there's some new upstream changes that liblockdep needs to adapt to. Right now there's a new build breakage even with all your fixes applied:
thule:~/tip/tools/lib/lockdep> make CC common.o In file included from ../../include/linux/lockdep.h:24, from common.c:5: ../../include/linux/../../../include/linux/lockdep.h:13:10: fatal error: linux/lockdep_types.h: No such file or directory 13 | #include <linux/lockdep_types.h> | ^~~~~~~~~~~~~~~~~~~~~~~
At which point we need to step back and analyze the development model: this comparatively high rate of breakage derives from the unorthodox direct coupling of a kernel subsystem to a user-space library.
The solution for that would be to use the method how perf syncs to kernel space headers, by maintaining a 100% copy in tools/include/ and having automatic mechanism that warns about out of sync headers but doesn't break functionality.
See tools/perf/check-headers.sh for details.
I believe this same half-automated sync-on-upstream-changes model could be used for liblockdep as well, i.e. lets copy kernel/lockdep.c and lockdep*.h over to tools/lib/lockdep/, and reuse the perf header syncing method to keep it synchronized from that point on.
That would result in a far more maintainable liblockdep end result IMO?
Thanks,
Ingo
| |