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/11085
Tested-by: Jenkins
Reviewed-by: Niu Yawei <yawei.niu@intel.com>
Reviewed-by: Fan Yong <fan.yong@intel.com>
Tested-by: Maloo <hpdd-maloo@intel.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 03f1d3f..b542546 100644 (file)
--- a/lustre/osd-ldiskfs/osd_handler.c
+++ b/lustre/osd-ldiskfs/osd_handler.c
@@ -1742,7 +1742,7 @@ static int osd_declare_attr_set(const struct lu_env *env,
        qid_t                   gid;
        long long               bspace;
        int                     rc = 0;
-       bool                    allocated;
+       bool                    enforce;
        ENTRY;
 
        LASSERT(dt != NULL);
@@ -1770,18 +1770,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 != (uid = i_uid_read(obj->oo_inode))) {
+       if (attr->la_valid & LA_UID || attr->la_valid & LA_GID) {
+               /* USERQUOTA */
+               uid = i_uid_read(obj->oo_inode);
                qi->lqi_type = USRQUOTA;
-
+               enforce = (attr->la_valid & LA_UID) && (attr->la_uid != 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)
@@ -1790,7 +1791,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 = 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)
@@ -1799,38 +1800,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 = 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 != (gid = i_gid_read(obj->oo_inode))) {
+               /* GROUP QUOTA */
+               gid = i_gid_read(obj->oo_inode);
                qi->lqi_type = GRPQUOTA;
+               enforce = (attr->la_valid & LA_GID) && (attr->la_gid != 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)
@@ -1839,7 +1842,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 = 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)
@@ -1848,20 +1851,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 = 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)
@@ -1927,6 +1929,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;
@@ -1991,7 +1994,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)
@@ -2381,7 +2383,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);
 
@@ -2452,12 +2454,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, i_uid_read(inode), i_gid_read(inode),
-                                  -1, oh, false, true, NULL, false);
+                                  -1, oh, obj, false, NULL, false);
        if (rc)
                RETURN(rc);
        /* data to be truncated */
        rc = osd_declare_inode_qid(env, i_uid_read(inode), i_gid_read(inode),
-                                  0, oh, true, true, NULL, false);
+                                  0, oh, obj, true, NULL, false);
        RETURN(rc);
 }
 
@@ -2943,6 +2945,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);
 
@@ -2958,13 +2961,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);
@@ -3074,6 +3080,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);
@@ -3083,6 +3090,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;
 }
@@ -3552,7 +3564,7 @@ static int osd_index_declare_ea_delete(const struct lu_env *env,
        LASSERT(inode);
 
        rc = osd_declare_inode_qid(env, i_uid_read(inode), i_gid_read(inode),
-                                  0, oh, true, true, NULL, false);
+                                  0, oh, osd_dt_obj(dt), true, NULL, false);
        RETURN(rc);
 }
 
@@ -4404,8 +4416,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, i_uid_read(inode),
-                                          i_gid_read(inode), 0,
-                                          oh, true, true, NULL, false);
+                                          i_gid_read(inode), 0, 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 ce78da1..af14812 100644 (file)
--- a/lustre/osd-ldiskfs/osd_internal.h
+++ b/lustre/osd-ldiskfs/osd_internal.h
@@ -707,10 +707,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 ccb210c..9765595 100644 (file)
--- a/lustre/osd-ldiskfs/osd_io.c
+++ b/lustre/osd-ldiskfs/osd_io.c
@@ -1116,8 +1116,8 @@ static int osd_declare_write_commit(const struct lu_env *env,
        lnb[0].lnb_flags &= ~(OBD_BRW_OVER_USRQUOTA | OBD_BRW_OVER_GRPQUOTA);
 
        rc = osd_declare_inode_qid(env, i_uid_read(inode), i_gid_read(inode),
-                                  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
@@ -1536,8 +1536,8 @@ out:
         * objects, so always set the lqi_space as 0. */
        if (inode != NULL)
                rc = osd_declare_inode_qid(env, i_uid_read(inode),
-                                          i_gid_read(inode), 0, oh, true,
-                                          true, NULL, false);
+                                          i_gid_read(inode), 0, oh, obj, true,
+                                          NULL, false);
        RETURN(rc);
 }
 
@@ -1702,7 +1702,7 @@ static int osd_declare_punch(const struct lu_env *env, struct dt_object *dt,
        LASSERT(inode);
 
        rc = osd_declare_inode_qid(env, i_uid_read(inode), i_gid_read(inode),
-                                  0, oh, true, true, NULL, false);
+                                  0, oh, 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 8513bfa..4051640 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
- *                  group quota, or sync commit flags to the caller
+ * \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 */