Messages in this thread | | | Date | Tue, 19 Mar 2013 16:00:14 -0700 | From | Andrew Morton <> | Subject | Re: gen_pool_add broken with LPAE based systems |
| |
On Tue, 19 Mar 2013 15:49:24 -0700 Laura Abbott <lauraa@codeaurora.org> wrote:
> On 3/19/2013 2:54 PM, Andrew Morton wrote: > > On Thu, 14 Mar 2013 16:05:27 -0700 Laura Abbott <lauraa@codeaurora.org> wrote: > > > >> Hi, > >> > >> We use genalloc for managing certain pools of physical memory. genalloc > >> currently uses unsigned long for virtual addresses and phys_addr_t for > >> physical addresses. Our ARM LPAE systems have 64-bit physical addresses > >> but unsigned long is still 32 bits. Using gen_pool_add breaks with > >> addresses > 4G because gen_pool_add treats the address passed in as the > >> virtual address. gen_pool allocates internally based on the 32 bit > >> virtual address as well so everything is broken if we want to be able to > >> manage the full address space after 4G. I see a couple of options: > > > > The above only makes sense if ARM LPAE has 64-bit (actually >= 33-bit) > > virtual addresses. If so, I don't understand how ARM LPAE can work at > > all - the core MM assumes that addresses-fit-in-ulongs in eleventy > > trillion places. > > > > I think we need a better description of the problem, please. > > > > Sorry, let me clarify. ARM LPAE still has 32 bit virtual addresses. > > Change 3c8f370ded3483b27f1218ff0051fcf0c7a2facd (lib/genalloc.c: add > support for specifying the physical address) added support for using > genalloc to know about both physical addresses and virtual addresses. > Allocation in gen_pool is still based on the virtual address though. > > The problem is we've been using genalloc to allocate physical addresses, > not virtual ones so allocating and returning an unsigned long breaks > with sizeof(phys_addr_t) > sizeof(unsigned long). It looks like genalloc > was added and extended with virtual addresses in mind but apart from the > address size limitation right now it should be able to work just fine > for physical addresses. > > There seem to be a few other clients scattered about who are using > genalloc for physical addresses as well (although all are 32 bit systems > right now)
I see. So genpool has never worked properly for this application?
> A better subject would be 'genalloc broken on LPAE systems when used to > allocate physical addresses instead of virtual addresses'
I'd say "extend genpool so we can use physical addresses instead of virtual addresses" ;)
> >> 1) Change gen_pool_add to use physical addresses and allocate based on > >> physical addresses instead of virtual addresses > >> 2) Change the virtual address to be a 64 bit type or something > >> selectable to a 64 bit type. > >> 3) Allow a flag per pool to select whether the allocator is virtual or > >> physical and switch between those. > >> 4) Split the APIs into virtual <-> physical and physical only and have > >> separate types for each. > >> > >> Any of these suggestions seem reasonable or is there another option to > >> consider? > > > > 2) sounds least intrusive but I can't think with my head spinning so fast.
I suppose using a bare u64 for `addr' would fix things up.
I think it's rather regrettable that genpool.c contains terms like "addr", "phys" and "virt" at all. It's in lib/ and it's a general-purpose container thing. It should know whether it's operating on addresses or bananas or whatever. A better layering would be to weed all that out of there, implement a truly general-purpose container and then add convenience wrappers around that for each particular application.
| |