Re: [PATCH 3/3] xfs: null unused quota inodes when quota is on

[Dave Chinner <david@fromorbit.com>]

On Fri, Jul 11, 2014 at 05:30:10PM +0200, Arkadiusz Miśkiewicz wrote:
> On Friday 11 of July 2014, Arkadiusz Miśkiewicz wrote:
> > On Friday 11 of July 2014, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > > 
> > > When quota is on, it is expected that unused quota inodes have a
> > > value of NULLFSINO. The changes to support a separate project quota
> > > in 3.12 broken this rule for non-project quota inode enabled
> > > filesystem, as the code now refuses to write the group quota inode
> > > if neither group or project quotas are enabled. This regression was
> > > introduced by commit d892d58 ("xfs: Start using pquotaino from the
> > > superblock").
> > > 
> > > In this case, we should be writing NULLFSINO rather than nothing to
> > > ensure that we leave the group quota inode in a valid state while
> > > quotas are enabled.
> > > 
> > > Failure to do so doesn't cause a current kernel to break - the
> > > separate project quota inodes introduced translation code to always
> > > treat a zero inode as NULLFSINO. This was introduced by commit
> > > 0102629 ("xfs: Initialize all quota inodes to be NULLFSINO") with is
> > > also in 3.12 but older kernels do not do this and hence taking a
> > > filesystem back to an older kernel can result in quotas failing
> > > initialisation at mount time. When that happens, we see this in
> > > dmesg:
> > > 
> > > [ 1649.215390] XFS (sdb): Mounting Filesystem
> > > [ 1649.316894] XFS (sdb): Failed to initialize disk quotas.
> > > [ 1649.316902] XFS (sdb): Ending clean mount
> > > 
> > > By ensuring that we write NULLFSINO to quota inodes that aren't
> > > active, we avoid this problem.
> > > 
> > > Signed-off-by: Dave Chinner <dchinner@redhat.com>
.....
> Actually there is a problem with different options, so likely more fixes is 
> needed. gquotinode doesn't get allocated if mounted with grpquota:
> 
> 3.16git+patch:
> [arekm@t400 test]$ rm 50M-image 
> [arekm@t400 test]$ truncate -s 50M 50M-image
> [arekm@t400 test]$ mkfs.xfs -f 50M-image 
> meta-data=50M-image              isize=256    agcount=2, agsize=6400 blks
>          =                       sectsz=512   attr=2, projid32bit=1
>          =                       crc=0        finobt=0
> data     =                       bsize=4096   blocks=12800, imaxpct=25
>          =                       sunit=0      swidth=0 blks
> naming   =version 2              bsize=4096   ascii-ci=0 ftype=0
> log      =internal log           bsize=4096   blocks=853, version=2
>          =                       sectsz=512   sunit=0 blks, lazy-count=1
> realtime =none                   extsz=4096   blocks=0, rtextents=0
> $ xfs_db 50M-image  -c "sb 0" -c "print" |grep quot
> uquotino = 0
> gquotino = 0
> pquotino = 0
> $ sudo mount 50M-image /media/floppy/ -o grpquota
> $ sudo umount /media/floppy/
> $ xfs_db 50M-image  -c "sb 0" -c "print" |grep quot
> uquotino = null
> gquotino = null
> pquotino = 0

Ok, I can reproduce that.

What a freakin' mess.

We can't unconditionally write NULLFSINO to the field because of the
way the updates and logging of the superblock buffer work - it only
updates the fields that are changed, so most updates (which occur
during mount or unmount) don't set the quota inode fields at all.

In fact, the whole "only write the specific sb fields" code seems
like an optimisation that is no longer needed, given that we never
actually update or log the superblock buffer in a fast path anymore.

So, for the dev tree, I'm just going to rip all this crap out. As to
a small, targeted regression fix, well, I haven't worked that out
yet....

-Dave.
-- 
Dave Chinner
david@fromorbit.com

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