login
Search
x86.git testing found the following build failure in -git: ERROR: "led_classdev_register" [drivers/input/joystick/xpad.ko] undefined! ERROR: "led_classdev_unregister" [drivers/input/joystick/xpad.ko] undefined! which triggers with the following config: http://redhat.com/~mingo/misc/config-Wed_Apr_30_21_43_17_CEST_2008.bad the reason is dependency on NEW_LEDS that was not spelled out in the Kconfig entry of JOYSTICK_XPAD. Signed-off-by: Ingo Molnar <mingo@elte.hu> --- drivers/input/joystick/Kconfig | 1 + 1 file changed, 1 insertion(+) Index: linux/drivers/input/joystick/Kconfig =================================================================== --- linux.orig/drivers/input/joystick/Kconfig +++ linux/drivers/input/joystick/Kconfig @@ -268,6 +268,7 @@ config JOYSTICK_JOYDUMP config JOYSTICK_XPAD tristate "X-Box gamepad support" depends on USB_ARCH_HAS_HCD + depends on NEW_LEDS select USB help Say Y here if you want to use the X-Box pad with your computer. --
Xpad can be compiled without LED support so this dependancy is incorrect. JOYSTICK_XPAD_LEDS has proper dependancy on LEDS_CLASS, the rest is Kconfig breakage. -- Dmitry --
no, you are wrong, read the current Kconfig rules again. If the user can create a .config that does not build, it is driver breakage. It always was, and has been in the past 15 years. Kconfig might be extended to make dependencies easier to manage for developers but until that is implemented you have to craft your driver's dependencies with the current tools in a way that doesnt break the build. Ingo --
Ingo, there are areas in the kernel you know well.
But kconfig is not among them.
This breakage with your .config is *not* caused by an input bug as you
wrongly claim (I'll send a correct fix after some testing).
And the commit that broke it even contains a
Signed-off-by: Ingo Molnar <mingo@elte.hu>
I would really appreciate it if you could send the error message and the
.config but not quick kconfig patches that are often wrong and that you
try to push through the maintainers as you tried here.
Toralf, who does randconfig testing for some years, does exactly this,
and the maintainers and/or I usually pick up his bug reports and work on
Thanks in advance
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
--
I have no opinion about where the bug is (input or leds or elsewhere) - my only opinion is that the kernel must not stay build-broken - and the discussion with Dmitry was about that. I'm not interested in trivial patches as those issues are much more efficiently handled by the person who maintains that code (i.e. Dmitry) and who intimately knows the dependencies and expectations of that code. That's why i sent the bugreport and patch to Dmitry. Ingo --
Why is Dmitry responsible for a bug introduced by a commit *you*
Signed-off in a subsystem that lists *you* as the maintainer?
The bug is in arch/x86/Kconfig .
Caused by commit 4cf31841762954ad2868156ccba94d798a16630f
(x86: mach-rdc321x Kconfig fix).
That Dmitrys code broke was just a side effect of your bug.
Roman's patch to remove the need to select NEW_LEDS that just appeared
in another thread will actually also fix your bug (and makes my idea
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
--
ah, indeed - Florian Cc:-ed. Preliminary quick hack below. Ingo ---------------> Subject: rdc: leds hack From: Ingo Molnar <mingo@elte.hu> Date: Thu May 01 03:46:22 CEST 2008 Signed-off-by: Ingo Molnar <mingo@elte.hu> --- arch/x86/Kconfig | 1 + 1 file changed, 1 insertion(+) Index: linux/arch/x86/Kconfig =================================================================== --- linux.orig/arch/x86/Kconfig +++ linux/arch/x86/Kconfig @@ -322,6 +322,7 @@ config X86_RDC321X select GENERIC_GPIO select LEDS_CLASS select LEDS_GPIO + select NEW_LEDS help This option is needed for RDC R-321x system-on-chip, also known as R-8610-(G). --
Why couldn't you *read* my email completely before sending yet another
kconfig patch?
I said:
Roman's patch to remove the need to select NEW_LEDS that just appeared
in another thread will actually also fix your bug (and makes my idea
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
--
hey, sorry about invading your turf of trivial patches ;) I dont see it as a problem that the thought process and the initial patch is incomplete and ad-hoc. My preference is to work with people out in the open, even on trivial issues. Dmitry is a capable maintainer who understands his code very well and he'll resist me if i'm full of it. Just like i resisted you when you were full of it. That's what maintainers do, their job is to know their code. And, occasionally, as in this case, i might end up being faced with a bug in the code i maintain ;) Ingo --
You completely miss my point.
You wrongly (and loudly) blamed Dmitry for something you broke yourself.
And if I hadn't stopped you pointing to what actually was broken you
might have annoyed Dmitry until he'd have merged your wrong patch.
I don't claim any knowledge about scheduler or x86 internals, but I
claim that I might be the person with the second-best understanding
of our kconfig files.
And I'd really prefer being able to take some time searching for a
correct solution over having to quickly defeat the mess you create
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
--
i didnt. Read what i wrote: || no, you are wrong, read the current Kconfig rules again. If the user || can create a .config that does not build, it is driver breakage. It || always was, and has been in the past 15 years. || || Kconfig might be extended to make dependencies easier to manage for || developers but until that is implemented you have to craft your || driver's dependencies with the current tools in a way that doesnt || break the build. and that's exactly what happens with Roman's patch: a Kconfig subsystem design bug (its inability to properly propagate the dependencies of select's) is worked around in the driver space: by the LEDS_CORE driver config introduction and no user-visible. Roman's patch is obviously cleaner than my hack (i just fixed a single instantiation of the problem, while he changed the LEDS driver dependency structure), but it's still a workaround for a Kconfig subsystem bug and the same problem could reoccur elsewhere. It could hit anytime dual dependencies are introduced in a driver accidentally. As Sam said it, fixing that Kconfig design bug would be "nice" - but unfortunately the Kconfig subsystem is not actively developed anymore. Would you like to volunteer for that? It would be a _very_ useful contribution. One such fix could avoid hundreds or even thousands of trivial problems in the future - it could avoid having to make hundreds or thousands of trivial patches in the future. Ingo --
Ingo, perhaps it helps if I put it in caps:
YOU TRIED TO PUSH AN INPUT PATCH FOR AN X86 BUG.
Roman's patch is better than adding a select, but your patch would have
added the select in the completely wrong subsystem.
Different to you I actually have some basic understanding how kconfig
works.
I'm not even sure the semantics of "select follows dependencies"
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
--
Adrian, your tone is getting more and more abusive, and while i can easily ignore your abuse you are not just doing it towards me but towards other kernel developers as well. You need to stop that. Yes, the incomplete (and buggy) select came from an x86 Kconfig file but you missed the real argument. Half a dozen other times similar bugs came from other portions of the tree so it's a reoccuring pattern of bugs. And had you read the exchange more carefully you'd notice that the discussion was about the Kconfig subsystem bug which is causing all these other bugs - which Kconfig subsystem bug is still unfixed. In the discussion Dmitry assumed the obvious: that a select on a component will also select the sub-components. The problem is - select does not do sure, and that's exactly what i said above: "Roman's patch is obviously cleaner than my hack". It avoids this problem by creating a single target to select for. A wrong/buggy select _somewhere_ (this time the bug indeed was in an x86 subarch Kconfig - but often it was just in a driver that tried to enable LEDS support) breaks the build - instead of propagating dependencies or at least warning about the problem. That's a Kconfig subsystem design bug and it has been known for years. it's now worked around by Roman's patch by making the LEDS Kconfig structure simpler so there's just a single select target. But the root cause was not fixed and similar issues could hit the kernel anytime in the future. So it's not a real fix - it just prolonges the real fix some how many times do i have to repeat it that yes it was a hack and that it great, does this mean we'll see fixes for select's misbehavior, along the lines of Sam's suggestions? at minimum a warning needs to be emitted by the kconfig tool if such incomplete selects are used. I've stopped counting the number of times such issues have broken the build and have held up kernel development. All the information is already in the Kconfig files ...
In the case of the problem here it would have turned one problem into
another, and Roman's patch is the correct solution no matter whether
It might held up your randconfig compiles.
Actual kernel development isn't much affected.
Before you started your randconfig builds and sending (often buggy)
kconfig patches like crazy there wasn't a problem (and Toralf's
That's nonsense.
Describe the semantics you want for "bool selects tristate", and no
matter which you choose I'll tell you a case where it breaks.
You've shown with another of your recent kconfig patches that you don't
even understand how "depends on" works, and you are _very_ far from
making claims like the one you just made.
Please accept the fact that while I'm (at least at the moment) not
qualified to make any deeply technical remarks on e.g. the CPU scheduler
or x86 details, you are (at least at the moment) not qualified to make
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
--
Hi Adrian It would be nice to know what was wrong with my suggestion. You have done your share of kconfig patches so you have a good grip on the problems we face. So any input on how we can improve kconfig so we can actually provide what people often expects or requests or need would be nice. I have so far not done any hacking on the core of the backend of kconfig but if I one day find more than one hour in row where I can do some kernel stuff I may actually try it out. And then it would be nice to have a sketch ready how to solve the issues. I would though request you to start a new mail thread for this and include both linux-kbuild@vger.kernel.org and Roman Zippel in the loop. Sam --
If you've sent a suggestion that included a description of the exact
semantics you want to implement I must have missed it.
Can you send me a pointer or a description of the semantics?
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
--
I took the original idea and added a few more details. Please see mail named "http://lkml.org/lkml/2008/5/4/18" http://lkml.org/lkml/2008/5/4/18 Sam --
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
--
uhm, you are quite wrong - countless times have people been bitten by select's breakages in the past, and not via randconfig. That's the main reason why select use in Kconfig was not encouraged for a long time. Select does make sense in some situations but it's a double-edged sword: kconfig does not warn at all about the situations where it's "unsafe" to use it - while it has all the information in the Kconfig files to emit that warning. Instead we get build breakages not visible when an incorrect select is added, but much later, if someone happens to stumble on the wrong kind of .config. That is obviously harmful. My larger point is that this kconfig tool bug breeds a constant stream of avoidable breakages, which causes lost manpower and causes a stream of trivial patches hindering maintainers all around the tree. Because every such trivial patch has to be reviewed, tested, it clogs the commit logs, etc. So the more trivial patches we _avoid_ having to do in the future, the better. I'm not sure why you are even arguing against this this rather simple point - your arguments are rather hard to understand. Wouldnt you be happier if a whole category of trivial breakages was avoided and if you didnt have to deal with and waste your time on that category of trivial patches anymore? Most of the time reoccuring trivial patches are an indicator of some deeper structural problem - as in this case. Ingo --
But it has not "held up kernel development".
Send a bug report to linux-kernel or with a Cc to me and I'll look at
the bug.
And how would a warning help?
From an UI perspective we often want to select options that have
Your conclusions are based on an assumption that isn't true.
"trivial patches" are the patches you send.
But they are often bogus.
Fixing these issues properly often requires a deeper understanding of
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
--
I suspect that Ingo is however correct - although a *proper* fix of one of these bugs requires human-intelligence to figure out what's *really* intended, the kconfig program *does* have enough information available to issue a a clear warning: "Yo doodz - I don't know *what* you intended here, but this SELECT is just waiting to sink its teeth into somebody's posterior. You might want to fix it somehow before somebody needs rabies shots..." There's a long tradition of toolchains issuing warnings about things that a toolchain can detect, but cannot correct - for instance, the gcc 'variable is used uninitialized' warning (distinct from the broken "may be used" warning). Given the truly insane amounts of code we merge every cycle, we should take advantage of every opportunity for automated detection of things that can't possibly be right. (Ingo - flame me if I've misread your position here...)
Ingo claims the problem was trivial since the patches were trivial.
But fact is they aren't trivial - as you can e.g. see on Ingos patch
And what do you want to do in such a case?
Kconfig is a user interface, and we actually need such cases you want to
warn for for getting a good UI.
We already know what can cause problems.
But as far as I know there are no such problems users actually ran into
in recent stable kernels - and most of the problems (like the one in
this thread) are pathological cornercases you only see with randconfig.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
--
Interesting. You confidently define how kernel development works and what affects it and what not. I just caught a build wreckage via randconfig builds on a patch which was sent through Andrew to x86 for merging into mainline. So you think that we should stop sharing with the wider kernel community the fixes that result out of our automated randconfig builds and should instead wait for Joe User or another kernel developer to trip into some subtle missing Kconfig dependency, just because a fix might not be the right one and might need another iteration before it can go upstream? You are wrong as usual. I for one prefer getting a (even wrong) patch which points to the FYI, Ingo has been doing randconfig build and boot tests for a long, long time. There are dozens and dozens of (non-build) fixes in the upstream kernel that resulted from that effort - it's really valuable in practice. So what you say is complete nonsense. The RDC subarch commit was buggy, but that does not justify your insulting and self-righteous behaviour at all. If you think that your behaviour vs. Ingo is improving kernel development, then you are on the completely wrong track. It might earn you an entry in his killfile, but nothing else. Thanks, tglx --
Ingo' message that caused me to change the subject of this thread was http://lkml.org/lkml/2008/4/30/446 Ingo's patch did not point at the actual problem at all (it was a x86 bug and Ingo's patch was for input). But he tried to push his bogus patch through Dmitry who had correctly stated that the patch was wrong. This way of "sharing fixes" even against a correct maintainer NAK is what annoys me. Am I really wrong as usual on that? And most issues randconfig finds are some cornercases unlikely to happen for users (hey, this thread is about a bug only occuring in configurations that include support for an X-Box gamepad in a kernel for an RDC R-321x SoC), so there's usually no need to hurry but time to look Since when is he actually doing regular randconfig builds? The only person from whom I'm aware of regularly seeing reports of build When talking about "self-righteous behaviour" please also talk about how Ingo rants about kconfig and makes statements like "All the information is already in the Kconfig files for the kconfig tool/subsystem to make cu Adrian [1] http://lkml.org/lkml/2008/5/3/215 -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed --
I think it would. But how would that actually work? By displaying all relevant dependency chains and asking the user to select one? -- Krzysztof Halasa --
Let's look at the problem this thread is about:
menuconfig NEW_LEDS
bool "LED Support"
config LEDS_CLASS
tristate "LED Class Support"
depends on NEW_LEDS # actually an "if", but that's
just syntactical sugar
config X86_RDC321X
bool "RDC R-321x SoC"
select LEDS_CLASS
If you select LEDS_CLASS "select follows dependencies" would let inherit
X86_RDC321X the dependencies of LEDS_CLASS, IOW treat it as:
config X86_RDC321X
bool "RDC R-321x SoC"
select LEDS_CLASS
depends on NEW_LEDS
That might make the randconfig crowd happy.
But from an UI perspective it's not an improvement.
And regarding "displaying all relevant dependency chains" to the user -
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
--
I see, "follows" the other way around. That fixes the breakage but I guess it's not exactly what the people doing make menuconfig want. I fear it too but perhaps there is some sane way? I don't know, asking (recursively?) interactively? Like: QWERTY requires at least one, select (Y/M): - ARCH_X86 && NET_ETHERNET, or - USB4_0 && !SMP, or - XXX then: NET_ETHERNET requires at lest one, select (Y/M): - PCI128 - ISA256 then (info): PCI128 requires PCI and it's selected automatically. I don't know. It could be useful even with a single dependency when configuring as the module, the user could select Y or M for dependency interactively (people may want to have E1000E=m and TULIP=m but MII=y). Not sure if still on the same planet, though :-) -- Krzysztof Halasa --
Kconfig is an interface for users.
From a developer perspective if might be the easiest to simply remove
FB_SGIVW requires X86_VISWS and it's selected automatically.
Why do we have to bother users with the MII option at all?
"E1000E=m and TULIP=m but MII=y" works, but it doesn't really make
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
--
Not sure what do you mean, it currently seem to "depend" on X86_VISWS:
config FB_SGIVW
tristate "SGI Visual Workstation framebuffer support"
depends on FB && X86_VISWS
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
(Obviously it assumes both TULIP and E1000E required MII which is not
exactly the case)
But it makes a perfect sense, I can have modular drivers for (few)
hardware devices (just in case I want to rmmod etc) but most of the
kernel may be not modular. It would be nice if the Kconfig ask me if
I want to "select" the dependency Y or M, even if there is only one
way to make "select" dependencies happy (not counting "Y vs "M").
--
Krzysztof Halasa
--
Unless I misunderstand your proposal you want to resolve dependencies
reversely.
And if you want to only do this for select's: FB_SGIVW could also be
Getting the cases that really matter working would already be hard
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
--
Right. Sure, if something selects FB_SGIVW then the user would be presented a list of dependencies and would have to resolve them to be allowed to continue. -- Krzysztof Halasa --
