On Tue, Jun 07, 2011 at 06:51:31PM +0200, Christian Ehrhardt wrote:
> 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);
> }
That looks like a genuine bug, yes.
> 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.
That's correct.
--
Ariane
