Re: checkpatch, a patch checking script.

From: Andi Kleen
Date: Sat Apr 28 2007 - 05:54:43 EST


Roland Dreier <rdreier@xxxxxxxxx> writes:

> > Checking patches/git-infiniband.patch: signoffs = 113
> > Use WARN_ON & Recovery code rather than BUG() and BUG_ON()
> > 8143:+ BUG_ON(mlx4_ib_alloc_db_from_pgdir(pgdir, db, order));
> > 12629:+ BUG_ON(cmd->free_head < 0);
> > 16580:+ BUG_ON(index < dev->caps.num_mgms);
> > 16665:+ BUG_ON(amgm_index_to_free < dev->caps.num_mgms);
> > 16681:+ BUG_ON(index < dev->caps.num_mgms);
>
> I agree -- killing the kernel for a driver bug is dump. I'll remove
> all these BUGs before merging.

So you prefer to corrupt data instead? I don't think that's a good idea.
Don't do it. Silent failures are really bad.

BUG_ONs are only fatal when you're in interrupt context anyways because
it often ends up trying to kill the idle task. In process context
while there might be some lost locks in most cases the system continues
running happily.

At some point I had a hack to just restart idle to handle the
interrupt case too. Perhaps that might be worth resurrecting if you
want to solve this properly.

Would be imho far preferable over not having the BUGs imho.

-Andi
-
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/