All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/4] quota: add project quota support
@ 2014-09-10  3:52 Li Xi
  2014-09-10 16:45 ` Jan Kara
  0 siblings, 1 reply; 6+ messages in thread
From: Li Xi @ 2014-09-10  3:52 UTC (permalink / raw)
  To: linux-fsdevel, Ext4 Developers List, Theodore Ts'o,
	Andreas Dilger, viro, hch, Jan Kara, Dmitry Monakhov

Adds general codes to enforces project quota limits

This patch adds support for a new quota type PRJQUOTA for project quota
enforcement. Also a new method get_projid() is added into dquot_operations
structure.

Signed-off-by: Li Xi <lixi <at> ddn.com>
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
Index: linux.git/fs/quota/dquot.c
===================================================================
--- linux.git.orig/fs/quota/dquot.c
+++ linux.git/fs/quota/dquot.c
@@ -161,6 +161,19 @@ static struct quota_module_name module_n
 /* SLAB cache for dquot structures */
 static struct kmem_cache *dquot_cachep;

+static inline unsigned long compat_qtype2bits(int type)
+{
+#ifdef CONFIG_QUOTA_PROJECT
+    unsigned long qtype_bits = QUOTA_ALL_BIT;
+#else
+    unsigned long qtype_bits = QUOTA_USR_BIT | QUOTA_GRP_BIT;
+#endif
+    if (type != -1) {
+        qtype_bits = 1 << type;
+    }
+    return qtype_bits;
+}
+
 int register_quota_format(struct quota_format_type *fmt)
 {
     spin_lock(&dq_list_lock);
@@ -250,7 +263,8 @@ struct dqstats dqstats;
 EXPORT_SYMBOL(dqstats);

 static qsize_t inode_get_rsv_space(struct inode *inode);
-static void __dquot_initialize(struct inode *inode, int type);
+static void __dquot_initialize(struct inode *inode,
+                   unsigned long qtype_bits);

 static inline unsigned int
 hashfn(const struct super_block *sb, struct kqid qid)
@@ -513,7 +527,8 @@ static inline void do_destroy_dquot(stru
  * just deleted or pruned by prune_icache() (those are not attached to any
  * list) or parallel quotactl call. We have to wait for such users.
  */
-static void invalidate_dquots(struct super_block *sb, int type)
+static void invalidate_dquots(struct super_block *sb,
+                  unsigned long qtype_bits)
 {
     struct dquot *dquot, *tmp;

@@ -522,7 +537,7 @@ restart:
     list_for_each_entry_safe(dquot, tmp, &inuse_list, dq_inuse) {
         if (dquot->dq_sb != sb)
             continue;
-        if (dquot->dq_id.type != type)
+        if (((1 << dquot->dq_id.type) & qtype_bits) == 0)
             continue;
         /* Wait for dquot users */
         if (atomic_read(&dquot->dq_count)) {
@@ -605,7 +620,8 @@ out:
 EXPORT_SYMBOL(dquot_scan_active);

 /* Write all dquot structures to quota files */
-int dquot_writeback_dquots(struct super_block *sb, int type)
+static int __dquot_writeback_dquots(struct super_block *sb,
+                    unsigned long qtype_bits)
 {
     struct list_head *dirty;
     struct dquot *dquot;
@@ -615,7 +631,7 @@ int dquot_writeback_dquots(struct super_

     mutex_lock(&dqopt->dqonoff_mutex);
     for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-        if (type != -1 && cnt != type)
+        if (((1 << cnt) & qtype_bits) == 0)
             continue;
         if (!sb_has_quota_active(sb, cnt))
             continue;
@@ -645,7 +661,7 @@ int dquot_writeback_dquots(struct super_
     }

     for (cnt = 0; cnt < MAXQUOTAS; cnt++)
-        if ((cnt == type || type == -1) && sb_has_quota_active(sb, cnt)
+        if (((1 << cnt) & qtype_bits) && sb_has_quota_active(sb, cnt)
             && info_dirty(&dqopt->info[cnt]))
             sb->dq_op->write_info(sb, cnt);
     dqstats_inc(DQST_SYNCS);
@@ -653,16 +669,23 @@ int dquot_writeback_dquots(struct super_

     return ret;
 }
+
+int dquot_writeback_dquots(struct super_block *sb, int type)
+{
+    unsigned long qtype_bits = compat_qtype2bits(type);
+    return __dquot_writeback_dquots(sb, qtype_bits);
+}
 EXPORT_SYMBOL(dquot_writeback_dquots);

 /* Write all dquot structures to disk and make them visible from userspace */
-int dquot_quota_sync(struct super_block *sb, int type)
+static int __dquot_quota_sync(struct super_block *sb,
+                  unsigned long qtype_bits)
 {
     struct quota_info *dqopt = sb_dqopt(sb);
     int cnt;
     int ret;

-    ret = dquot_writeback_dquots(sb, type);
+    ret = dquot_writeback_dquots(sb, qtype_bits);
     if (ret)
         return ret;
     if (dqopt->flags & DQUOT_QUOTA_SYS_FILE)
@@ -681,7 +704,7 @@ int dquot_quota_sync(struct super_block
      */
     mutex_lock(&dqopt->dqonoff_mutex);
     for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-        if (type != -1 && cnt != type)
+        if (((1 << cnt) & qtype_bits) == 0)
             continue;
         if (!sb_has_quota_active(sb, cnt))
             continue;
@@ -693,6 +716,12 @@ int dquot_quota_sync(struct super_block

     return 0;
 }
+
+int dquot_quota_sync(struct super_block *sb, int type)
+{
+    unsigned long qtype_bits = compat_qtype2bits(type);
+    return __dquot_quota_sync(sb, qtype_bits);
+}
 EXPORT_SYMBOL(dquot_quota_sync);

 static unsigned long
@@ -897,17 +926,18 @@ out:
 }
 EXPORT_SYMBOL(dqget);

-static int dqinit_needed(struct inode *inode, int type)
+static int dqinit_needed(struct inode *inode, unsigned long qtype_bits)
 {
     int cnt;

     if (IS_NOQUOTA(inode))
         return 0;
-    if (type != -1)
-        return !inode->i_dquot[type];
-    for (cnt = 0; cnt < MAXQUOTAS; cnt++)
+    for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
+        if (((1 << cnt) & qtype_bits) == 0)
+            continue;
         if (!inode->i_dquot[cnt])
             return 1;
+    }
     return 0;
 }

@@ -924,7 +954,7 @@ static void add_dquot_ref(struct super_b
         spin_lock(&inode->i_lock);
         if ((inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) ||
             !atomic_read(&inode->i_writecount) ||
-            !dqinit_needed(inode, type)) {
+            !dqinit_needed(inode, 1 << type)) {
             spin_unlock(&inode->i_lock);
             continue;
         }
@@ -937,7 +967,7 @@ static void add_dquot_ref(struct super_b
             reserved = 1;
 #endif
         iput(old_inode);
-        __dquot_initialize(inode, type);
+        __dquot_initialize(inode, 1 << type);

         /*
          * We hold a reference to 'inode' so it couldn't have been
@@ -1170,8 +1200,12 @@ static int need_print_warning(struct dqu
             return uid_eq(current_fsuid(), warn->w_dq_id.uid);
         case GRPQUOTA:
             return in_group_p(warn->w_dq_id.gid);
-        case PRJQUOTA:    /* Never taken... Just make gcc happy */
+        case PRJQUOTA:
+#ifdef CONFIG_QUOTA_PROJECT
+            return 1;
+#else
             return 0;
+#endif
     }
     return 0;
 }
@@ -1400,7 +1434,7 @@ static int dquot_active(const struct ino
  * It is better to call this function outside of any transaction as it
  * might need a lot of space in journal for dquot structure allocation.
  */
-static void __dquot_initialize(struct inode *inode, int type)
+static void __dquot_initialize(struct inode *inode, unsigned long qtype_bits)
 {
     int cnt;
     struct dquot *got[MAXQUOTAS];
@@ -1415,8 +1449,13 @@ static void __dquot_initialize(struct in
     /* First get references to structures we might need. */
     for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
         struct kqid qid;
+#ifdef CONFIG_QUOTA_PROJECT
+        kprojid_t projid;
+        int rc;
+#endif
+
         got[cnt] = NULL;
-        if (type != -1 && cnt != type)
+        if (((1 << cnt) & qtype_bits) == 0)
             continue;
         switch (cnt) {
         case USRQUOTA:
@@ -1425,6 +1464,19 @@ static void __dquot_initialize(struct in
         case GRPQUOTA:
             qid = make_kqid_gid(inode->i_gid);
             break;
+        case PRJQUOTA:
+#ifdef CONFIG_QUOTA_PROJECT
+            /* Project ID is not supported */
+            if (!inode->i_sb->dq_op->get_projid)
+                continue;
+            rc = inode->i_sb->dq_op->get_projid(inode, &projid);
+            if (rc)
+                continue;
+            qid = make_kqid_projid(projid);
+#else
+            continue;
+#endif
+            break;
         }
         got[cnt] = dqget(sb, qid);
     }
@@ -1433,7 +1485,7 @@ static void __dquot_initialize(struct in
     if (IS_NOQUOTA(inode))
         goto out_err;
     for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-        if (type != -1 && cnt != type)
+        if (((1 << cnt) & qtype_bits) == 0)
             continue;
         /* Avoid races with quotaoff() */
         if (!sb_has_quota_active(sb, cnt))
@@ -1464,7 +1516,12 @@ out_err:

 void dquot_initialize(struct inode *inode)
 {
-    __dquot_initialize(inode, -1);
+#ifdef CONFIG_QUOTA_PROJECT
+    unsigned long qtype_bits = QUOTA_ALL_BIT;
+#else
+    unsigned long qtype_bits = QUOTA_USR_BIT | QUOTA_GRP_BIT;
+#endif
+    __dquot_initialize(inode, qtype_bits);
 }
 EXPORT_SYMBOL(dquot_initialize);

@@ -2005,9 +2062,10 @@ int dquot_file_open(struct inode *inode,
 EXPORT_SYMBOL(dquot_file_open);

 /*
- * Turn quota off on a device. type == -1 ==> quotaoff for all types (umount)
+ * Turn quota off on a device. (umount)
  */
-int dquot_disable(struct super_block *sb, int type, unsigned int flags)
+static int __dquot_disable(struct super_block *sb, unsigned long qtype_bits,
+               unsigned int flags)
 {
     int cnt, ret = 0;
     struct quota_info *dqopt = sb_dqopt(sb);
@@ -2034,7 +2092,7 @@ int dquot_disable(struct super_block *sb
     }
     for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
         toputinode[cnt] = NULL;
-        if (type != -1 && cnt != type)
+        if (((1 << cnt) & qtype_bits) == 0)
             continue;
         if (!sb_has_quota_loaded(sb, cnt))
             continue;
@@ -2066,7 +2124,7 @@ int dquot_disable(struct super_block *sb

         /* Note: these are blocking operations */
         drop_dquot_ref(sb, cnt);
-        invalidate_dquots(sb, cnt);
+        invalidate_dquots(sb, 1 << cnt);
         /*
          * Now all dquots should be invalidated, all writes done so we
          * should be only users of the info. No locks needed.
@@ -2136,6 +2194,13 @@ put_inodes:
         }
     return ret;
 }
+int dquot_disable(struct super_block *sb, int type,
+          unsigned int flags)
+{
+    unsigned long qtype_bits = compat_qtype2bits(type);
+    return __dquot_disable(sb, qtype_bits, flags);
+}
+
 EXPORT_SYMBOL(dquot_disable);

 int dquot_quota_off(struct super_block *sb, int type)
@@ -2264,7 +2329,7 @@ out_fmt:
 }

 /* Reenable quotas on remount RW */
-int dquot_resume(struct super_block *sb, int type)
+static int __dquot_resume(struct super_block *sb, unsigned long qtype_bits)
 {
     struct quota_info *dqopt = sb_dqopt(sb);
     struct inode *inode;
@@ -2272,7 +2337,7 @@ int dquot_resume(struct super_block *sb,
     unsigned int flags;

     for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-        if (type != -1 && cnt != type)
+        if (((1 << cnt) & qtype_bits) == 0)
             continue;

         mutex_lock(&dqopt->dqonoff_mutex);
@@ -2298,6 +2363,12 @@ int dquot_resume(struct super_block *sb,

     return ret;
 }
+
+int dquot_resume(struct super_block *sb, int type)
+{
+    unsigned long qtype_bits = compat_qtype2bits(type);
+    return __dquot_resume(sb, qtype_bits);
+}
 EXPORT_SYMBOL(dquot_resume);

 int dquot_quota_on(struct super_block *sb, int type, int format_id,
Index: linux.git/fs/quota/quotaio_v2.h
===================================================================
--- linux.git.orig/fs/quota/quotaio_v2.h
+++ linux.git/fs/quota/quotaio_v2.h
@@ -13,12 +13,14 @@
  */
 #define V2_INITQMAGICS {\
     0xd9c01f11,    /* USRQUOTA */\
-    0xd9c01927    /* GRPQUOTA */\
+    0xd9c01927,    /* GRPQUOTA */\
+    0xd9c03f14    /* PRJQUOTA */\
 }

 #define V2_INITQVERSIONS {\
     1,        /* USRQUOTA */\
-    1        /* GRPQUOTA */\
+    1,        /* GRPQUOTA */\
+    1        /* PRJQUOTA */\
 }

 /* First generic header */
Index: linux.git/include/linux/quota.h
===================================================================
--- linux.git.orig/include/linux/quota.h
+++ linux.git/include/linux/quota.h
@@ -50,12 +50,18 @@

 #undef USRQUOTA
 #undef GRPQUOTA
+#undef PRJQUOTA
 enum quota_type {
     USRQUOTA = 0,        /* element used for user quotas */
     GRPQUOTA = 1,        /* element used for group quotas */
     PRJQUOTA = 2,        /* element used for project quotas */
 };

+#define QUOTA_USR_BIT (1 << USRQUOTA)
+#define QUOTA_GRP_BIT (1 << GRPQUOTA)
+#define QUOTA_PRJ_BIT (1 << PRJQUOTA)
+#define QUOTA_ALL_BIT (QUOTA_USR_BIT | QUOTA_GRP_BIT | QUOTA_PRJ_BIT)
+
 typedef __kernel_uid32_t qid_t; /* Type in which we store ids in memory */
 typedef long long qsize_t;    /* Type in which we store sizes */

@@ -312,6 +318,9 @@ struct dquot_operations {
     /* get reserved quota for delayed alloc, value returned is managed by
      * quota code only */
     qsize_t *(*get_reserved_space) (struct inode *);
+#ifdef CONFIG_QUOTA_PROJECT
+    int (*get_projid) (struct inode *, kprojid_t *);/* Get project ID */
+#endif
 };

 struct path;
Index: linux.git/include/uapi/linux/quota.h
===================================================================
--- linux.git.orig/include/uapi/linux/quota.h
+++ linux.git/include/uapi/linux/quota.h
@@ -36,11 +36,12 @@
 #include <linux/errno.h>
 #include <linux/types.h>

-#define __DQUOT_VERSION__    "dquot_6.5.2"
+#define __DQUOT_VERSION__    "dquot_6.6.0"

-#define MAXQUOTAS 2
+#define MAXQUOTAS 3
 #define USRQUOTA  0        /* element used for user quotas */
 #define GRPQUOTA  1        /* element used for group quotas */
+#define PRJQUOTA  2        /* element used for project quotas */

 /*
  * Definitions for the default names of the quotas files.
@@ -48,6 +49,7 @@
 #define INITQFNAMES { \
     "user",    /* USRQUOTA */ \
     "group",   /* GRPQUOTA */ \
+    "project", /* PRJQUOTA */ \
     "undefined", \
 };

Index: linux.git/fs/quota/Kconfig
===================================================================
--- linux.git.orig/fs/quota/Kconfig
+++ linux.git/fs/quota/Kconfig
@@ -17,6 +17,15 @@ config QUOTA
       with the quota tools. Probably the quota support is only useful for
       multi user systems. If unsure, say N.

+config QUOTA_PROJECT
+    bool "Enable project quota"
+    depends on QUOTA
+    default y
+    help
+      This option enables project inode identifier. Project id
+      may be used as auxiliary owner specifier in addition to
+      standard uid/gid.
+
 config QUOTA_NETLINK_INTERFACE
     bool "Report quota messages through netlink interface"
     depends on QUOTACTL && NET
Index: linux.git/fs/quota/quota.c
===================================================================
--- linux.git.orig/fs/quota/quota.c
+++ linux.git/fs/quota/quota.c
@@ -30,7 +30,10 @@ static int check_quotactl_permission(str
     case Q_XGETQSTATV:
     case Q_XQUOTASYNC:
         break;
-    /* allow to query information for dquots we "own" */
+    /*
+     * allow to query information for dquots we "own"
+     * always allow quota check for project quota
+     */
     case Q_GETQUOTA:
     case Q_XGETQUOTA:
         if ((type == USRQUOTA && uid_eq(current_euid(),
make_kuid(current_user_ns(), id))) ||

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/4] quota: add project quota support
  2014-09-10  3:52 [PATCH v3 1/4] quota: add project quota support Li Xi
@ 2014-09-10 16:45 ` Jan Kara
  2014-09-16  8:15   ` Li Xi
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kara @ 2014-09-10 16:45 UTC (permalink / raw)
  To: Li Xi
  Cc: linux-fsdevel, Ext4 Developers List, Theodore Ts'o,
	Andreas Dilger, viro, hch, Jan Kara, Dmitry Monakhov

On Wed 10-09-14 11:52:35, Li Xi wrote:
> Adds general codes to enforces project quota limits
> 
> This patch adds support for a new quota type PRJQUOTA for project quota
> enforcement. Also a new method get_projid() is added into dquot_operations
> structure.
> 
  One general note:
  Your mail client has apparently mangled the patch (replaced tabs with
spaces). Please resend patches using git-send-email or some other email
client that doesn't do this so that they can be applied cleanly.

  Some other comments below.

> Signed-off-by: Li Xi <lixi <at> ddn.com>
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> ---
> Index: linux.git/fs/quota/dquot.c
> ===================================================================
> --- linux.git.orig/fs/quota/dquot.c
> +++ linux.git/fs/quota/dquot.c
> @@ -161,6 +161,19 @@ static struct quota_module_name module_n
>  /* SLAB cache for dquot structures */
>  static struct kmem_cache *dquot_cachep;
> 
> +static inline unsigned long compat_qtype2bits(int type)
> +{
> +#ifdef CONFIG_QUOTA_PROJECT
  I don't find CONFIG_QUOTA_PROJECT all that useful. If someone is building
a kernel with CONFIG_QUOTA enabled (i.e., a kernel for a server), he can
well spare those few kilobytes for project quota support and it makes the
code somewhat nicer. So I would just remove this config option.

> +    unsigned long qtype_bits = QUOTA_ALL_BIT;
> +#else
> +    unsigned long qtype_bits = QUOTA_USR_BIT | QUOTA_GRP_BIT;
> +#endif
> +    if (type != -1) {
> +        qtype_bits = 1 << type;
> +    }
> +    return qtype_bits;
> +}
> +
>  int register_quota_format(struct quota_format_type *fmt)
>  {
>      spin_lock(&dq_list_lock);
> @@ -250,7 +263,8 @@ struct dqstats dqstats;
>  EXPORT_SYMBOL(dqstats);
> 
>  static qsize_t inode_get_rsv_space(struct inode *inode);
> -static void __dquot_initialize(struct inode *inode, int type);
> +static void __dquot_initialize(struct inode *inode,
> +                   unsigned long qtype_bits);
  I've noticed you are changing interface of several functions from taking
a type number (or -1) to taking a bitmask. I guess it's because of
CONFIG_QUOTA_PROJECT or is there also any other reason? If we get rid of
that config, we won't need this change either, right?

...
> Index: linux.git/include/uapi/linux/quota.h
> ===================================================================
> --- linux.git.orig/include/uapi/linux/quota.h
> +++ linux.git/include/uapi/linux/quota.h
> @@ -36,11 +36,12 @@
>  #include <linux/errno.h>
>  #include <linux/types.h>
> 
> -#define __DQUOT_VERSION__    "dquot_6.5.2"
> +#define __DQUOT_VERSION__    "dquot_6.6.0"
> 
> -#define MAXQUOTAS 2
> +#define MAXQUOTAS 3
  Hum, actually this isn't so simple. MAXQUOTAS is used in several
filesystems - ext3, ext4, ocfs2, reiserfs, gfs2 - and just bumping up
MAXQUOTAS can have unexpected consequences for them (they won't have
properly initialized data structures for new quota type). So what we have
to do as a preparatory step is to make these filesystems define their own
MAXQUOTAS value (like EXT3_MAXQUOTAS, ...). I'll take care of that.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/4] quota: add project quota support
  2014-09-10 16:45 ` Jan Kara
@ 2014-09-16  8:15   ` Li Xi
  2014-09-16 19:58     ` Jan Kara
  0 siblings, 1 reply; 6+ messages in thread
From: Li Xi @ 2014-09-16  8:15 UTC (permalink / raw)
  To: Jan Kara
  Cc: linux-fsdevel, Ext4 Developers List, Theodore Ts'o,
	Andreas Dilger, viro, hch, Dmitry Monakhov

On Thu, Sep 11, 2014 at 12:45 AM, Jan Kara <jack@suse.cz> wrote:
> On Wed 10-09-14 11:52:35, Li Xi wrote:
>> Adds general codes to enforces project quota limits
>>
>> This patch adds support for a new quota type PRJQUOTA for project quota
>> enforcement. Also a new method get_projid() is added into dquot_operations
>> structure.
>>
>   One general note:
>   Your mail client has apparently mangled the patch (replaced tabs with
> spaces). Please resend patches using git-send-email or some other email
> client that doesn't do this so that they can be applied cleanly.
>
>   Some other comments below.
>
>> Signed-off-by: Li Xi <lixi <at> ddn.com>
>> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
>> ---
>> Index: linux.git/fs/quota/dquot.c
>> ===================================================================
>> --- linux.git.orig/fs/quota/dquot.c
>> +++ linux.git/fs/quota/dquot.c
>> @@ -161,6 +161,19 @@ static struct quota_module_name module_n
>>  /* SLAB cache for dquot structures */
>>  static struct kmem_cache *dquot_cachep;
>>
>> +static inline unsigned long compat_qtype2bits(int type)
>> +{
>> +#ifdef CONFIG_QUOTA_PROJECT
>   I don't find CONFIG_QUOTA_PROJECT all that useful. If someone is building
> a kernel with CONFIG_QUOTA enabled (i.e., a kernel for a server), he can
> well spare those few kilobytes for project quota support and it makes the
> code somewhat nicer. So I would just remove this config option.
>
>> +    unsigned long qtype_bits = QUOTA_ALL_BIT;
>> +#else
>> +    unsigned long qtype_bits = QUOTA_USR_BIT | QUOTA_GRP_BIT;
>> +#endif
>> +    if (type != -1) {
>> +        qtype_bits = 1 << type;
>> +    }
>> +    return qtype_bits;
>> +}
>> +
>>  int register_quota_format(struct quota_format_type *fmt)
>>  {
>>      spin_lock(&dq_list_lock);
>> @@ -250,7 +263,8 @@ struct dqstats dqstats;
>>  EXPORT_SYMBOL(dqstats);
>>
>>  static qsize_t inode_get_rsv_space(struct inode *inode);
>> -static void __dquot_initialize(struct inode *inode, int type);
>> +static void __dquot_initialize(struct inode *inode,
>> +                   unsigned long qtype_bits);
>   I've noticed you are changing interface of several functions from taking
> a type number (or -1) to taking a bitmask. I guess it's because of
> CONFIG_QUOTA_PROJECT or is there also any other reason? If we get rid of
> that config, we won't need this change either, right?
>
> ...
>> Index: linux.git/include/uapi/linux/quota.h
>> ===================================================================
>> --- linux.git.orig/include/uapi/linux/quota.h
>> +++ linux.git/include/uapi/linux/quota.h
>> @@ -36,11 +36,12 @@
>>  #include <linux/errno.h>
>>  #include <linux/types.h>
>>
>> -#define __DQUOT_VERSION__    "dquot_6.5.2"
>> +#define __DQUOT_VERSION__    "dquot_6.6.0"
>>
>> -#define MAXQUOTAS 2
>> +#define MAXQUOTAS 3
>   Hum, actually this isn't so simple. MAXQUOTAS is used in several
> filesystems - ext3, ext4, ocfs2, reiserfs, gfs2 - and just bumping up
> MAXQUOTAS can have unexpected consequences for them (they won't have
> properly initialized data structures for new quota type). So what we have
> to do as a preparatory step is to make these filesystems define their own
> MAXQUOTAS value (like EXT3_MAXQUOTAS, ...). I'll take care of that.
Yeah, you are right. It is likely that a new MAXQUOTAS value will hurt other
file systems. And I saw your patch for it. I will use EXT4_MAXQUOTAS
in Ext4 instead.

However, the general codes in fs/quota or head files like quotaops.h use
MAXQUOTAS heavily too. I guess I have no better choice but replace
MAXQUOTAS there with a new macro, e.g. MAXQUOTAS_NEW (3). I will handle
the interfaces carefully so that they remain exactly the same semantics. Do you
have any better idea?

The reason why I replaced the sepcail type number (i.e. -1) with a bitmask is
that I thought there might be some cases when we want to operate on some
of the quota types rather than all of them. When the number of quota
types was 2,
-1 is alright. But since we are using 3 quota types, it becomes a
problem. In order
to keep the compatibility of quota interfaces, we need to pass only
QUOTA_USR_BIT | QUOTA_GRP_BIT to functions like __dquot_initialize().
But for ext4, we need to pass QUOTA_ALL_BIT. This is an important use case
which -1 quota type is not able to satisfy.

So, in general, I will:
1) keep MAXQUOTAS value as 2;
2) replace MAXQUOTAS in genral codes with MAXQUOTAS_NEW (3);
3) use EXT4_MAXQUOTAS in Ext4 codes.

