Hi Andrew, "Andrew Galante" <haiku.galante@xxxxxxxxx> wrote: > 1) I don't recall using anything C++ specific, other than the .cpp > extension - and I didn't realize we were trying to keep C++ out of > the > kernel. We're not trying to keep C++ out of the kernel - we're trying to keep it internally. You were not using the C++ header guards (ie. #ifdef __cplusplus extern "C" ...) and you have overloaded get_datastore() which is a feature not available in C. > 2) I thought it could be useful in other areas, so I made it > independent of the net_buffer implementation. It's okay to make it independent from the net_buffer implementation - and we're even using something similar for the ports and pipefs implementation (cbuf, which you could have used as well), but it's also going to be rethought when we switch to the slab allocator. That's why I would have preferred you would have asked before you got to work - you've wasted some time, unfortunately. [fixed 4 MB memory] > 3) True, but this was something I planned on changing later No need to anymore. [fixed unpageable kernel memory wasted] > 4) Again, this could be changed as necessary, eventually computed on > the fly, for low memory systems and such. Sure, but even then networking should work flawlessly :) [physical addresses] > 5) It didn't seem necessary, as the datastore manages this > internally. No it doesn't, it doesn't manage this at all; it doesn't even need to, because this kind of knowledge is only useful for the networking drivers. Currently, they have to call get_memory_map() to get the physical memory location, and they have to do this for every buffer that's going in or out - and they currently even usually do it in the interrupt routine. It would be nice if the net_buffer implementation could manage this as it recycles the same buffers over and over, and can easily make this address available for lookup at any time. > Now, looking at what a slab allocator really is (which I should have > done before I started writing this), some of the stuff you originally > did is making more sense. (As I told myself before the ci, "If it's > terrible, the old version is still there on svn") Sure, it's no problem you replaced it, it's just that you wasted some time :-) Anyway, there is another problem of the implementation (http://haiku-os.org/trac/ticket/875) but you couldn't even know about that: our current allocator is very basic; if you allocate a region greater than 64 kB the memory cannot be tracked anymore, and will never be freed (and thus wasting some bytes of the fixed size kernel heap). Of course, this problem will also go away when we switch to the slab allocator; we know about that for quite some time (too long if you ask me) :) Anyway, I'll fix that one for you, as it's so simple. Bye, Axel.