Re: [PATCH 1/2] quota: reorder flags in quota state
From: Konstantin Khlebnikov
Date: Thu Feb 12 2015 - 10:46:40 EST
On 12.02.2015 18:05, Jan Kara wrote:
On Thu 12-02-15 12:36:44, Konstantin Khlebnikov wrote:
Flags in struct quota_state keep flags for each quota type and
some common flags. This patch reorders typed flags:
Before:
0 USRQUOTA DQUOT_USAGE_ENABLED
1 USRQUOTA DQUOT_LIMITS_ENABLED
2 USRQUOTA DQUOT_SUSPENDED
3 GRPQUOTA DQUOT_USAGE_ENABLED
4 GRPQUOTA DQUOT_LIMITS_ENABLED
5 GRPQUOTA DQUOT_SUSPENDED
6 DQUOT_QUOTA_SYS_FILE
7 DQUOT_NEGATIVE_USAGE
After:
0 USRQUOTA DQUOT_USAGE_ENABLED
1 GRPQUOTA DQUOT_USAGE_ENABLED
2 USRQUOTA DQUOT_LIMITS_ENABLED
3 GRPQUOTA DQUOT_LIMITS_ENABLED
4 USRQUOTA DQUOT_SUSPENDED
5 GRPQUOTA DQUOT_SUSPENDED
6 DQUOT_QUOTA_SYS_FILE
7 DQUOT_NEGATIVE_USAGE
Now we can get bitmap of all enabled/suspended quota types without loop.
For example suspended: (flags / DQUOT_SUSPENDED) & ((1 << MAXQUOTAS) - 1).
add/remove: 0/1 grow/shrink: 3/11 up/down: 56/-215 (-159)
function old new delta
__dquot_initialize 423 447 +24
dquot_transfer 181 197 +16
dquot_alloc_inode 286 302 +16
dquot_reclaim_space_nodirty 316 313 -3
dquot_claim_space_nodirty 314 311 -3
dquot_resume 286 281 -5
dquot_free_inode 332 324 -8
__dquot_alloc_space 500 492 -8
dquot_disable 1944 1929 -15
dquot_quota_enable 252 236 -16
__dquot_free_space 750 734 -16
dquot_writeback_dquots 625 608 -17
__dquot_transfer 1186 1154 -32
dquot_quota_sync 299 261 -38
dquot_active.isra 54 - -54
Two minor comments below. Otherwise the patch looks good.
Signed-off-by: Konstantin Khlebnikov <khlebnikov@xxxxxxxxxxxxxx>
---
include/linux/quota.h | 18 ++++++++++++------
include/linux/quotaops.h | 10 ++--------
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/include/linux/quota.h b/include/linux/quota.h
index d534e8e..205b4f7 100644
--- a/include/linux/quota.h
+++ b/include/linux/quota.h
@@ -398,9 +398,9 @@ enum {
* memory to turn them on */
_DQUOT_STATE_FLAGS
};
Please explain how the flags are exactly laid out in a comment in
include/linux/quota.h at the enum definition above.
-#define DQUOT_USAGE_ENABLED (1 << _DQUOT_USAGE_ENABLED)
-#define DQUOT_LIMITS_ENABLED (1 << _DQUOT_LIMITS_ENABLED)
-#define DQUOT_SUSPENDED (1 << _DQUOT_SUSPENDED)
+#define DQUOT_USAGE_ENABLED (1 << _DQUOT_USAGE_ENABLED * MAXQUOTAS)
+#define DQUOT_LIMITS_ENABLED (1 << _DQUOT_LIMITS_ENABLED * MAXQUOTAS)
+#define DQUOT_SUSPENDED (1 << _DQUOT_SUSPENDED * MAXQUOTAS)
#define DQUOT_STATE_FLAGS (DQUOT_USAGE_ENABLED | DQUOT_LIMITS_ENABLED | \
DQUOT_SUSPENDED)
/* Other quota flags */
@@ -414,15 +414,21 @@ enum {
*/
#define DQUOT_NEGATIVE_USAGE (1 << (DQUOT_STATE_LAST + 1))
/* Allow negative quota usage */
-
static inline unsigned int dquot_state_flag(unsigned int flags, int type)
{
- return flags << _DQUOT_STATE_FLAGS * type;
+ return flags << type;
}
static inline unsigned int dquot_generic_flag(unsigned int flags, int type)
{
- return (flags >> _DQUOT_STATE_FLAGS * type) & DQUOT_STATE_FLAGS;
+ return (flags >> type) & DQUOT_STATE_FLAGS;
+}
+
+/* Bitmap of quota types where flag is set in flags */
+static __always_inline unsigned dquot_state_types(unsigned flags, unsigned flag)
Why do you have __always_inline here? Just use inline if the function
works standalone (which seems to be the case here).
It works better if "flag" is constant -- dividing turns into bit shift.
And I'm not sure how BUILD_BUG_ON* work for non-constant expressions.
+{
+ BUILD_BUG_ON_NOT_POWER_OF_2(flag);
+ return (flags / flag) & ((1 << MAXQUOTAS) - 1);
}
#ifdef CONFIG_QUOTA_NETLINK_INTERFACE
Honza
--
Konstantin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/