Please let me know if you have any suggestions.

Regards,
                                                         - Li Xi

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/4] quota: add project quota support
  2014-09-16  8:15   ` Li Xi
@ 2014-09-16 19:58     ` Jan Kara
  2014-09-17  3:02       ` Li Xi
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kara @ 2014-09-16 19:58 UTC (permalink / raw)
  To: Li Xi
  Cc: Jan Kara, linux-fsdevel, Ext4 Developers List, Theodore Ts'o,
	Andreas Dilger, viro, hch, Dmitry Monakhov

On Tue 16-09-14 16:15:35, Li Xi wrote:
> On Thu, Sep 11, 2014 at 12:45 AM, Jan Kara <jack@suse.cz> wrote:
> >> Index: linux.git/include/uapi/linux/quota.h
> >> ===================================================================
> >> --- linux.git.orig/include/uapi/linux/quota.h
> >> +++ linux.git/include/uapi/linux/quota.h
> >> @@ -36,11 +36,12 @@
> >>  #include <linux/errno.h>
> >>  #include <linux/types.h>
> >>
> >> -#define __DQUOT_VERSION__    "dquot_6.5.2"
> >> +#define __DQUOT_VERSION__    "dquot_6.6.0"
> >>
> >> -#define MAXQUOTAS 2
> >> +#define MAXQUOTAS 3
> >   Hum, actually this isn't so simple. MAXQUOTAS is used in several
> > filesystems - ext3, ext4, ocfs2, reiserfs, gfs2 - and just bumping up
> > MAXQUOTAS can have unexpected consequences for them (they won't have
> > properly initialized data structures for new quota type). So what we have
> > to do as a preparatory step is to make these filesystems define their own
> > MAXQUOTAS value (like EXT3_MAXQUOTAS, ...). I'll take care of that.
> Yeah, you are right. It is likely that a new MAXQUOTAS value will hurt other
> file systems. And I saw your patch for it. I will use EXT4_MAXQUOTAS
> in Ext4 instead.
> 
> However, the general codes in fs/quota or head files like quotaops.h use
> MAXQUOTAS heavily too. I guess I have no better choice but replace
> MAXQUOTAS there with a new macro, e.g. MAXQUOTAS_NEW (3). I will handle
> the interfaces carefully so that they remain exactly the same semantics. Do you
> have any better idea?
  The idea is that MAXQUOTAS is the number of quota types supported by VFS.
So you should really increase MAXQUOTAS in your patch because VFS will now
support three quota types. You should make sure that all places that use
MAXQUOTAS in fs/quota/ are safe with that change and fix them if not.

> The reason why I replaced the sepcail type number (i.e. -1) with a
> bitmask is that I thought there might be some cases when we want to
> operate on some of the quota types rather than all of them. When the
> number of quota types was 2, -1 is alright. But since we are using 3
> quota types, it becomes a problem. In order to keep the compatibility of
> quota interfaces, we need to pass only QUOTA_USR_BIT | QUOTA_GRP_BIT to
> functions like __dquot_initialize().  But for ext4, we need to pass
> QUOTA_ALL_BIT. This is an important use case which -1 quota type is not
> able to satisfy.
  Yes, I understand current calling convention isn't able to express
"QUOTA_USR_BIT | QUOTA_GRP_BIT" but I don't see a place where we would
really need that. All the places I'm aware of either want just one quota
type or all of them. That's why I'm asking whether you really need to
express just two types out of three.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/4] quota: add project quota support
  2014-09-16 19:58     ` Jan Kara
