Re: [PATCH v3 1/3] btrfs: Introduce per-profile available space facility

David Sterba
2020-01-08T15:04:55Z

On Tue, Jan 07, 2020 at 10:13:43AM +0800, Qu Wenruo wrote:
> >> + devices_info = kcalloc(fs_devices->rw_devices, sizeof(*devices_info),
> >> + GFP_NOFS);
> >
> > Calling kcalloc is another potential slowdown, for the statfs
> > considerations.
>
> That's also what we did in statfs() before, so it shouldn't cause extra
> problem.
> Furthermore, we didn't use calc_one_profile_avail() directly in statfs()
> directly.
>
> Although I get your point, and personally speaking the memory allocation
> and extra in-memory device iteration should be pretty fast compared to
> __btrfs_alloc_chunk().
>
> Thus I don't think this memory allocation would cause extra trouble,
> except the error handling mentioned below.

Right, current statfs also does allocation via
btrfs_calc_avail_data_space, so it's the same as before.

> [...]
> >> + ret = calc_per_profile_avail(fs_info);
> >
> > Adding new failure modes
>
> Another solution I have tried is make calc_per_profile_avail() return
> void, ignoring the ENOMEM error, and just set the affected profile to 0
> available space.
>
> But that method is just delaying ENOMEM, and would cause strange
> pre-profile values until next successful update or mount cycle.
>
> Any idea on which method is less worse?

Better to return the error than wrong values in this case. As the
numbers are sort of a cache and the mount cycle to get them fixed is not
very user friendly, we need some other way. As this is a global state, a
bit in fs_info::flags can be set and full recalculation attempted at
some point until it succeeds. This would leave the counters stale for
some time but I think still better than if they're suddenly 0.