Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 11 Oct 1999 14:06:38 +0800
From:      Peter Wemm <peter@netplex.com.au>
To:        Matthew Dillon <dillon@apollo.backplane.com>
Cc:        Poul-Henning Kamp <phk@critter.freebsd.dk>, "Daniel C. Sobral" <dcs@newsguy.com>, cvs-committers@FreeBSD.ORG, cvs-all@FreeBSD.ORG
Subject:   Re: cvs commit: src/sys/vm vm_swap.c 
Message-ID:  <19991011060638.B3D8D1CCD@overcee.netplex.com.au>
In-Reply-To: Your message of "Sun, 10 Oct 1999 18:04:12 MST." <199910110104.SAA18509@apollo.backplane.com> 

next in thread | previous in thread | raw e-mail | index | archive | help
Matthew Dillon wrote:
> :>    If you implement a means to create unassociated (i.e. no major/minor
> :>    number at all) devices, the swap device can be converted to one 
> :>    internally.  We aren't going to get rid of the cdevsw structure, though
    .
> :>    Creating totally custom I/O interfaces is never a good idea.
> :
> :You don't need a dev_t, in fact one or two levels of indirection through
> :vnop/devsw can be ripped right out by calling swstrategy() directly.
> :
> :--
> :Poul-Henning Kamp             FreeBSD coreteam member
> 
>     We're not removing the device interface.  It provides a reasonable
>     abstraction and a nice demark between swap_pager.c and vm_swap.c,
>     as well as potential flexibility that could be useful in the future
>     Removing it will not save time or much in the way of code space.

The swap *device* is non-functional and doesn't *do* anything except
provide a subroutine call in a very roundabout way.  I think it would be
far better to avoid an indirection via the VOP_* system for no useful gain
and do something like the aooended patch (which works perfectly here BTW,
even under heavy swap load on multiple disks).  Further (micro)
optimizations are possible, for example pbgetvp() is used to get a p-buffer
that's associated with swapdev_vp and the device.  The device isn't used
and presently vn_todev(vp) ends up returning NODEV.  swapdev_vp is kinda
orphaned with this change but still works ok. p-buffers are created being
assigned to it and then are pbreassignbuf'ed to the real backing vnode in
swstrategy().

Index: swap_pager.c
===================================================================
RCS file: /home/ncvs/src/sys/vm/swap_pager.c,v
retrieving revision 1.126
diff -u -r1.126 swap_pager.c
--- swap_pager.c	1999/09/17 05:09:24	1.126
+++ swap_pager.c	1999/10/11 05:33:27
@@ -1130,11 +1130,11 @@
 	 * The other pages in our m[] array are also released on completion,
 	 * so we cannot assume they are valid anymore either.
 	 *
-	 * NOTE: b_blkno is destroyed by the call to VOP_STRATEGY
+	 * NOTE: b_blkno is destroyed by the call to swstrategy()
 	 */
 
 	BUF_KERNPROC(bp);
