[haiku-gsoc] Re: XSI semaphore implamentation patch #2

  • From: "Axel Dörfler" <axeld@xxxxxxxxxxxxxxxx>
  • To: haiku-gsoc@xxxxxxxxxxxxx
  • Date: Tue, 29 Jul 2008 14:12:07 +0200 CEST

Hi Salvatore,

"Salvatore Benedetto" <emitrax@xxxxxxxxx> wrote:
> attached is a second patch of xsi semaphore implementation.
> It includes the SEM_UNDO features.
> 
> I actually spent more time than I though on this one, because
> the sem_undo list get accesed not only when a process exit,
> thus executing all its sem_undo request, but also when calling
> semctl with SETVAL or SETALL on a semaphore (all sem_undo request
> must be cleared, that is deleted).
> 
> I reworked it three times in order to keep it as simple as possible
> and eventually came up with a single global sem_undo list 
> (sUndoList).
> I wanted to add as little as possible to the team structure.
> 
> The complexity of the algorithm when removing an element is of course 
> O(n)
> as I iterated through the list but only a simple short value has been
> to the team
> structure (at first it was a boolean value) and speed shouldn't 
> really
> be a problem
> as I think we won't have many semaphores in the systems.
>
> Anyway, if it happens to be a problem, I'll try with another 
> solution.

Well, that indeed doesn't sound like a nice solution, but at least it 
should be good enough for the time being, and to run further tests.
Anyway, since it's easier to read what you changed instead of the whole 
patch every time, I applied the patch with a few minor changes now in 
r26676; since no one is using the XSI semaphores, it shouldn't hurt 
anyway.
Please have a look at the SVN commit message to see what I changed. 
Most of this has been mentioned previously to you but has not yet been 
picked up by you.

> The code has been tested with bonnie++, which uses the SEM_UNDO 
> feature, and
> it seems to works fine.
> 
> I've also fixed some bugs that weren't easy to discover by simple
> reviewing the code in the first patch and bonnie++ helped with this. 
> In fact it now runs
> in multiprocesses mode.
> 
> For those who want to test it, the following run three processes that
> synchronized themself
> with the semaphores
> bonnie++ -p 3 -u 0
> bonnie++ -y -u 0 &
> bonnie++ -y -u 0 &
> bonnie++ -y -u 0 &
> 
> I've been running some test with bonnie++ in multiprocesses mode
> and no file system corruption has happened after last week commits.

Nice!

> However the software quits thinking that there is not more space
> available where there actually is.
> The same behavior does't happened with dd though.

That error could also happen when there is no way to grow the file 
stream anymore - with a single threaded 'dd' this is very unlikely to 
happen, while bonnie++ should be able to run into this much earlier. 
But maybe you want to check where this error happens to be sure that is 
the cause of this; maybe always returning E2BIG would be the better 
alternative for the latter (ie. in case meta data couldn't be allocated 
anymore in contrast to the data stream itself).

> Also some other panic have happened during the test, but not bfs
> related (at least it seems),
> like
> PANIC: _mutex_lock(): double lock of 0x90cc4ed0 by thread 3595
> where the mutex is vm_cache and thread is grep

A stack crawl would be most helpful in this case.

> Anyway, I'll investigate on these bugs once this code is complete and 
> committed.

There are still a number of compiler warnings left to be fixed in your 
code. I've fixed those in xsi_sem.cpp due to the missing _kern_*() 
prototypes in syscalls.h, but xsi_semaphore.cpp also produces some; 
please take care of them first.

Have you written or found any other test suite for XSI that would be 
worth ending up in our repository?

Bye,
   Axel.


Other related posts: