On Mon, Oct 05, 2020 at 07:18:17PM +0200, Claudio Imbrenda wrote: > On Mon, 5 Oct 2020 18:53:11 +0200 > Andrew Jones <drjones@xxxxxxxxxx> wrote: > > That won't work for 32-bit processors that support physical addresses > > that are larger than 32-bits - if the target has more than 4G and the > > unit test wants to access that high memory. Using phys_addr_t > > everywhere you want a physical address avoids that problem. > > no it doesn't. you can't even handle the freelist if it's in high > memory. consider that the current allocator also does not handle high > memory. and there is no infrastructure to shortly map high memory. > > if a 32 bit test wants to access high memory, it will have to manage it > manually But the point of this series is to rewrite the allocator in order to add features such as allocations from different memory zones, like high memory, and also to fix its shortcomings. It seems like we should start by fixing the current allocator's lack of concern for physical and virtual address distinctions. > > Using a list member for list operations and container_of() to get > > back to the object would help readability. > > except that the "object" is the block of free memory, and the only > thing inside it is the list object at the very beginning. I think it's > overkill to use container_of in that case, since the only thing is the > list itself, and it's at the beginning of the free block. Fair enough, but I'm also thinking it might be overkill to introduce a general list API at all if all we need to do is insert and delete blocks, and ones where we're aware of where the pointers are. > so what I gather at the end is that I have to write a physical memory > allocator for a complex OS that has to seamlessly access high memory. We definitely don't want complexity. Trade-offs for simplicity are good, but they should be implemented in a way that assert when assumptions fail. If we want to limit memory access to 4G for 32-bit architectures, then we should add some code to clamp upper physical addresses and some asserts for unit tests that try to use them. If the allocator wants to depend on the identity map to simplify its implementation, then that's probably OK too, but I'd still keep the interfaces clean: physical addresses should be phys_addr_t, virtual addresses that should be the identity mapping of physical addresses should be checked, e.g. assert(virt_to_phys(vaddr) == vaddr) > > I'll try to come up with something > Thanks, drew
