Re: [PATCH v2 04/13] SoftiWarp object management

[Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1489 bytes --]

On 10/8/2017 8:28 AM, Leon Romanovsky wrote:
>> +	} else if (id == -ENOSPC && pre_id != 1) {
>> +		pre_id = 1;
>> +		goto again;
>> +	} else {
>> +		BUG_ON(id == 0);
> No BUG_ON in new code.
> 
>> +		dprint(DBG_OBJ|DBG_ON, "(OBJ??): IDR New Object failed!\n");
>> +	}
>> +	return id > 0 ? 0 : id;
>> +}

Not to mention that this segment displays why BUG_ON() is so bad.  You
have issued a BUG_ON() and then on the very next line you have a
dprint() as if there is anything else that happens after a BUG_ON().
There isn't.  By definition, with a BUG_ON(), you have killed the entire
machine.  You will never get to the dprint() line, it is completely
unreachable code.  Unless you run across something the likes of a
hardware device that is completely out of control, that is DMAing to
random address, and that you can't stop, BUG_ON is not an appropriate
solution.  You should only BUG_ON if continuing to run will render the
machine randomly corrupted and jeopardize writing that corruption to
disk and wiping the persistent system data out.  Short of that, you need
to find a way to gracefully handle the situation and continue.  It may
mean shutting your device down forcefully so that the rest of the
machine may live, but that's OK.  Shutting the entire machine down is not.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG Key ID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD


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