Re: WABPL tstile/performance issues...[LONG]

[Greg Oster <oster%cs.usask.ca@localhost>]

Juergen Hannken-Illjes writes:
> On Sun, Jan 25, 2009 at 08:29:01PM -0600, Greg Oster wrote:
> [snip]
> 
> > (gdb) list *0xc0352316
> > 0xc0352316 is in ffs_full_fsync (/u1/devel/current2/src/sys/ufs/ffs/ffs_vno
> ps.c:477).
> > 472                      * Don't flush the log if the vnode being flushed
> > 473                      * contains no dirty buffers that could be in the l
> og.
> > 474                      */
> > 475                     if (!((flags & FSYNC_RECLAIM) &&
> > 476                         LIST_EMPTY(&vp->v_dirtyblkhd))) {
> > 477                             error = wapbl_flush(mp->mnt_wapbl, 0);
> 
> If I get this condition right, wapbl_flush() will be called if either
> the FSYNC_RECLAIM flag is not set (this is the common case) or the
> vnode has dirty buffers.
> 
> What is the reason for wapbl_flush() to depend on FSYNC_RECLAIM that is only
> set for vinvalbuf(..., V_SAVE, ...).

So I'd like to know this too.....  What I think is happening here is 
that (flags & FSYNC_RECLAIM) is almost always false, resulting in the 
flush *always* being called, whether or not there are are any items 
on the &vp->v_dirtyblkhd list.  When I get rid of that part of the 
condition, and just run with:

                if (!(LIST_EMPTY(&vp->v_dirtyblkhd))) {
                        error = wapbl_flush(mp->mnt_wapbl, 0);
                        if (error)
                                return error;
                }

the whole "silly behavior" goes away... (or at least has gone looking 
for a different place to show up..)

Ok.. a bit more discussion: 

As far as I can tell,  FSYNC_RECLAIM is set *only* by the call to 
ffs_full_fsync() from within vinvalbuf().  I believe the original 
code is an optimization for the case where ffs_full_fsync() is being 
called via vinvalbuf() -- in that case, we know we're doing 
FSYNC_RECLAIM, and there is no point in calling wapbl_flush() if the 
&vp->v_dirtyblkhd list is emtpy.  That part is all fine and well.
The problem comes in when FSYNC_RECLAIM is *not* set.  That means 
that wapbl_flush() is being called for every vp that comes in, 
regardless of what is in the &vp->v_dirtyblkhd list.

Additional thoughts?  Is nuking the FSYNC_RECLAIM the right check?
(the above code will still do the right thing if FSYNC_RECLAIM is 
set, since it will still be keying off of anything being in the 
&vp->v_dirtyblkhd list)

Later...

Greg Oster