[lkml]   [2008]   [Apr]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [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

    > 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

    > 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
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2008-04-01 10:39    [W:0.035 / U:5.004 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site