----- On Oct 26, 2017, at 12:20 AM, Stephen Bates sba...@raithlin.com wrote:
>> I found that genalloc is very slow for large chunk sizes because >> bitmap_find_next_zero_area has to grind through that entire bitmap. >> Hence, I recommend avoiding genalloc for large chunk sizes. > > Thanks for the feedback Daniel! We have been doing 16GiB without any > noticeable > issues. > >> I'm thinking how this would behave on a 32 bit ARM platform > > I don’t think people would be doing such big allocations on 32 bit (ARM > systems). It would not make sense for them to be doing >4GB anyway. > >>> --- a/lib/genalloc.c >>> +++ b/lib/genalloc.c >>> @@ -194,7 +194,7 @@ int gen_pool_add_virt(struct gen_pool *pool, unsigned >>> long >>> virt, phys_addr_t phy >>> chunk->phys_addr = phys; >>> chunk->start_addr = virt; >>> chunk->end_addr = virt + size - 1; >>> - atomic_set(&chunk->avail, size); >>> + atomic64_set(&chunk->avail, size); > >> Isn't size defined as a size_t type which is 32 bit wide on ARM? How >> can you ever set chunk->avail to anything larger than 2^32 - 1? > > I did consider changing this type but it seems like there would never be a > need > to set this value to more than 4GiB on 32 bit systems. > >>> @@ -464,7 +464,7 @@ size_t gen_pool_avail(struct gen_pool *pool) >>> >>> rcu_read_lock(); >>> list_for_each_entry_rcu(chunk, &pool->chunks, next_chunk) >>> - avail += atomic_read(&chunk->avail); >>> + avail += atomic64_read(&chunk->avail); >> >>avail is defined as size_t (32 bit). Aren't you going to overflow that >>variable? > > Again, I don’t think people on 32 bit systems will be doing >4GB assignments > so > it would not be an issue. We have atomic_long_t for that. Please use it instead. It will be 64-bit on 64-bit archs, and 32-bit on 32-bit archs, which seems to fit your purpose here. Thanks, Mathieu > > Stephen -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
