[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <84144f020908242126n5c7d93aah7305f4da64f6965@mail.gmail.com>
Date: Tue, 25 Aug 2009 07:26:53 +0300
From: Pekka Enberg <penberg@...helsinki.fi>
To: ngupta@...are.org
Cc: akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
linux-mm@...ck.org, linux-mm-cc@...top.org,
hugh.dickins@...cali.co.uk
Subject: Re: [PATCH 1/4] compcache: xvmalloc memory allocator
Hi Nitin,
On Tue, Aug 25, 2009 at 12:16 AM, Nitin Gupta<ngupta@...are.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists