[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2f9d6549-47cf-fb71-3bff-50b51093e757@toxicpanda.com>
Date: Wed, 18 Dec 2019 11:38:18 -0500
From: Josef Bacik <josef@...icpanda.com>
To: Aditya Pakki <pakki001@....edu>
Cc: kjlu@....edu, Chris Mason <clm@...com>,
David Sterba <dsterba@...e.com>, linux-btrfs@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] btrfs: remove BUG_ON used as assertions
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@....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
Powered by blists - more mailing lists