LU-5040 osd: fix osd declare credit for quota

osd_attr_set() always calls ll_vfs_dq_init() to initialize dquot for
the inode, while in some cases osd_declare_attr_set() does not reserve
credit for it.

This patch fixes this issue. This patch also corrects the quota credit
accounting in osd_declare_qid() by judging whether the inode's
i_dquot[quota_type] has been allocated, while old code judgement is
based on the file uid/gid existence, which is not correct.

Signed-off-by: Bobi Jam <bobijam.xu@intel.com>
Change-Id: I16555cb1097e1a3e75cdcb4852a2c5e1772ddd88
Reviewed-on: http://review.whamcloud.com/11097
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Jian Yu <jian.yu@intel.com>
Reviewed-by: Niu Yawei <yawei.niu@intel.com>
Reviewed-by: James Simmons <uja.ornl@gmail.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>

diff --git a/lustre/osd-ldiskfs/osd_handler.c b/lustre/osd-ldiskfs/osd_handler.c
index 5c2fcc7..e7cef42 100644 (file)
--- a/lustre/osd-ldiskfs/osd_handler.c
+++ b/lustre/osd-ldiskfs/osd_handler.c
@@ -1661,7 +1661,7 @@ static int osd_declare_attr_set(const struct lu_env *env,
        struct lquota_id_info  *qi = &info->oti_qi;
        long long               bspace;
        int                     rc = 0;
-       bool                    allocated;
+       bool                    enforce;
        ENTRY;
 
        LASSERT(dt != NULL);
@@ -1689,18 +1689,19 @@ static int osd_declare_attr_set(const struct lu_env *env,
         * We still need to call the osd_declare_qid() to calculate the journal
         * credits for updating quota accounting files and to trigger quota
         * space adjustment once the operation is completed.*/
-       if ((attr->la_valid & LA_UID) != 0 &&
-            attr->la_uid != obj->oo_inode->i_uid) {
+       if (attr->la_valid & LA_UID || attr->la_valid & LA_GID) {
+               /* USERQUOTA */
                qi->lqi_type = USRQUOTA;
-
+               enforce = (attr->la_valid & LA_UID) &&
+                         (attr->la_uid != obj->oo_inode->i_uid);
                /* inode accounting */
                qi->lqi_is_blk = false;
 
-               /* one more inode for the new owner ... */
+               /* one more inode for the new uid ... */
                qi->lqi_id.qid_uid = attr->la_uid;
                qi->lqi_space      = 1;
-               allocated = (attr->la_uid == 0) ? true : false;
-               rc = osd_declare_qid(env, oh, qi, allocated, NULL);
+               /* Reserve credits for the new uid */
+               rc = osd_declare_qid(env, oh, qi, NULL, enforce, NULL);
                if (rc == -EDQUOT || rc == -EINPROGRESS)
                        rc = 0;
                if (rc)
@@ -1709,7 +1710,7 @@ static int osd_declare_attr_set(const struct lu_env *env,
                /* and one less inode for the current uid */
                qi->lqi_id.qid_uid = obj->oo_inode->i_uid;
                qi->lqi_space      = -1;
-               rc = osd_declare_qid(env, oh, qi, true, NULL);
+               rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL);
                if (rc == -EDQUOT || rc == -EINPROGRESS)
                        rc = 0;
                if (rc)
@@ -1718,38 +1719,40 @@ static int osd_declare_attr_set(const struct lu_env *env,
                /* block accounting */
                qi->lqi_is_blk = true;
 
-               /* more blocks for the new owner ... */
+               /* more blocks for the new uid ... */
                qi->lqi_id.qid_uid = attr->la_uid;
                qi->lqi_space      = bspace;
-               allocated = (attr->la_uid == 0) ? true : false;
-               rc = osd_declare_qid(env, oh, qi, allocated, NULL);
+               /*
+                * Credits for the new uid has been reserved, re-use "obj"
+                * to save credit reservation.
+                */
+               rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL);
                if (rc == -EDQUOT || rc == -EINPROGRESS)
                        rc = 0;
                if (rc)
                        RETURN(rc);
 
-               /* and finally less blocks for the current owner */
+               /* and finally less blocks for the current uid */
                qi->lqi_id.qid_uid = obj->oo_inode->i_uid;
                qi->lqi_space      = -bspace;
-               rc = osd_declare_qid(env, oh, qi, true, NULL);
+               rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL);
                if (rc == -EDQUOT || rc == -EINPROGRESS)
                        rc = 0;
                if (rc)
                        RETURN(rc);
-       }
 
-       if (attr->la_valid & LA_GID &&
-           attr->la_gid != obj->oo_inode->i_gid) {
+               /* GROUP QUOTA */
                qi->lqi_type = GRPQUOTA;
+               enforce = (attr->la_valid & LA_GID) &&
+                         (attr->la_gid != obj->oo_inode->i_gid);
 
                /* inode accounting */
                qi->lqi_is_blk = false;
 
-               /* one more inode for the new group owner ... */
+               /* one more inode for the new gid ... */
                qi->lqi_id.qid_gid = attr->la_gid;
                qi->lqi_space      = 1;
-               allocated = (attr->la_gid == 0) ? true : false;
-               rc = osd_declare_qid(env, oh, qi, allocated, NULL);
+               rc = osd_declare_qid(env, oh, qi, NULL, enforce, NULL);
                if (rc == -EDQUOT || rc == -EINPROGRESS)
                        rc = 0;
                if (rc)
@@ -1758,7 +1761,7 @@ static int osd_declare_attr_set(const struct lu_env *env,
                /* and one less inode for the current gid */
                qi->lqi_id.qid_gid = obj->oo_inode->i_gid;
                qi->lqi_space      = -1;
-               rc = osd_declare_qid(env, oh, qi, true, NULL);
+               rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL);
                if (rc == -EDQUOT || rc == -EINPROGRESS)
                        rc = 0;
                if (rc)
@@ -1767,20 +1770,19 @@ static int osd_declare_attr_set(const struct lu_env *env,
                /* block accounting */
                qi->lqi_is_blk = true;
 
-               /* more blocks for the new owner ... */
+               /* more blocks for the new gid ... */
                qi->lqi_id.qid_gid = attr->la_gid;
                qi->lqi_space      = bspace;
-               allocated = (attr->la_gid == 0) ? true : false;
-               rc = osd_declare_qid(env, oh, qi, allocated, NULL);
+               rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL);
                if (rc == -EDQUOT || rc == -EINPROGRESS)
                        rc = 0;
                if (rc)
                        RETURN(rc);
 
-               /* and finally less blocks for the current owner */
+               /* and finally less blocks for the current gid */
                qi->lqi_id.qid_gid = obj->oo_inode->i_gid;
                qi->lqi_space      = -bspace;
-               rc = osd_declare_qid(env, oh, qi, true, NULL);
+               rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL);
                if (rc == -EDQUOT || rc == -EINPROGRESS)
                        rc = 0;
                if (rc)
@@ -1841,6 +1843,7 @@ static int osd_quota_transfer(struct inode *inode, const struct lu_attr *attr)
                struct iattr    iattr;
                int             rc;
 
+               ll_vfs_dq_init(inode);
                iattr.ia_valid = 0;
                if (attr->la_valid & LA_UID)
                        iattr.ia_valid |= ATTR_UID;
@@ -1905,7 +1908,6 @@ static int osd_attr_set(const struct lu_env *env,
        }
 
         inode = obj->oo_inode;
-       ll_vfs_dq_init(inode);
 
        rc = osd_quota_transfer(inode, attr);
        if (rc)
@@ -2299,7 +2301,7 @@ static int osd_declare_object_create(const struct lu_env *env,
                RETURN(0);
 
        rc = osd_declare_inode_qid(env, attr->la_uid, attr->la_gid, 1, oh,
-                                  false, false, NULL, false);
+                                  osd_dt_obj(dt), false, NULL, false);
        if (rc != 0)
                RETURN(rc);
 
@@ -2380,12 +2382,12 @@ static int osd_declare_object_destroy(const struct lu_env *env,
                             osd_dto_credits_noquota[DTO_INDEX_DELETE] + 3);
        /* one less inode */
        rc = osd_declare_inode_qid(env, inode->i_uid, inode->i_gid, -1, oh,
-                                  false, true, NULL, false);
+                                  obj, false, NULL, false);
        if (rc)
                RETURN(rc);
        /* data to be truncated */
        rc = osd_declare_inode_qid(env, inode->i_uid, inode->i_gid, 0, oh,
-                                  true, true, NULL, false);
+                                  obj, true, NULL, false);
        RETURN(rc);
 }
 
