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 <andreas.rohner@xxxxxxx>
>> ---
>> 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_t ns_flushed_device;
>
> Andreas, can you implement this with just an integer variable like below ?
>
> int ns_flushed_device
>
> I want to limit the use of atomic_t type just for counters such as
> ref-counts, statistics information, and so forth.
Yes sure. I thought it is a good idea to use atomic_t, because
atomic_read() and atomic_set() are implemented with simple inline functions:
static inline int atomic_read(const atomic_t *v)
{
return (*(volatile int *)&(v)->counter);
}
static inline void atomic_set(atomic_t *v, int i)
{
v->counter = i;
}
So the performance would be essentially the same as with int. No extra
locks or memory barriers at least on architectures where an int is
always stored atomically. I thought it is good practice to use the
inline functions in this case.
> The load/store operations for this value is done atomically because
> the_nilfs structure is natually aligned without "packed" attribute
> though it may need that smp barriers are inserted properly.
>
>> struct block_device *ns_bdev;
>> struct rw_semaphore ns_sem;
>> --
>> 2.1.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
