Date: Sun, 01 Mar 1998 13:32:57 +0300 From: Dmitrij Tejblum <dima@tejblum.dnttm.rssi.ru> To: Terry Lambert <tlambert@primenet.com> Cc: FreeBSD-current@FreeBSD.ORG Subject: Re: VM: Process hangs sleeping on vmpfw Message-ID: <199803011032.NAA02269@tejblum.dnttm.rssi.ru> In-Reply-To: Your message of "Sat, 28 Feb 1998 22:57:42 GMT." <199802282257.PAA08480@usr08.primenet.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199803011032.NAA02269>