@ 2014-09-17  3:02       ` Li Xi
  2014-09-17 12:17         ` Jan Kara
  0 siblings, 1 reply; 6+ messages in thread
From: Li Xi @ 2014-09-17  3:02 UTC (permalink / raw)
  To: Jan Kara
  Cc: linux-fsdevel, Ext4 Developers List, Theodore Ts'o,
	Andreas Dilger, viro, hch, Dmitry Monakhov

On Wed, Sep 17, 2014 at 3:58 AM, Jan Kara <jack@suse.cz> wrote:
> On Tue 16-09-14 16:15:35, Li Xi wrote:
>> On Thu, Sep 11, 2014 at 12:45 AM, Jan Kara <jack@suse.cz> wrote:
>> >> Index: linux.git/include/uapi/linux/quota.h
>> >> ===================================================================
>> >> --- linux.git.orig/include/uapi/linux/quota.h
>> >> +++ linux.git/include/uapi/linux/quota.h
>> >> @@ -36,11 +36,12 @@
>> >>  #include <linux/errno.h>
>> >>  #include <linux/types.h>
>> >>
>> >> -#define __DQUOT_VERSION__    "dquot_6.5.2"
>> >> +#define __DQUOT_VERSION__    "dquot_6.6.0"
>> >>
>> >> -#define MAXQUOTAS 2
>> >> +#define MAXQUOTAS 3
>> >   Hum, actually this isn't so simple. MAXQUOTAS is used in several
>> > filesystems - ext3, ext4, ocfs2, reiserfs, gfs2 - and just bumping up
>> > MAXQUOTAS can have unexpected consequences for them (they won't have
>> > properly initialized data structures for new quota type). So what we have
>> > to do as a preparatory step is to make these filesystems define their own
>> > MAXQUOTAS value (like EXT3_MAXQUOTAS, ...). I'll take care of that.
>> Yeah, you are right. It is likely that a new MAXQUOTAS value will hurt other
>> file systems. And I saw your patch for it. I will use EXT4_MAXQUOTAS
>> in Ext4 instead.
>>
>> However, the general codes in fs/quota or head files like quotaops.h use
>> MAXQUOTAS heavily too. I guess I have no better choice but replace
>> MAXQUOTAS there with a new macro, e.g. MAXQUOTAS_NEW (3). I will handle
>> the interfaces carefully so that they remain exactly the same semantics. Do you
>> have any better idea?
>   The idea is that MAXQUOTAS is the number of quota types supported by VFS.
> So you should really increase MAXQUOTAS in your patch because VFS will now
> support three quota types. You should make sure that all places that use
> MAXQUOTAS in fs/quota/ are safe with that change and fix them if not.
Sorry, I am confused here. Would you please explain more? As you mentioned
before, MAXQUOTAS is a so critial macro that we can't change it without taking
care of other file systems. So, should I update MAXQUOTAS directly or not?

I would prefer to update MAXQUOTAS to 3 rather than add MAXQUOTAS_NEW
or hack in other ways. I understand that MAXQUOTAS has been 2 from the first
begining, and it is possible that some codes take its invariance for granted for
too long. But if a file system is using MAXQUOTAS in a wrong way, maybe it
is possible for us to find and fix the problems after this patch is merged, yet
before Linux is released as next major version?

Another solution would be to replace MAXQUOTAS in these file systems to
${FS}_MAXQUOTAS. It won't cost much time to prepare those patches. And
If these patches can be accepted and merged quickly, it would a safer
solution. For example, that would prevent Ext3 from allocating unnecessary
transaction blocks because MAXQUOTAS is increased. I checked other
places where MAXQUOTAS is used. I haven't found any problems yet. But
as you said, we are not sure whether there will be any critical problem.

I am currently hesitating between choices. So, what should I do exactly in
your opinion?
>
>> The reason why I replaced the sepcail type number (i.e. -1) with a
>> bitmask is that I thought there might be some cases when we want to
>> operate on some of the quota types rather than all of them. When the
>> number of quota types was 2, -1 is alright. But since we are using 3
>> quota types, it becomes a problem. In order to keep the compatibility of
>> quota interfaces, we need to pass only QUOTA_USR_BIT | QUOTA_GRP_BIT to
>> functions like __dquot_initialize().  But for ext4, we need to pass
>> QUOTA_ALL_BIT. This is an important use case which -1 quota type is not
>> able to satisfy.
>   Yes, I understand current calling convention isn't able to express
> "QUOTA_USR_BIT | QUOTA_GRP_BIT" but I don't see a place where we would
> really need that. All the places I'm aware of either want just one quota
> type or all of them. That's why I'm asking whether you really need to
> express just two types out of three.
Sure, I will avoid changing these codes if it turns out unnecessary.

Regards,
Li Xi

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/4] quota: add project quota support
  2014-09-17  3:02       ` Li Xi
@ 2014-09-17 12:17         ` Jan Kara
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2014-09-17 12:17 UTC (permalink / raw)
  To: Li Xi
  Cc: Jan Kara, linux-fsdevel, Ext4 Developers List, Theodore Ts'o,
	Andreas Dilger, viro, hch, Dmitry Monakhov

On Wed 17-09-14 11:02:01, Li Xi wrote:
> On Wed, Sep 17, 2014 at 3:58 AM, Jan Kara <jack@suse.cz> wrote:
> > On Tue 16-09-14 16:15:35, Li Xi wrote:
> >> On Thu, Sep 11, 2014 at 12:45 AM, Jan Kara <jack@suse.cz> wrote:
> >> >> Index: linux.git/include/uapi/linux/quota.h
> >> >> ===================================================================
> >> >> --- linux.git.orig/include/uapi/linux/quota.h
> >> >> +++ linux.git/include/uapi/linux/quota.h
> >> >> @@ -36,11 +36,12 @@
> >> >>  #include <linux/errno.h>
> >> >>  #include <linux/types.h>
> >> >>
> >> >> -#define __DQUOT_VERSION__    "dquot_6.5.2"
> >> >> +#define __DQUOT_VERSION__    "dquot_6.6.0"
> >> >>
> >> >> -#define MAXQUOTAS 2
> >> >> +#define MAXQUOTAS 3
> >> >   Hum, actually this isn't so simple. MAXQUOTAS is used in several
> >> > filesystems - ext3, ext4, ocfs2, reiserfs, gfs2 - and just bumping up
> >> > MAXQUOTAS can have unexpected consequences for them (they won't have
> >> > properly initialized data structures for new quota type). So what we have
> >> > to do as a preparatory step is to make these filesystems define their own
> >> > MAXQUOTAS value (like EXT3_MAXQUOTAS, ...). I'll take care of that.
> >> Yeah, you are right. It is likely that a new MAXQUOTAS value will hurt other
> >> file systems. And I saw your patch for it. I will use EXT4_MAXQUOTAS
> >> in Ext4 instead.
> >>
> >> However, the general codes in fs/quota or head files like quotaops.h use
> >> MAXQUOTAS heavily too. I guess I have no better choice but replace
> >> MAXQUOTAS there with a new macro, e.g. MAXQUOTAS_NEW (3). I will handle
> >> the interfaces carefully so that they remain exactly the same semantics. Do you
> >> have any better idea?
> >   The idea is that MAXQUOTAS is the number of quota types supported by VFS.
> > So you should really increase MAXQUOTAS in your patch because VFS will now
> > support three quota types. You should make sure that all places that use
> > MAXQUOTAS in fs/quota/ are safe with that change and fix them if not.
> Sorry, I am confused here. Would you please explain more? As you mentioned
> before, MAXQUOTAS is a so critial macro that we can't change it without taking
> care of other file systems. So, should I update MAXQUOTAS directly or not?
  So as you noted, I've already posted patches for all filesystems to stop
