On Wed, Feb 12, 2014 at 02:28:48PM -0800, Ben Widawsky wrote:
> - for (i = first_pte; i < last_pte; i++)
> + for (i = which_pte; i < last_pte; i++) {
> pt_vaddr[i] = scratch_pte;
> + num_entries--;
> + BUG_ON(num_entries < 0);
> + }
>
> kunmap_atomic(pt_vaddr);
>
> - num_entries -= last_pte - first_pte;
I'm going to moan about this being replaced by a BUG_ON inside the inner
loop.
> - first_pte = 0;
> - act_pt++;
> + which_pte = 0;
> + if (which_pde + 1 == GEN8_PDES_PER_PAGE)
> + which_pdpe++;
> + which_pde = (which_pde + 1) & GEN8_PDE_MASK;
I think this would be clearer written as
if (++which_pde == GEN8_PDES_PER_PAGE) {
which_pdpe++;
which_pde = 0;
}
as then the relationship between pdpe and pde is much more apparent to
me. Do we feel that which_pte, which_pde, which_pdpe are really any
better than pte, pde, pdpe? Or is it important to question ourselves
every step of the way?
And I may as well moan about having to preallocate everything. ;-)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