@@ -2858,6 +2860,7 @@ static int osd_declare_xattr_set(const struct lu_env *env,
 {
        struct osd_thandle *oh;
        int credits;
+       struct super_block *sb = osd_sb(osd_dev(dt->do_lu.lo_dev));
 
        LASSERT(handle != NULL);
 
@@ -2873,13 +2876,16 @@ static int osd_declare_xattr_set(const struct lu_env *env,
        } else if (strcmp(name, XATTR_NAME_VERSION) == 0) {
                credits = 1;
        } else {
-               struct osd_device  *osd = osd_dev(dt->do_lu.lo_dev);
-               struct super_block *sb = osd_sb(osd);
                credits = osd_dto_credits_noquota[DTO_XATTR_SET];
                if (buf && buf->lb_len > sb->s_blocksize) {
                        credits *= (buf->lb_len + sb->s_blocksize - 1) >>
                                        sb->s_blocksize_bits;
                }
+               /*
+                * xattr set may involve inode quota change, reserve credits for
+                * dquot_initialize()
+                */
+               oh->ot_credits += LDISKFS_MAXQUOTAS_INIT_BLOCKS(sb);
        }
 
        osd_trans_declare_op(env, oh, OSD_OT_XATTR_SET, credits);
@@ -2971,6 +2977,7 @@ static int osd_declare_xattr_del(const struct lu_env *env,
                                  struct thandle *handle)
 {
         struct osd_thandle *oh;
+       struct super_block *sb = osd_sb(osd_dev(dt->do_lu.lo_dev));
 
        LASSERT(dt_object_exists(dt) && !dt_object_remote(dt));
         LASSERT(handle != NULL);
@@ -2980,6 +2987,11 @@ static int osd_declare_xattr_del(const struct lu_env *env,
 
        osd_trans_declare_op(env, oh, OSD_OT_XATTR_SET,
                             osd_dto_credits_noquota[DTO_XATTR_SET]);
+       /*
+        * xattr del may involve inode quota change, reserve credits for
+        * dquot_initialize()
+        */
+       oh->ot_credits += LDISKFS_MAXQUOTAS_INIT_BLOCKS(sb);
 
        return 0;
 }
@@ -3440,7 +3452,7 @@ static int osd_index_declare_ea_delete(const struct lu_env *env,
        LASSERT(inode);
 
        rc = osd_declare_inode_qid(env, inode->i_uid, inode->i_gid, 0, oh,
-                                  true, true, NULL, false);
+                                  osd_dt_obj(dt), true, NULL, false);
        RETURN(rc);
 }
 
@@ -4231,7 +4243,8 @@ static int osd_index_declare_ea_insert(const struct lu_env *env,
                 * calculate how many blocks will be consumed by this index
                 * insert */
                rc = osd_declare_inode_qid(env, inode->i_uid, inode->i_gid, 0,
-                                          oh, true, true, NULL, false);
+                                          oh, osd_dt_obj(dt), true, NULL,
+                                          false);
        }
 
        if (fid == NULL)

diff --git a/lustre/osd-ldiskfs/osd_internal.h b/lustre/osd-ldiskfs/osd_internal.h
index 904ef5b..b1b05f9 100644 (file)
--- a/lustre/osd-ldiskfs/osd_internal.h
+++ b/lustre/osd-ldiskfs/osd_internal.h
@@ -712,10 +712,12 @@ loff_t find_tree_dqentry(const struct lu_env *env,
                          struct osd_it_quota *it);
 /* osd_quota.c */
 int osd_declare_qid(const struct lu_env *env, struct osd_thandle *oh,
-                   struct lquota_id_info *qi, bool allocated, int *flags);
+                   struct lquota_id_info *qi, struct osd_object *obj,
+                   bool enforce, int *flags);
 int osd_declare_inode_qid(const struct lu_env *env, qid_t uid, qid_t gid,
                          long long space, struct osd_thandle *oh,
-                         bool is_blk, bool allocated, int *flags, bool force);
+                         struct osd_object *obj, bool is_blk, int *flags,
+                         bool force);
 const struct dt_rec *osd_quota_pack(struct osd_object *obj,
                                    const struct dt_rec *rec,
                                    union lquota_rec *quota_rec);

