| Message ID | 20150212093644.6055.17019.stgit@buzz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@yandex-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
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@yandex-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). 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 >
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 }; -#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) +{ + BUILD_BUG_ON_NOT_POWER_OF_2(flag); + return (flags / flag) & ((1 << MAXQUOTAS) - 1); } #ifdef CONFIG_QUOTA_NETLINK_INTERFACE diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h index df73258..8778ec4 100644 --- a/include/linux/quotaops.h +++ b/include/linux/quotaops.h @@ -134,10 +134,7 @@ static inline bool sb_has_quota_suspended(struct super_block *sb, int type) static inline unsigned sb_any_quota_suspended(struct super_block *sb) { - unsigned type, tmsk = 0; - for (type = 0; type < MAXQUOTAS; type++) - tmsk |= sb_has_quota_suspended(sb, type) << type; - return tmsk; + return dquot_state_types(sb_dqopt(sb)->flags, DQUOT_SUSPENDED); } /* Does kernel know about any quota information for given sb + type? */ @@ -149,10 +146,7 @@ static inline bool sb_has_quota_loaded(struct super_block *sb, int type) static inline unsigned sb_any_quota_loaded(struct super_block *sb) { - unsigned type, tmsk = 0; - for (type = 0; type < MAXQUOTAS; type++) - tmsk |= sb_has_quota_loaded(sb, type) << type; - return tmsk; + return dquot_state_types(sb_dqopt(sb)->flags, DQUOT_USAGE_ENABLED); } static inline bool sb_has_quota_active(struct super_block *sb, int type)
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 Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> --- include/linux/quota.h | 18 ++++++++++++------ include/linux/quotaops.h | 10 ++-------- 2 files changed, 14 insertions(+), 14 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html