Re: Packet loss every 30.999 seconds

[Kostik Belousov]

On Wed, Dec 19, 2007 at 08:11:59PM +0200, Kostik Belousov wrote:
> On Wed, Dec 19, 2007 at 09:13:31AM -0800, David G Lawrence wrote:
> > > >Try it with "find / -type f >/dev/null" to duplicate the problem  
> > > >almost
> > > >instantly.
> > > 
> > > I was able to verify last night that (cd /; tar -cpf -) > all.tar would
> > > trigger the problem.  I'm working getting a test running with
> > > David's ffs_sync() workaround now, adding a few counters there should
> > > get this narrowed down a little more.
> > 
> >    Unfortunately, the version of the patch that I sent out isn't going to
> > help your problem. It needs to yield at the top of the loop, but vp isn't
> > necessarily valid after the wakeup from the msleep. That's a problem that
> > I'm having trouble figuring out a solution to - the solutions that come
> > to mind will all significantly increase the overhead of the loop.
> >    As a very inadequate work-around, you might consider lowering
> > kern.maxvnodes to something like 20000 - that might be low enough to
> > not trigger the problem, but also be high enough to not significantly
> > affect system I/O performance.
> 
> I think the following may be safe. It counts only the clean scanned vnodes
> and does not evaluate the vp, that indeed may be reclaimed, after the sleep.
> 
> I never booted with the change.
> 
> diff --git a/sys/ufs/ffs/ffs_vfsops.c b/sys/ufs/ffs/ffs_vfsops.c
> index cbccc62..e686b97 100644

Or, better to use uio_yield(). See below.

diff --git a/sys/ufs/ffs/ffs_vfsops.c b/sys/ufs/ffs/ffs_vfsops.c
index cbccc62..5d2535f 100644
--- a/sys/ufs/ffs/ffs_vfsops.c
+++ b/sys/ufs/ffs/ffs_vfsops.c
@@ -1176,6 +1176,7 @@ ffs_sync(mp, waitfor, td)
        struct ufsmount *ump = VFSTOUFS(mp);
        struct fs *fs;
        int error, count, wait, lockreq, allerror = 0;
+       int yield_count;
        int suspend;
        int suspended;
        int secondary_writes;
@@ -1216,6 +1217,7 @@ loop:
        softdep_get_depcounts(mp, &softdep_deps, &softdep_accdeps);
        MNT_ILOCK(mp);
 
+       yield_count = 0;
        MNT_VNODE_FOREACH(vp, mp, mvp) {
                /*
                 * Depend on the mntvnode_slock to keep things stable enough
@@ -1233,6 +1235,12 @@ loop:
                    (IN_ACCESS | IN_CHANGE | IN_MODIFIED | IN_UPDATE)) == 0 &&
                    vp->v_bufobj.bo_dirty.bv_cnt == 0)) {
                        VI_UNLOCK(vp);
+                       if (yield_count++ == 500) {
+                               MNT_IUNLOCK(mp);
+                               yield_count = 0;
+                               uio_yield();
+                               goto relock_mp;
+                       }
                        continue;
                }
                MNT_IUNLOCK(mp);
@@ -1247,6 +1255,7 @@ loop:
                if ((error = ffs_syncvnode(vp, waitfor)) != 0)
                        allerror = error;
                vput(vp);
+       relock_mp:
                MNT_ILOCK(mp);
        }
        MNT_IUNLOCK(mp);

pgpmpNKoG2bTI.pgp
Description: PGP signature