Re: RFC: merging the quota source files

[Dave Chinner <david@fromorbit.com>]

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@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs