Re: xfstests: generic/342 run failed in f2fs

Theodore Ts'o
2017-12-28T03:41:54Z

On Thu, Dec 28, 2017 at 11:17:09AM +0800, Chao Yu wrote:
> > Indeed. Actually, since one of our goals was to reduce fsync latencies for
> > Android, we decided to support posix in a minimum way. In order to avoid
>
> Agreed, in order to maximize performance of fsync regular file, we'd better
> not change current sematic of f2fs' fsync so far, moreover, there is no
> such requirement from upper layer application.

The cost of forcing an fsync on the parent directory only if the
regular file was just created should really be minimal. Most of the
performance critical use cases for fsync() is after writing data on an
already-existing file.

Even in the case where a package manager is writing, say, lots of
newly created files followed by an fsync (such as a 21 megabyte docker
binary) the overhead of including a handful of 4k metadata blocks for
the containing directory to make sure /usr/bin/docker exists as part
of the fsync() is in the noise compared to all of the other data
blocks being written by the package manager.

So I suspect the concerns of "oh, we can't provide the same guarantees
as ext4 and f2fs" are a bit overblown here. But ultimately, that's up
to the f2fs developers...

> > IMHO, as a quick fix, it seems we need to handle MS_DIRSYNC likewse xfs.
> > #define MS_DIRSYNC 128 /* Directory modifications are synchronous */
>
> We have supported that yet in commit b7e1d800031c ("f2fs: implement -o
> dirsync"). ;)

To be clear, we're not talking about dirsync mode where *all*
directory modifications are synchronous. What we're talking about
here is the case where if you fsync() a regular file, and the regular
file is newly created --- AND ONLY THAT CASE --- to make sure the
parent directory is included in the set of metadata blocks updated by
the fsync() against the regular file.

Best regards,

- Ted