using MAXQUOTAS. So after these patches are merged you are safe to change
MAXQUOTAS as you need.

> I would prefer to update MAXQUOTAS to 3 rather than add MAXQUOTAS_NEW or
> hack in other ways. I understand that MAXQUOTAS has been 2 from the first
> begining, and it is possible that some codes take its invariance for
> granted for too long. But if a file system is using MAXQUOTAS in a wrong
> way, maybe it is possible for us to find and fix the problems after this
> patch is merged, yet before Linux is released as next major version?
  As I wrote above filesystems won't be using MAXQUOTAS anymore. Each
filesystem now has its own define. Thus it is enough to audit VFS for
MAXQUOTAS usage and that you have to do to allow for project quotas anyway.

> Another solution would be to replace MAXQUOTAS in these file systems to
> ${FS}_MAXQUOTAS. It won't cost much time to prepare those patches. And
> If these patches can be accepted and merged quickly, it would a safer
> solution. For example, that would prevent Ext3 from allocating unnecessary
> transaction blocks because MAXQUOTAS is increased. I checked other
> places where MAXQUOTAS is used. I haven't found any problems yet. But
> as you said, we are not sure whether there will be any critical problem.
  This is exactly what I meant and patches for this are already sitting in
maintainer's trees. So in the next merge window this will be done.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-09-17 12:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-10  3:52 [PATCH v3 1/4] quota: add project quota support Li Xi
2014-09-10 16:45 ` Jan Kara
2014-09-16  8:15   ` Li Xi
2014-09-16 19:58     ` Jan Kara
2014-09-17  3:02       ` Li Xi
2014-09-17 12:17         ` Jan Kara

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.