Re: [PATCH VER 4] Extend project quotas to support 32bit project identificators.

[Arkadiusz Miskiewicz <arekm@maven.pl>]

On Wednesday 22 of September 2010, Christoph Hellwig wrote:
> On Wed, Sep 22, 2010 at 07:42:23PM +0200, Arkadiusz Mi??kiewicz wrote:
> >  	if (mask & FSX_PROJID) {
> > 
> > +		/*
> > +		 * Switch on the PROJID32BIT superblock bit when needed
> > +		 * (implies also FEATURES2)
> > +		 */
> > +		if (!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb) &&
> > +				fa->fsx_projid > (__uint16_t)-1)
> > +			xfs_addprojid32bit(tp, ip);
> 
> Didn't we agree that we want to enable this feature explicitly via
> xfs_admin (or mkfs.xfs)?

Actually there was no agreement on this. Some think that it's good to do that 
automaticly (so user doesn't have to do anything and has everything working) 
and others think that it should be turned on explictly by xfs_admin/mkfs.xfs.

For me both methods are fine, both have some advantages and disadvantages.

(There was an agreement that separate projid (from group) quota should be 
turned on manually but was no such agreement on projid32bit)

> > -	__u16		bs_projid;	/* project id			*/
> > +	__u16		bs_projid_lo;	/* lower part of project id	*/
> > 
> >  	__u16		bs_forkoff;	/* inode fork offset in bytes	*/
> > 
> > -	unsigned char	bs_pad[12];	/* pad space, unused		*/
> > +	__u16		bs_projid_hi;	/* higher part of project id	*/
> > +	unsigned char	bs_pad[10];	/* pad space, unused		*/
> 
> Unlike in the inode we can't just rename the lo field here - that would
> break the compilation of existing applications.

Ok but maybe breaking these is good? So these can be extended to support 32bit 
projid. Otherwise these will get crap if 32bit projid is enabled (actually 
already built binaries will still get crap for > 16bit values with projid32bit 
patch).

> > +{
> > +	spin_lock(&ip->i_mount->m_sb_lock);
> > +	if (!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb)) {
> 
> Wedon't need the lock around the check, it's enough if it's inside the
> conditional.

Hm, xfs_bump_ino_vers2 does that exactly that way (for addnlink) thus I 
thought there is a reason for that. 

> > +		xfs_sb_version_addprojid32bit(&ip->i_mount->m_sb);
> > +		spin_unlock(&ip->i_mount->m_sb_lock);
> > +		xfs_mod_sb(tp,
> > +				XFS_SB_VERSIONNUM | XFS_SB_FEATURES2);

-- 
Arkadiusz Miśkiewicz        PLD/Linux Team
arekm / maven.pl            http://ftp.pld-linux.org/

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