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 <bou...@antioche.eu.org> > 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
