Re: WAPBL patch for testing

[Juergen Hannken-Illjes <hannken%eis.cs.tu-bs.de@localhost>]

On Sat, Nov 01, 2008 at 12:29:32AM +0100, Joerg Sonnenberger wrote:
> On Fri, Oct 31, 2008 at 10:01:36PM +0100, Joerg Sonnenberger wrote:
> > On Fri, Oct 31, 2008 at 08:03:32PM +0100, Juergen Hannken-Illjes wrote:
> > > The reason for these wapbl locks is ffs snapshots.  VOP_PUTPAGES() may
> > > need to copy-on-write which needs to run inside a wapbl transaction
> > > as we allocate blocks here.
> > 
> > Hm. I see. Can you try the attached patch?
> 
> ...and a version that doesn't crash immediate if non-ufs filesystems are
> used.
> 
> Joerg

This would bring back a deadlock (see Rev. 1.12 of genfs_io.c).  We have
to take the wapbl LOCK before we lock pages.

-- 
Juergen Hannken-Illjes - hannken%eis.cs.tu-bs.de@localhost - TU Braunschweig 
(Germany)

> Index: genfs_io.c
> ===================================================================
> RCS file: /home/joerg/repo/netbsd/src/sys/miscfs/genfs/genfs_io.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 genfs_io.c
> --- genfs_io.c        19 Oct 2008 18:17:13 -0000      1.13
> +++ genfs_io.c        31 Oct 2008 23:22:36 -0000
> @@ -779,7 +779,6 @@ genfs_do_putpages(struct vnode *vp, off_
>       int flags;
>       int dirtygen;
>       bool modified;
> -     bool need_wapbl;
>       bool has_trans;
>       bool cleanall;
>       bool onworklst;
> @@ -794,8 +793,6 @@ genfs_do_putpages(struct vnode *vp, off_
>           vp, uobj->uo_npages, startoff, endoff - startoff);
>  
>       has_trans = false;
> -     need_wapbl = (!pagedaemon && vp->v_mount && vp->v_mount->mnt_wapbl &&
> -         (origflags & PGO_JOURNALLOCKED) == 0);
>  
>  retry:
>       modified = false;
> @@ -809,8 +806,6 @@ retry:
>                               vn_syncer_remove_from_worklist(vp);
>               }
>               if (has_trans) {
> -                     if (need_wapbl)
> -                             WAPBL_END(vp->v_mount);
>                       fstrans_done(vp->v_mount);
>               }
>               mutex_exit(slock);
> @@ -829,13 +824,6 @@ retry:
>                               return error;
>               } else
>                       fstrans_start(vp->v_mount, FSTRANS_LAZY);
> -             if (need_wapbl) {
> -                     error = WAPBL_BEGIN(vp->v_mount);
> -                     if (error) {
> -                             fstrans_done(vp->v_mount);
> -                             return error;
> -                     }
> -             }
>               has_trans = true;
>               mutex_enter(slock);
>               goto retry;
> @@ -1210,8 +1198,6 @@ skip_scan:
>       }
>  
>       if (has_trans) {
> -             if (need_wapbl)
> -                     WAPBL_END(vp->v_mount);
>               fstrans_done(vp->v_mount);
>       }
>  
> @@ -1283,6 +1269,8 @@ genfs_do_io(struct vnode *vp, off_t off,
>       struct vnode *devvp;
>       bool async = (flags & PGO_SYNCIO) == 0;
>       bool write = rw == UIO_WRITE;
> +     bool need_wapbl = (write && vp->v_mount && vp->v_mount->mnt_wapbl &&
> +         (flags & PGO_JOURNALLOCKED) == 0);
>       int brw = write ? B_WRITE : B_READ;
>       UVMHIST_FUNC(__func__); UVMHIST_CALLED(ubchist);
>  
> @@ -1304,6 +1292,12 @@ genfs_do_io(struct vnode *vp, off_t off,
>       skipbytes = 0;
>       KASSERT(bytes != 0);
>  
> +     if (need_wapbl) {
> +             error = WAPBL_BEGIN(vp->v_mount);
> +             if (error)
> +                     return error;
> +     }
> +
>       if (write) {
>               mutex_enter(&vp->v_interlock);
>               vp->v_numoutput += 2;
> @@ -1380,14 +1374,19 @@ genfs_do_io(struct vnode *vp, off_t off,
>       nestiobuf_done(mbp, skipbytes, error);
>       if (async) {
>               UVMHIST_LOG(ubchist, "returning 0 (async)", 0,0,0,0);
> -             return (0);
> +             error = 0;
> +     } else {
> +             UVMHIST_LOG(ubchist, "waiting for mbp %p", mbp,0,0,0);
> +             error = biowait(mbp);
> +             s = splbio();
> +             (*iodone)(mbp);
> +             splx(s);
> +             UVMHIST_LOG(ubchist, "returning, error %d", error,0,0,0);
>       }
> -     UVMHIST_LOG(ubchist, "waiting for mbp %p", mbp,0,0,0);
> -     error = biowait(mbp);
> -     s = splbio();
> -     (*iodone)(mbp);
> -     splx(s);
> -     UVMHIST_LOG(ubchist, "returning, error %d", error,0,0,0);
> +
> +     if (need_wapbl)
> +             WAPBL_END(vp->v_mount);
> +
>       return (error);
>  }
>  
> @@ -1585,12 +1584,6 @@ genfs_directio(struct vnode *vp, struct 
>               return;
>       }
>  
> -     if ((ioflag & IO_JOURNALLOCKED) == 0) {
> -             error = WAPBL_BEGIN(vp->v_mount);
> -             if (error)
> -                     return;
> -     }
> -
>       /*
>        * Do as much of the uio as possible with direct I/O.
>        */
> @@ -1636,9 +1629,6 @@ genfs_directio(struct vnode *vp, struct 
>               uio->uio_offset += len;
>               uio->uio_resid -= len;
>       }
> -
> -     if ((ioflag & IO_JOURNALLOCKED) == 0)
> -             WAPBL_END(vp->v_mount);
>  }
>  
>  /*