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

[Filipe Manana <fdmanana@kernel.org>]

On Mon, Nov 9, 2020 at 10:34 AM Nikolay Borisov <nborisov@suse.com> wrote:
>
>
>
> 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) ?

Because the synchronization is only meant for the races with stat(2),
and don't want to break existing users of inode_get_bytes(), specially
those outside btrfs.

Thanks.

>
> >
> > 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>