diff --git a/lustre/osd-ldiskfs/osd_io.c b/lustre/osd-ldiskfs/osd_io.c
index 7e0bab8..4c59cee 100644 (file)
--- a/lustre/osd-ldiskfs/osd_io.c
+++ b/lustre/osd-ldiskfs/osd_io.c
@@ -711,8 +711,8 @@ static int osd_declare_write_commit(const struct lu_env *env,
        lnb[0].flags &= ~(OBD_BRW_OVER_USRQUOTA | OBD_BRW_OVER_GRPQUOTA);
 
        rc = osd_declare_inode_qid(env, inode->i_uid, inode->i_gid,
-                                  quota_space, oh, true, true, &flags,
-                                  ignore_quota);
+                                  quota_space, oh, osd_dt_obj(dt), true,
+                                  &flags, ignore_quota);
 
        /* we need only to store the overquota flags in the first lnb for
         * now, once we support multiple objects BRW, this code needs be
@@ -1119,7 +1119,7 @@ out:
         * objects, so always set the lqi_space as 0. */
        if (inode != NULL)
                rc = osd_declare_inode_qid(env, inode->i_uid, inode->i_gid,
-                                          0, oh, true, true, NULL, false);
+                                          0, oh, obj, true, NULL, false);
        RETURN(rc);
 }
 
