2005-05-16 20:23:24

by Patrick Plattes

Subject: semaphore understanding: sys_semtimedop()

Hello,

i have a question about this little code snippet, found in
sys_semtimedop() (ipc/sem.c):

for (sop = sops; sop < sops + nsops; sop++) {
if (sop->sem_num >= max)
max = sop->sem_num;
if (sop->sem_flg & SEM_UNDO)
undos++;
if (sop->sem_op < 0)
decrease = 1;
if (sop->sem_op > 0)
alter = 1;
}
alter |= decrease;

The variable decrease will never be used again in this
function, so why this intricate code? Isn't this much easier and works
also:

for (sop = sops; sop < sops + nsops; sop++) {
if (sop->sem_num >= max)
max = sop->sem_num;
if (sop->sem_flg & SEM_UNDO)
undos++;
if (sop->sem_op != 0)
alter = 1;
}

Maybe i'm totally wrong, so please correct me and don't shoot me up,
'cause i'm not a os developer.

Thanks,
Patrick

2005-05-22 05:21:59

by Randy Dunlap

Subject: Re: semaphore understanding: sys_semtimedop()

On Mon, 16 May 2005 22:17:04 +0200 Patrick Plattes wrote:

| Hello,
|
| i have a question about this little code snippet, found in
| sys_semtimedop() (ipc/sem.c):
|
| for (sop = sops; sop < sops + nsops; sop++) {
| if (sop->sem_num >= max)
| max = sop->sem_num;
| if (sop->sem_flg & SEM_UNDO)
| undos++;
| if (sop->sem_op < 0)
| decrease = 1;
| if (sop->sem_op > 0)
| alter = 1;
| }
| alter |= decrease;
|
| The variable decrease will never be used again in this
| function, so why this intricate code? Isn't this much easier and works
| also:
|
| for (sop = sops; sop < sops + nsops; sop++) {
| if (sop->sem_num >= max)
| max = sop->sem_num;
| if (sop->sem_flg & SEM_UNDO)
| undos++;
| if (sop->sem_op != 0)
| alter = 1;
| }
|
| Maybe i'm totally wrong, so please correct me and don't shoot me up,
| 'cause i'm not a os developer.

Looks like a reasonable and correct optimization to me.

Let's see what Manfred thinks or has to say...

---
~Randy

2005-05-22 06:49:57

by Manfred Spraul

Subject: Re: semaphore understanding: sys_semtimedop()

randy_dunlap wrote:

>On Mon, 16 May 2005 22:17:04 +0200 Patrick Plattes wrote:
>
>| The variable decrease will never be used again in this
>| function, so why this intricate code? Isn't this much easier and works
>| also:
>|
>| for (sop = sops; sop < sops + nsops; sop++) {
>| if (sop->sem_num >= max)
>| max = sop->sem_num;
>| if (sop->sem_flg & SEM_UNDO)
>| undos++;
>| if (sop->sem_op != 0)
>| alter = 1;
>| }
>|
>| Maybe i'm totally wrong, so please correct me and don't shoot me up,
>| 'cause i'm not a os developer.
>
>Looks like a reasonable and correct optimization to me.
>
>
>
It looks correct:
decrease was added during 2.1 development: it's not in 2.0.40, it's in
2.2.26: I think the idea was that operations with decrease can cause
other threads to block, therefore a different wakeup strategy was used.
The wakeup implementation was rewritten, but the loop remained unchanged.
I'll write a patch, thanks Patrick.

--
Manfred