Messages in this thread | | | Date | Tue, 01 Apr 2008 11:32:58 +0300 | From | Artem Bityutskiy <> | Subject | Re: [RFC PATCH 25/26] UBIFS: add debugging stuff |
| |
Pekka, I still do not see why you are opposed to assertions so much :-)
Pekka Enberg wrote: > Why would you want to have assertions that are compiled out by default?
Because we want to have a way to catch bugs and to quickly fix them. This is why we injected many assertions all over the place. Enabling them by default is inefficient and makes the code larger, which is not good especially for small embedded systems.
If someone reports us an obscure oops, and we have no idea why it happened, and we cannot reproduce it on our setup, we ask the reporter to enable debugging and report us results. This helps us to figure out what was the reason and to quickly fix the bug. I do not see why you want to prevent us from doing this.
> Either you handle the error or don't (and have an assertion). We handle all errors. Errors are things like I/O failures, memory allocation failures, unexpected behavior. We do handle this. Assertion are about _debugging_, when you already know you have a problem.
Indeed, bugs may be tricky. An oops may happen because half an hour ago a function craped out something. Assertions allow us to catch problems on _early_ stage, instead of dealing with consequences and scratching the head what was the reason.
But I do agree we have too much of that. We will lessen the amount of course.
> 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.
I am not sure what you mean. I would not want to delve into a general discussion of the debugging stuff. I would better talk about specific things. I'll just point you examples of debugging stuff in the kernel in other subsystems which exists and does not hurt anyone. And I believe it is helpful. It is compiled out by default and is enable when it is needed to hunt a bug.
fs/ext2: ea_idebug(), EXT2FS_DEBUG fs/xfs: #ifdef DEBUG, XFS_LOUD_RECOVERY and so on fs/ocfs2: OCFS2_DEBUG_FS fs/jfs: CONFIG_JFS_DEBUG, assert(), etc fs: DEBUG_EPOLL, #ifdef DEBUG fs/jbd2: assert_spin_locked(), CONFIG_JBD2_DEBUG, etc mm: CONFIG_SLUB_DEBUG, SLABDEBUG, CONFIG_DEBUG_VM, and so on
> Note that they're also a total pain in the ass to enable for anyone not > intimately familiar with your code.
Of course. People who are not familiar with the code send bug reports and we have to fix the problem quickly, and debugging stuff helps.
> 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.
It is OK to have few BUG_ON() checks, and we should probably turn few assertions into BUG_ON(). But only few.
> 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.
If something unexpected happens, UBIFS will just return -EINVAL in the most cases, because one of the function will find out that something is going wrong. Assertions have nothing to do with this. The help to _fix_ bugs which were hit in certain circumstances.
> 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. We have heavy checks, right. They are expensive, so disabled by default. Why can't assertions be similar?
-- Best Regards, Artem Bityutskiy (Артём Битюцкий) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |