From owner-freebsd-current Sun Mar 1 03:38:55 1998 Return-Path: Received: (from majordom@localhost) by hub.freebsd.org (8.8.8/8.8.8) id DAA01754 for freebsd-current-outgoing; Sun, 1 Mar 1998 03:38:55 -0800 (PST) (envelope-from owner-freebsd-current@FreeBSD.ORG) Received: from helios.dnttm.ru (root@dnttm-gw.rssi.ru [193.232.0.205]) by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id DAA01738 for ; Sun, 1 Mar 1998 03:38:47 -0800 (PST) (envelope-from dima@tejblum.dnttm.rssi.ru) Received: (from uucp@localhost) by helios.dnttm.ru (8.8.5/8.8.5/IP-3) with UUCP id NAA11917; Sun, 1 Mar 1998 13:24:39 +0300 Received: from tejblum.dnttm.rssi.ru (localhost [127.0.0.1]) by tejblum.dnttm.rssi.ru (8.8.8/8.8.7) with ESMTP id NAA02269; Sun, 1 Mar 1998 13:32:58 +0300 (MSK) (envelope-from dima@tejblum.dnttm.rssi.ru) Message-Id: <199803011032.NAA02269@tejblum.dnttm.rssi.ru> X-Mailer: exmh version 2.0gamma 1/27/96 To: Terry Lambert cc: FreeBSD-current@FreeBSD.ORG Subject: Re: VM: Process hangs sleeping on vmpfw In-reply-to: Your message of "Sat, 28 Feb 1998 22:57:42 GMT." <199802282257.PAA08480@usr08.primenet.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Date: Sun, 01 Mar 1998 13:32:57 +0300 From: Dmitrij Tejblum Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG Terry Lambert wrote: > > First, why default/standard/generic getpages/putpages routines does not > > have interface of VOP_GETPAGES/VOP_PUTPAGES vnode operations? It would be > > easier for a filesystem to just add some entries to their operations > > tables than also cut&paste implementation (even trivial) of these > > operations from ffs. > > Look at /sys/ufs/ufs/ufs_readwrite.c: > > ========================================================================== [...] > int > ffs_putpages(ap) > struct vop_putpages_args *ap; > { > return vnode_pager_generic_putpages(ap->a_vp, ap->a_m, ap->a_count, > ap->a_sync, ap->a_rtvals); > } > ========================================================================== Yes, I seen it. It is too complex. I guess you will understand my C better than my English. See a patch below. > > Second, why don't put the operations to default_vnodeop_entries? It is > > used exactly by local media filesystems. Stacking layers use bypass > > routines instead (unionfs is an exception). So, filesystems even would > > not notice this change, until they really want their own implementation > > of getpages/putpages > > > > What is wrong in the above? > > Unionfs. Specifically, the point is that the unionfs implementation > should fan out to the correct underlying implementation. This means > it shouldn't go into the default. But you must write correct getpages/putpages for unionfs in any case. Or, better, make a bypass routine for unionfs, to avoid similar problems with future new vnode operations :-). > Secondly, you can't make FS-specific optimizations. Nothing prevent a filesystem to implement its own getpages/putpages and override the default. > > I can send a patch for you... It is indeed pretty easy... > > See above for the patch. It's the identification problem and the > stacking problem that I wanted to handle on a case-by-case basis. I don't see why default ops doesn't solve stacking problems. > As I said, it's possible to make this change go transparent (#2, > above); I would actually prefer an implementation that does *not* > define the generic code as default ops, so if it has to go > transparent, it should go transparent that way, not the default > ops way, so at least there are warning messages emitted. If you want list of all filesystems to fix them you can do 'ls /sys', 'ls /sys/gnu', and 'ls /sys/miscfs' and exclude some non-filesystem directories. Also, if you really don't want the generic code be a default ops, you can make it non-default ops and insert the ops to each vnode table that require it. > You should also follow the NFS case in vnode_pager_generic_getpages > and look at all the other vp->v_mount references. Look what the > change means to VOP_BMAP, specifically with regard to the assumptions > comment in vnode_pager_generic_getpages and vnode_pager_input_smlfs's > being called -- with the same assumptions but without the test. It's > pretty obvious that the VOP_BMAP return test is equal to the NFS > test. This code is at the heart of a lot of problems, and I'd like > to take it slow... TBH, I dont understand why the generic_getpages mention NFS at all. NFS has its own getpages. Dima Here is my patch against -current as of yesterday. --- vm/vnode_pager.c Sat Feb 28 16:45:49 1998 +++ vm/vnode_pager.c Sun Mar 1 13:05:11 1998 @@ -514,14 +514,6 @@ vnode_pager_input_old(object, m) * generic vnode pager input routine */ -/* - * EOPNOTSUPP is no longer legal. For local media VFS's that do not - * implement their own VOP_GETPAGES, their VOP_GETPAGES should call to - * vnode_pager_generic_getpages() to implement the previous behaviour. - * - * All other FS's should use the bypass to get to the local media - * backing vp's VOP_GETPAGES. - */ static int vnode_pager_getpages(object, m, count, reqpage) vm_object_t object; @@ -529,46 +521,41 @@ vnode_pager_getpages(object, m, count, r int count; int reqpage; { - int rtval; - struct vnode *vp; - - vp = object->handle; - /* - * XXX temporary diagnostic message to help track stale FS code, - * Returning EOPNOTSUPP from here may make things unhappy. - */ - rtval = VOP_GETPAGES(vp, m, count*PAGE_SIZE, reqpage, 0); - if (rtval == EOPNOTSUPP) - printf("vnode_pager: *** WARNING *** stale FS code in system.\n"); - return rtval; + return (VOP_GETPAGES(object->handle, m, count*PAGE_SIZE, reqpage, 0)); } /* - * This is now called from local media FS's to operate against their - * own vnodes if they fail to implement VOP_GETPAGES. + * This is the standard implementation of VOP_GETPAGES. */ int -vnode_pager_generic_getpages(vp, m, bytecount, reqpage) - struct vnode *vp; - vm_page_t *m; - int bytecount; - int reqpage; +vop_stdgetpages(ap) + struct vop_getpages_args /* { + struct vnode *a_vp; + vm_page_t *a_m; + int a_count; + int a_reqpage; + vm_ooffset_t a_offset; + }*/ *ap; { vm_object_t object; vm_offset_t kva; + vm_page_t *m; off_t foff; int i, size, bsize, first, firstaddr; - struct vnode *dp; + struct vnode *dp, *vp; int runpg; int runend; struct buf *bp; int s; int count; + int reqpage = ap->a_reqpage; int error = 0; + vp = ap->a_vp; object = vp->v_object; - count = bytecount / PAGE_SIZE; + count = ap->a_count / PAGE_SIZE; + m = ap->a_m; if (vp->v_mount == NULL) return VM_PAGER_BAD; @@ -785,14 +772,6 @@ vnode_pager_generic_getpages(vp, m, byte return (error ? VM_PAGER_ERROR : VM_PAGER_OK); } -/* - * EOPNOTSUPP is no longer legal. For local media VFS's that do not - * implement their own VOP_PUTPAGES, their VOP_PUTPAGES should call to - * vnode_pager_generic_putpages() to implement the previous behaviour. - * - * All other FS's should use the bypass to get to the local media - * backing vp's VOP_PUTPAGES. - */ static int vnode_pager_putpages(object, m, count, sync, rtvals) vm_object_t object; @@ -801,38 +780,44 @@ vnode_pager_putpages(object, m, count, s boolean_t sync; int *rtvals; { - int rtval; - struct vnode *vp; - - vp = object->handle; - return VOP_PUTPAGES(vp, m, count*PAGE_SIZE, sync, rtvals, 0); + return (VOP_PUTPAGES(object->handle, m, count*PAGE_SIZE, sync, rtvals, 0)); } /* - * This is now called from local media FS's to operate against their - * own vnodes if they fail to implement VOP_GETPAGES. + * This is the standard implementation of VOP_PUTPAGES. */ int -vnode_pager_generic_putpages(vp, m, bytecount, sync, rtvals) - struct vnode *vp; - vm_page_t *m; - int bytecount; - boolean_t sync; - int *rtvals; +vop_stdputpages(ap) + struct vop_putpages_args /* { + struct vnode *a_vp; + vm_page_t *a_m; + int a_count; + int a_sync; + int *a_rtvals; + vm_ooffset_t a_offset; + } */ *ap; { int i; vm_object_t object; + struct vnode *vp; int count; int maxsize, ncount; vm_ooffset_t poffset; + vm_page_t *m; + boolean_t sync; struct uio auio; struct iovec aiov; + int *rtvals; int error; - object = vp->v_object; - count = bytecount / PAGE_SIZE; + vp = ap->a_vp; + object = ap->a_vp->v_object; + count = ap->a_count / PAGE_SIZE; + sync = ap->a_sync; + m = ap->a_m; + rtvals = ap->a_rtvals; for (i = 0; i < count; i++) rtvals[i] = VM_PAGER_AGAIN; --- vm/vnode_pager.h Sat Feb 28 16:45:56 1998 +++ vm/vnode_pager.h Sat Feb 28 17:10:31 1998 @@ -46,16 +46,6 @@ vm_object_t vnode_pager_alloc __P((void *, vm_size_t, vm_prot_t, vm_ooffset_t)); void vnode_pager_freepage __P((vm_page_t m)); struct vnode *vnode_pager_lock __P((vm_object_t)); - -/* - * XXX Generic routines; currently called by badly written FS code; these - * XXX should go away soon. - */ -int vnode_pager_generic_getpages __P((struct vnode *vp, vm_page_t *m, - int count, int reqpage)); -int vnode_pager_generic_putpages __P((struct vnode *vp, vm_page_t *m, - int count, boolean_t sync, - int *rtvals)); #endif #endif /* _VNODE_PAGER_ */ --- ufs/ffs/ffs_vnops.c Sat Feb 28 17:14:10 1998 +++ ufs/ffs/ffs_vnops.c Sat Feb 28 19:04:26 1998 @@ -62,7 +62,6 @@ static int ffs_fsync __P((struct vop_fsync_args *)); static int ffs_getpages __P((struct vop_getpages_args *)); -static int ffs_putpages __P((struct vop_putpages_args *)); static int ffs_read __P((struct vop_read_args *)); static int ffs_write __P((struct vop_write_args *)); @@ -72,7 +71,6 @@ static struct vnodeopv_entry_desc ffs_vn { &vop_default_desc, (vop_t *) ufs_vnoperate }, { &vop_fsync_desc, (vop_t *) ffs_fsync }, { &vop_getpages_desc, (vop_t *) ffs_getpages }, - { &vop_putpages_desc, (vop_t *) ffs_putpages }, { &vop_read_desc, (vop_t *) ffs_read }, { &vop_reallocblks_desc, (vop_t *) ffs_reallocblks }, { &vop_write_desc, (vop_t *) ffs_write }, --- ufs/ufs/ufs_readwrite.c Sat Feb 28 17:15:11 1998 +++ ufs/ufs/ufs_readwrite.c Sat Feb 28 17:15:38 1998 @@ -540,16 +540,3 @@ ffs_getpages(ap) return (rtval); } -/* - * put page routine - * - * XXX By default, wimp out... note that a_offset is ignored (and always - * XXX has been). - */ -int -ffs_putpages(ap) - struct vop_putpages_args *ap; -{ - return vnode_pager_generic_putpages(ap->a_vp, ap->a_m, ap->a_count, - ap->a_sync, ap->a_rtvals); -} --- kern/vfs_default.c Sat Feb 28 18:59:26 1998 +++ kern/vfs_default.c Sat Feb 28 19:01:51 1998 @@ -64,6 +64,7 @@ static struct vnodeopv_entry_desc defaul { &vop_bwrite_desc, (vop_t *) vop_stdbwrite }, { &vop_close_desc, (vop_t *) vop_null }, { &vop_fsync_desc, (vop_t *) vop_null }, + { &vop_getpages_desc, (vop_t *) vop_stdgetpages }, { &vop_ioctl_desc, (vop_t *) vop_enotty }, { &vop_islocked_desc, (vop_t *) vop_noislocked }, { &vop_lease_desc, (vop_t *) vop_null }, @@ -72,6 +73,7 @@ static struct vnodeopv_entry_desc defaul { &vop_open_desc, (vop_t *) vop_null }, { &vop_pathconf_desc, (vop_t *) vop_einval }, { &vop_poll_desc, (vop_t *) vop_nopoll }, + { &vop_putpages_desc, (vop_t *) vop_stdputpages }, { &vop_readlink_desc, (vop_t *) vop_einval }, { &vop_reallocblks_desc, (vop_t *) vop_eopnotsupp }, { &vop_revoke_desc, (vop_t *) vop_revoke }, --- sys/vnode.h Sat Feb 28 17:11:08 1998 +++ sys/vnode.h Sat Feb 28 19:30:09 1998 @@ -520,6 +520,8 @@ int vop_einval __P((struct vop_generic_a int vop_enotty __P((struct vop_generic_args *ap)); int vop_defaultop __P((struct vop_generic_args *ap)); int vop_null __P((struct vop_generic_args *ap)); +int vop_stdgetpages __P((struct vop_getpages_args *ap)); +int vop_stdputpages __P((struct vop_putpages_args *ap)); struct vnode * checkalias __P((struct vnode *vp, dev_t nvp_rdev, struct mount *mp)); To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message