Re: [PATCH 6/6] btrfs: Simplify code flow in btrfs_delayed_inode_reserve_metadata

[David Sterba <dsterba@suse.cz>]

On Mon, Mar 01, 2021 at 06:20:29PM +0200, Nikolay Borisov wrote:
> 
> 
> On 1.03.21 г. 18:15 ч., David Sterba wrote:
> > On Mon, Feb 22, 2021 at 06:40:47PM +0200, Nikolay Borisov wrote:
> >> btrfs_block_rsv_add can return only ENOSPC since it's called with
> >> NO_FLUSH modifier. This so simplify the logic in
> >> btrfs_delayed_inode_reserve_metadata to exploit this invariant.
> > 
> > This seems quite fragile, it's not straightforward to see from the
> > context that the NO_FLUSH code will always return ENOSPC. I followed a
> > few calls down from btrfs_block_rsv_add and it's well hidden inside
> > __reserve_bytes. So in case it's an invariant I'd rather add an
> > assertion, ie. ASSERT(ret == 0 || ret == -ENOSPC) so at least we know
> > when this gets broken. Otherwise looks ok.
> 
> Fair enough, I'm fine with it. In any case we no longer return eagain
> when reserving. either we succeed or we return ENOSPC - either because
> we don't have space and we can't flush or because even after the
> flushing machinery did its work a ticket still couldn't be satisfied, in
> which case we failed it hence ENOSPC got returned.

Yeah but this is a precaution when somebody reworks the flushing logic
yet another time and suddenly EAGAIN or whatever else can become the
return code again. Hunting such bugs can be quite difficult as it
depends on the runtime state and the space stat on disk etc.