Currently each quota modification result in write_dquot()
and later dquot_commit(). This means what each quota modification
function must wait for dqio_mutex. Which is *huge* performance
penalty on big SMP systems. ASAIU The core idea of this implementation
is to guarantee that each quota modification will be written to journal
atomically. But in fact this is not always true, because dquot may be
changed after dquot modification, but before it was committed to disk.
| Task 1 | Task 2 |
| alloc_space(nr) | |
| ->spin_lock(dq_data_lock) | |
| ->curspace += nr | |
| ->spin_unlock(dq_data_lock) | |
| ->mark_dirty() | free_space(nr) |
| -->write_dquot() | ->spin_lock(dq_data_lock) |
| --->dquot_commit() | ->curspace -= nr |
| ---->commit_dqblk() | ->spin_unlock(dq_data_lock) |
| ----->spin_lock(dq_data_lock) | |
| ----->mem2disk_dqblk(ddq, dquot) | <<< Copy updated value |
| ----->spin_unlock(dq_data_lock) | |
| ----->quota_write() | |
Quota corruption not happens only because quota modification caller
started journal already. And ext3/4 allow only one running quota
at a time. Let's exploit this fact and avoid writing quota each time.
Act similar to dirty_for_io in general write_back path in page-cache.
If we have found what other task already started on copy and write the
dquot then we skip actual quota_write stage. And let that task do the job.
Side effect: Task which skip real quota_write() will not get an error
(if any). But this is not big deal because:
1) Any error has global consequences (RO_remount, err_print, etc).
2) Real IO is differed till the journall_commit.
Signed-off-by: Dmitry Monakhov <dmonakhov@xxxxxxxxxx>
---
fs/ext4/super.c | 37 ++++++++++++++++++++++++++++++-------
fs/quota/dquot.c | 16 +++++++++++++---
include/linux/quotaops.h | 1 +
3 files changed, 44 insertions(+), 10 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 3929091..164217e 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3777,14 +3777,37 @@ static int ext4_release_dquot(struct dquot *dquot)
static int ext4_mark_dquot_dirty(struct dquot *dquot)
{
- /* Are we journaling quotas? */
- if (EXT4_SB(dquot->dq_sb)->s_qf_names[USRQUOTA] ||
- EXT4_SB(dquot->dq_sb)->s_qf_names[GRPQUOTA]) {
- dquot_mark_dquot_dirty(dquot);
- return ext4_write_dquot(dquot);
- } else {
+ int ret, err;
+ handle_t *handle;
+ struct inode *inode;
+
+ /* Are we not journaling quotas? */
+ if (!EXT4_SB(dquot->dq_sb)->s_qf_names[USRQUOTA] &&
+ !EXT4_SB(dquot->dq_sb)->s_qf_names[GRPQUOTA])
return dquot_mark_dquot_dirty(dquot);
- }
+
+ /* journaling quotas case */
+ inode = dquot_to_inode(dquot);
+ handle = ext4_journal_start(inode,
+ EXT4_QUOTA_TRANS_BLOCKS(dquot->dq_sb));
+ if (IS_ERR(handle))
+ return PTR_ERR(handle);
+ if (!__dquot_mark_dquot_dirty(dquot))
+ ret = dquot_commit(dquot);
+ else
+ /*
+ * Dquot was already dirty. This means that other task already
+ * started a transaction but not clear dirty bit yet (see
+ * dquot_commit). Since the only one running transaction is
+ * possible at a time. Then that task belongs to the same
+ * transaction. We don'n have to actually write dquot changes
+ * because that task will write it for for us.
+ */
+ ret = 0;
+ err = ext4_journal_stop(handle);
+ if (!ret)
+ ret = err;
+ return ret;
}
static int ext4_write_info(struct super_block *sb, int type)
diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 6979224..e03eea0 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -311,14 +311,24 @@ static inline int mark_dquot_dirty(struct dquot *dquot)
{
return dquot->dq_sb->dq_op->mark_dirty(dquot);
}
-
-int dquot_mark_dquot_dirty(struct dquot *dquot)
+/* mark dquot dirty in atomic meaner, and return old dirty state */
+int __dquot_mark_dquot_dirty(struct dquot *dquot)
{
+ int ret = 1;
spin_lock(&dq_list_lock);
- if (!test_and_set_bit(DQ_MOD_B, &dquot->dq_flags))
+ if (!test_and_set_bit(DQ_MOD_B, &dquot->dq_flags)) {
list_add(&dquot->dq_dirty, &sb_dqopt(dquot->dq_sb)->
info[dquot->dq_type].dqi_dirty_list);
+ ret = 0;
+ }
spin_unlock(&dq_list_lock);
+ return ret;
+}
+EXPORT_SYMBOL(__dquot_mark_dquot_dirty);
+
+int dquot_mark_dquot_dirty(struct dquot *dquot)
+{
+ __dquot_mark_dquot_dirty(dquot);
return 0;
}
EXPORT_SYMBOL(dquot_mark_dquot_dirty);
diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h
index ae7ef25..c950f7d 100644
--- a/include/linux/quotaops.h
+++ b/include/linux/quotaops.h
@@ -56,6 +56,7 @@ int dquot_commit(struct dquot *dquot);
int dquot_acquire(struct dquot *dquot);
int dquot_release(struct dquot *dquot);
int dquot_commit_info(struct super_block *sb, int type);
+int __dquot_mark_dquot_dirty(struct dquot *dquot);
int dquot_mark_dquot_dirty(struct dquot *dquot);
int vfs_quota_on(struct super_block *sb, int type, int format_id,
--
1.6.0.4
--
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
