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.
