Re: Making use of string items in the capability system.

[Marcus Brinkmann]

(Matthieu, the mailing list is l4-hurd and not l4_hurd.)

At Sun, 20 Feb 2005 20:55:13 +0000,
Matthieu Lemerre <address@hidden> wrote:
> Marcus Brinkmann <address@hidden> writes:
> >   
> >   You do not check the return value of allocate_string_item_buffers
> >   when you call it.
> >
> 
> OK.  What should I do in manage_mt_worker if allocate_string_item_buffers 
> fails? 

Good catch.  Uhm, this is troublesome.  The current code pretty much
assumes that the startup of a worker thread can't fail.  I would hate
to break that assumption.

So, I think the right thing to do is to allocate the buffers in the
manager thread already, where it at least will know about the failure
before starting the worker thread.  You need to pass down the
information somehow to the worker then, in a per-worker structure
(which can be alloc'ed and then freed by the worker).  This is a bit
of extra work, but I really think all errors should be catched in the
manager before starting the worker.

Note that this extra complexity could be saved if we decide to
allocate the memory on the stack of the worker (in case it is not too
much).

So, yes, I am afraid this needs to be rearranged a bit to properly
deal with errors.

BTW, if allocation fails in the manager, it is still not entirely
clear what should be done!  But at least that's a possibility we have
to deal with anyway (in case pthread_create fails).  So, at least all
the problems are in one place.

> >   Can you add a function flush_string_item_buffers() that would do
> >munmap/mmap for now? (and will later use our own pager feature to
> >just flush the underlying memory)
> 
> I added that function but doing a munmap/mmap on these buffers would
> change the addresses of them. This is annoying because it would force
> to remake a new l4_msg_buffer and accept it. I believe that flushing
> memory would keep the same buffer addresses, so these operations would
> be unnecessary at the end. So just re-using the buffers seem fine to
> me, but maybe I'm wrong. If so, tell me because I made this assumption
> in my patch.

True, good observation.  Actually, you can specify addresses with
mmap, but as the whole operation wouldn't be atomic, there is a good
chance it could fail.  Well, reloading the BRs is really fast, and
marginal compared to the cost of the munmap/mmap.  But you are right,
it doesn't make much sense as a flush operation is so much better.  I
guess it is ok to just defer this until the flush operation is
available.

> OK.  I thought I read somewhere that 0 was better than NULL, but I
> checked and it surely isn't in the GNU coding standards.

Well, technically, NULL and 0 are identical.  But using NULL conveys
more information, possibly to the compiler (ie, you can get pointer to
integer conversion warnings), but definitely to the human reading the
code.
 
Have not looked at the details of the patch yet.

Thanks,
Marcus