On Wed, May 18, 2011 at 06:48:02PM +0200, Ariane van der Steldt wrote:
> Pmemrange being a difficult algorithm and reports of dma controllers
> being unhappy with what it serves up, plus some prodding from theo,
> caused me to write this little validation step.
>
> Very low overhead, but guaranteed to catch bugs when they happen,
> instead of when finally a dma controller does its own validation.
>
> Ok?
I'd still like an ok for this diff.
I added one fix (as comment) below.
> Index: uvm_pmemrange.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_pmemrange.c,v
> retrieving revision 1.22
> diff -u -d -p -r1.22 uvm_pmemrange.c
> --- uvm_pmemrange.c 6 Apr 2011 12:31:10 -0000 1.22
> +++ uvm_pmemrange.c 18 May 2011 16:42:36 -0000
> @@ -746,6 +746,9 @@ uvm_pmr_getpages(psize_t count, paddr_t
> int memtype; /* Requested memtype. */
> int memtype_init; /* Best memtype. */
> int desperate; /* True if allocation failed. */
> +#ifdef DIAGNOSTIC
> + struct vm_page *diag_prev; /* Used during validation. */
> +#endif /* DIAGNOSTIC */
>
> /*
> * Validate arguments.
> @@ -1048,6 +1051,11 @@ out:
> uvm_unlock_fpageq();
>
> /* Update statistics and zero pages if UVM_PLA_ZERO. */
> +#ifdef DIAGNOSTIC
> + fnsegs = 0;
> + fcount = 0;
> + diag_prev = NULL;
> +#endif /* DIAGNOSTIC */
> TAILQ_FOREACH(found, result, pageq) {
> atomic_clearbits_int(&found->pg_flags,
> PG_PMAP0|PG_PMAP1|PG_PMAP2|PG_PMAP3);
> @@ -1074,7 +1082,36 @@ out:
> */
> KDASSERT(start == 0 || atop(VM_PAGE_TO_PHYS(found)) >= start);
> KDASSERT(end == 0 || atop(VM_PAGE_TO_PHYS(found)) < end);
> +
> +#ifdef DIAGNOSTIC
> + /*
> + * Update fcount (# found pages) and
> + * fnsegs (# found segments) counters.
> + */
> + if (diag_prev == NULL ||
> + /* new segment if it contains a hole */
> + atop(VM_PAGE_TO_PHYS(diag_prev)) + 1 !=
> + atop(VM_PAGE_TO_PHYS(found)) ||
> + /* new segment if it crosses boundary */
> + (atop(VM_PAGE_TO_PHYS(diag_prev)) & ~(boundary - 1)) !=
> + (atop(VM_PAGE_TO_PHYS(found)) & ~(boundary - 1)))
> + fnsegs++;
> + fcount++;
> +
> + diag_prev = found;
> +#endif /* DIAGNOSTIC */
> }
> +
> +#ifdef DIAGNOSTIC
> + /*
> + * Panic on algorithm failure.
> + */
> + if (fcount > count || fnsegs > maxseg)
That should be fcount != count ofcourse.
> + panic("pmemrange allocation error: "
> + "allocated %ld pages in %d segments, "
> + "but request was %ld pages in %d segments",
> + fcount, fnsegs, count, maxseg);
> +#endif /* DIAGNOSTIC */
>
> return 0;
> }
>
--
Ariane
