Re: [PATCH] btrfs: remove BUG_ON used as assertions

[Josef Bacik <josef@toxicpanda.com>]

On 12/15/19 12:12 PM, Aditya Pakki wrote:
> alloc_extent_state_atomic() allocates extents via GFP_ATOMIC flag
> and cannot fail. There are multiple invocations of BUG_ON on the
> return value to check for failure. The patch replaces certain
> invocations of BUG_ON by returning the error upstream.
> 
> Signed-off-by: Aditya Pakki <pakki001@umn.edu>

I already tried this a few months ago and gave up.  There are a few things if 
you want to tackle something like this

1) use bpf's error injection thing to make sure you handle every path that can 
error out.  This is the script I wrote to do just that

https://github.com/josefbacik/debug-scripts/blob/master/error-injection-stress.py

2) We actually can't fail here.  We would need to go back and make _all_ callers 
of lock_extent_bits() handle the allocation error.  This is theoretically 
possible, but a giant pain in the ass.  In general we can make allocations here 
and we need to be able to make them.

3) We should probably mark this path with __GFP_NOFAIL because again, this is 
locking and we need locking to succeed.

Thanks,

Josef