Weird behavior writing to SSD on 2013 MacBook

[Konstantin Belousov]

On Thu, Feb 05, 2015 at 12:45:55AM -0800, Alfred Perlstein wrote:
> 
> 
> On 2/5/15 12:30 AM, Konstantin Belousov wrote:
> > On Thu, Feb 05, 2015 at 08:56:59AM +0100, Dimitry Andric wrote:
> >> If you let bsdtar continue, and press control-T a few times, does the
> >> user time (u) increase at all?  Does it ever go any further, if you let
> >> it run for a very long time?
> >>
> >> I believe a problem may have been introduced by r277922, leading to
> >> filesystem hangs in some scenarios.  It looks like this commit is also
> >> in dumbbell's github fork:
> >>
> >> https://github.com/dumbbell/freebsd/commit/83723416a6bb8695d60c6573722a81086899f521
> >>
> >
> > Would be nice if you mailed me with your findings.
> >
> > Please try this.
> >
> > diff --git a/sys/ufs/ffs/ffs_softdep.c b/sys/ufs/ffs/ffs_softdep.c
> > index 79783c8..700854e 100644
> > --- a/sys/ufs/ffs/ffs_softdep.c
> > +++ b/sys/ufs/ffs/ffs_softdep.c
> > @@ -1393,7 +1393,7 @@ softdep_flush(addr)
> >   		    VFSTOUFS(mp)->softdep_jblocks->jb_suspended))
> >   			kthread_suspend_check();
> >   		ACQUIRE_LOCK(ump);
> > -		while ((ump->softdep_flags & (FLUSH_CLEANUP | FLUSH_EXIT)) == 0)
> > +		if ((ump->softdep_flags & (FLUSH_CLEANUP | FLUSH_EXIT)) == 0)
> >   			msleep(&ump->softdep_flushtd, LOCK_PTR(ump), PVM,
> >   			    "sdflush", hz / 2);
> >   		ump->softdep_flags &= ~FLUSH_CLEANUP;
> > @@ -1423,10 +1423,9 @@ worklist_speedup(mp)
> >
> >   	ump = VFSTOUFS(mp);
> >   	LOCK_OWNED(ump);
> > -	if ((ump->softdep_flags & (FLUSH_CLEANUP | FLUSH_EXIT)) == 0) {
> > +	if ((ump->softdep_flags & (FLUSH_CLEANUP | FLUSH_EXIT)) == 0)
> >   		ump->softdep_flags |= FLUSH_CLEANUP;
> > -		wakeup(&ump->softdep_flushtd);
> > -	}
> > +	wakeup(&ump->softdep_flushtd);
> >   }
> >
> >   static int
> > @@ -1471,11 +1470,10 @@ softdep_speedup(ump)
> >   			TAILQ_INSERT_TAIL(&softdepmounts, sdp, sd_next);
> >   			FREE_GBLLOCK(&lk);
> >   			if ((altump->softdep_flags &
> > -			    (FLUSH_CLEANUP | FLUSH_EXIT)) == 0) {
> > +			    (FLUSH_CLEANUP | FLUSH_EXIT)) == 0)
> >   				altump->softdep_flags |= FLUSH_CLEANUP;
> > -				altump->um_softdep->sd_cleanups++;
> > -				wakeup(&altump->softdep_flushtd);
> > -			}
> > +			altump->um_softdep->sd_cleanups++;
> > +			wakeup(&altump->softdep_flushtd);
> >   			FREE_LOCK(altump);
> >   		}
> >   	}
> > _______________________________________________
> 
> Why the conversion of while() to if()?
> 
> 
> The reason for a while() when doing msleep/wakeup is typically to 
> prevent superfluous wakeups from signalling early.

if()->while() was one of the changes in r277922, and I suspect that it
is the cause of the issue.  I.e. my thought right now is that
softdep_process_worklist() does not keep up with the requests.

If this is true, then real fix is somewhere else, but restoring
pre-r277922 behaviour should get rid of immediate pain.