Messages in this thread | | | Date | Tue, 25 Aug 2009 07:26:53 +0300 | Subject | Re: [PATCH 1/4] compcache: xvmalloc memory allocator | From | Pekka Enberg <> |
| |
Hi Nitin,
On Tue, Aug 25, 2009 at 12:16 AM, Nitin Gupta<ngupta@vflare.org> wrote: > Now, if code cleanup is the aim rather that reducing the no. of conversions, > then I think use of PFNs is still preferred due to minor implementation > details mentioned above. > > So, I think the interface should be left in its current state.
I don't agree. For example, grow_pool() does xv_alloc_page() and immediately passes the PFN to get_ptr_atomic() which does conversion back to struct page. Passing PFNs around is not a good idea because it's very non-obvious, potentially broken (the 64-bit issue Hugh mentioned), and you lose type checking. The whole wrapper thing around kmap() (which is also duplicated in the actual driver) is a pretty clear indication that you're doing it the wrong way.
So again, _storing_ PFNs in internal data structures is probably a reasonable optimization (given the 64-bit issues are sorted out) but making the APIs work on them is not. It's much cleaner to have few places that do page_to_pfn() on stores and pass struct pages around.
Pekka
| |