On Mon, Mar 01, 2021 at 06:20:29PM +0200, Nikolay Borisov wrote:

On 1.03.21 г. 18:15 ч., David Sterba wrote:

quoted

On Mon, Feb 22, 2021 at 06:40:47PM +0200, Nikolay Borisov wrote:

quoted

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.