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).
> +{
> + BUILD_BUG_ON_NOT_POWER_OF_2(flag);
> + return (flags / flag) & ((1 << MAXQUOTAS) - 1);
> }
>
> #ifdef CONFIG_QUOTA_NETLINK_INTERFACE
Honza
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
