Cleanup quota header files
Subsystems (5)
Jan Kara
reviewerThe series of patches below cleans up quota header files. It does several things:
1) Rename functions from uppercase to lowercase (compatibility macros introduced)
2) Remove some unnecessary definitions
3) Split quota.h into two files so that we can properly declare inline functions.
4) Change macros to inline functions.
So far no filesystem is converted from old function names to new ones -
that is an independent step and I'll hopefully get to that later :). Andrew, would you merge this series?
Honza
Jan Kara
reviewerPATCH 1/1 quota: Rename quota functions from upper case, make bigger ones non-inline
Cleanup quotaops.h: Rename functions from uppercase to lowercase (and define backward compatibility macros), move larger functions to dquot.c and make them non-inline.
--- fs/dquot.c | 53 +++++++++++ include/linux/quotaops.h | 226 ++++++++++++++++++++++------------------------ 2 files changed, 160 insertions(+), 119 deletions(-)
fs/dquot.c +53 -0
@@ -1139,6 +1139,28 @@ int dquot_drop(struct inode *inode)return 0 ; } +/* Wrapper to remove references to quota structures from inode */ +void vfs_dq_drop (struct inode *inode) +{ +/* Here we can get arbitrary inode from clear_inode() so we have +* to be careful. OTOH we don't need locking as quota operations +* are allowed to change only at mount time */ +if (!IS_NOQUOTA (inode) && inode->i_sb && inode->i_sb->dq_op + && inode->i_sb->dq_op->drop) { +int cnt; +/* Test before calling to rule out calls from proc and such +* where we are not allowed to block. Note that this is +* actually reliable test even without the lock - the caller +* must assure that nobody can come after the DQUOT_DROP and +* add quota pointers back anyway */ +for (cnt =0 ; cnt <MAXQUOTAS ; cnt++) +if (inode->i_dquot[cnt] !=NODQUOT ) +break ; +if (cnt <MAXQUOTAS ) + inode->i_sb->dq_op->drop (inode); + } +} +/* * Following four functions update i_blocks+i_bytes fields and * quota information (together with appropriate checks)
@@ -1412,6 +1434,18 @@ warn_put_all:return ret; } +/* Wrapper for transferring ownership of an inode */ +int vfs_dq_transfer (struct inode *inode,struct iattr *iattr) +{ +if (sb_any_quota_enabled (inode->i_sb) && !IS_NOQUOTA (inode)) { +vfs_dq_init (inode); +if (inode->i_sb->dq_op->transfer (inode, iattr) ==NO_QUOTA ) +return 1 ; + } +return 0 ; +} + +/* * Write info of quota file to disk */
@@ -1752,6 +1786,22 @@ out:return error; } +/* Wrapper to turn on quotas when remounting rw */ +int vfs_dq_quota_on_remount (struct super_block *sb) +{ +int cnt; +int ret =0 , err; + +if (!sb->s_qcop || !sb->s_qcop->quota_on) +return -ENOSYS ; +for (cnt =0 ; cnt <MAXQUOTAS ; cnt++) { + err = sb->s_qcop->quota_on (sb, cnt,0 ,NULL ,1 ); +if (err <0 && !ret) + ret = err; + } +return ret; +} +/* Generic routine for getting common part of quota structure */ static void do_get_dqblk (struct dquot *dquot,struct if_dqblk *di) {
@@ -2087,8 +2137,11 @@ EXPORT_SYMBOL(dquot_release);EXPORT_SYMBOL (dquot_mark_dquot_dirty);EXPORT_SYMBOL (dquot_initialize);EXPORT_SYMBOL (dquot_drop); +EXPORT_SYMBOL (vfs_dq_drop);EXPORT_SYMBOL (dquot_alloc_space);EXPORT_SYMBOL (dquot_alloc_inode);EXPORT_SYMBOL (dquot_free_space);EXPORT_SYMBOL (dquot_free_inode);EXPORT_SYMBOL (dquot_transfer); +EXPORT_SYMBOL (vfs_dq_transfer); +EXPORT_SYMBOL (vfs_dq_quota_on_remount);
include/linux/quotaops.h +107 -119
388 lines not on this page · raw message
Signature · 1 line
1.5.2.4
Jan Kara
reviewerPATCH 1/1 quota: Remove use of info_any_dirty()
Since there is only a single place which uses info_any_dirty() and that is a trivial macro, just remove the use of this macro completely.
--- fs/quota.c | 7 +++++-- include/linux/quota.h | 2 -- 2 files changed, 5 insertions(+), 4 deletions(-)
fs/quota.c +5 -2
@@ -199,8 +199,11 @@ restart:list_for_each_entry (sb, &super_blocks, s_list) {/* This test just improves performance so it needn't be reliable... */ for (cnt =0 , dirty =0 ; cnt <MAXQUOTAS ; cnt++) -if ((type == cnt || type ==-1 ) &&sb_has_quota_enabled (sb, cnt) - &&info_any_dirty (&sb_dqopt (sb)->info[cnt])) +if ((type == cnt || type ==-1 ) + &&sb_has_quota_enabled (sb, cnt) + && (info_dirty (&sb_dqopt (sb)->info[cnt]) + || !list_empty (&sb_dqopt (sb)-> + info[cnt].dqi_dirty_list))) dirty =1 ;if (!dirty)continue ;
include/linux/quota.h +0 -2
@@ -224,8 +224,6 @@ struct super_block;extern void mark_info_dirty (struct super_block *sb,int type);#define info_dirty (info) test_bit(DQF_INFO_DIRTY_B, &(info)->dqi_flags) -#define info_any_dquot_dirty (info) (!list_empty(&(info)->dqi_dirty_list)) -#define info_any_dirty (info) (info_dirty(info) || info_any_dquot_dirty(info))#define sb_dqopt (sb) (&(sb)->s_dquot)#define sb_dqinfo (sb, type) (sb_dqopt(sb)->info+(type))
Signature · 1 line
1.5.2.4
Jan Kara
reviewerPATCH 1/1 ext4: Add explicit include of <linux/quota.h> into ext4_jbd2.h
We need definitions from quota.h. So far we depended on fs.h on including this file for us and this is going to change.
--- fs/ext4/ext4_jbd2.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
fs/ext4/ext4_jbd2.h +1 -0
@@ -17,6 +17,7 @@#include <linux/fs.h> #include <linux/jbd2.h> +#include <linux/quota.h> #include "ext4.h" #define EXT4_JOURNAL (inode) (EXT4_SB((inode)->i_sb)->s_journal)
Signature · 1 line
1.5.2.4
Jan Kara
reviewerPATCH 1/1 ext2: Add explicit include of <linux/quota.h> into super.c
We need definitions from quota.h and so far we depended on fs.h to include this file for us. This will be no longer true.
--- fs/ext2/super.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
fs/ext2/super.c +1 -0
@@ -31,6 +31,7 @@#include <linux/seq_file.h> #include <linux/mount.h> #include <linux/log2.h> +#include <linux/quota.h> #include <asm/uaccess.h> #include "ext2.h" #include "xattr.h"
Signature · 1 line
1.5.2.4
Jan Kara
reviewerPATCH 1/1 quota: Split out quota_struct.h out of quota.h
Split some structure definitions from quota.h to quota_struct.h so that they can be separately included in fs.h. This makes it possible to declare some useful inline functions in quota.h because it can now include fs.h.
--- include/linux/fs.h | 2 +- include/linux/quota.h | 193 +--------------------------------------------- include/linux/quotaops.h | 1 + include/linux/syscalls.h | 2 +- 4 files changed, 5 insertions(+), 193 deletions(-)
include/linux/fs.h +1 -1
@@ -364,7 +364,7 @@ struct iattr {/* * Includes for diskquotas. */ -#include <linux/quota.h> +#include <linux/quota_struct.h> /** * enum positive_aop_returns - aop return codes with specific semantics
include/linux/quota.h +2 -191
237 lines not on this page · raw message
include/linux/quotaops.h +1 -0
@@ -13,6 +13,7 @@#include <linux/smp_lock.h> #include <linux/fs.h> +#include <linux/quota.h> #if defined(CONFIG_QUOTA )
include/linux/syscalls.h +1 -1
@@ -62,7 +62,7 @@ struct getcpu_cache;#include <linux/sem.h> #include <asm/siginfo.h> #include <asm/signal.h> -#include <linux/quota.h> +#include <linux/quota_struct.h> #include <linux/key.h> asmlinkage long sys_time (time_t __user *tloc);
Signature · 1 line
1.5.2.4
Jan Kara
reviewerPATCH 1/1 quota: Convert macros to inline functions
--- include/linux/quota.h | 53 +++++++++++++++++++++++++++++++++++------------- 1 files changed, 38 insertions(+), 15 deletions(-)
include/linux/quota.h +38 -15
@@ -125,10 +125,20 @@ extern spinlock_t dq_data_lock;#define DQUOT_DEL_REWRITE max(V1_DEL_REWRITE, V2_DEL_REWRITE)void mark_info_dirty (struct super_block *sb,int type); -#define info_dirty (info) test_bit(DQF_INFO_DIRTY_B, &(info)->dqi_flags) +static inline int info_dirty (struct mem_dqinfo *info) +{ +return test_bit (DQF_INFO_DIRTY_B , &info->dqi_flags); +} -#define sb_dqopt (sb) (&(sb)->s_dquot) -#define sb_dqinfo (sb, type) (sb_dqopt(sb)->info+(type)) +static inline struct quota_info *sb_dqopt (struct super_block *sb) +{ +return &sb->s_dquot; +} + +static inline struct mem_dqinfo *sb_dqinfo (struct super_block *sb,int type) +{ +return sb_dqopt (sb)->info + type; +}struct dqstats {int lookups;
@@ -146,18 +156,31 @@ extern struct dqstats dqstats;#define QUOTA_OK 0#define NO_QUOTA 1 -#define sb_has_quota_enabled (sb, type) ((type)==USRQUOTA ? \ - (sb_dqopt(sb)->flags & DQUOT_USR_ENABLED) : (sb_dqopt(sb)->flags & DQUOT_GRP_ENABLED)) - -#define sb_any_quota_enabled (sb) (sb_has_quota_enabled(sb, USRQUOTA) | \ - sb_has_quota_enabled(sb, GRPQUOTA)) - -#define sb_has_quota_suspended (sb, type) \ - ((type) == USRQUOTA ? (sb_dqopt(sb)->flags & DQUOT_USR_SUSPENDED) : \ - (sb_dqopt(sb)->flags & DQUOT_GRP_SUSPENDED)) - -#define sb_any_quota_suspended (sb) (sb_has_quota_suspended(sb, USRQUOTA) | \ - sb_has_quota_suspended(sb, GRPQUOTA)) +static inline int sb_has_quota_enabled (struct super_block *sb,int type) +{ +if (type ==USRQUOTA ) +return sb_dqopt (sb)->flags &DQUOT_USR_ENABLED ; +return sb_dqopt (sb)->flags &DQUOT_GRP_ENABLED ; +} + +static inline int sb_any_quota_enabled (struct super_block *sb) +{ +return sb_has_quota_enabled (sb,USRQUOTA ) || +sb_has_quota_enabled (sb,GRPQUOTA ); +} + +static inline int sb_has_quota_suspended (struct super_block *sb,int type) +{ +if (type ==USRQUOTA ) +return sb_dqopt (sb)->flags &DQUOT_USR_SUSPENDED ; +return sb_dqopt (sb)->flags &DQUOT_GRP_SUSPENDED ; +} + +static inline int sb_any_quota_suspended (struct super_block *sb) +{ +return sb_has_quota_suspended (sb,USRQUOTA ) || +sb_has_quota_suspended (sb,GRPQUOTA ); +}int register_quota_format (struct quota_format_type *fmt);void unregister_quota_format (struct quota_format_type *fmt);
Signature · 1 line
1.5.2.4
Re: [PATCH] quota: Split out quota_struct.h out of quota.h
In reply to Jan KaraQuoted 4 lines · Jan Kara
Split some structure definitions from
quota.htoquota_struct.hso that they can be separately included infs.h. This makes it possible to declare some useful inline functions inquota.hbecause it can now includefs.h.
Quoted 8 lines
Signed-off-by Jan Kara <[email protected]>--- include/linux/fs.h | 2 +- include/linux/quota.h | 193 +--------------------------------------------- include/linux/quotaops.h | 1 + include/linux/syscalls.h | 2 +- 4 files changed, 5 insertions(+), 193 deletions(-)...
Missing "git-add include/linux/quota_struct.h" ?
include/linux/quota.h line 40
Quoted 8 lines
include/linux/quota.h +1
-0@@ -37,13 +37,11 @@#include <linux/errno.h> #include <linux/types.h> +#include <linux/quota_struct.h>
This breaks our userspace headers.
Please always run "make headers_check" after touching #include's under include/
cu
Adrian
Signature · 4 lines
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon SeedJan Kara
reviewerRe: [PATCH] quota: Split out quota_struct.h out of quota.h
Quoted 14 lines · Adrian Bunk
Quoted 4 lines · Jan Kara
Split some structure definitions from
quota.htoquota_struct.hso that they can be separately included infs.h. This makes it possible to declare some useful inline functions inquota.hbecause it can now includefs.h.Quoted 8 lines
Signed-off-by Jan Kara <[email protected]>--- include/linux/fs.h | 2 +- include/linux/quota.h | 193 +--------------------------------------------- include/linux/quotaops.h | 1 + include/linux/syscalls.h | 2 +- 4 files changed, 5 insertions(+), 193 deletions(-)...
Quoted 1 line
Missing "git-add
include/linux/quota_struct.h" ?
Probably yes, I already noticed this and mailed Andrew a patch with this
file included.
Quoted 11 lines
include/linux/quota.h line 40
Quoted 8 lines
include/linux/quota.h +1
-0@@ -37,13 +37,11 @@#include <linux/errno.h> #include <linux/types.h> +#include <linux/quota_struct.h> This breaks our userspace headers.
Quoted 2 lines
Please always run "make
headers_check" after touching #include's underinclude/
I didn't know this existed :) I'll use it in future. Thanks. The question
is how to fix this. Probably the best would be if didn't have to export new header file, wouldn't it? We can split the headers into quota.h containing structures and all other userspace stuff and quota_func.h carrying other kernel-internal stuff like inline functions etc. Oh well...
Honza
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
Re: [PATCH] quota: Split out quota_struct.h out of quota.h
Quoted 14 lines · Jan Kara
Quoted 1 line · Adrian Bunk
On Mon, Jun 02, 2008 at 07:11:10PM +0200, Jan Kara wrote:
...
Quoted 11 lines
include/linux/quota.h line 40
Quoted 8 lines
include/linux/quota.h +1
-0@@ -37,13 +37,11 @@#include <linux/errno.h> #include <linux/types.h> +#include <linux/quota_struct.h> This breaks our userspace headers.
Quoted 7 lines
Quoted 2 lines
Please always run "make
headers_check" after touching #include's underinclude/I didn't know this existed :) I'll use it in future. Thanks. The question
is how to fix this. Probably the best would be if didn't have to export new header file, wouldn't it? We can split the headers intoquota.hcontaining structures and all other userspace stuff andquota_func.hcarrying other kernel-internal stuff like inline functions etc. Oh well...
Considering that quota.h #include's quota_struct.h it wouldn't be a problem if the new header also gets exported to userspace.
At least I do not have any strong opinion regarding how to best organize the quota headers - David (Cc'ed) might have a more input on this.
cu
Adrian
Signature · 4 lines
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon SeedRe: [PATCH] quota: Split out quota_struct.h out of quota.h
Quoted 23 lines · Adrian Bunk
Quoted 14 lines · Jan Kara
Quoted 1 line · Adrian Bunk
On Mon, Jun 02, 2008 at 07:11:10PM +0200, Jan Kara wrote:
...
Quoted 11 lines
include/linux/quota.h line 40
Quoted 8 lines
include/linux/quota.h +1
-0@@ -37,13 +37,11 @@#include <linux/errno.h> #include <linux/types.h> +#include <linux/quota_struct.h> This breaks our userspace headers.
Quoted 7 lines
Quoted 2 lines
Please always run "make
headers_check" after touching #include's underinclude/I didn't know this existed :) I'll use it in future. Thanks. The question
is how to fix this. Probably the best would be if didn't have to export new header file, wouldn't it? We can split the headers intoquota.hcontaining structures and all other userspace stuff andquota_func.hcarrying other kernel-internal stuff like inline functions etc. Oh well...
Quoted 5 lines
Considering that
quota.h#include'squota_struct.hit wouldn't be a problem if the new header also gets exported to userspace.At least I do not have any strong opinion regarding how to best organize the quota headers - David (Cc'ed) might have a more input on this.
To a large extent it doesn't actually matter for userspace. All userspace will be using <sys/quota.h> instead, because for a very long period in our history, <linux/quota.h> would be completely broken for userspace because it would give the v1 quota stuff. I believe that was because we were depending on some CONFIG_xxx definition, which doesn't exist in userspace (we _really_ should add that to headers_check some time).
That said, the patch looks OK as long as you remember to add the new file to unifdef-y, and as long as you have #ifdef __KERNEL__ in the correct places in it (I can't see the updated patch with the new file actually included, so I haven't checked).
Signature · 1 line
dwmw2
Jan Kara
reviewerRe: [PATCH] quota: Split out quota_struct.h out of quota.h
Quoted 38 lines · David Woodhouse
Quoted 23 lines · Adrian Bunk
Quoted 14 lines · Jan Kara
Quoted 1 line · Adrian Bunk
On Mon, Jun 02, 2008 at 07:11:10PM +0200, Jan Kara wrote:
...
Quoted 11 lines
include/linux/quota.h line 40
Quoted 8 lines
include/linux/quota.h +1
-0@@ -37,13 +37,11 @@#include <linux/errno.h> #include <linux/types.h> +#include <linux/quota_struct.h> This breaks our userspace headers.
Quoted 7 lines
Quoted 2 lines
Please always run "make
headers_check" after touching #include's underinclude/I didn't know this existed :) I'll use it in future. Thanks. The question
is how to fix this. Probably the best would be if didn't have to export new header file, wouldn't it? We can split the headers intoquota.hcontaining structures and all other userspace stuff andquota_func.hcarrying other kernel-internal stuff like inline functions etc. Oh well...Quoted 5 lines
Considering that
quota.h#include'squota_struct.hit wouldn't be a problem if the new header also gets exported to userspace.At least I do not have any strong opinion regarding how to best organize the quota headers - David (Cc'ed) might have a more input on this.
To a large extent it doesn't actually matter for userspace. All userspace will be using <
sys/quota.h> instead, because for a very long period in our history, <linux/quota.h> would be completely broken for userspace because it would give the v1 quota stuff. I believe that was because we were depending on someCONFIG_xxxdefinition, which doesn't exist in userspace (we _really_ should add that toheaders_checksome time).
Quoted 2 lines
That said, the patch looks OK as long as you remember to add the new file to unifdef-y, and as long as you have #ifdef
__KERNEL__in the
Sorry, what does "add the new file to unifdef-y" mean?
Quoted 2 lines
correct places in it (I can't see the updated patch with the new file actually included, so I haven't checked).
The updated patch is attached if you're interested. I think __KERNEL__ is
there where it should be...
Thanks for reply.
Honza
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
Quoted 1 line
From
2281fe38459b2968869a144462a8890ccd319c1dMon Sep 17 00:00:00 2001
From: Jan Kara <[email protected]>
Date: Mon, 2 Jun 2008 18:41:58 +0200
Subject: [PATCH] quota: Split out quota_struct.h out of quota.h
Split some structure definitions from quota.h to quota_struct.h so that they can be separately included in fs.h. This makes it possible to declare some useful inline functions in quota.h because it can now include fs.h.
--- include/linux/fs.h | 2 +- include/linux/quota.h | 193 +--------------------------------- include/linux/quota_struct.h | 240 ++++++++++++++++++++++++++++++++++++++++++ include/linux/quotaops.h | 1 + include/linux/syscalls.h | 2 +- 5 files changed, 245 insertions(+), 193 deletions(-) create mode 100644 include/linux/quota_struct.h
include/linux/fs.h +1 -1
@@ -364,7 +364,7 @@ struct iattr {/* * Includes for diskquotas. */ -#include <linux/quota.h> +#include <linux/quota_struct.h> /** * enum positive_aop_returns - aop return codes with specific semantics
include/linux/quota.h +2 -191
237 lines not on this page · raw message
include/linux/quota_struct.h new file +240 -0
240 lines not on this page · raw message
include/linux/quotaops.h +1 -0
@@ -13,6 +13,7 @@#include <linux/smp_lock.h> #include <linux/fs.h> +#include <linux/quota.h> #if defined(CONFIG_QUOTA )
include/linux/syscalls.h +1 -1
@@ -62,7 +62,7 @@ struct getcpu_cache;#include <linux/sem.h> #include <asm/siginfo.h> #include <asm/signal.h> -#include <linux/quota.h> +#include <linux/quota_struct.h> #include <linux/key.h> asmlinkage long sys_time (time_t __user *tloc);
Signature · 1 line
1.5.2.4
Re: [PATCH] quota: Remove use of info_any_dirty()
In reply to Jan KaraHi,
Quoted 15 lines · Jan Kara
Since there is only a single place which uses
info_any_dirty()and that is a trivial macro, just remove the use of this macro completely.Signed-off-by Jan Kara <[email protected]>--- fs/quota.c | 7 +++++-- include/linux/quota.h | 2 -- 2 files changed, 5 insertions(+), 4 deletions(-)fs/quota.c +0
-0@@ -199,8 +199,11 @@ restart:list_for_each_entry (sb, &super_blocks, s_list) {
fs/quota.c restart: · line 207
Quoted 10 lines
fs/quota.c +5
-2/* This test just improves performance so it needn't be reliable... */ for (cnt =0 , dirty =0 ; cnt <MAXQUOTAS ; cnt++) -if ((type == cnt || type ==-1 ) &&sb_has_quota_enabled (sb, cnt) - &&info_any_dirty (&sb_dqopt (sb)->info[cnt])) +if ((type == cnt || type ==-1 ) + &&sb_has_quota_enabled (sb, cnt) + && (info_dirty (&sb_dqopt (sb)->info[cnt]) + || !list_empty (&sb_dqopt (sb)-> + info[cnt].dqi_dirty_list))) dirty =1 ;
This is really too hideous in my opinion and looks like a candidate for its own static inline function.
Or you can try to rewrite the boolean expression on multiple lines using continue, something like:
- for (cnt = 0, dirty = 0; cnt < MAXQUOTAS; cnt++)
- if ((type == cnt || type == -1) && sb_has_quota_enabled(
- && info_any_dirty(&sb_dqopt(sb)->info[cnt]))
- dirty = 1;
+ for (cnt = 0, dirty = 0; cnt < MAXQUOTAS; cnt++) {
+ if (type != cnt && type != -1)
+ continue;
+ if (!sb_has_quota_enabled(sb, cnt))
+ continue;
+ if (!info_any_dirty(&sb_dqopt(sb)->info[cnt]))
+ continue;
+ dirty = 1;
+ }
(This uses the original macro, I know. How about moving that from the header to a new inline function just above this function?)
What do you think?
Vegard
PS: This is a really good clean-up effort. Good work!
Signature · 4 lines
"The animistic metaphor of the bug that maliciously sneaked in while the programmer was not looking is intellectually dishonest as it disguises that the error is the programmer's own creation." -- E. W. Dijkstra, EWD1036
Jan Kara
reviewerRe: [PATCH] quota: Remove use of info_any_dirty()
Hi,
Thanks for reading the patches.
Quoted 47 lines · Jan Kara
Quoted 15 lines
Since there is only a single place which uses
info_any_dirty()and that is a trivial macro, just remove the use of this macro completely.Signed-off-by Jan Kara <[email protected]>--- fs/quota.c | 7 +++++-- include/linux/quota.h | 2 -- 2 files changed, 5 insertions(+), 4 deletions(-)fs/quota.c +0
-0@@ -199,8 +199,11 @@ restart:list_for_each_entry (sb, &super_blocks, s_list) {fs/quota.c restart: · line 207
Quoted 10 lines
fs/quota.c +5
-2/* This test just improves performance so it needn't be reliable... */ for (cnt =0 , dirty =0 ; cnt <MAXQUOTAS ; cnt++) -if ((type == cnt || type ==-1 ) &&sb_has_quota_enabled (sb, cnt) - &&info_any_dirty (&sb_dqopt (sb)->info[cnt])) +if ((type == cnt || type ==-1 ) + &&sb_has_quota_enabled (sb, cnt) + && (info_dirty (&sb_dqopt (sb)->info[cnt]) + || !list_empty (&sb_dqopt (sb)-> + info[cnt].dqi_dirty_list))) dirty =1 ;This is really too hideous in my opinion and looks like a candidate for its own static inline function.
Or you can try to rewrite the boolean expression on multiple lines using continue, something like:
(quoted diff) +9
-4- for (cnt = 0, dirty = 0; cnt < MAXQUOTAS; cnt++) - if ((type == cnt || type == -1) && sb_has_quota_enabled( - && info_any_dirty(&sb_dqopt(sb)->info[cnt])) - dirty = 1; + for (cnt = 0, dirty = 0; cnt < MAXQUOTAS; cnt++) { + if (type != cnt && type != -1) + continue; + if (!sb_has_quota_enabled(sb, cnt)) + continue; + if (!info_any_dirty(&sb_dqopt(sb)->info[cnt])) + continue; + dirty = 1; + }
Quoted 2 lines
(This uses the original macro, I know. How about moving that from the header to a new inline function just above this function?)
I like this one better. I've rewritten it and you don't even need the
'dirty' variable. The resulting patch is below.
Quoted 1 line
PS: This is a really good clean-up effort. Good work!
Thanks. I'm just cleaning up my own mess... ;)
Honza
Signature · 3 lines
Jan Kara <[email protected]> SUSE Labs, CR ---
Quoted 1 line
From
7509c2a0cf5d64687b1b68bde5ec2f7cb625688eMon Sep 17 00:00:00 2001
From: Jan Kara <[email protected]>
Date: Mon, 2 Jun 2008 18:27:15 +0200
Subject: [PATCH] quota: Cleanup loop in sync_dquots()
Make loop in sync_dquots() checking whether there's something to write more readable, remove useless variable and macro info_any_dirty() which is used only in this place.
--- fs/quota.c | 18 ++++++++++++------ include/linux/quota.h | 2 -- 2 files changed, 12 insertions(+), 8 deletions(-)
fs/quota.c +12 -6
@@ -186,7 +186,7 @@ static void quota_sync_sb(struct super_block *sb, int type)void sync_dquots (struct super_block *sb,int type) { -int cnt, dirty; +int cnt;if (sb) {if (sb->s_qcop->quota_sync)
@@ -198,11 +198,17 @@ void sync_dquots(struct super_block *sb, int type) restart:list_for_each_entry (sb, &super_blocks, s_list) {/* This test just improves performance so it needn't be reliable... */ -for (cnt =0 , dirty =0 ; cnt <MAXQUOTAS ; cnt++) -if ((type == cnt || type ==-1 ) &&sb_has_quota_enabled (sb, cnt) - &&info_any_dirty (&sb_dqopt (sb)->info[cnt])) - dirty =1 ; -if (!dirty) +for (cnt =0 ; cnt <MAXQUOTAS ; cnt++) { +if (type !=-1 && type != cnt) +continue ; +if (!sb_has_quota_enabled (sb, cnt)) +continue ; +if (!info_dirty (&sb_dqopt (sb)->info[cnt]) && +list_empty (&sb_dqopt (sb)->info[cnt].dqi_dirty_list)) +continue ; +break ; + } +if (cnt ==MAXQUOTAS )continue ; sb->s_count++;spin_unlock (&sb_lock);
include/linux/quota.h +0 -2
@@ -224,8 +224,6 @@ struct super_block;extern void mark_info_dirty (struct super_block *sb,int type);#define info_dirty (info) test_bit(DQF_INFO_DIRTY_B, &(info)->dqi_flags) -#define info_any_dquot_dirty (info) (!list_empty(&(info)->dqi_dirty_list)) -#define info_any_dirty (info) (info_dirty(info) || info_any_dquot_dirty(info))#define sb_dqopt (sb) (&(sb)->s_dquot)#define sb_dqinfo (sb, type) (sb_dqopt(sb)->info+(type))
Signature · 1 line
1.5.2.4