Re: checkpatch, a patch checking script.

Andi Kleen
2007-04-28T09:54:29Z

Roland Dreier <rdreier@cisco.com> 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