Re: [PATCH 4/4] btrfs: update the number of bytes used by an inode atomically

[Nikolay Borisov <nborisov@suse.com>]

On 4.11.20 г. 13:07 ч., fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 

<snip>

> So fix this by:
> 
> 1) Making btrfs_drop_extents() not decrement the vfs inode's number of
>    bytes, and instead return the number of bytes;
> 
> 2) Making any code that drops extents and adds new extents update the
>    inode's number of bytes atomically, while holding the btrfs inode's
>    spinlock, which is also used by the stat(2) callback to get the inode's
>    number of bytes;

Since synchronization is going to be provided by the btrfs_inode's lock,
then how about switching to __inode_(sub|add)_bytes functions which do
not take the vfs_inode's lock? Or do we need both (btrfs' and vfs' inode
locks) ?

> 
> 3) For ranges in the inode's iotree that are marked as 'delalloc new',
>    corresponding to previously unallocated ranges, increment the inode's
>    number of bytes when clearing the 'delalloc new' bit from the range,
>    in the same critical section that decrements the inode's
>    'new_delalloc_bytes' counter, delimited by the btrfs inode's spinlock.
> 
> An alternative would be to have btrfs_getattr() wait for any IO (ordered
> extents in progress) and locking the whole range (0 to (u64)-1) while it
> it computes the number of blocks used. But that would mean blocking
> stat(2), which is a very used syscall and expected to be fast, waiting
> for writes, clone/dedupe, fallocate, page reads, fiemap, etc.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
<snip>