[LU-6378] Quota performance issue for 2.7 Created: 18/Mar/15 Updated: 12/May/16 Resolved: 29/Apr/15 |
|
| Status: | Resolved |
| Project: | Lustre |
| Component/s: | None |
| Affects Version/s: | Lustre 2.7.0, Lustre 2.8.0 |
| Fix Version/s: | Lustre 2.8.0 |
| Type: | Bug | Priority: | Major |
| Reporter: | Di Wang | Assignee: | nasf (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Issue Links: |
|
||||||||
| Severity: | 3 | ||||||||
| Rank (Obsolete): | 9223372036854775807 | ||||||||
| Description |
|
In lustre 2.7, inode I_NEW state is cleared in the later phase, which cause the following owner/group changing (in osd_attr_init0()) will be performed in I_NEW state, which means the dquot_initialize() won't be skipped for these operations, 2.7.0 /**
* Helper function for osd_object_create()
*
* \retval 0, on success
*/
static int __osd_object_create(struct osd_thread_info *info,
struct osd_object *obj, struct lu_attr *attr,
struct dt_allocation_hint *hint,
struct dt_object_format *dof,
struct thandle *th)
{
int result;
__u32 umask;
/* we drop umask so that permissions we pass are not affected */
umask = current->fs->umask;
current->fs->umask = 0;
result = osd_create_type_f(dof->dof_type)(info, obj, attr, hint, dof,
th);
if (result == 0) {
osd_attr_init(info, obj, attr, dof);
osd_object_init0(obj);
}
if (obj->oo_inode != NULL) {
LASSERT(obj->oo_inode->i_state & I_NEW);
unlock_new_inode(obj->oo_inode); --->It will unlock new inode here, so it will cause the above osd_attr_init to call quota_initialization (with I_NEW). But in 2.6.0, we used to unlock the new inode in ldiskfs_new_inode.
}
/* restore previous umask value */
current->fs->umask = umask;
return result;
}
It is caused by this patch |
| Comments |
| Comment by Niu Yawei (Inactive) [ 18/Mar/15 ] |
|
Actually, the quota-avoid-dqget-calls.patch shouldn't rely on I_NEW to skip the dqget()/dqput() calls, instead, it should check the i_dquot pointer directly. |
| Comment by nasf (Inactive) [ 18/Mar/15 ] |
|
Here is the patch: |
| Comment by Andreas Dilger [ 18/Mar/15 ] |
|
I think the correct fix is as Niu writes. The patch that was landed upstream didn't check I_NEW, but instead directly checked if the inode already had the quota initialized. The lustre/kernel_patches/patches/quota-avoid-dqget-calls patches should be updated to use the same code as was committed to the upstream kernel. commit 1ea06bec78a128adc995ca32bd906a6c9bb9cf91
Author: Niu Yawei <yawei.niu@gmail.com>
Date: Wed Jun 4 12:20:30 2014 +0800
quota: avoid unnecessary dqget()/dqput() calls
Avoid unnecessary dqget()/dqput() calls in __dquot_initialize(),
that will introduce global lock contention otherwise.
Signed-off-by: Lai Siyao <lai.siyao@intel.com>
Signed-off-by: Niu Yawei <yawei.niu@intel.com>
Signed-off-by: Jan Kara <jack@suse.cz>
diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 7f30bdc..2517719 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -1402,7 +1402,7 @@ static int dquot_active(const struct inode *inode)
*/
static void __dquot_initialize(struct inode *inode, int type)
{
- int cnt;
+ int cnt, init_needed = 0;
struct dquot *got[MAXQUOTAS];
struct super_block *sb = inode->i_sb;
qsize_t rsv;
@@ -1418,6 +1418,15 @@ static void __dquot_initialize(struct inode *inode, int type)
got[cnt] = NULL;
if (type != -1 && cnt != type)
continue;
+ /*
+ * The i_dquot should have been initialized in most cases,
+ * we check it without locking here to avoid unnecessary
+ * dqget()/dqput() calls.
+ */
+ if (inode->i_dquot[cnt])
+ continue;
+ init_needed = 1;
+
switch (cnt) {
case USRQUOTA:
qid = make_kqid_uid(inode->i_uid);
@@ -1429,6 +1438,10 @@ static void __dquot_initialize(struct inode *inode, int type)
got[cnt] = dqget(sb, qid);
}
+ /* All required i_dquot has been initialized */
+ if (!init_needed)
+ return;
+
down_write(&sb_dqopt(sb)->dqptr_sem);
if (IS_NOQUOTA(inode))
goto out_err;
|
| Comment by nasf (Inactive) [ 19/Mar/15 ] |
|
Since it has been committed to upstream, why still patch the kernel? As for the locked inode returned by ldiskfs_create_inode(), we can unlock it after the LDISKFS_STATE_LUSTRE_NOSCRUB set. It is unnecessary to hold the lock for osd_attr_init(). Anyway, I do not mind the way of fixing quota patch. |
| Comment by Andreas Dilger [ 19/Mar/15 ] |
|
It is only in the upstream kernel after 3.16 or so, but we are applying the older patch to RHEL 6 and SLES 11 that checks I_NEW. If the current patch doesn't cause problems with LFSCK, then I don't mind to land it also, because holding a lock for less time is always better (assuming it is still correct). I'd still like to fix the quota patch, because without that the performance will still be bad for non-root users. |
| Comment by nasf (Inactive) [ 19/Mar/15 ] |
|
I have refreshed the patch http://review.whamcloud.com/#/c/14102/ with this LU ticket. Niu, would you please to port your quota patch to master branch? Thanks! |
| Comment by Gerrit Updater [ 23/Mar/15 ] |
|
Niu Yawei (yawei.niu@intel.com) uploaded a new patch: http://review.whamcloud.com/14135 |
| Comment by Niu Yawei (Inactive) [ 23/Mar/15 ] |
Patch updated: http://review.whamcloud.com/14135 |
| Comment by Gerrit Updater [ 26/Mar/15 ] |
|
Oleg Drokin (oleg.drokin@intel.com) merged in patch http://review.whamcloud.com/14135/ |
| Comment by nasf (Inactive) [ 20/Apr/15 ] |
|
Oleg, would you please to consider the patch http://review.whamcloud.com/#/c/14102/, then we can close this ticket. Thanks! |
| Comment by Gerrit Updater [ 28/Apr/15 ] |
|
Oleg Drokin (oleg.drokin@intel.com) merged in patch http://review.whamcloud.com/14102/ |
| Comment by nasf (Inactive) [ 29/Apr/15 ] |
|
The patches have been landed to master. |