On Aug 16, 2017, at 9:41 AM, Jan Kara <jack@xxxxxxx> wrote:
>
> Convert dqio_mutex to rwsem and call it dqio_sem. No functional changes
> yet.
>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
> ---
> fs/ext4/super.c | 4 ++--
> fs/ocfs2/quota_global.c | 20 ++++++++++----------
> fs/ocfs2/quota_local.c | 10 +++++-----
> fs/quota/dquot.c | 28 ++++++++++++++--------------
> fs/quota/quota_tree.c | 2 +-
> fs/super.c | 2 +-
> include/linux/quota.h | 2 +-
> 7 files changed, 34 insertions(+), 34 deletions(-)
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index d61a70e2193a..8e0c27387ab7 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -5268,8 +5268,8 @@ static int ext4_statfs(struct dentry *dentry, struct kstatfs *buf)
> * Process 1 Process 2
> * ext4_create() quota_sync()
> * jbd2_journal_start() write_dquot()
> - * dquot_initialize() down(dqio_mutex)
> - * down(dqio_mutex) jbd2_journal_start()
> + * dquot_initialize() down(dqio_sem)
> + * down(dqio_sem) jbd2_journal_start()
Not a big deal, since this is a comment, but it should probably be changed
to down_write(dqio_sem) on both lines, as this deadlock wouldn't happen if
it was down_read(dqio_sem), and it is more consistent with the new usage.
That said, this comment doesn't appear to be relevant anymore. At least I
couldn't find where in those callpaths dqio_mutex/dqio_sem is used anymore.
Cheers, Andreas
> *
> */
>
> diff --git a/fs/ocfs2/quota_global.c b/fs/ocfs2/quota_global.c
> index cec495a921e3..4134d557a8e5 100644
> --- a/fs/ocfs2/quota_global.c
> +++ b/fs/ocfs2/quota_global.c
> @@ -33,7 +33,7 @@
> * Locking of quotas with OCFS2 is rather complex. Here are rules that
> * should be obeyed by all the functions:
> * - any write of quota structure (either to local or global file) is protected
> - * by dqio_mutex or dquot->dq_lock.
> + * by dqio_sem or dquot->dq_lock.
> * - any modification of global quota file holds inode cluster lock, i_mutex,
> * and ip_alloc_sem of the global quota file (achieved by
> * ocfs2_lock_global_qf). It also has to hold qinfo_lock.
> @@ -42,9 +42,9 @@
> *
> * A rough sketch of locking dependencies (lf = local file, gf = global file):
> * Normal filesystem operation:
> - * start_trans -> dqio_mutex -> write to lf
> + * start_trans -> dqio_sem -> write to lf
> * Syncing of local and global file:
> - * ocfs2_lock_global_qf -> start_trans -> dqio_mutex -> qinfo_lock ->
> + * ocfs2_lock_global_qf -> start_trans -> dqio_sem -> qinfo_lock ->
> * write to gf
> * -> write to lf
> * Acquire dquot for the first time:
> @@ -60,7 +60,7 @@
> * Recovery:
> * inode cluster lock of recovered lf
> * -> read bitmaps -> ip_alloc_sem of lf
> - * -> ocfs2_lock_global_qf -> start_trans -> dqio_mutex -> qinfo_lock ->
> + * -> ocfs2_lock_global_qf -> start_trans -> dqio_sem -> qinfo_lock ->
> * write to gf
> */
>
> @@ -611,7 +611,7 @@ static int ocfs2_sync_dquot_helper(struct dquot *dquot, unsigned long type)
> mlog_errno(status);
> goto out_ilock;
> }
> - mutex_lock(&sb_dqopt(sb)->dqio_mutex);
> + down_write(&sb_dqopt(sb)->dqio_sem);
> status = ocfs2_sync_dquot(dquot);
> if (status < 0)
> mlog_errno(status);
> @@ -619,7 +619,7 @@ static int ocfs2_sync_dquot_helper(struct dquot *dquot, unsigned long type)
> status = ocfs2_local_write_dquot(dquot);
> if (status < 0)
> mlog_errno(status);
> - mutex_unlock(&sb_dqopt(sb)->dqio_mutex);
> + up_write(&sb_dqopt(sb)->dqio_sem);
> ocfs2_commit_trans(osb, handle);
> out_ilock:
> ocfs2_unlock_global_qf(oinfo, 1);
> @@ -666,9 +666,9 @@ static int ocfs2_write_dquot(struct dquot *dquot)
> mlog_errno(status);
> goto out;
> }
> - mutex_lock(&sb_dqopt(dquot->dq_sb)->dqio_mutex);
> + down_write(&sb_dqopt(dquot->dq_sb)->dqio_sem);
> status = ocfs2_local_write_dquot(dquot);
> - mutex_unlock(&sb_dqopt(dquot->dq_sb)->dqio_mutex);
> + up_write(&sb_dqopt(dquot->dq_sb)->dqio_sem);
> ocfs2_commit_trans(osb, handle);
> out:
> return status;
> @@ -939,7 +939,7 @@ static int ocfs2_mark_dquot_dirty(struct dquot *dquot)
> mlog_errno(status);
> goto out_ilock;
> }
> - mutex_lock(&sb_dqopt(sb)->dqio_mutex);
> + down_write(&sb_dqopt(sb)->dqio_sem);
> status = ocfs2_sync_dquot(dquot);
> if (status < 0) {
> mlog_errno(status);
> @@ -948,7 +948,7 @@ static int ocfs2_mark_dquot_dirty(struct dquot *dquot)
> /* Now write updated local dquot structure */
> status = ocfs2_local_write_dquot(dquot);
> out_dlock:
> - mutex_unlock(&sb_dqopt(sb)->dqio_mutex);
> + up_write(&sb_dqopt(sb)->dqio_sem);
> ocfs2_commit_trans(osb, handle);
> out_ilock:
> ocfs2_unlock_global_qf(oinfo, 1);
> diff --git a/fs/ocfs2/quota_local.c b/fs/ocfs2/quota_local.c
> index 32c5a40c1257..1311eff1c050 100644
> --- a/fs/ocfs2/quota_local.c
> +++ b/fs/ocfs2/quota_local.c
> @@ -520,7 +520,7 @@ static int ocfs2_recover_local_quota_file(struct inode *lqinode,
> mlog_errno(status);
> goto out_drop_lock;
> }
> - mutex_lock(&sb_dqopt(sb)->dqio_mutex);
> + down_write(&sb_dqopt(sb)->dqio_sem);
> spin_lock(&dq_data_lock);
> /* Add usage from quota entry into quota changes
> * of our node. Auxiliary variables are important
> @@ -553,7 +553,7 @@ static int ocfs2_recover_local_quota_file(struct inode *lqinode,
> unlock_buffer(qbh);
> ocfs2_journal_dirty(handle, qbh);
> out_commit:
> - mutex_unlock(&sb_dqopt(sb)->dqio_mutex);
> + up_write(&sb_dqopt(sb)->dqio_sem);
> ocfs2_commit_trans(OCFS2_SB(sb), handle);
> out_drop_lock:
> ocfs2_unlock_global_qf(oinfo, 1);
> @@ -693,7 +693,7 @@ static int ocfs2_local_read_info(struct super_block *sb, int type)
>
> /* We don't need the lock and we have to acquire quota file locks
> * which will later depend on this lock */
> - mutex_unlock(&sb_dqopt(sb)->dqio_mutex);
> + up_write(&sb_dqopt(sb)->dqio_sem);
> info->dqi_max_spc_limit = 0x7fffffffffffffffLL;
> info->dqi_max_ino_limit = 0x7fffffffffffffffLL;
> oinfo = kmalloc(sizeof(struct ocfs2_mem_dqinfo), GFP_NOFS);
> @@ -772,7 +772,7 @@ static int ocfs2_local_read_info(struct super_block *sb, int type)
> goto out_err;
> }
>
> - mutex_lock(&sb_dqopt(sb)->dqio_mutex);
> + down_write(&sb_dqopt(sb)->dqio_sem);
> return 0;
> out_err:
> if (oinfo) {
> @@ -786,7 +786,7 @@ static int ocfs2_local_read_info(struct super_block *sb, int type)
> kfree(oinfo);
> }
> brelse(bh);
> - mutex_lock(&sb_dqopt(sb)->dqio_mutex);
> + down_write(&sb_dqopt(sb)->dqio_sem);
> return -1;
> }
>
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 53a17496c5c5..29d447598642 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -120,7 +120,7 @@
> * spinlock to internal buffers before writing.
> *
> * Lock ordering (including related VFS locks) is the following:
> - * s_umount > i_mutex > journal_lock > dquot->dq_lock > dqio_mutex
> + * s_umount > i_mutex > journal_lock > dquot->dq_lock > dqio_sem
> */
>
> static __cacheline_aligned_in_smp DEFINE_SPINLOCK(dq_list_lock);
> @@ -406,7 +406,7 @@ int dquot_acquire(struct dquot *dquot)
> struct quota_info *dqopt = sb_dqopt(dquot->dq_sb);
>
> mutex_lock(&dquot->dq_lock);
> - mutex_lock(&dqopt->dqio_mutex);
> + down_write(&dqopt->dqio_sem);
> if (!test_bit(DQ_READ_B, &dquot->dq_flags))
> ret = dqopt->ops[dquot->dq_id.type]->read_dqblk(dquot);
> if (ret < 0)
> @@ -436,7 +436,7 @@ int dquot_acquire(struct dquot *dquot)
> smp_mb__before_atomic();
> set_bit(DQ_ACTIVE_B, &dquot->dq_flags);
> out_iolock:
> - mutex_unlock(&dqopt->dqio_mutex);
> + up_write(&dqopt->dqio_sem);
> mutex_unlock(&dquot->dq_lock);
> return ret;
> }
> @@ -450,7 +450,7 @@ int dquot_commit(struct dquot *dquot)
> int ret = 0;
> struct quota_info *dqopt = sb_dqopt(dquot->dq_sb);
>
> - mutex_lock(&dqopt->dqio_mutex);
> + down_write(&dqopt->dqio_sem);
> spin_lock(&dq_list_lock);
> if (!clear_dquot_dirty(dquot)) {
> spin_unlock(&dq_list_lock);
> @@ -464,7 +464,7 @@ int dquot_commit(struct dquot *dquot)
> else
> ret = -EIO;
> out_sem:
> - mutex_unlock(&dqopt->dqio_mutex);
> + up_write(&dqopt->dqio_sem);
> return ret;
> }
> EXPORT_SYMBOL(dquot_commit);
> @@ -481,7 +481,7 @@ int dquot_release(struct dquot *dquot)
> /* Check whether we are not racing with some other dqget() */
> if (atomic_read(&dquot->dq_count) > 1)
> goto out_dqlock;
> - mutex_lock(&dqopt->dqio_mutex);
> + down_write(&dqopt->dqio_sem);
> if (dqopt->ops[dquot->dq_id.type]->release_dqblk) {
> ret = dqopt->ops[dquot->dq_id.type]->release_dqblk(dquot);
> /* Write the info */
> @@ -493,7 +493,7 @@ int dquot_release(struct dquot *dquot)
> ret = ret2;
> }
> clear_bit(DQ_ACTIVE_B, &dquot->dq_flags);
> - mutex_unlock(&dqopt->dqio_mutex);
> + up_write(&dqopt->dqio_sem);
> out_dqlock:
> mutex_unlock(&dquot->dq_lock);
> return ret;
> @@ -2060,9 +2060,9 @@ int dquot_commit_info(struct super_block *sb, int type)
> int ret;
> struct quota_info *dqopt = sb_dqopt(sb);
>
> - mutex_lock(&dqopt->dqio_mutex);
> + down_write(&dqopt->dqio_sem);
> ret = dqopt->ops[type]->write_file_info(sb, type);
> - mutex_unlock(&dqopt->dqio_mutex);
> + up_write(&dqopt->dqio_sem);
> return ret;
> }
> EXPORT_SYMBOL(dquot_commit_info);
> @@ -2076,9 +2076,9 @@ int dquot_get_next_id(struct super_block *sb, struct kqid *qid)
> return -ESRCH;
> if (!dqopt->ops[qid->type]->get_next_id)
> return -ENOSYS;
> - mutex_lock(&dqopt->dqio_mutex);
> + down_write(&dqopt->dqio_sem);
> err = dqopt->ops[qid->type]->get_next_id(sb, qid);
> - mutex_unlock(&dqopt->dqio_mutex);
> + up_write(&dqopt->dqio_sem);
> return err;
> }
> EXPORT_SYMBOL(dquot_get_next_id);
> @@ -2328,15 +2328,15 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
> dqopt->info[type].dqi_format = fmt;
> dqopt->info[type].dqi_fmt_id = format_id;
> INIT_LIST_HEAD(&dqopt->info[type].dqi_dirty_list);
> - mutex_lock(&dqopt->dqio_mutex);
> + down_write(&dqopt->dqio_sem);
> error = dqopt->ops[type]->read_file_info(sb, type);
> if (error < 0) {
> - mutex_unlock(&dqopt->dqio_mutex);
> + up_write(&dqopt->dqio_sem);
> goto out_file_init;
> }
> if (dqopt->flags & DQUOT_QUOTA_SYS_FILE)
> dqopt->info[type].dqi_flags |= DQF_SYS_FILE;
> - mutex_unlock(&dqopt->dqio_mutex);
> + up_write(&dqopt->dqio_sem);
> spin_lock(&dq_state_lock);
> dqopt->flags |= dquot_state_flag(flags, type);
> spin_unlock(&dq_state_lock);
> diff --git a/fs/quota/quota_tree.c b/fs/quota/quota_tree.c
> index 0738972e8d3f..54f85eb2609c 100644
> --- a/fs/quota/quota_tree.c
> +++ b/fs/quota/quota_tree.c
> @@ -379,7 +379,7 @@ int qtree_write_dquot(struct qtree_mem_dqinfo *info, struct dquot *dquot)
> if (!ddquot)
> return -ENOMEM;
>
> - /* dq_off is guarded by dqio_mutex */
> + /* dq_off is guarded by dqio_sem */
> if (!dquot->dq_off) {
> ret = dq_insert_tree(info, dquot);
> if (ret < 0) {
> diff --git a/fs/super.c b/fs/super.c
> index 6bc3352adcf3..221cfa1f4e92 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -242,7 +242,7 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,
> atomic_set(&s->s_active, 1);
> mutex_init(&s->s_vfs_rename_mutex);
> lockdep_set_class(&s->s_vfs_rename_mutex, &type->s_vfs_rename_key);
> - mutex_init(&s->s_dquot.dqio_mutex);
> + init_rwsem(&s->s_dquot.dqio_sem);
> s->s_maxbytes = MAX_NON_LFS;
> s->s_op = &default_op;
> s->s_time_gran = 1000000000;
> diff --git a/include/linux/quota.h b/include/linux/quota.h
> index bfd077ca6ac3..3a6df7461642 100644
> --- a/include/linux/quota.h
> +++ b/include/linux/quota.h
> @@ -521,7 +521,7 @@ static inline void quota_send_warning(struct kqid qid, dev_t dev,
>
> struct quota_info {
> unsigned int flags; /* Flags for diskquotas on this device */
> - struct mutex dqio_mutex; /* lock device while I/O in progress */
> + struct rw_semaphore dqio_sem; /* Lock quota file while I/O in progress */
> struct inode *files[MAXQUOTAS]; /* inodes of quotafiles */
> struct mem_dqinfo info[MAXQUOTAS]; /* Information for each quota type */
> const struct quota_format_ops *ops[MAXQUOTAS]; /* Operations for each type */
> --
> 2.12.3
>
Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
