Re: [PATCH] Btrfs: add support for asserts

[Jeff Mahoney <jeffm@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 1856 bytes --]

On 8/27/13 4:56 PM, Josef Bacik wrote:
> On Tue, Aug 27, 2013 at 03:28:24PM -0400, Jeff Mahoney wrote:
>> On 8/26/13 4:56 PM, Josef Bacik wrote:
>>> One of the complaints we get a lot is how many BUG_ON()'s we have.  So to help
>>> with this I'm introducing a kconfig option to enable/disable a new ASSERT()
>>> mechanism much like what XFS does.  This will allow us developers to still get
>>> our nice panics but allow users/distros to compile them out.  With this we can
>>> go through and convert any BUG_ON()'s that we have to catch actual programming
>>> mistakes to the new ASSERT() and then fix everybody else to return errors.  This
>>> will also allow developers to leave sanity checks in their new code to make sure
>>> we don't trip over problems while testing stuff and vetting new features.
>>> Thanks,
>>
>> I don't think the complaint is so much about the number of BUG_ONs, but
>> that there's no distinction between something that is supposed to be
>> impossible and something that is improbable. The BUG_ONs to keep code
>> correctness are good and are littered all over the kernel with positive
>> results. The BUG_ONs that are there in place of real error handling
>> served their purpose and need to be replaced.
>>
>> So, I don't know if it's a net win to compile the "good" BUG_ONs out of
>> the code. Especially if a user runs into something strange yet familiar
>> and the first response is "oh, huh, can you rebuild with asserts enabled?"
>>
> 
> Either I provide an option for it or distros do it themselves, this cuts out the
> middle man.  I'd really rather they just be on all the time since they aren't
> things we should hit anyway, but at least this way people have a choice.

Ok. With my distro hat on, I can tell you I'll be leaving them on. :)

-Jeff


-- 
Jeff Mahoney
SUSE Labs


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 841 bytes --]