On Tue 15-09-15 17:02:06, Dongsheng Yang wrote:
> There are only two places in whole kernel to call ->sync_fs directly. It
> will sync fs even in read-only mode. It's not a good idea and some filesystem
> would warn out if you are syncing in read-only mode. But sync_filesystem()
> does filter this case out. Let's call sync_filesystem() here instead.
>
> Signed-off-by: Dongsheng Yang <yangds.fnst@xxxxxxxxxxxxxx>
So I'd prefer ubifs used hidden system inodes for quota files, set
DQUOT_QUOTA_SYS_FILE flag and so this code calling sync_fs() could be
completely avoided.
I don't like calling sync_filesystem() from quota code. Mainly, it does
much more work than ->sync_fs() for most filesystems (it flushes all files
to disk). Just adding a check for read-only filesystem would look better to
me.
Honza
> ---
> fs/quota/dquot.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 140397a..0bda7fa 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -668,8 +668,7 @@ int dquot_quota_sync(struct super_block *sb, int type)
> /* This is not very clever (and fast) but currently I don't know about
> * any other simple way of getting quota data to disk and we must get
> * them there for userspace to be visible... */
> - if (sb->s_op->sync_fs)
> - sb->s_op->sync_fs(sb, 1);
> + sync_filesystem(sb);
> sync_blockdev(sb->s_bdev);
>
> /*
> @@ -2043,7 +2042,7 @@ int dquot_disable(struct super_block *sb, int type, unsigned int flags)
> /*
> * Skip everything if there's nothing to do. We have to do this because
> * sometimes we are called when fill_super() failed and calling
> - * sync_fs() in such cases does no good.
> + * sync_filesystem() in such cases does no good.
> */
> if (!sb_any_quota_loaded(sb)) {
> mutex_unlock(&dqopt->dqonoff_mutex);
> @@ -2110,8 +2109,7 @@ int dquot_disable(struct super_block *sb, int type, unsigned int flags)
>
> /* Sync the superblock so that buffers with quota data are written to
> * disk (and so userspace sees correct data afterwards). */
> - if (sb->s_op->sync_fs)
> - sb->s_op->sync_fs(sb, 1);
> + sync_filesystem(sb);
> sync_blockdev(sb->s_bdev);
> /* Now the quota files are just ordinary files and we can set the
> * inode flags back. Moreover we discard the pagecache so that
> --
> 1.8.4.2
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
