On Mon, Mar 08, 2021 at 11:28:41AM +0000, David Howells wrote: > Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > > But after I've written and sync'd the data, I set the xattr to mark the > > > file not open. At the moment I'm doing this too lazily, only doing it > > > when a netfs file gets evicted or when the cache gets withdrawn, but I > > > really need to add a queue of objects to be sealed as they're closed. The > > > balance is working out how often to do the sealing as something like a > > > shell script can do a lot of consecutive open/write/close ops. > > > > You could add an internal vfs API wait_for_multiple_inodes_to_be_synced(). > > For example, xfs keeps the "LSN" on each inode, so once the transaction > > with some LSN has been committed, all the relevant inodes, if not dirty, can > > be declared as synced, without having to call fsync() on any file and without > > having to force transaction commit or any IO at all. > > > > Since fscache takes care of submitting the IO, and it shouldn't care about any > > specific time that the data/metadata hits the disk(?), you can make use of the > > existing periodic writeback and rolling transaction commit and only ever need > > to wait for that to happen before marking cache files "closed". > > > > There was a discussion about fsyncing a range of files on LSFMM [1]. > > In the last comment on the article dchinner argues why we already have that > > API (and now also with io_uring(), but AFAIK, we do not have a useful > > wait_for_sync() API. And it doesn't need to be exposed to userspace at all. > > > > [1] https://lwn.net/Articles/789024/ > > This sounds like an interesting idea. Actually, what I probably want is a > notification to say that a particular object has been completely sync'd to > disk, metadata and all. This isn't hard to do yourself in the kernel. All it takes is a workqueue to run vfs_fsync() calls asynchronously and for the work to queue a local notification/wakeup when the fsync completes... That's all aio_fsync() does - the notification it queues on completion is the AIO completion event for userspace - so I think you could do this in about 50 lines of code if you really needed it... > However, there are some performance problems are arising in my fscache-iter > branch: > > (1) It's doing a lot of synchronous metadata operations (tmpfile, truncate, > setxattr). Async pipelines using unbound workqueues are your friend. > > (2) It's retaining a lot of open file structs on cache files. Cachefiles > opens the file when it's first asked to access it and retains that till > the cookie is relinquished or the cache withdrawn (the file* doesn't > contribute to ENFILE/EMFILE but it still eats memory). Sounds similar to the problem that the NFSd open file cache solves. (fs/nfsd/filecache.c) > (3) Trimming excess data from the end of the cache file. The problem with > using DIO to write to the cache is that the write has to be rounded up to > a multiple of the backing fs DIO blocksize, Actually, a multiple of the logical sector size of the backing device behind the backing filesystem. > but if the file is truncated > larger, that excess data now becomes part of the file. Keep the actual file size in your tracking xattr. > Possibly it's sufficient to just clear the excess page space before > writing, but that doesn't necessarily stop a writable mmap from > scribbling on it. We can't stop mmap from scribbling in it. All filesystems have this problem, so to prevent data leaks we have to zero the post-eof tail region on every write of the EOF block, anyway. > (4) Committing outstanding cache metadata at cache withdrawal or netfs > unmount. I've previously mentioned this: it ends up with a whole slew of > synchronous metadata changes being committed to the cache in one go > (truncates, fallocates, fsync, xattrs, unlink+link of tmpfile) - and this > can take quite a long time. The cache needs to be more proactive in > getting stuff committed as it goes along. Workqueues give you an easy mechanism for async dispatch and concurrency for synchronous operations. This is a largely solved problem... > (5) Attaching to an object requires a pathwalk to it (normally only two > steps) and then reading various xattrs on it - all synchronous, but can > be punted to a background threadpool. a.k.a. punting to a workqueue :) Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx