On Sat, Mar 22, 2014 at 01:21:39PM -0700, Ben Widawsky wrote: > On Tue, Mar 18, 2014 at 09:14:09AM +0000, Chris Wilson wrote: > > On Mon, Mar 17, 2014 at 10:48:47PM -0700, Ben Widawsky wrote: > > > As we move toward dynamic page table allocation, it becomes much easier > > > to manage our data structures if break do things less coarsely by > > > breaking up all of our actions into individual tasks. This makes the > > > code easier to write, read, and verify. > > > > > > Aside from the dissection of the allocation functions, the patch > > > statically allocates the page table structures without a page directory. > > > This remains the same for all platforms, > > > > > > The patch itself should not have much functional difference. The primary > > > noticeable difference is the fact that page tables are no longer > > > allocated, but rather statically declared as part of the page directory. > > > This has non-zero overhead, but things gain non-trivial complexity as a > > > result. > > > > We increase overhead for increased complexity. What's the selling point > > of this patch then? > > I'd argue about the complexity. Personally, I think the result is easier > to read. > > I'll add this all to the commit message, but hopefully you agree: > > 1. Splitting out the functions allows easily combining GEN6 and GEN8 > code. Page tables have no difference based on GEN8. As we'll see in a > future patch when we add the dma mappings to the allocations, it > requires only one small change to make work, and error handling should > just fall into place. > > 2. Unless we always want to allocate all page tables under a given PDE, > we'll have to eventually break this up into an array of pointers (or > pointer to pointer). > > 3. Having the discrete functions is easier to review, and understand. > All allocations and frees now take place in just a couple of locations. > Reviewing, and cathcing leaks should be easy. > > 4. Less important: the gfp flags are confined to one location, which > makes playing around with such things trivial.o > > Hopefully you're more convinced after you went through more of the patch > series. Right, the patches and the resulting code look good. I just felt the changelog here was self-contradictory. And now we have a great spiel to include ;-) > If you want to try to optimize the overhead of managing the page tables, > I think this is a worthy thing to do (for instance, not statically > declaring the array). It takes a little more work though, and I'd prefer > to do it after the code is doing what it's supposed to do. Agreed. I'm still watching PT pages come and go with great joy, and not yet worrying about the impact. (Though I think I did notices some side-effect when a reap of the userspace cache lead to a sudden release of lots of pages, and a dip in throughput.) -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
