On Aug 19, 2022, at 5:09 PM, Eric Biggers <ebiggers@xxxxxxxxxx> wrote: > > On Tue, Aug 16, 2022 at 10:42:29AM -0600, Andreas Dilger wrote: >> >> IMHO, this whole discussion is putting the cart before the horse. >> Changing existing (and useful) IO behavior to accommodate an API that >> nobody has ever used, and is unlikely to even be widely used, doesn't >> make sense to me. Most applications won't check or care about the new >> DIO size fields, since they've lived this long without statx() returning >> this info, and will just pick a "large enough" size (4KB, 1MB, whatever) >> that gives them the performance they need. They *WILL* care if the app >> is suddenly unable to read data from a file in ways that have worked for >> a long time. >> >> Even if apps are modified to check these new DIO size fields, and then >> try to DIO write to a file in f2fs that doesn't allow it, then f2fs will >> return an error, which is what it would have done without the statx() >> changes, so no harm done AFAICS. >> >> Even with a more-complex DIO status return that handles a "direction" >> field (which IMHO is needlessly complex), there is always the potential >> for a TOCTOU race where a file changes between checking and access, so >> the userspace code would need to handle this. > > I'm having trouble making sense of your argument here; you seem to be saying > that STATX_DIOALIGN isn't useful, so it doesn't matter if we design it > correctly? That line of reasoning is concerning, as it's certainly intended > to be useful, and if it's not useful there's no point in adding it. > > Are there any specific concerns that you have, besides TOCTOU races and the > lack of support for read-only DIO? My main concern is disabling useful functionality that exists today to appease the new DIO size API. Whether STATX_DIOALIGN will become widely used by applications or not is hard to say at this point. If there were separate STATX_DIOREAD and STATX_DIOWRITE flags in the returned data, and the alignment is provided as it is today, that would be enough IMHO to address the original use case without significant complexity. > I don't think that TOCTOU races are a real concern here. Generally DIO > constraints would only change if the application doing DIO intentionally does > something to the file, or if there are changes that involve the filesystem > being taken offline, e.g. the filesystem being mounted with significantly > different options or being moved to a different block device. And, well, > everything else in stat()/statx() is subject to TOCTOU as well, but is still > used... I was thinking of background filesystem operations like compression, LVM migration to new storage with a different sector size, etc. that may change the DIO characteristics of the file even while it is open. Not that I think this will happen frequently, but it is possible, and applications shouldn't explode if the DIO parameters change and they get an error. Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
