FreeLists / openbeosnetteam / [openbeosnetteam] Re: datastore issues

["Axel Dörfler" <axeld@xxxxxxxxxxxxxxxx>]

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.