From owner-freebsd-current Wed Jun 14 13:29:53 2000 Delivered-To: freebsd-current@freebsd.org Received: from critter.freebsd.dk (critter.freebsd.dk [212.242.40.131]) by hub.freebsd.org (Postfix) with ESMTP id A674337C209 for ; Wed, 14 Jun 2000 13:29:37 -0700 (PDT) (envelope-from phk@critter.freebsd.dk) Received: from critter.freebsd.dk (localhost [127.0.0.1]) by critter.freebsd.dk (8.9.3/8.9.3) with ESMTP id WAA18320 for ; Wed, 14 Jun 2000 22:29:32 +0200 (CEST) (envelope-from phk@critter.freebsd.dk) To: current@freebsd.org Subject: HEADSUP: bioops patch. From: Poul-Henning Kamp Date: Wed, 14 Jun 2000 22:29:32 +0200 Message-ID: <18317.961014572@critter.freebsd.dk> Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG This patch virtualizes & untangles the bioops operations vector. Background: The bioops operation vector is a list of OO-like operations which can be performed on struct buf. They are used by the softupdates code to handle dependencies. Ideally struct buf should have had a real OO like operations vector like vnodes have it, and struct bioops is the first step towards that. One of the reasons we should have OO-like struct buf, is that as long as bwrite(bp) "knows" that the buffer is backed by a disk device, we cannot use the UFS layer on top of a storage manager which isn't based on disk-devices: When UFS modifies a directory inode, it will call bwrite(bp) on the buffer with the data. This would not work if the backing were based on malloc(9) or anonymous swap-backed VM objects for instance. In other words: this is the main reason why we don't have a decent tmpfs in FreeBSD. Instead of just assuming that it works on a disk, bwrite(bp) should do a "bp->b_ops->bwrite(bp)" so that each buffer could have its own private idea of how to write itself, depending on what backing it has. So in order to move bioops closer to become a bp->b_ops, this patch takes two entries out of bioops: the "sync" and the "fsync" items and virtualizes the rest of the elements a bit. The "sync" item is called only from the syncer, and is a call to the softupdates code to do what it needs to do for periodic syncing. The real way of doing that would be to have an event-handler for this since other code could need to be part of the sync trafic, raid5 private parity caches could be one example. I have not done this yet, since currently softupdates is the only client. The fsync item really doesn't belong in the fsync system call, it belongs in ffs_fsync, and has been moved there. To give the right behaviour when SOFTUPDATES is not compiled in, stubs for both of these functions have been added to ffs_softdep_stub.c Finally all the checks to see if the bioops vector is populated has been centralized in in-line functions in thereby paving the road for the global bioops to become bp->b_ops. Comments, reviews, tests please Poul-Henning Index: contrib/softupdates/ffs_softdep.c =================================================================== RCS file: /home/ncvs/src/sys/contrib/softupdates/ffs_softdep.c,v retrieving revision 1.64 diff -u -r1.64 ffs_softdep.c --- contrib/softupdates/ffs_softdep.c 2000/05/26 02:01:59 1.64 +++ contrib/softupdates/ffs_softdep.c 2000/06/14 19:26:46 @@ -222,8 +222,6 @@ softdep_disk_io_initiation, /* io_start */ softdep_disk_write_complete, /* io_complete */ softdep_deallocate_dependencies, /* io_deallocate */ - softdep_fsync, /* io_fsync */ - softdep_process_worklist, /* io_sync */ softdep_move_dependencies, /* io_movedeps */ softdep_count_dependencies, /* io_countdeps */ }; Index: kern/vfs_bio.c =================================================================== RCS file: /home/ncvs/src/sys/kern/vfs_bio.c,v retrieving revision 1.258 diff -u -r1.258 vfs_bio.c --- kern/vfs_bio.c 2000/05/26 02:04:39 1.258 +++ kern/vfs_bio.c 2000/06/14 19:00:56 @@ -616,8 +616,8 @@ newbp->b_flags &= ~B_INVAL; /* move over the dependencies */ - if (LIST_FIRST(&bp->b_dep) != NULL && bioops.io_movedeps) - (*bioops.io_movedeps)(bp, newbp); + if (LIST_FIRST(&bp->b_dep) != NULL) + buf_movedeps(bp, newbp); /* * Initiate write on the copy, release the original to @@ -673,10 +673,10 @@ /* * Process dependencies then return any unfinished ones. */ - if (LIST_FIRST(&bp->b_dep) != NULL && bioops.io_complete) - (*bioops.io_complete)(bp); - if (LIST_FIRST(&bp->b_dep) != NULL && bioops.io_movedeps) - (*bioops.io_movedeps)(bp, origbp); + if (LIST_FIRST(&bp->b_dep) != NULL) + buf_complete(bp); + if (LIST_FIRST(&bp->b_dep) != NULL) + buf_movedeps(bp, origbp); /* * Clear the BX_BKGRDINPROG flag in the original buffer * and awaken it if it is waiting for the write to complete. @@ -939,8 +939,8 @@ * cache the buffer. */ bp->b_flags |= B_INVAL; - if (LIST_FIRST(&bp->b_dep) != NULL && bioops.io_deallocate) - (*bioops.io_deallocate)(bp); + if (LIST_FIRST(&bp->b_dep) != NULL) + buf_deallocate(bp); if (bp->b_flags & B_DELWRI) { --numdirtybuffers; numdirtywakeup(); @@ -1570,8 +1570,8 @@ crfree(bp->b_wcred); bp->b_wcred = NOCRED; } - if (LIST_FIRST(&bp->b_dep) != NULL && bioops.io_deallocate) - (*bioops.io_deallocate)(bp); + if (LIST_FIRST(&bp->b_dep) != NULL) + buf_deallocate(bp); if (bp->b_xflags & BX_BKGRDINPROG) panic("losing buffer 3"); LIST_REMOVE(bp, b_hash); @@ -1848,9 +1848,8 @@ break; } if (LIST_FIRST(&bp->b_dep) != NULL && - bioops.io_countdeps && (bp->b_flags & B_DEFERRED) == 0 && - (*bioops.io_countdeps)(bp, 0)) { + buf_countdeps(bp, 0)) { TAILQ_REMOVE(&bufqueues[QUEUE_DIRTY], bp, b_freelist); TAILQ_INSERT_TAIL(&bufqueues[QUEUE_DIRTY], @@ -2664,8 +2663,8 @@ splx(s); return; } - if (LIST_FIRST(&bp->b_dep) != NULL && bioops.io_complete) - (*bioops.io_complete)(bp); + if (LIST_FIRST(&bp->b_dep) != NULL) + buf_complete(bp); if (bp->b_flags & B_VMIO) { int i, resid; Index: kern/vfs_cluster.c =================================================================== RCS file: /home/ncvs/src/sys/kern/vfs_cluster.c,v retrieving revision 1.98 diff -u -r1.98 vfs_cluster.c --- kern/vfs_cluster.c 2000/05/05 09:58:25 1.98 +++ kern/vfs_cluster.c 2000/06/14 19:01:13 @@ -805,9 +805,8 @@ splx(s); } /* end of code for non-first buffers only */ /* check for latent dependencies to be handled */ - if ((LIST_FIRST(&tbp->b_dep)) != NULL && - bioops.io_start) - (*bioops.io_start)(tbp); + if ((LIST_FIRST(&tbp->b_dep)) != NULL) + buf_start(tbp); /* * If the IO is via the VM then we do some * special VM hackery. (yuck) Index: kern/vfs_subr.c =================================================================== RCS file: /home/ncvs/src/sys/kern/vfs_subr.c,v retrieving revision 1.257 diff -u -r1.257 vfs_subr.c --- kern/vfs_subr.c 2000/05/26 02:04:39 1.257 +++ kern/vfs_subr.c 2000/06/14 19:56:33 @@ -1029,8 +1029,7 @@ /* * Do soft update processing. */ - if (bioops.io_sync) - (*bioops.io_sync)(NULL); + softdep_process_worklist(NULL); /* * The variable rushjob allows the kernel to speed up the Index: kern/vfs_syscalls.c =================================================================== RCS file: /home/ncvs/src/sys/kern/vfs_syscalls.c,v retrieving revision 1.153 diff -u -r1.153 vfs_syscalls.c --- kern/vfs_syscalls.c 2000/05/05 09:58:27 1.153 +++ kern/vfs_syscalls.c 2000/06/14 19:38:24 @@ -2545,10 +2545,7 @@ vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, p); if (vp->v_object) vm_object_page_clean(vp->v_object, 0, 0, 0); - if ((error = VOP_FSYNC(vp, fp->f_cred, MNT_WAIT, p)) == 0 && - vp->v_mount && (vp->v_mount->mnt_flag & MNT_SOFTDEP) && - bioops.io_fsync) - error = (*bioops.io_fsync)(vp); + error = VOP_FSYNC(vp, fp->f_cred, MNT_WAIT, p); VOP_UNLOCK(vp, 0, p); return (error); } Index: miscfs/devfs/devfs_vnops.c =================================================================== RCS file: /home/ncvs/src/sys/miscfs/devfs/devfs_vnops.c,v retrieving revision 1.96 diff -u -r1.96 devfs_vnops.c --- miscfs/devfs/devfs_vnops.c 2000/05/05 09:58:29 1.96 +++ miscfs/devfs/devfs_vnops.c 2000/06/14 19:02:35 @@ -1570,9 +1570,8 @@ return error; - if ((bp->b_iocmd == BIO_WRITE) && - (LIST_FIRST(&bp->b_dep)) != NULL && bioops.io_start) - (*bioops.io_start)(bp); + if ((bp->b_iocmd == BIO_WRITE) && (LIST_FIRST(&bp->b_dep)) != NULL) + buf_start(bp); switch (vp->v_type) { case VCHR: (*vp->v_rdev->si_devsw->d_strategy)(&bp->b_io); Index: miscfs/specfs/spec_vnops.c =================================================================== RCS file: /home/ncvs/src/sys/miscfs/specfs/spec_vnops.c,v retrieving revision 1.138 diff -u -r1.138 spec_vnops.c --- miscfs/specfs/spec_vnops.c 2000/06/12 10:20:18 1.138 +++ miscfs/specfs/spec_vnops.c 2000/06/14 19:02:49 @@ -417,9 +417,8 @@ struct mount *mp; bp = ap->a_bp; - if ((bp->b_iocmd == BIO_WRITE) && - (LIST_FIRST(&bp->b_dep)) != NULL && bioops.io_start) - (*bioops.io_start)(bp); + if ((bp->b_iocmd == BIO_WRITE) && (LIST_FIRST(&bp->b_dep)) != NULL) + buf_start(bp); /* * Collect statistics on synchronous and asynchronous read Index: sys/buf.h =================================================================== RCS file: /home/ncvs/src/sys/sys/buf.h,v retrieving revision 1.103 diff -u -r1.103 buf.h --- sys/buf.h 2000/05/26 02:06:53 1.103 +++ sys/buf.h 2000/06/14 19:26:59 @@ -64,8 +64,6 @@ void (*io_start) __P((struct buf *)); void (*io_complete) __P((struct buf *)); void (*io_deallocate) __P((struct buf *)); - int (*io_fsync) __P((struct vnode *)); - int (*io_sync) __P((struct mount *)); void (*io_movedeps) __P((struct buf *, struct buf *)); int (*io_countdeps) __P((struct buf *, int)); } bioops; @@ -405,6 +403,43 @@ #define BUF_WRITE(bp) VOP_BWRITE((bp)->b_vp, (bp)) #define BUF_STRATEGY(bp) VOP_STRATEGY((bp)->b_vp, (bp)) + +static __inline void +buf_start(struct buf *bp) +{ + if (bioops.io_start) + (*bioops.io_start)(bp); +} + +static __inline void +buf_complete(struct buf *bp) +{ + if (bioops.io_complete) + (*bioops.io_complete)(bp); +} + +static __inline void +buf_deallocate(struct buf *bp) +{ + if (bioops.io_deallocate) + (*bioops.io_deallocate)(bp); +} + +static __inline void +buf_movedeps(struct buf *bp, struct buf *bp2) +{ + if (bioops.io_movedeps) + (*bioops.io_movedeps)(bp, bp2); +} + +static __inline int +buf_countdeps(struct buf *bp, int i) +{ + if (bioops.io_countdeps) + return ((*bioops.io_countdeps)(bp, i)); + else + return (0); +} #endif /* _KERNEL */ Index: sys/mount.h =================================================================== RCS file: /home/ncvs/src/sys/sys/mount.h,v retrieving revision 1.91 diff -u -r1.91 mount.h --- sys/mount.h 2000/05/26 02:06:55 1.91 +++ sys/mount.h 2000/06/14 19:58:22 @@ -447,6 +447,7 @@ int vfs_stdextattrctl __P((struct mount *mp, int cmd, const char *attrname, caddr_t arg, struct proc *p)); +void softdep_process_worklist __P((struct mount *)); #else /* !_KERNEL */ #include Index: ufs/ffs/ffs_softdep_stub.c =================================================================== RCS file: /home/ncvs/src/sys/ufs/ffs/ffs_softdep_stub.c,v retrieving revision 1.8 diff -u -r1.8 ffs_softdep_stub.c --- ufs/ffs/ffs_softdep_stub.c 2000/04/19 14:58:25 1.8 +++ ufs/ffs/ffs_softdep_stub.c 2000/06/14 19:33:11 @@ -254,4 +254,20 @@ return (0); } + +int +softdep_fsync(vp) + struct vnode *vp; /* the "in_core" copy of the inode */ +{ + + return (0); +} + +int +softdep_process_worklist(matchmnt) + struct mount *matchmnt; +{ + return (0); +} + #endif /* SOFTUPDATES not configured in */ Index: ufs/ffs/ffs_vnops.c =================================================================== RCS file: /home/ncvs/src/sys/ufs/ffs/ffs_vnops.c,v retrieving revision 1.66 diff -u -r1.66 ffs_vnops.c --- ufs/ffs/ffs_vnops.c 2000/05/05 09:59:05 1.66 +++ ufs/ffs/ffs_vnops.c 2000/06/14 19:38:13 @@ -175,7 +175,7 @@ continue; if (!wait && LIST_FIRST(&bp->b_dep) != NULL && (bp->b_flags & B_DEFERRED) == 0 && - bioops.io_countdeps && (*bioops.io_countdeps)(bp, 0)) { + buf_countdeps(bp, 0)) { bp->b_flags |= B_DEFERRED; continue; } @@ -278,5 +278,8 @@ } } splx(s); - return (UFS_UPDATE(vp, wait)); + error = UFS_UPDATE(vp, wait); + if (error == 0 && vp->v_mount && (vp->v_mount->mnt_flag & MNT_SOFTDEP)) + error = softdep_fsync(vp); + return (error); } -- Poul-Henning Kamp | UNIX since Zilog Zeus 3.20 phk@FreeBSD.ORG | TCP/IP since RFC 956 FreeBSD coreteam member | BSD since 4.3-tahoe Never attribute to malice what can adequately be explained by incompetence. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message