Re: very bad behavior on overquota writes

[Chuck Silvers <chuq%chuq.com@localhost>]

On Thu, Nov 22, 2012 at 06:34:55PM +0100, Manuel Bouyer wrote:
> On Thu, Nov 22, 2012 at 12:46:54PM +0100, Manuel Bouyer wrote:
> > Here's another patch, after comments from chs@ in a private mail.
> > 
> > There really are 2 parts here:
> > - avoid unecessery list walk at truncate time. This is the change to
> >   uvm_vnp_setsize() (truncate between pgend and oldsize instead of 0, which
> >   always triggers a list walk - we know there is nothing beyond oldsize)
> >   and ffs_truncate() (vtruncbuf() is needed only for non-VREG files,
> >   and will always trigger a list walk).
> > 
> > This help for the local write case, where on my test system syscalls/s rise
> > from 170 (when write succeed) to 570 (when EDQUOT is returned). Without
> > this patch, the syscalls/s would fall to less than 20.
> > 
> > But this doesn't help much for nfsd, which can get write requests way beyond
> > the real end of file (the client's idea of end of file is wrong).
> > In such a case, the distance between pgend and oldsize in uvm_vnp_setsize()
> > is large enough to trigger a list walk, even through there is really only
> > a few pages to free. I guess fixing this would require an interface change
> > to pass to uvm_vnp_setsize() for which the allocation did really take place.
> > But I found a workaround: when a write error occurs, free all the
> > pages associated with the vnode in ffs_write(). This way, on subsequent 
> > writes
> > only new pages should be in the list, and freeing them is fast.
> > 
> > With this change, the nfsd performances don't change when the quota limit
> > is hit.
> 
> In fact, with this patch alone, the nfsd performances drops
> from about 1250 writes/s to 700 when the quota limit is hit (and we keep
> a disk activity of 140MB/s when no data write occurs). To preserve the
> performance, the attached patch is also needed. What this does is detect early
> that we'll have an allocation failure in ffs_balloc(), and bail out
> before we allocated any indirect block that we would need to freed in the
> error path. 
> This needs an additionnal flag to to chkdq(), to test a quota allocation
> without updating the quota values.
> 
> -- 
> Manuel Bouyer <bouyer%antioche.eu.org@localhost>
>      NetBSD: 26 ans d'experience feront toujours la difference
> --


this looks fine to me, but please consolidate the two copies of
the new pre-check into a new function.

-Chuck