Date: Sun, 14 Feb 2016 15:04:35 -0800 From: Gleb Kurtsou <gleb@freebsd.org> To: Gleb Smirnoff <glebius@FreeBSD.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r292373 - in head: share/man/man9 sys/cddl/contrib/opensolaris/uts/common/fs/zfs sys/dev/drm2/i915 sys/dev/drm2/ttm sys/dev/md sys/fs/fuse sys/fs/nfsclient sys/fs/smbfs sys/fs/tmpfs sys... Message-ID: <20160214230435.GA4547@reks> In-Reply-To: <201512162130.tBGLUjPj083575@repo.freebsd.org> References: <201512162130.tBGLUjPj083575@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--7AUc2qLy4jB3hD7Z Content-Type: text/plain; charset=utf-8 Content-Disposition: inline On (16/12/2015 21:30), Gleb Smirnoff wrote: > Author: glebius > Date: Wed Dec 16 21:30:45 2015 > New Revision: 292373 > URL: https://svnweb.freebsd.org/changeset/base/292373 > > Log: > A change to KPI of vm_pager_get_pages() and underlying VOP_GETPAGES(). > > o With new KPI consumers can request contiguous ranges of pages, and > unlike before, all pages will be kept busied on return, like it was > done before with the 'reqpage' only. Now the reqpage goes away. With > new interface it is easier to implement code protected from race > conditions. > > Such arrayed requests for now should be preceeded by a call to > vm_pager_haspage() to make sure that request is possible. This > could be improved later, making vm_pager_haspage() obsolete. vm_pager_haspage is essentially wrapper around VOP_BMAP. VOP_BMAP is a stub for all non UFS-like file systems. E.g. it's return (0) in zfs and return (EOPNOTSUPP) in tmpfs. Could you elaborate on how strong the requirement of "should be preceded by a call to vm_pager_haspage" is. It's also not clear how to approach it if file system doesn't have bmap and getpages/putpages, but uses standard fallback pager through read/write. You've added vm_pager_has_page to exec_map_first_page. Should we now assume that vm_pager_get_pages(VM_INITIAL_PAGEIN) may fail if 'after' returned by vm_pager_has_page is less than VM_INITIAL_PAGEIN? Could you please take a look at 2 patches attached. I'd like to commit the one fixing vnode_pager_haspage, but I'm not sure about vm_pager_has_page usage in exec_map_first_page. 0001-Emulate-vop_stdbmap-in-vnode_pager_haspage-if-bmap-i.patch 'after' will be uninitialized if VOP_BMAP returns error. KASSERT in exec_map_first_page may fail because of it. I'm not sure if after = 0 is currently expected in exec_map_first_page. Extend the logic to treat EOPNOTSUPP as vop_stdbmap (set before and after to 0). Then extend both to fs block size. 0002-Handle-vm_pager_has_page-failure-during-exec.patch Patch may be dropped if vm_pager_has_page is required to succeed as described above. Thanks, Gleb. --7AUc2qLy4jB3hD7Z Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0001-Emulate-vop_stdbmap-in-vnode_pager_haspage-if-bmap-i.patch" >From 40978eba392bcb20bf59704eac3d744d15f1e080 Mon Sep 17 00:00:00 2001 From: Gleb Kurtsou <gleb.kurtsou@gmail.com> Date: Sat, 13 Feb 2016 23:00:00 -0800 Subject: [PATCH 1/2] Emulate vop_stdbmap in vnode_pager_haspage if bmap is not supported fs. Reset 'before' and 'after' to zero if VOP_BMAP fails. Assume no error if bmap is not supported by file system and adjust 'before', 'after' accordingly. --- sys/vm/vnode_pager.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/sys/vm/vnode_pager.c b/sys/vm/vnode_pager.c index 66dd29d7686..063d8d55495 100644 --- a/sys/vm/vnode_pager.c +++ b/sys/vm/vnode_pager.c @@ -321,32 +321,39 @@ vnode_pager_haspage(vm_object_t object, vm_pindex_t pindex, int *before, if (IDX_TO_OFF(pindex) >= object->un_pager.vnp.vnp_size) return FALSE; bsize = vp->v_mount->mnt_stat.f_iosize; pagesperblock = bsize / PAGE_SIZE; blocksperpage = 0; if (pagesperblock > 0) { reqblock = pindex / pagesperblock; } else { blocksperpage = (PAGE_SIZE / bsize); reqblock = pindex * blocksperpage; } VM_OBJECT_WUNLOCK(object); err = VOP_BMAP(vp, reqblock, NULL, &bn, after, before); VM_OBJECT_WLOCK(object); - if (err) - return TRUE; + if (err) { + if (before) + *before = 0; + if (after) + *after = 0; + if (err != EOPNOTSUPP) + return TRUE; + bn = reqblock; + } if (bn == -1) return FALSE; if (pagesperblock > 0) { poff = pindex - (reqblock * pagesperblock); if (before) { *before *= pagesperblock; *before += poff; } if (after) { /* * The BMAP vop can report a partial block in the * 'after', but must not report blocks after EOF. * Assert the latter, and truncate 'after' in case * of the former. */ -- 2.4.2 --7AUc2qLy4jB3hD7Z Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0002-Handle-vm_pager_has_page-failure-during-exec.patch" >From 0d4ed2d975a796b612914ee70b82064ba5fa19e3 Mon Sep 17 00:00:00 2001 From: Gleb Kurtsou <gleb.kurtsou@gmail.com> Date: Sat, 13 Feb 2016 23:07:50 -0800 Subject: [PATCH 2/2] Handle vm_pager_has_page failure during exec. Relax exec_map_first_page dependency on vm_pager_has_page to calculate initial number of pages. r292373 changed behavior to completely rely on vm_pager_has_page(). Fallback to VM_INITIAL_PAGEIN if vm_pager_has_page fails or number of pages reported as available ('after' argument) is zero. --- sys/kern/kern_exec.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c index 741bc3e48c6..e71a32fca09 100644 --- a/sys/kern/kern_exec.c +++ b/sys/kern/kern_exec.c @@ -954,39 +954,34 @@ exec_map_first_page(imgp) vm_page_t ma[VM_INITIAL_PAGEIN]; vm_object_t object; if (imgp->firstpage != NULL) exec_unmap_first_page(imgp); object = imgp->vp->v_object; if (object == NULL) return (EACCES); VM_OBJECT_WLOCK(object); #if VM_NRESERVLEVEL > 0 vm_object_color(object, 0); #endif ma[0] = vm_page_grab(object, 0, VM_ALLOC_NORMAL); if (ma[0]->valid != VM_PAGE_BITS_ALL) { - if (!vm_pager_has_page(object, 0, NULL, &after)) { - vm_page_lock(ma[0]); - vm_page_free(ma[0]); - vm_page_unlock(ma[0]); - vm_page_xunbusy(ma[0]); - VM_OBJECT_WUNLOCK(object); - return (EIO); - } - initial_pagein = min(after, VM_INITIAL_PAGEIN); + if (vm_pager_has_page(object, 0, NULL, &after) && after > 0) + initial_pagein = min(after, VM_INITIAL_PAGEIN); + else + initial_pagein = min(object->size, VM_INITIAL_PAGEIN); KASSERT(initial_pagein <= object->size, ("%s: initial_pagein %d object->size %ju", __func__, initial_pagein, (uintmax_t )object->size)); for (i = 1; i < initial_pagein; i++) { if ((ma[i] = vm_page_next(ma[i - 1])) != NULL) { if (ma[i]->valid) break; if (vm_page_tryxbusy(ma[i])) break; } else { ma[i] = vm_page_alloc(object, i, VM_ALLOC_NORMAL | VM_ALLOC_IFNOTCACHED); if (ma[i] == NULL) break; } -- 2.4.2 --7AUc2qLy4jB3hD7Z--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160214230435.GA4547>