Re: [PATCH 1/1] nilfs2: add missing blkdev_issue_flush() to nilfs_sync_fs()
On 2014-09-09 21:18, Ryusuke Konishi wrote: > On Tue, 9 Sep 2014 18:35:40 +0200, Andreas Rohner wrote: >> Under normal circumstances nilfs_sync_fs() writes out the super block, >> which causes a flush of the underlying block device. But this depends on >> the THE_NILFS_SB_DIRTY flag, which is only set if the pointer to the >> last segment crosses a segment boundary. So if only a small amount of >> data is written before the call to nilfs_sync_fs(), no flush of the >> block device occurs. >> >> In the above case an additional call to blkdev_issue_flush() is needed. >> To prevent unnecessary overhead, the new flag nilfs->ns_flushed_device >> is introduced, which is cleared whenever new logs are written and set >> whenever the block device is flushed. >> >> Signed-off-by: Andreas Rohner >> --- >> fs/nilfs2/file.c | 6 +- >> fs/nilfs2/ioctl.c | 6 +- >> fs/nilfs2/segment.c | 4 >> fs/nilfs2/super.c | 12 >> fs/nilfs2/the_nilfs.c | 1 + >> fs/nilfs2/the_nilfs.h | 2 ++ >> 6 files changed, 29 insertions(+), 2 deletions(-) >> >> diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c >> index 2497815..8a3e702 100644 >> --- a/fs/nilfs2/file.c >> +++ b/fs/nilfs2/file.c >> @@ -56,7 +56,11 @@ int nilfs_sync_file(struct file *file, loff_t start, >> loff_t end, int datasync) >> mutex_unlock(&inode->i_mutex); >> >> nilfs = inode->i_sb->s_fs_info; >> -if (!err && nilfs_test_opt(nilfs, BARRIER)) { >> +if (!err && nilfs_test_opt(nilfs, BARRIER) && >> +!atomic_read(&nilfs->ns_flushed_device)) { >> +atomic_set(&nilfs->ns_flushed_device, 1); >> +smp_mb__after_atomic(); >> + >> err = blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL); >> if (err != -EIO) >> err = 0; >> diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c >> index 422fb54..47fe7cf 100644 >> --- a/fs/nilfs2/ioctl.c >> +++ b/fs/nilfs2/ioctl.c >> @@ -1022,7 +1022,11 @@ static int nilfs_ioctl_sync(struct inode *inode, >> struct file *filp, >> return ret; >> >> nilfs = inode->i_sb->s_fs_info; >> -if (nilfs_test_opt(nilfs, BARRIER)) { >> +if (nilfs_test_opt(nilfs, BARRIER) && >> +!atomic_read(&nilfs->ns_flushed_device)) { >> +atomic_set(&nilfs->ns_flushed_device, 1); >> +smp_mb__after_atomic(); >> + >> ret = blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL); >> if (ret == -EIO) >> return ret; >> diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c >> index a1a1916..3119b64 100644 >> --- a/fs/nilfs2/segment.c >> +++ b/fs/nilfs2/segment.c >> @@ -1997,6 +1997,10 @@ static int nilfs_segctor_do_construct(struct >> nilfs_sc_info *sci, int mode) >> err = nilfs_segctor_wait(sci); >> if (err) >> goto failed_to_write; >> + >> +if (test_bit(NILFS_SC_SUPER_ROOT, &sci->sc_flags) || >> +mode == SC_LSEG_DSYNC) >> +atomic_set(&nilfs->ns_flushed_device, 0); >> } >> } while (sci->sc_stage.scnt != NILFS_ST_DONE); >> >> diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c >> index 228f5bd..74a9930 100644 >> --- a/fs/nilfs2/super.c >> +++ b/fs/nilfs2/super.c >> @@ -310,6 +310,8 @@ int nilfs_commit_super(struct super_block *sb, int flag) >> nilfs->ns_sbsize)); >> } >> clear_nilfs_sb_dirty(nilfs); >> +atomic_set(&nilfs->ns_flushed_device, 1); >> +smp_mb__after_atomic(); >> return nilfs_sync_super(sb, flag); >> } >> >> @@ -514,6 +516,16 @@ static int nilfs_sync_fs(struct super_block *sb, int >> wait) >> } >> up_write(&nilfs->ns_sem); >> >> +if (wait && !err && nilfs_test_opt(nilfs, BARRIER) && >> +!atomic_read(&nilfs->ns_flushed_device)) { >> +atomic_set(&nilfs->ns_flushed_device, 1); >> +smp_mb__after_atomic(); >> + >> +err = blkdev_issue_flush(sb->s_bdev, GFP_KERNEL, NULL); >> +if (err != -EIO) >> +err = 0; >> +} >> + >> return err; >> } >> >> diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c >> index 9da25fe..d37c50b 100644 >> --- a/fs/nilfs2/the_nilfs.c >> +++ b/fs/nilfs2/the_nilfs.c >> @@ -74,6 +74,7 @@ struct the_nilfs *alloc_nilfs(struct block_device *bdev) >> return NULL; >> >> nilfs->ns_bdev = bdev; >> +atomic_set(&nilfs->ns_flushed_device, 0); >> atomic_set(&nilfs->ns_ndirtyblks, 0); >> init_rwsem(&nilfs->ns_sem); >> mutex_init(&nilfs->ns_snapshot_mount_mutex); >> diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h >> index d01ead1..ec53958 100644 >> --- a/fs/nilfs2/the_nilfs.h >> +++ b/fs/nilfs2/the_nilfs.h >> @@ -45,6 +45,7 @@ enum { >> >> /** >> * struct the_nilfs - struct to supervise multiple nilfs mount points >> + * @ns_flushed
Re: [PATCH 1/1] nilfs2: add missing blkdev_issue_flush() to nilfs_sync_fs()
On Tue, 9 Sep 2014 18:35:40 +0200, Andreas Rohner wrote: > Under normal circumstances nilfs_sync_fs() writes out the super block, > which causes a flush of the underlying block device. But this depends on > the THE_NILFS_SB_DIRTY flag, which is only set if the pointer to the > last segment crosses a segment boundary. So if only a small amount of > data is written before the call to nilfs_sync_fs(), no flush of the > block device occurs. > > In the above case an additional call to blkdev_issue_flush() is needed. > To prevent unnecessary overhead, the new flag nilfs->ns_flushed_device > is introduced, which is cleared whenever new logs are written and set > whenever the block device is flushed. > > Signed-off-by: Andreas Rohner > --- > fs/nilfs2/file.c | 6 +- > fs/nilfs2/ioctl.c | 6 +- > fs/nilfs2/segment.c | 4 > fs/nilfs2/super.c | 12 > fs/nilfs2/the_nilfs.c | 1 + > fs/nilfs2/the_nilfs.h | 2 ++ > 6 files changed, 29 insertions(+), 2 deletions(-) > > diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c > index 2497815..8a3e702 100644 > --- a/fs/nilfs2/file.c > +++ b/fs/nilfs2/file.c > @@ -56,7 +56,11 @@ int nilfs_sync_file(struct file *file, loff_t start, > loff_t end, int datasync) > mutex_unlock(&inode->i_mutex); > > nilfs = inode->i_sb->s_fs_info; > - if (!err && nilfs_test_opt(nilfs, BARRIER)) { > + if (!err && nilfs_test_opt(nilfs, BARRIER) && > + !atomic_read(&nilfs->ns_flushed_device)) { > + atomic_set(&nilfs->ns_flushed_device, 1); > + smp_mb__after_atomic(); > + > err = blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL); > if (err != -EIO) > err = 0; > diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c > index 422fb54..47fe7cf 100644 > --- a/fs/nilfs2/ioctl.c > +++ b/fs/nilfs2/ioctl.c > @@ -1022,7 +1022,11 @@ static int nilfs_ioctl_sync(struct inode *inode, > struct file *filp, > return ret; > > nilfs = inode->i_sb->s_fs_info; > - if (nilfs_test_opt(nilfs, BARRIER)) { > + if (nilfs_test_opt(nilfs, BARRIER) && > + !atomic_read(&nilfs->ns_flushed_device)) { > + atomic_set(&nilfs->ns_flushed_device, 1); > + smp_mb__after_atomic(); > + > ret = blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL); > if (ret == -EIO) > return ret; > diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c > index a1a1916..3119b64 100644 > --- a/fs/nilfs2/segment.c > +++ b/fs/nilfs2/segment.c > @@ -1997,6 +1997,10 @@ static int nilfs_segctor_do_construct(struct > nilfs_sc_info *sci, int mode) > err = nilfs_segctor_wait(sci); > if (err) > goto failed_to_write; > + > + if (test_bit(NILFS_SC_SUPER_ROOT, &sci->sc_flags) || > + mode == SC_LSEG_DSYNC) > + atomic_set(&nilfs->ns_flushed_device, 0); > } > } while (sci->sc_stage.scnt != NILFS_ST_DONE); > > diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c > index 228f5bd..74a9930 100644 > --- a/fs/nilfs2/super.c > +++ b/fs/nilfs2/super.c > @@ -310,6 +310,8 @@ int nilfs_commit_super(struct super_block *sb, int flag) > nilfs->ns_sbsize)); > } > clear_nilfs_sb_dirty(nilfs); > + atomic_set(&nilfs->ns_flushed_device, 1); > + smp_mb__after_atomic(); > return nilfs_sync_super(sb, flag); > } > > @@ -514,6 +516,16 @@ static int nilfs_sync_fs(struct super_block *sb, int > wait) > } > up_write(&nilfs->ns_sem); > > + if (wait && !err && nilfs_test_opt(nilfs, BARRIER) && > + !atomic_read(&nilfs->ns_flushed_device)) { > + atomic_set(&nilfs->ns_flushed_device, 1); > + smp_mb__after_atomic(); > + > + err = blkdev_issue_flush(sb->s_bdev, GFP_KERNEL, NULL); > + if (err != -EIO) > + err = 0; > + } > + > return err; > } > > diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c > index 9da25fe..d37c50b 100644 > --- a/fs/nilfs2/the_nilfs.c > +++ b/fs/nilfs2/the_nilfs.c > @@ -74,6 +74,7 @@ struct the_nilfs *alloc_nilfs(struct block_device *bdev) > return NULL; > > nilfs->ns_bdev = bdev; > + atomic_set(&nilfs->ns_flushed_device, 0); > atomic_set(&nilfs->ns_ndirtyblks, 0); > init_rwsem(&nilfs->ns_sem); > mutex_init(&nilfs->ns_snapshot_mount_mutex); > diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h > index d01ead1..ec53958 100644 > --- a/fs/nilfs2/the_nilfs.h > +++ b/fs/nilfs2/the_nilfs.h > @@ -45,6 +45,7 @@ enum { > > /** > * struct the_nilfs - struct to supervise multiple nilfs mount points > + * @ns_flushed_device: flag indicating if all volatile data was flushed > * @ns_flags: flags > * @ns_bdev: block device > * @n
[PATCH 1/1] nilfs2: add missing blkdev_issue_flush() to nilfs_sync_fs()
Under normal circumstances nilfs_sync_fs() writes out the super block, which causes a flush of the underlying block device. But this depends on the THE_NILFS_SB_DIRTY flag, which is only set if the pointer to the last segment crosses a segment boundary. So if only a small amount of data is written before the call to nilfs_sync_fs(), no flush of the block device occurs. In the above case an additional call to blkdev_issue_flush() is needed. To prevent unnecessary overhead, the new flag nilfs->ns_flushed_device is introduced, which is cleared whenever new logs are written and set whenever the block device is flushed. Signed-off-by: Andreas Rohner --- fs/nilfs2/file.c | 6 +- fs/nilfs2/ioctl.c | 6 +- fs/nilfs2/segment.c | 4 fs/nilfs2/super.c | 12 fs/nilfs2/the_nilfs.c | 1 + fs/nilfs2/the_nilfs.h | 2 ++ 6 files changed, 29 insertions(+), 2 deletions(-) diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c index 2497815..8a3e702 100644 --- a/fs/nilfs2/file.c +++ b/fs/nilfs2/file.c @@ -56,7 +56,11 @@ int nilfs_sync_file(struct file *file, loff_t start, loff_t end, int datasync) mutex_unlock(&inode->i_mutex); nilfs = inode->i_sb->s_fs_info; - if (!err && nilfs_test_opt(nilfs, BARRIER)) { + if (!err && nilfs_test_opt(nilfs, BARRIER) && + !atomic_read(&nilfs->ns_flushed_device)) { + atomic_set(&nilfs->ns_flushed_device, 1); + smp_mb__after_atomic(); + err = blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL); if (err != -EIO) err = 0; diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c index 422fb54..47fe7cf 100644 --- a/fs/nilfs2/ioctl.c +++ b/fs/nilfs2/ioctl.c @@ -1022,7 +1022,11 @@ static int nilfs_ioctl_sync(struct inode *inode, struct file *filp, return ret; nilfs = inode->i_sb->s_fs_info; - if (nilfs_test_opt(nilfs, BARRIER)) { + if (nilfs_test_opt(nilfs, BARRIER) && + !atomic_read(&nilfs->ns_flushed_device)) { + atomic_set(&nilfs->ns_flushed_device, 1); + smp_mb__after_atomic(); + ret = blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL); if (ret == -EIO) return ret; diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c index a1a1916..3119b64 100644 --- a/fs/nilfs2/segment.c +++ b/fs/nilfs2/segment.c @@ -1997,6 +1997,10 @@ static int nilfs_segctor_do_construct(struct nilfs_sc_info *sci, int mode) err = nilfs_segctor_wait(sci); if (err) goto failed_to_write; + + if (test_bit(NILFS_SC_SUPER_ROOT, &sci->sc_flags) || + mode == SC_LSEG_DSYNC) + atomic_set(&nilfs->ns_flushed_device, 0); } } while (sci->sc_stage.scnt != NILFS_ST_DONE); diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c index 228f5bd..74a9930 100644 --- a/fs/nilfs2/super.c +++ b/fs/nilfs2/super.c @@ -310,6 +310,8 @@ int nilfs_commit_super(struct super_block *sb, int flag) nilfs->ns_sbsize)); } clear_nilfs_sb_dirty(nilfs); + atomic_set(&nilfs->ns_flushed_device, 1); + smp_mb__after_atomic(); return nilfs_sync_super(sb, flag); } @@ -514,6 +516,16 @@ static int nilfs_sync_fs(struct super_block *sb, int wait) } up_write(&nilfs->ns_sem); + if (wait && !err && nilfs_test_opt(nilfs, BARRIER) && + !atomic_read(&nilfs->ns_flushed_device)) { + atomic_set(&nilfs->ns_flushed_device, 1); + smp_mb__after_atomic(); + + err = blkdev_issue_flush(sb->s_bdev, GFP_KERNEL, NULL); + if (err != -EIO) + err = 0; + } + return err; } diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c index 9da25fe..d37c50b 100644 --- a/fs/nilfs2/the_nilfs.c +++ b/fs/nilfs2/the_nilfs.c @@ -74,6 +74,7 @@ struct the_nilfs *alloc_nilfs(struct block_device *bdev) return NULL; nilfs->ns_bdev = bdev; + atomic_set(&nilfs->ns_flushed_device, 0); atomic_set(&nilfs->ns_ndirtyblks, 0); init_rwsem(&nilfs->ns_sem); mutex_init(&nilfs->ns_snapshot_mount_mutex); diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h index d01ead1..ec53958 100644 --- a/fs/nilfs2/the_nilfs.h +++ b/fs/nilfs2/the_nilfs.h @@ -45,6 +45,7 @@ enum { /** * struct the_nilfs - struct to supervise multiple nilfs mount points + * @ns_flushed_device: flag indicating if all volatile data was flushed * @ns_flags: flags * @ns_bdev: block device * @ns_sem: semaphore for shared states @@ -103,6 +104,7 @@ enum { */ struct the_nilfs { unsigned long ns_flags; + atomic_tns_flushed_device; struct block_device*n
[PATCH 1/1] nilfs2: add missing blkdev_issue_flush() to nilfs_sync_fs()
This patch adds a call to blkdev_issue_flush() to nilfs_sync_fs(), which is the nilfs implementation of the sync() syscall. If the BARRIER mount option is set, both the nilfs implementation of fsync() and nilfs' custom ioctl version of sync() used by the cleaner, use blkdev_issue_flush() to guarantee that the data is written to the underlying device. To get the same behaviour and guarantees for the sync() syscall, blkdev_issue_flush() should also be called in nilfs_sync_fs(). Signed-off-by: Andreas Rohner --- fs/nilfs2/super.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c index 228f5bd..1f21e81 100644 --- a/fs/nilfs2/super.c +++ b/fs/nilfs2/super.c @@ -514,6 +514,12 @@ static int nilfs_sync_fs(struct super_block *sb, int wait) } up_write(&nilfs->ns_sem); + if (wait && !err && nilfs_test_opt(nilfs, BARRIER)) { + err = blkdev_issue_flush(sb->s_bdev, GFP_KERNEL, NULL); + if (err != -EIO) + err = 0; + } + return err; } -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html