On Fri, Jan 17, 2020 at 10:22:46PM +0800, Qu Wenruo wrote: > >>>> If it gets removed you are trading one bug for another. With the changed > >>>> logic in the referenced commit, the metadata exhaustion is more likely > >>>> but it's also temporary. > >> > >> Furthermore, the point of the patch is, current check doesn't play well > >> with metadata over-commit. > > > > The recent overcommit updates broke statfs in a new way and left us > > almost nothing to make it better. > > It's not impossible to solve in fact. > > Exporting can_overcommit() can do pretty well in this particular case. can_overcommit will be exported by the block group reado-only fixes, pending for 5.6, so it might be used for statfs if need be. > >> If it's before v5.4, I won't touch the check considering it will never > >> hit anyway. > >> > >> But now for v5.4, either: > >> - We over-commit metadata > >> Meaning we have unallocated space, nothing to worry > > > > Can we estimate how much unallocated data are there? I don't know how, > > and "nothing to worry" always worries me. > > Data never over-commit. We always ensure there are enough data chunk > allocated before we allocate data extents. > > > > >> - No more space for over-commit > >> But in that case, we still have global rsv to update essential trees. > >> Please note that, btrfs should never fall into a status where no files > >> can be deleted. > > > > Of course, the global reserve is there for last resort actions and will > > be used for deletion and updating essential trees. What statfs says is > > how much data is there left for the user. New files, writing more data > > etc. > > > >> Consider all these, we're no longer able to really hit that case. > >> > >> So that's why I'm purposing deleting that. I see no reason why that > >> magic number 4M would still work nowadays. > > > > So, the corner case that resulted in the guesswork needs to be > > reevaluated then, the space reservations and related updates clearly > > affect that. That's out of 5.5-rc timeframe though. > > Although we can still solve the problem only using facility in v5.5, I'm > still not happy enough with the idea of "one exhausted resource would > result a different resource exhausted" > > I still believe in that we should report different things independently. > (Which obviously makes our lives easier in statfs case). > > That's also why we require reporters to include 'btrfs fi df' result > other than vanilla 'df', because we have different internals. > > Or, can we reuse the f_files/f_free facility to report metadata space, > and forgot all these mess? Requiring filesystem-specific interpretation of f_files is a mess too. That statfs, which is a syscall and we can't change anything on the interface level, is a severe limitation for presenting the space is a well known problem, yeah. The patch is still in game, I got a feedback some feedback on IRC. Comparing the 2 corner cases, the one I was aiming to fix is harder to hit than the inflated metadata during balance.