@@ -1284,7 +1284,7 @@ static int osd_declare_punch(const struct lu_env *env, struct dt_object *dt,
        LASSERT(inode);
 
        rc = osd_declare_inode_qid(env, inode->i_uid, inode->i_gid, 0, oh,
-                                  true, true, NULL, false);
+                                  osd_dt_obj(dt), true, NULL, false);
        RETURN(rc);
 }
 

diff --git a/lustre/osd-ldiskfs/osd_quota.c b/lustre/osd-ldiskfs/osd_quota.c
index 917491d..c18d672 100644 (file)
--- a/lustre/osd-ldiskfs/osd_quota.c
+++ b/lustre/osd-ldiskfs/osd_quota.c
@@ -493,23 +493,26 @@ static inline void osd_qid_set_type(struct osd_thandle *oh, int i, int type)
  * Reserve journal credits for quota files update first, then call
  * ->op_begin() to perform quota enforcement.
  *
- * \param  env    - the environment passed by the caller
- * \param  oh     - osd transaction handle
- * \param  qi     - quota id & space required for this operation
- * \param  allocated - dquot entry in quota accounting file has been allocated
- * \param  flags  - if the operation is write, return no user quota, no
+ * \param  env     - the environment passed by the caller
+ * \param  oh      - osd transaction handle
+ * \param  qi      - quota id & space required for this operation
+ * \param  obj     - osd object, could be NULL when it's under create
+ * \param  enforce - whether to perform quota enforcement
+ * \param  flags   - if the operation is write, return no user quota, no
  *                  group quota, or sync commit flags to the caller
  *
