Re: [RFC PATCH 25/26] UBIFS: add debugging stuff

[Pekka Enberg]

Hi Artem,

Artem Bityutskiy wrote:
> Well, I do not see a big reason not to get rid of this harmless stuff.
> Many kernel subsystems have their debugging, why not? Using BUG_ON() is
> OK in few most important places. But we want to have more assertions
> which are compiled-out by default, why can't we?. Similar is for prints.

Why would you want to have assertions that are compiled out by default? 
Either you handle the error or don't (and have an assertion). The reason 
some subsystems have had their own asserts is because they go overboard 
with defensive checks as they haven't bothered to think through a 
reasonable error handling strategy. The downside? It clutters the code 
and causes the (compiled out) assertions to bit-rot.

Note that they're also a total pain in the ass to enable for anyone not 
intimately familiar with your code. Not to mention you're now making the 
lives of those crazy embedded folks that disable CONFIG_BUG for smaller 
kernel size harder as well.

Do you know why we don't have compiled out asserts in the core kernel? 
That's because it simply can't just roll-over and die if something 
unexpected happens and your filesystem shouldn't probably do that 
either. Sure, if you have some debugging checks that are way too 
expensive for production use, you might want to have a 
CONFIG_UBIFS_DEBUG but that shouldn't happen at assertion level but 
rather at much higher level.

And btw, for optional printks, we have a lot of tracing infrastructure 
in the kernel already (kprobes, relayfs, ftrace probably soon), so if 
you want to have tracing for UBIFS (you probably don't), don't invent 
your mechanism. But for most printks, they're either useful or they're 
not. Again, I do see the potential need for CONFIG_UBIFS_DEBUG here, but 
doing that at printk-level is also too low-level.

			Pekka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/