-	VOP_STRATEGY(bp->b_vp, bp);
+	swstrategy(bp);
 
 	/*
 	 * wait for the page we want to complete.  PG_SWAPINPROG is always
@@ -1187,7 +1187,7 @@
  *	We support both OBJT_DEFAULT and OBJT_SWAP objects.  DEFAULT objects
  *	are automatically converted to SWAP objects.
  *
- *	In a low memory situation we may block in VOP_STRATEGY(), but the new 
+ *	In a low memory situation we may block in swstrategy(), but the new 
  *	vm_page reservation system coupled with properly written VFS devices 
  *	should ensure that no low-memory deadlock occurs.  This is an area
  *	which needs work.
@@ -1381,13 +1381,13 @@
 		/*
 		 * asynchronous
 		 *
-		 * NOTE: b_blkno is destroyed by the call to VOP_STRATEGY
+		 * NOTE: b_blkno is destroyed by the call to swstrategy()
 		 */
 
 		if (sync == FALSE) {
 			bp->b_iodone = swp_pager_async_iodone;
 			BUF_KERNPROC(bp);
-			VOP_STRATEGY(bp->b_vp, bp);
+			swstrategy(bp);
 
 			for (j = 0; j < n; ++j)
 				rtvals[i+j] = VM_PAGER_PEND;
@@ -1397,11 +1397,11 @@
 		/*
 		 * synchronous
 		 *
-		 * NOTE: b_blkno is destroyed by the call to VOP_STRATEGY
+		 * NOTE: b_blkno is destroyed by the call to swstrategy()
 		 */
 
 		bp->b_iodone = swp_pager_sync_iodone;
-		VOP_STRATEGY(bp->b_vp, bp);
+		swstrategy(bp);
 
 		/*
 		 * Wait for the sync I/O to complete, then update rtvals.
Index: swap_pager.h
===================================================================
RCS file: /home/ncvs/src/sys/vm/swap_pager.h,v
retrieving revision 1.26
diff -u -r1.26 swap_pager.h
--- swap_pager.h	1999/09/17 05:09:24	1.26
+++ swap_pager.h	1999/10/11 05:33:27
@@ -104,6 +104,10 @@
 
 void swap_pager_page_removed __P((vm_page_t, vm_object_t));
 
+/* choose underlying swap device and queue up I/O */
+struct buf;
+void swstrategy __P((struct buf *bp));	/* probably needs to move elsewhere */
+
 #endif
 
 #endif				/* _SWAP_PAGER_ */
Index: vm_swap.c
===================================================================
RCS file: /home/ncvs/src/sys/vm/vm_swap.c,v
retrieving revision 1.89
diff -u -r1.89 vm_swap.c
--- vm_swap.c	1999/10/08 19:10:18	1.89
+++ vm_swap.c	1999/10/11 05:33:27
@@ -55,40 +55,6 @@
 #include <vm/swap_pager.h>
 
 /*
- * "sw" is a fake device implemented
- * in vm_swap.c and used only internally to get to swstrategy.
- * It cannot be provided to the users, because the
- * swstrategy routine munches the b_dev and b_blkno entries
- * before calling the appropriate driver.  This would horribly
- * confuse, e.g. the hashing routines. Instead, /dev/drum is
- * provided as a character (raw) device.
- */
-
-static	d_open_t	swopen;
-static	d_strategy_t    swstrategy;
-
-#define CDEV_MAJOR 4
-#define BDEV_MAJOR 26
-
-static struct cdevsw sw_cdevsw = {
-	/* open */	swopen,
-	/* close */	nullclose,
-	/* read */	noread,
-	/* write */	nowrite,
-	/* ioctl */	noioctl,
-	/* poll */	nopoll,
-	/* mmap */	nommap,
-	/* strategy */	swstrategy,
-	/* name */	"sw",
-	/* maj */	CDEV_MAJOR,
-	/* dump */	nodump,
-	/* psize */	nopsize,
-	/* flags */	D_DISK,
-	/* bmaj */	BDEV_MAJOR
-};
-
-
-/*
  * Indirect driver for multi-controller paging.
  */
 
@@ -109,22 +75,8 @@
  *
  *	The bp is expected to be locked and *not* B_DONE on call.
  */
-
-static int
-swopen(dev, flag, mode, p)
-	dev_t           dev;
-	int             flag;
-	int             mode;
-	struct proc     *p;
-{
 
-	if (mode == S_IFBLK || minor(dev) != 0)
-		return (ENXIO);
-	return (0);
-}
-
-
-static void
+void
 swstrategy(bp)
 	register struct buf *bp;
 {
@@ -132,17 +84,6 @@
 	register struct swdevt *sp;
 	struct vnode *vp;
 
-	/*
-	 * XXX: if we allow userland to come through, it will panic
-	 * XXX: so use minor zero for userland and fail it right here
-	 * XXX: and in noread/nowrite.
-	 */
-	if (minor(bp->b_dev) == 0) {
-		bp->b_error = EINVAL;
-		bp->b_flags |= B_ERROR;
-		biodone(bp);
-		return;
-	}
 	sz = howmany(bp->b_bcount, PAGE_SIZE);
 	/*
 	 * Convert interleaved swap into per-device swap.  Note that
@@ -224,12 +165,7 @@
 	dev_t dev;
 	struct nameidata nd;
 	int error;
-	static int once;
 
-	if (!once) {
-		make_dev(&sw_cdevsw, 0, UID_ROOT, GID_KMEM, 0640, "drum");
-		once++;
-	}
 	error = suser(p);
 	if (error)
 		return (error);
@@ -361,19 +297,11 @@
 	}
 
 	if (!swapdev_vp) {
-		struct vnode *vp1;
-		struct vnode *nvp;
-		dev_t dev;
-
 		error = getnewvnode(VT_NON, (struct mount *) 0,
-		    spec_vnodeop_p, &nvp);
+		    spec_vnodeop_p, &swapdev_vp);
 		if (error)
 			panic("Cannot get vnode for swapdev");
-		vp1 = nvp;
-		vp1->v_type = VBLK;
-		dev = make_dev(&sw_cdevsw, 1, UID_ROOT, GID_KMEM, 0640, "swapdev");
-		addalias(vp1, dev);
-		swapdev_vp = vp1;
+		swapdev_vp->v_type = VBLK;
 	}
 	return (0);
 }


Cheers,
-Peter



To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?19991011060638.B3D8D1CCD>