On Tue, Jun 07, 2011 at 06:51:31PM +0200, Christian Ehrhardt wrote:
> Hi,
>
> while reading through uvm code I stubled accross a piece of code that
> appears to be buggy. Here's the proposed patch, rational follows:
>
> Index: uvm_vnode.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_vnode.c,v
> retrieving revision 1.71
> diff -u -r1.71 uvm_vnode.c
> --- uvm_vnode.c 18 May 2010 04:41:14 -0000 1.71
> +++ uvm_vnode.c 7 Jun 2011 16:42:15 -0000
> @@ -924,8 +924,8 @@
> */
>
> if (flags & PGO_DEACTIVATE) {
> - if ((pp->pg_flags & PQ_INACTIVE) == 0 &&
> - pp->wire_count == 0) {
> + if ((ptmp->pg_flags & PQ_INACTIVE) == 0 &&
> + ptmp->wire_count == 0) {
> pmap_page_protect(ptmp,
VM_PROT_NONE); >
uvm_pagedeactivate(ptmp); > }
This does look correct to me. In very *very* rare cases where one page was
wired and the other was not (playing with mlock() could cause this
synthetically with luck) this could cause us to hit a KASSERT in
uvm_pagedeactivate().
Note that the PQ_INACTIVE bit isn't needed (pagedeactivate already
checks that) but the wire count check is still necessary.
Anyone willing to ok this diff? I'll take care of it.
-0-
>
> This code handles (among other things) write back of dirty pages to disk.
> It calls uvm_pager_put to do the actual IO (one call per page). To improve
> performance, uvm_pager_put is allowed to do IO for adjacent pages as well
> and returns an array of all pages that must be unbusied by the caller
> (uvn_flush).
>
> The code above is part of the loop that does this. pp is the initial page
> passed to uvm_pager_put and ptmp is the page that we are about to unbusy.
> Thus we should IMHO check the PQ_INACTIVE flag and the wire_count of ptmp
> instead of pp.
>
--
They told me I was gullible ... and I believed them!
