On Aug 16, 2017, at 9:41 AM, Jan Kara <jack@xxxxxxx> wrote:
>
> Push down acquisition of dqio_sem into ->write_file_info() callback.
> Mostly for consistency with other operations.
>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
> ---
> fs/ocfs2/quota_global.c | 9 +++++++--
> fs/quota/dquot.c | 10 +---------
> fs/quota/quota_v1.c | 2 ++
> fs/quota/quota_v2.c | 5 ++++-
> 4 files changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/fs/ocfs2/quota_global.c b/fs/ocfs2/quota_global.c
> index 4134d557a8e5..6f7c5bd3cbc6 100644
> --- a/fs/ocfs2/quota_global.c
> +++ b/fs/ocfs2/quota_global.c
> @@ -443,13 +443,18 @@ static int __ocfs2_global_write_info(struct super_block *sb, int type)
> int ocfs2_global_write_info(struct super_block *sb, int type)
Would be nice if this was named ocfs2_global_write_file_info() so that
it showed up in searches more easily, though not a huge deal. I guess
the same for ocfs2_local_read_file_info() and ocfs2_local_free_file_info(),
if we wanted to be consistent here, but that should go into a separate
patch.
> {
> int err;
> - struct ocfs2_mem_dqinfo *info = sb_dqinfo(sb, type)->dqi_priv;
> + struct quota_info *dqopt = sb_dqopt(sb);
> + struct ocfs2_mem_dqinfo *info = dqopt->info[type].dqi_priv;
>
> + down_write(&dqopt->dqio_sem);
> err = ocfs2_qinfo_lock(info, 1);
> - if (err < 0)
> + if (err < 0) {
> + up_write(&dqopt->dqio_sem);
> return err;
> + }
goto out_sem?
> err = __ocfs2_global_write_info(sb, type);
> ocfs2_qinfo_unlock(info, 1);
out_sem:
Other than this minor suggestion,
Reviewed-by: Andreas Dilger <adilger@xxxxxxxxx>
> + up_write(&dqopt->dqio_sem);
> return err;
> }
>
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 332f7026edad..1e1ff97098ec 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -415,10 +415,8 @@ int dquot_acquire(struct dquot *dquot)
> ret = dqopt->ops[dquot->dq_id.type]->commit_dqblk(dquot);
> /* Write the info if needed */
> if (info_dirty(&dqopt->info[dquot->dq_id.type])) {
> - down_write(&dqopt->dqio_sem);
> ret2 = dqopt->ops[dquot->dq_id.type]->write_file_info(
> dquot->dq_sb, dquot->dq_id.type);
> - up_write(&dqopt->dqio_sem);
> }
> if (ret < 0)
> goto out_iolock;
> @@ -482,10 +480,8 @@ int dquot_release(struct dquot *dquot)
> ret = dqopt->ops[dquot->dq_id.type]->release_dqblk(dquot);
> /* Write the info */
> if (info_dirty(&dqopt->info[dquot->dq_id.type])) {
> - down_write(&dqopt->dqio_sem);
> ret2 = dqopt->ops[dquot->dq_id.type]->write_file_info(
> dquot->dq_sb, dquot->dq_id.type);
> - up_write(&dqopt->dqio_sem);
> }
> if (ret >= 0)
> ret = ret2;
> @@ -2054,13 +2050,9 @@ EXPORT_SYMBOL(dquot_transfer);
> */
> int dquot_commit_info(struct super_block *sb, int type)
> {
> - int ret;
> struct quota_info *dqopt = sb_dqopt(sb);
>
> - down_write(&dqopt->dqio_sem);
> - ret = dqopt->ops[type]->write_file_info(sb, type);
> - up_write(&dqopt->dqio_sem);
> - return ret;
> + return dqopt->ops[type]->write_file_info(sb, type);
> }
> EXPORT_SYMBOL(dquot_commit_info);
>
> diff --git a/fs/quota/quota_v1.c b/fs/quota/quota_v1.c
> index 12d69cda57cc..fe68bf544b29 100644
> --- a/fs/quota/quota_v1.c
> +++ b/fs/quota/quota_v1.c
> @@ -186,6 +186,7 @@ static int v1_write_file_info(struct super_block *sb, int type)
> struct v1_disk_dqblk dqblk;
> int ret;
>
> + down_write(&dqopt->dqio_sem);
> dqopt->info[type].dqi_flags &= ~DQF_INFO_DIRTY;
> ret = sb->s_op->quota_read(sb, type, (char *)&dqblk,
> sizeof(struct v1_disk_dqblk), v1_dqoff(0));
> @@ -203,6 +204,7 @@ static int v1_write_file_info(struct super_block *sb, int type)
> else if (ret > 0)
> ret = -EIO;
> out:
> + up_write(&dqopt->dqio_sem);
> return ret;
> }
>
> diff --git a/fs/quota/quota_v2.c b/fs/quota/quota_v2.c
> index f82638e43c07..5e47012d2f57 100644
> --- a/fs/quota/quota_v2.c
> +++ b/fs/quota/quota_v2.c
> @@ -154,10 +154,12 @@ static int v2_read_file_info(struct super_block *sb, int type)
> static int v2_write_file_info(struct super_block *sb, int type)
> {
> struct v2_disk_dqinfo dinfo;
> - struct mem_dqinfo *info = sb_dqinfo(sb, type);
> + struct quota_info *dqopt = sb_dqopt(sb);
> + struct mem_dqinfo *info = &dqopt->info[type];
> struct qtree_mem_dqinfo *qinfo = info->dqi_priv;
> ssize_t size;
>
> + down_write(&dqopt->dqio_sem);
> spin_lock(&dq_data_lock);
> info->dqi_flags &= ~DQF_INFO_DIRTY;
> dinfo.dqi_bgrace = cpu_to_le32(info->dqi_bgrace);
> @@ -170,6 +172,7 @@ static int v2_write_file_info(struct super_block *sb, int type)
> dinfo.dqi_free_entry = cpu_to_le32(qinfo->dqi_free_entry);
> size = sb->s_op->quota_write(sb, type, (char *)&dinfo,
> sizeof(struct v2_disk_dqinfo), V2_DQINFOOFF);
> + up_write(&dqopt->dqio_sem);
> if (size != sizeof(struct v2_disk_dqinfo)) {
> quota_error(sb, "Can't write info structure");
> return -1;
> --
> 2.12.3
>
Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