- * \retval 0      - success
- * \retval -ve    - failure
+ * \retval 0       - success
+ * \retval -ve     - failure
  */
 int osd_declare_qid(const struct lu_env *env, struct osd_thandle *oh,
-                    struct lquota_id_info *qi, bool allocated, int *flags)
+                   struct lquota_id_info *qi, struct osd_object *obj,
+                   bool enforce, int *flags)
 {
        struct osd_thread_info  *info = osd_oti_get(env);
        struct osd_device       *dev = info->oti_dev;
        struct qsd_instance     *qsd = dev->od_quota_slave;
-       int                      i, rc;
+       struct inode            *inode = NULL;
+       int                      i, rc = 0;
        bool                     found = false;
        ENTRY;
 
@@ -532,8 +535,12 @@ int osd_declare_qid(const struct lu_env *env, struct osd_thandle *oh,
                        RETURN(-EOVERFLOW);
                }
 
+               if (obj != NULL)
+                       inode = obj->oo_inode;
                osd_trans_declare_op(env, oh, OSD_OT_QUOTA,
-                                    (allocated || qi->lqi_id.qid_uid == 0) ?
+                                    (qi->lqi_id.qid_uid == 0 ||
+                                     (inode != NULL &&
+                                      inode->i_dquot[qi->lqi_type] != NULL)) ?
                                     1: LDISKFS_QUOTA_INIT_BLOCKS(osd_sb(dev)));
 
                oh->ot_id_array[i] = qi->lqi_id.qid_uid;
@@ -546,7 +553,8 @@ int osd_declare_qid(const struct lu_env *env, struct osd_thandle *oh,
                RETURN(0);
 
        /* check quota */
-       rc = qsd_op_begin(env, qsd, oh->ot_quota_trans, qi, flags);
+       if (enforce)
+               rc = qsd_op_begin(env, qsd, oh->ot_quota_trans, qi, flags);
        RETURN(rc);
 }
 
@@ -558,8 +566,8 @@ int osd_declare_qid(const struct lu_env *env, struct osd_thandle *oh,
  * \param  gid    - group id of the inode
  * \param  space  - how many blocks/inodes will be consumed/released
  * \param  oh     - osd transaction handle
+ * \param  obj    - osd object, could be NULL when it's under create
  * \param  is_blk - block quota or inode quota?
- * \param  allocated - dquot entry in quota accounting file has been allocated
  * \param  flags  - if the operation is write, return no user quota, no
  *                  group quota, or sync commit flags to the caller
  * \param force   - set to 1 when changes are performed by root user and thus
@@ -570,7 +578,8 @@ int osd_declare_qid(const struct lu_env *env, struct osd_thandle *oh,
  */
 int osd_declare_inode_qid(const struct lu_env *env, qid_t uid, qid_t gid,
                          long long space, struct osd_thandle *oh,
-                         bool is_blk, bool allocated, int *flags, bool force)
+                         struct osd_object *obj, bool is_blk, int *flags,
+                         bool force)
 {
        struct osd_thread_info  *info = osd_oti_get(env);
        struct lquota_id_info   *qi = &info->oti_qi;
@@ -582,7 +591,7 @@ int osd_declare_inode_qid(const struct lu_env *env, qid_t uid, qid_t gid,
        qi->lqi_type       = USRQUOTA;
        qi->lqi_space      = space;
        qi->lqi_is_blk     = is_blk;
-       rcu = osd_declare_qid(env, oh, qi, allocated, flags);
+       rcu = osd_declare_qid(env, oh, qi, obj, true, flags);
 
        if (force && (rcu == -EDQUOT || rcu == -EINPROGRESS))
                /* ignore EDQUOT & EINPROGRESS when changes are done by root */
@@ -598,7 +607,7 @@ int osd_declare_inode_qid(const struct lu_env *env, qid_t uid, qid_t gid,
        /* and now group quota */
        qi->lqi_id.qid_gid = gid;
        qi->lqi_type       = GRPQUOTA;
-       rcg = osd_declare_qid(env, oh, qi, allocated, flags);
+       rcg = osd_declare_qid(env, oh, qi, obj, true, flags);
 
        if (force && (rcg == -EDQUOT || rcg == -EINPROGRESS))
                /* as before, ignore EDQUOT & EINPROGRESS for root */