Re: [PATCH 1/1] nilfs2: add missing blkdev_issue_flush() to nilfs_sync_fs()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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




[Index of Archives]     [Linux Filesystem Development]     [Linux BTRFS]     [Linux CIFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux