Messages in this thread | | | Date | Tue, 15 Nov 2011 11:00:14 +1100 | From | Ryan Mallon <> | Subject | Re: [PATCH] net/packet: remove dead code and unneeded variable from prb_setup_retire_blk_timer() |
| |
On 15/11/11 10:48, Jesper Juhl wrote: > On Tue, 15 Nov 2011, Ryan Mallon wrote: > >> On 15/11/11 10:37, Jesper Juhl wrote: >> >>> On Mon, 14 Nov 2011, Ryan Mallon wrote: >>> >>>> On 14/11/11 08:55, Jesper Juhl wrote: >>>> >>>>> We test for 'tx_ring' being != zero and BUG() if that's the case. So after >>>>> that check there is no way that 'tx_ring' could be anything _but_ zero, so >>>>> testing it again is just dead code. Once that dead code is removed, the >>>>> 'pkc' local variable becomes entirely redundant, so remove that as well. >>>> >>>> What if CONFIG_BUG=n? >>>> >>> Arrgh, I didn't consider that (should have, but didn't).. In that case >>> we'll have >>> #define BUG() do {} while(0) >>> which is not going to work all that peachy with my change... >>> >>> David: You may want to pass on this one. I obviously didn't think it >>> through properly - sorry :-( >>> >> >> It's something I've never been entirely clear on. How are you meant to >> handle something which really is a fatal bug if CONFIG_BUG=n? >> >> I think there are several places in the kernel where the expectation is >> that BUG() causes a panic that probably don't behave nicely at all >> (though I guess that might be the expected behaviour) if CONFIG_BUG=n. >> > You have a point. Perhaps the proper solution would be to remove > CONFIG_BUG so that it is always enabled...? As you say, if something is a > fatal bug there is no sane way to continue, so why do we even allow > people/systems to try??? > That way lies madness it would seem... > The Kconfig looks like this:
config BUG bool "BUG() support" if EXPERT default y help Disabling this option eliminates support for BUG and WARN, reducing the size of your kernel image and potentially quietly ignoring numerous fatal conditions. You should only consider disabling this option for embedded systems with no facilities for reporting errors. Just say Y.
So you can only disable it if you have CONFIG_EXPERT (read embedded I think). It says that you should only disable it for embedded systems where you have no error reporting facilities, but you probably still want the system to hang/die though since you might have a watchdog which can reset the machine if it hangs. With CONFIG_BUG=n you could potentially hit a BUG() and manage to carry on for sometime even though the system is now unstable. Maybe BUG() should at least drop in to an infinite loop in the case of CONFIG_BUG=n. I might try and come up with a patch later tonight.
~Ryan
| |