| Message ID | 20170816154127.7048-6-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Aug 16, 2017, at 9:41 AM, Jan Kara <jack@suse.cz> wrote: > > Currently dquot writeout is only protected by dqio_sem held for writing. > As we transition to a finer grained locking we will use dquot->dq_lock > instead. So acquire it in dquot_commit() and move dqio_sem just around > ->commit_dqblk() call as it is still needed to serialize quota file > changes. > > Signed-off-by: Jan Kara <jack@suse.cz> > --- > fs/quota/dquot.c | 27 +++++++++++++-------------- > 1 file changed, 13 insertions(+), 14 deletions(-) > > diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c > index 3852a3c79ac9..99693c6d5dae 100644 > --- a/fs/quota/dquot.c > +++ b/fs/quota/dquot.c > @@ -110,14 +110,11 @@ > * sure they cannot race with quotaon which first sets S_NOQUOTA flag and > * then drops all pointers to dquots from an inode. > * > - * Each dquot has its dq_lock mutex. Locked dquots might not be referenced > - * from inodes (dquot_alloc_space() and such don't check the dq_lock). > - * Currently dquot is locked only when it is being read to memory (or space for > - * it is being allocated) on the first dqget() and when it is being released on > - * the last dqput(). The allocation and release oparations are serialized by > - * the dq_lock and by checking the use count in dquot_release(). Write > - * operations on dquots don't hold dq_lock as they copy data under dq_data_lock > - * spinlock to internal buffers before writing. > + * Each dquot has its dq_lock mutex. Dquot is locked when it is being read to > + * memory (or space for it is being allocated) on the first dqget(), when it is > + * being written out, and when it is being released on the last dqput(). The > + * allocation and release operations are serialized by the dq_lock and by > + * checking the use count in dquot_release(). > * > * Lock ordering (including related VFS locks) is the following: > * s_umount > i_mutex > journal_lock > dquot->dq_lock > dqio_sem > @@ -453,21 +450,23 @@ int dquot_commit(struct dquot *dquot) > int ret = 0; > struct quota_info *dqopt = sb_dqopt(dquot->dq_sb); > > - down_write(&dqopt->dqio_sem); > + mutex_lock(&dquot->dq_lock); > spin_lock(&dq_list_lock); > if (!clear_dquot_dirty(dquot)) { > spin_unlock(&dq_list_lock); > - goto out_sem; > + goto out_lock; > } > spin_unlock(&dq_list_lock); > /* Inactive dquot can be only if there was error during read/init > * => we have better not writing it */ > - if (test_bit(DQ_ACTIVE_B, &dquot->dq_flags)) > + if (test_bit(DQ_ACTIVE_B, &dquot->dq_flags)) { > + down_write(&dqopt->dqio_sem); > ret = dqopt->ops[dquot->dq_id.type]->commit_dqblk(dquot); > - else > + up_write(&dqopt->dqio_sem); > + } else > ret = -EIO; (style) typically braces are put around both branches of an if/else if either one needs it. Otherwise, looks fine to me. Reviewed-by: Andreas Dilger <adilger@dilger.ca> > -out_sem: > - up_write(&dqopt->dqio_sem); > +out_lock: > + mutex_unlock(&dquot->dq_lock); > return ret; > } > EXPORT_SYMBOL(dquot_commit); > -- > 2.12.3 > Cheers, Andreas
diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index 3852a3c79ac9..99693c6d5dae 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -110,14 +110,11 @@ * sure they cannot race with quotaon which first sets S_NOQUOTA flag and * then drops all pointers to dquots from an inode. * - * Each dquot has its dq_lock mutex. Locked dquots might not be referenced - * from inodes (dquot_alloc_space() and such don't check the dq_lock). - * Currently dquot is locked only when it is being read to memory (or space for - * it is being allocated) on the first dqget() and when it is being released on - * the last dqput(). The allocation and release oparations are serialized by - * the dq_lock and by checking the use count in dquot_release(). Write - * operations on dquots don't hold dq_lock as they copy data under dq_data_lock - * spinlock to internal buffers before writing. + * Each dquot has its dq_lock mutex. Dquot is locked when it is being read to + * memory (or space for it is being allocated) on the first dqget(), when it is + * being written out, and when it is being released on the last dqput(). The + * allocation and release operations are serialized by the dq_lock and by + * checking the use count in dquot_release(). * * Lock ordering (including related VFS locks) is the following: * s_umount > i_mutex > journal_lock > dquot->dq_lock > dqio_sem @@ -453,21 +450,23 @@ int dquot_commit(struct dquot *dquot) int ret = 0; struct quota_info *dqopt = sb_dqopt(dquot->dq_sb); - down_write(&dqopt->dqio_sem); + mutex_lock(&dquot->dq_lock); spin_lock(&dq_list_lock); if (!clear_dquot_dirty(dquot)) { spin_unlock(&dq_list_lock); - goto out_sem; + goto out_lock; } spin_unlock(&dq_list_lock); /* Inactive dquot can be only if there was error during read/init * => we have better not writing it */ - if (test_bit(DQ_ACTIVE_B, &dquot->dq_flags)) + if (test_bit(DQ_ACTIVE_B, &dquot->dq_flags)) { + down_write(&dqopt->dqio_sem); ret = dqopt->ops[dquot->dq_id.type]->commit_dqblk(dquot); - else + up_write(&dqopt->dqio_sem); + } else ret = -EIO; -out_sem: - up_write(&dqopt->dqio_sem); +out_lock: + mutex_unlock(&dquot->dq_lock); return ret; } EXPORT_SYMBOL(dquot_commit);
Currently dquot writeout is only protected by dqio_sem held for writing. As we transition to a finer grained locking we will use dquot->dq_lock instead. So acquire it in dquot_commit() and move dqio_sem just around ->commit_dqblk() call as it is still needed to serialize quota file changes. Signed-off-by: Jan Kara <jack@suse.cz> --- fs/quota/dquot.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)