linux-kernel - Re: [PATCH 1/2] quota: reorder flags in quota state

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 12 Feb 2015 16:05:40 +0100
From:	Jan Kara <jack@...e.cz>
To:	Konstantin Khlebnikov <khlebnikov@...dex-team.ru>
Cc:	linux-fsdevel@...r.kernel.org, Jan Kara <jack@...e.cz>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] quota: reorder flags in quota state

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@...dex-team.ru>
> ---
>  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@...e.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/