On Wed, Apr 19, 2023 at 10:26:02AM -0700, Darrick J. Wong wrote: > On Wed, Apr 19, 2023 at 12:17:34PM -0400, Mike Snitzer wrote: > > (And obviously needs fixing independent of this patchset) > > > > Shouldn't mkfs first check that the underlying storage supports > > REQ_OP_PROVISION by verifying > > /sys/block/<dev>/queue/provision_max_bytes exists and is not 0? > > (Just saying, we need to add new features more defensively.. you just > > made the case based on this scenario's implications alone) > > Not for fallocate -- for regular files, there's no way to check if the > filesystem actually supports the operation requested other than to try > it and see if it succeeds. We probably should've defined a DRY_RUN flag > for that purpose back when it was introduced. That ignores the fact that fallocate() was never intended to guarantee it will work in all contexts - it's an advisory interface at it's most basic level. If the call succeeds, then great, it does what is says on the box, but if it fails then it should have no visible effect on user data at all and the application still needs to perform whatever modification it needed done in some other way. IOWs, calling it one a block device without first checking if the block device supports that fallocate operation is exactly how it is supposed to be used. If the kernel can't handle this gracefully without corrupting data, then that's a kernel bug and not an application problem. > For fallocate calls to block devices, yes, the program can check the > queue limits in sysfs if fstat says the supplied path is a block device, > but I don't know that most programs are that thorough. The fallocate(1) > CLI program does not check. Right. fallocate() was intended to just do the right thing without the application having to jump thrown an unknown number of hoops to determine if fallocate() can be used or not in the context it is executing in. The kernel implementation is supposed to abstract all that context-dependent behaviour away from the application; all the application has to do is implement the fallocate() fast path and a single generic "do the right thing the slow way" fallback when the fallocate() method it called is not supported... -Dave. -- Dave Chinner david@xxxxxxxxxxxxx
