Subject: Re: kern/37109: fscow_run + swap on regular file deadlock
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Juergen Hannken-Illjes <hannken@eis.cs.tu-bs.de>
List: netbsd-bugs
Date: 10/13/2007 21:00:10
The following reply was made to PR kern/37109; it has been noted by GNATS.

From: Juergen Hannken-Illjes <hannken@eis.cs.tu-bs.de>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/37109: fscow_run + swap on regular file deadlock
Date: Sat, 13 Oct 2007 21:44:59 +0200

 On Sat, Oct 13, 2007 at 06:40:02PM +0000, YAMAMOTO Takashi wrote:
 > 
 >  >  With this diff VOP_STRATEGY() is no longer called from interrupt context.
 >  >  Yamt, ok to commit?
 >  
 >  i'd like to see workqueue_create/destroy in swapon/off.
 >  otherwise seems fine to me.
 >  
 >  YAMAMOTO Takashi
 
 Ok, new diff follows.
 
 -- 
 Juergen Hannken-Illjes - hannken@eis.cs.tu-bs.de - TU Braunschweig (Germany)
 
 Index: uvm_swap.c
 ===================================================================
 RCS file: /cvsroot/src/sys/uvm/uvm_swap.c,v
 retrieving revision 1.129
 diff -p -u -2 -r1.129 uvm_swap.c
 --- uvm_swap.c	29 Jul 2007 13:31:18 -0000	1.129
 +++ uvm_swap.c	13 Oct 2007 19:42:39 -0000
 @@ -61,4 +61,5 @@ __KERNEL_RCSID(0, "$NetBSD: uvm_swap.c,v
  #include <sys/kauth.h>
  #include <sys/sysctl.h>
 +#include <sys/workqueue.h>
  
  #include <uvm/uvm.h>
 @@ -221,4 +222,8 @@ static struct swap_priority swap_priorit
  static krwlock_t swap_syscall_lock;
  
 +/* workqueue and use counter for swap to regular files */
 +static int sw_reg_count = 0;
 +static struct workqueue *sw_reg_workqueue;
 +
  /*
   * prototypes
 @@ -237,5 +242,6 @@ static void uvm_swap_stats_locked(int, s
  
  static void sw_reg_strategy(struct swapdev *, struct buf *, int);
 -static void sw_reg_iodone(struct buf *);
 +static void sw_reg_biodone(struct buf *);
 +static void sw_reg_iodone(struct work *wk, void *dummy);
  static void sw_reg_start(struct swapdev *);
  
 @@ -959,4 +965,16 @@ swap_on(struct lwp *l, struct swapdev *s
  	if (result == 0)
  		panic("swapdrum_add");
 +	/*
 +	 * If this is the first regular swap create the workqueue.
 +	 * => Protected by swap_syscall_lock.
 +	 */
 +	if (vp->v_type != VBLK) {
 +		if (sw_reg_count++ == 0) {
 +			KASSERT(sw_reg_workqueue == NULL);
 +			if (workqueue_create(&sw_reg_workqueue, "swapiod",
 +			    sw_reg_iodone, NULL, PRIBIO, IPL_BIO, 0) != 0)
 +				panic("swap_add: workqueue_create failed");
 +		}
 +	}
  
  	sdp->swd_drumoffset = (int)result;
 @@ -1030,4 +1048,17 @@ swap_off(struct lwp *l, struct swapdev *
  
  	/*
 +	 * If this is the last regular swap destroy the workqueue.
 +	 * => Protected by swap_syscall_lock.
 +	 */
 +	if (sdp->swd_vp->v_type != VBLK) {
 +		KASSERT(sw_reg_count > 0);
 +		KASSERT(sw_reg_workqueue != NULL);
 +		if (--sw_reg_count == 0) {
 +			workqueue_destroy(sw_reg_workqueue);
 +			sw_reg_workqueue = NULL;
 +		}
 +	}
 +
 +	/*
  	 * done with the vnode.
  	 * drop our ref on the vnode before calling VOP_CLOSE()
 @@ -1288,5 +1319,5 @@ sw_reg_strategy(struct swapdev *sdp, str
  		nbp->vb_buf.b_blkno    = nbn + btodb(off);
  		nbp->vb_buf.b_rawblkno = nbp->vb_buf.b_blkno;
 -		nbp->vb_buf.b_iodone   = sw_reg_iodone;
 +		nbp->vb_buf.b_iodone   = sw_reg_biodone;
  		nbp->vb_buf.b_vp       = vp;
  		if (vp->v_type == VBLK) {
 @@ -1366,4 +1397,13 @@ sw_reg_start(struct swapdev *sdp)
  
  /*
 + * sw_reg_biodone: one of our i/o's has completed
 + */
 +static void
 +sw_reg_biodone(struct buf *bp)
 +{
 +	workqueue_enqueue(sw_reg_workqueue, &bp->b_work, NULL);
 +}
 +
 +/*
   * sw_reg_iodone: one of our i/o's has completed and needs post-i/o cleanup
   *
 @@ -1371,11 +1411,12 @@ sw_reg_start(struct swapdev *sdp)
   */
  static void
 -sw_reg_iodone(struct buf *bp)
 +sw_reg_iodone(struct work *wk, void *dummy)
  {
 -	struct vndbuf *vbp = (struct vndbuf *) bp;
 +	struct vndbuf *vbp = (void *)wk;
  	struct vndxfer *vnx = vbp->vb_xfer;
  	struct buf *pbp = vnx->vx_bp;		/* parent buffer */
  	struct swapdev	*sdp = vnx->vx_sdp;
  	int s, resid, error;
 +	KASSERT(&vbp->vb_buf.b_work == wk);
  	UVMHIST_FUNC("sw_reg_iodone"); UVMHIST_CALLED(pdhist);