nilfs2: avoid duplicate segment construction for fsync()
authorAndreas Rohner <andreas.rohner@gmx.net>
Wed, 10 Dec 2014 23:54:29 +0000 (15:54 -0800)
committerLinus Torvalds <torvalds@linux-foundation.org>
Thu, 11 Dec 2014 01:41:16 +0000 (17:41 -0800)
This patch removes filemap_write_and_wait_range() from nilfs_sync_file(),
because it triggers a data segment construction by calling
nilfs_writepages() with WB_SYNC_ALL.  A data segment construction does not
remove the inode from the i_dirty list and it does not clear the
NILFS_I_DIRTY flag.  Therefore nilfs_inode_dirty() still returns true,
which leads to an unnecessary duplicate segment construction in
nilfs_sync_file().

A call to filemap_write_and_wait_range() is not needed, because NILFS2
does not rely on the generic writeback mechanisms.  Instead it implements
its own mechanism to collect all dirty pages and write them into segments.
 It is more efficient to initiate the segment construction directly in
nilfs_sync_file() without the detour over filemap_write_and_wait_range().

Additionally the lock of i_mutex is not needed, because all code blocks
that are protected by i_mutex are also protected by a NILFS transaction:

  Function                i_mutex     nilfs_transaction
  ------------------------------------------------------
  nilfs_ioctl_setflags:   yes         yes
  nilfs_fiemap:           yes         no
  nilfs_write_begin:      yes         yes
  nilfs_write_end:        yes         yes
  nilfs_lookup:           yes         no
  nilfs_create:           yes         yes
  nilfs_link:             yes         yes
  nilfs_mknod:            yes         yes
  nilfs_symlink:          yes         yes
  nilfs_mkdir:            yes         yes
  nilfs_unlink:           yes         yes
  nilfs_rmdir:            yes         yes
  nilfs_rename:           yes         yes
  nilfs_setattr:          yes         yes

For nilfs_lookup() i_mutex is held for the parent directory, to protect it
from modification.  The segment construction does not modify directory
inodes, so no lock is needed.

nilfs_fiemap() reads the block layout on the disk, by using
nilfs_bmap_lookup_contig(). This is already protected by bmap->b_sem.

Signed-off-by: Andreas Rohner <andreas.rohner@gmx.net>
Signed-off-by: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
fs/nilfs2/file.c

index e9e3325f29f30cbe0c01c4433441803ff097df58..3a03e0aea1fb6b3cc9b00e1d3870719f13030c42 100644 (file)
@@ -39,21 +39,15 @@ int nilfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync)
         */
        struct the_nilfs *nilfs;
        struct inode *inode = file->f_mapping->host;
-       int err;
-
-       err = filemap_write_and_wait_range(inode->i_mapping, start, end);
-       if (err)
-               return err;
-       mutex_lock(&inode->i_mutex);
+       int err = 0;
 
        if (nilfs_inode_dirty(inode)) {
                if (datasync)
                        err = nilfs_construct_dsync_segment(inode->i_sb, inode,
-                                                           0, LLONG_MAX);
+                                                           start, end);
                else
                        err = nilfs_construct_segment(inode->i_sb);
        }
-       mutex_unlock(&inode->i_mutex);
 
        nilfs = inode->i_sb->s_fs_info;
        if (!err)