[haiku-gsoc] Re: XSI semaphore implamentation patch #2
- From: "Salvatore Benedetto" <emitrax@xxxxxxxxx>
- To: haiku-gsoc@xxxxxxxxxxxxx
- Date: Sat, 2 Aug 2008 08:35:30 +0000
2008/8/1 Ingo Weinhold <ingo_weinhold@xxxxxx>:
>> Hmm... Let's say I use the following structure, and you agree :-)
>>
>> struct undoRequest : DoublyLinkedList<undoRequest> {
>> union {
>> team *team; // Used in the sem list
>> XsiSemaphoreSet *semaphore_set; // Used in the team
>> } p;
>> short undo_value;
>> int semaphore_number;
>> }
>
> Nope, I don't agree. As I wrote, I would go with an entry per semaphore set
> and team:
>
> struct UndoEntry : DoublyLinkedListLinkImpl<UndoEntry> {
> DoublyLinkedListLink<UndoEntry> team_link;
> XsiSemaphoreSet* semaphore_set;
> struct team* team;
> int16 undo_values[0];
> };
>
> The entry would be in both the semaphore set and the team list. For each
> semaphore in the set there is an "undo_values" array element.
...
>
> One has to create only one entry which is added to both lists (currently you
> create on entry and add it to the global list and have to increment the
> counter in the team). If the semaphore set does already have an entry for the
> team, merely the array element for the concerned semaphore needs to be
> adjusted.
I though of adding the same object to both list, but that requires 4 links
in the same object instead of two, and I didn't think that would be
possible, but I see
you have added a double link element. I'll see how what works. Thanks!
> In the SETALL case, removing would be possible, but memset()ing the arrays to
> 0 is probably the better choice.
Ok.
>> > It doesn't make a difference when SETVAL/SETALL are called or whether
>> > there
>> > are undo entries for these semaphores, since you unconditionally iterate
>> > through the whole global undo entry list in any case.
>>
>> Exactly. And that can be easily fixed with a counter in the set.
>
> Yep, that can and probably should be done, if you don't implement my proposal.
I'll implement your proposal, after I'm done with testing.
>
> That reminds me: You have a locking order inversion in xsi_sem_undo(): It's
> sUndoListLock -> sXsiSemaphoreSetLock, while it is the other way around in
> _user_xsi_sem{op,ctl}().
I fixed that in the previous patch, which I'm going to commit. It
reverses the order in
xsi_sem_undo.
>
>> Seriously, I understand your concern about perfomances and I don't mean
>> in any way to waist your time, but I thought optimization in this situation
>> could be postponed.
>
> Yep, I'd much prefer unit tests first.
So I suppose those I've ported are not to be considered enough. Never mind,
I'll write an unit test more or like the one you wrote for the realtime sem.
>
> CU, Ingo
>
Thanks for your suggestions.
Regards,
--
Salvatore Benedetto (a.k.a. emitrax)
Student of Computer Engineer
University of Pisa
www.haiku-os.it
Other related posts:
