On Tue, Aug 30, 2011 at 08:11:12AM -0400, Christoph Hellwig wrote: > Hi, > > I've started a bit work on the quota code, and the split between the > implementation files is rather annoying. I'd suggest we merge at least > xfs_dquot.c and xfs_qm.c into a single file, as there is no visible > split. It seems to me to be a reasonable separation - xfs_qm.c is mostly global management functions e.g. quota cache initialisation, reclaim, quotacheck, etc, which xfs_dquot.c is all single dquot specific operations which don't have global scope. I'd argue that there are parts of xfs_qm.c that could be moved into xfs_dquot.c (like the xfs_qm_vop_* and xfs_qm_dq*attach* functions that only act on a specific dquots) which would make this separation much clearer. This would also reflect the same sort of separation as we have with inode operations vs cache infrastructure.... > Given how small and useless xfs_qm_bhv.c is I'd throw it in. Agreed, that should be merged into xfs_qm.c at least. > As a second pass I'd also like to kill xfs_qm_syscalls.c and merge > it into the new xfs_qm.c/xfs_dquot.c for the low-level helpers, and > xfs_quotaops.c for high-level code that deals directly with the Linux > interface. That seems like a good idea, too. > Does this a) sound good to others, and b) does anyone have a preference > for the name of the new quota implementation file? If you are going to rename xfs_qm.c, then xfs_quota.c seems like the best name to me. FWIW, this is what I'd like to seen done with the quota infrastructure in the medium term: 1. break up the global quota manager into per-xfs_mount structures (move into the quotainfo structure). 2. remove the lookup hash tables and replace it with something more flexible (btree/radix-tree/rbtree) for different cache sizes. 3. replace the weird, complex freelist and dquot recycling code with a standard LRU implementation. 4. clearly document the locking model and simplify it to remove all the unnecessary nested locking and try-locking that the current deeply nested model requires. This is appears to just be a replication of the locking model pattern already defined by the VFS dentry and inode caches... 4. change the shrinker implementation to be "normal" so that the dquot cache sizes respond to memory pressure correctly The gets rid of roughly ~50% of the infrastructure code that currently exists in xfs_qm.c, and remaining stuff like the sync infrastructure mostly goes away with AIL driven writeback, too. Some of the functionality remaining (like the shrinkers) can be placed in xfs_sync.c with the inode cache shrinkers, which further reduces the code in xfs_qm.c. At that point the delineation between xfs_qm.c and xfs_dquot.c will be much clearer, I think, especially if the changes I suggested above were made... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
