on 29/10/2010 20:51 Artemiev Igor said the following:
> On Fri, Oct 29, 2010 at 07:06:03PM +0300, Andriy Gapon wrote:
>> Probably yes, but have to be careful there.
>> First, do vm_page_grab only for UIO_NOCOPY case.
>> Second, the first page is already "shared busy" after vm_page_io_start()
>> call in
>> kern_sendfile; so you might need VM_ALLOC_IGN_SBUSY for that page to avoid a
>> deadlock.
>
> RELENG_8 doesn`t have VM_ALLOC_IGN_SBUSY, it appeared only in HEAD.
> Can you review this patch, Whether correctly I have understood? (didnt test
> it yet)
>
> --- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c.orig
> 2010-10-29 18:18:23.921078337 +0200
> +++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c
> 2010-10-29 19:23:48.142513084 +0200
> @@ -449,7 +449,7 @@
> int bytes = MIN(PAGESIZE - off, len);
>
> again:
> - if ((m = vm_page_lookup(obj, OFF_TO_IDX(start))) != NULL &&
> + if (uio->uio_segflg != UIO_NOCOPY && (m = vm_page_lookup(obj,
> OFF_TO_IDX(start))) != NULL &&
> vm_page_is_valid(m, off, bytes)) {
> if (vm_page_sleep_if_busy(m, FALSE, "zfsmrb"))
> goto again;
> @@ -464,7 +464,7 @@
> uiomove_fromphys(&m, off, bytes, uio);
> VM_OBJECT_LOCK(obj);
> vm_page_wakeup(m);
> - } else if (m != NULL && uio->uio_segflg == UIO_NOCOPY) {
> + } else if (uio->uio_segflg == UIO_NOCOPY) {
> /*
> * The code below is here to make sendfile(2) work
> * correctly with ZFS. As pointed out by ups@
> @@ -472,11 +472,9 @@
> * but it pessimize performance of sendfile/UFS, that's
> * why I handle this special case in ZFS code.
> */
> - KASSERT(off == 0,
> - ("unexpected offset in mappedread for sendfile"));
> - if (vm_page_sleep_if_busy(m, FALSE, "zfsmrb"))
> - goto again;
> - vm_page_busy(m);
> + if((m = vm_page_lookup(obj, OFF_TO_IDX(start))) == NULL
> || !vm_page_is_valid(m, off, bytes))
> + m = vm_page_grab(obj, OFF_TO_IDX(start),
> VM_ALLOC_NORMAL|VM_ALLOC_RETRY);
> +
> VM_OBJECT_UNLOCK(obj);
> if (dirbytes > 0) {
> error = dmu_read_uio(os, zp->z_id, uio,
Or maybe something like the following?
It looks a little bit cleaner to me, but still is not perfect, as I have not
handled unnecessary busy-ing of the pages where something more lightweight could
have sufficed (e.g. wiring and shared busying).
Index: sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c
===================================================================
--- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c (revision
214318)
+++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c (working copy)
@@ -67,6 +67,7 @@
#include <sys/sf_buf.h>
#include <sys/sched.h>
#include <sys/acl.h>
+#include <vm/vm_pageout.h>
/*
* Programming rules.
@@ -464,7 +465,7 @@
uiomove_fromphys(&m, off, bytes, uio);
VM_OBJECT_LOCK(obj);
vm_page_wakeup(m);
- } else if (m != NULL && uio->uio_segflg == UIO_NOCOPY) {
+ } else if (uio->uio_segflg == UIO_NOCOPY) {
/*
* The code below is here to make sendfile(2) work
* correctly with ZFS. As pointed out by ups@
@@ -476,6 +477,16 @@
("unexpected offset in mappedread for sendfile"));
if (vm_page_sleep_if_busy(m, FALSE, "zfsmrb"))
goto again;
+ if (m == NULL) {
+ m = vm_page_alloc(obj, OFF_TO_IDX(start),
+ VM_ALLOC_NOBUSY | VM_ALLOC_SYSTEM);
+ if (m == NULL) {
+ VM_OBJECT_UNLOCK(obj);
+ VM_WAIT;
+ VM_OBJECT_LOCK(obj);
+ goto again;
+ }
+ }
vm_page_busy(m);
VM_OBJECT_UNLOCK(obj);
if (dirbytes > 0) {
--
Andriy Gapon
_______________________________________________
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"
