In commit 0a2b9d4c,the update of semaphore's sem_otime(last semop time)
was removed because he wanted to move setting sem->sem_otime to one
place. But after that, the initial semop() will not set the otime
because its sem_op value is 0(in semtimedop,will not change
otime if alter == 1).
the error case:
process_a(server) process_b(client)
semget()
semctl(SETVAL)
semop()
semget()
setctl(IP_STAT)
for(;;) { <--not successful here
check until sem_otime > 0
}
provide test codes here:
$cat server.c
int semid;
int main()
{
int key;
struct semid64_ds sem_info;
union semun arg;
struct sembuf sop;
key = ftok(SEM_PATH,'a');
semid = semget(key,1,IPC_CREAT|IPC_EXCL|00666);
if(semid < 0)
perror("server:semget");
arg.val = 0;
if(semctl(semid,0,SETVAL,arg) == -1)
perror("semctl setval error");
sop.sem_num = 0;
sop.sem_op = 0;
sop.sem_flg = 0;
if (semop(semid, &sop, 1) == -1)
perror("semop error");
sleep(30);
if(semctl(semid, 0, IPC_RMID) == -1)
perror("semctl IPC_RMID");
else printf("remove sem ok\n");
}
$cat client.c
int semid;
int main()
{
int i;
int key;
union semun arg;
struct semid64_ds sem_info;
key = ftok(SEM_PATH,'a');
semid = semget(key,1,IPC_CREAT|00666);
if(semid < 0)
perror("client:semget");
for (i = 0; i < MAX_TRIES; i++)
{
arg.buf = &sem_info;
semctl(semid, 0, IPC_STAT, arg);
if (sem_info.sem_otime != 0) break;
sleep(1);
}
if(MAX_TRIES == i)
printf("error in opening a existed sem\n");
else
printf("open exsited sem sucessfully\n");
return 0;
}
the steps to test:
touch /tmp/my_sem
./server &
sleep 1
./client &
With the patch
1.test output:
error in opening a existed sem
2.cat /proc/sysvipc/sem
the field sem_otime is always zero
Without this patch
1.test output:
open exsited sem sucessfully
2.cat /proc/sysvipc/sem
the field sem_otime is not zero
Signed-off-by: Jia He <[email protected]>
---
ipc/sem.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/ipc/sem.c b/ipc/sem.c
index 69b6a21..8e01e76 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -590,6 +590,7 @@ static int perform_atomic_semop(struct sem_array *sma, struct sembuf *sops,
sop--;
}
+ sma->sem_base[sops[0].sem_num].sem_otime = get_seconds();
return 0;
out_of_range:
--
1.8.1.2
On Sun, 2013-09-22 at 10:11 +0800, Jia He wrote:
> In commit 0a2b9d4c,the update of semaphore's sem_otime(last semop time)
> was removed because he wanted to move setting sem->sem_otime to one
> place. But after that, the initial semop() will not set the otime
> because its sem_op value is 0(in semtimedop,will not change
> otime if alter == 1).
>
> the error case:
> process_a(server) process_b(client)
> semget()
> semctl(SETVAL)
> semop()
> semget()
> setctl(IP_STAT)
> for(;;) { <--not successful here
> check until sem_otime > 0
> }
Why not..
ipc,sem: Create semaphores with plausible sem_otime.
Signed-off-by: Mike Galbraith <[email protected]>
diff --git a/ipc/sem.c b/ipc/sem.c
index 4108889..f2564d7 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -471,19 +471,20 @@ static int newary(struct ipc_namespace *ns, struct
ipc_params *params)
ns->used_sems += nsems;
sma->sem_base = (struct sem *) &sma[1];
+ sma->complex_count = 0;
+ INIT_LIST_HEAD(&sma->pending_alter);
+ INIT_LIST_HEAD(&sma->pending_const);
+ INIT_LIST_HEAD(&sma->list_id);
+ sma->sem_nsems = nsems;
+ sma->sem_ctime = get_seconds();
for (i = 0; i < nsems; i++) {
INIT_LIST_HEAD(&sma->sem_base[i].pending_alter);
INIT_LIST_HEAD(&sma->sem_base[i].pending_const);
spin_lock_init(&sma->sem_base[i].lock);
+ sma->sem_base[i].sem_otime = sma->sem_ctime;
}
- sma->complex_count = 0;
- INIT_LIST_HEAD(&sma->pending_alter);
- INIT_LIST_HEAD(&sma->pending_const);
- INIT_LIST_HEAD(&sma->list_id);
- sma->sem_nsems = nsems;
- sma->sem_ctime = get_seconds();
sem_unlock(sma, -1);
rcu_read_unlock();
On Sun, 2013-09-22 at 10:17 +0200, Mike Galbraith wrote:
> On Sun, 2013-09-22 at 10:11 +0800, Jia He wrote:
> > In commit 0a2b9d4c,the update of semaphore's sem_otime(last semop time)
> > was removed because he wanted to move setting sem->sem_otime to one
> > place. But after that, the initial semop() will not set the otime
> > because its sem_op value is 0(in semtimedop,will not change
> > otime if alter == 1).
> >
> > the error case:
> > process_a(server) process_b(client)
> > semget()
> > semctl(SETVAL)
> > semop()
> > semget()
> > setctl(IP_STAT)
> > for(;;) { <--not successful here
> > check until sem_otime > 0
> > }
>
> Why not..
(pokes evolution's don't-munge-me button)
ipc,sem: Create semaphores with plausible sem_otime.
Signed-off-by: Mike Galbraith <[email protected]>
diff --git a/ipc/sem.c b/ipc/sem.c
index 4108889..f2564d7 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -471,19 +471,20 @@ static int newary(struct ipc_namespace *ns, struct ipc_params *params)
ns->used_sems += nsems;
sma->sem_base = (struct sem *) &sma[1];
+ sma->complex_count = 0;
+ INIT_LIST_HEAD(&sma->pending_alter);
+ INIT_LIST_HEAD(&sma->pending_const);
+ INIT_LIST_HEAD(&sma->list_id);
+ sma->sem_nsems = nsems;
+ sma->sem_ctime = get_seconds();
for (i = 0; i < nsems; i++) {
INIT_LIST_HEAD(&sma->sem_base[i].pending_alter);
INIT_LIST_HEAD(&sma->sem_base[i].pending_const);
spin_lock_init(&sma->sem_base[i].lock);
+ sma->sem_base[i].sem_otime = sma->sem_ctime;
}
- sma->complex_count = 0;
- INIT_LIST_HEAD(&sma->pending_alter);
- INIT_LIST_HEAD(&sma->pending_const);
- INIT_LIST_HEAD(&sma->list_id);
- sma->sem_nsems = nsems;
- sma->sem_ctime = get_seconds();
sem_unlock(sma, -1);
rcu_read_unlock();
Thanks for the comments, but pls add my email as "from [email protected]"
if you have a better implementation.U know, it is my first kernel patch, maybe
will give me a brilliant memory in the future :)
Anyway, your implementation looks not correct to me. Because from "man semop"
sem_otime will record the last sem operation time of semop. If you change the
otime in semget(), it changes the meanings in stardard, doesn't it?
On Sun, 22 Sep 2013 10:26:04 +0200 from [email protected] wrote:
> On Sun, 2013-09-22 at 10:17 +0200, Mike Galbraith wrote:
>> On Sun, 2013-09-22 at 10:11 +0800, Jia He wrote:
>>> In commit 0a2b9d4c,the update of semaphore's sem_otime(last semop time)
>>> was removed because he wanted to move setting sem->sem_otime to one
>>> place. But after that, the initial semop() will not set the otime
>>> because its sem_op value is 0(in semtimedop,will not change
>>> otime if alter == 1).
>>>
>>> the error case:
>>> process_a(server) process_b(client)
>>> semget()
>>> semctl(SETVAL)
>>> semop()
>>> semget()
>>> setctl(IP_STAT)
>>> for(;;) { <--not successful here
>>> check until sem_otime > 0
>>> }
>> Why not..
> (pokes evolution's don't-munge-me button)
>
> ipc,sem: Create semaphores with plausible sem_otime.
>
> Signed-off-by: Mike Galbraith <[email protected]>
>
> diff --git a/ipc/sem.c b/ipc/sem.c
> index 4108889..f2564d7 100644
> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -471,19 +471,20 @@ static int newary(struct ipc_namespace *ns, struct ipc_params *params)
> ns->used_sems += nsems;
>
> sma->sem_base = (struct sem *) &sma[1];
> + sma->complex_count = 0;
> + INIT_LIST_HEAD(&sma->pending_alter);
> + INIT_LIST_HEAD(&sma->pending_const);
> + INIT_LIST_HEAD(&sma->list_id);
> + sma->sem_nsems = nsems;
> + sma->sem_ctime = get_seconds();
>
> for (i = 0; i < nsems; i++) {
> INIT_LIST_HEAD(&sma->sem_base[i].pending_alter);
> INIT_LIST_HEAD(&sma->sem_base[i].pending_const);
> spin_lock_init(&sma->sem_base[i].lock);
> + sma->sem_base[i].sem_otime = sma->sem_ctime;
> }
>
> - sma->complex_count = 0;
> - INIT_LIST_HEAD(&sma->pending_alter);
> - INIT_LIST_HEAD(&sma->pending_const);
> - INIT_LIST_HEAD(&sma->list_id);
> - sma->sem_nsems = nsems;
> - sma->sem_ctime = get_seconds();
> sem_unlock(sma, -1);
> rcu_read_unlock();
>
>
>
>
On Sun, 2013-09-22 at 17:34 +0800, Jia He wrote:
> Thanks for the comments, but pls add my email as "from [email protected]"
> if you have a better implementation.U know, it is my first kernel patch, maybe
> will give me a brilliant memory in the future :)
You can have the blame if you like :)
> Anyway, your implementation looks not correct to me. Because from "man semop"
> sem_otime will record the last sem operation time of semop. If you change the
> otime in semget(), it changes the meanings in stardard, doesn't it?
A Linux kernel doing a semop in 1970 would be a kinda neat trick :)
-Mike
Hi all,
On 09/22/2013 10:26 AM, Mike Galbraith wrote:
> On Sun, 2013-09-22 at 10:17 +0200, Mike Galbraith wrote:
>> On Sun, 2013-09-22 at 10:11 +0800, Jia He wrote:
>>> In commit 0a2b9d4c,the update of semaphore's sem_otime(last semop time)
>>> was removed because he wanted to move setting sem->sem_otime to one
>>> place. But after that, the initial semop() will not set the otime
>>> because its sem_op value is 0(in semtimedop,will not change
>>> otime if alter == 1).
>>>
>>> the error case:
>>> process_a(server) process_b(client)
>>> semget()
>>> semctl(SETVAL)
>>> semop()
>>> semget()
>>> setctl(IP_STAT)
>>> for(;;) { <--not successful here
>>> check until sem_otime > 0
>>> }
Good catch:
Since commit 0a2b9d4c, wait-for-zero semops do not update sem_otime anymore.
Let's reverse that part of my commit and move the update of sem_otime
back into perform_atomic_semop().
Jia: If perform_atomic_semop() updates sem_otime, then the update in
do_smart_update() is not necessary anymore.
Thus the whole logic with passing arround "semop_completed" can be
removed, too.
Are you interested in writing that patch?
>> Why not..
> (pokes evolution's don't-munge-me button)
>
> ipc,sem: Create semaphores with plausible sem_otime.
Mike: no, your patch makes it worse:
- wait-for-zero semops still don't update sem_otime
- sem_otime is initialized to sem_ctime. That's not mentioned in the
sysv standard.
--
Manfred
On Sun, 22 Sep 2013 12:00:21 +0200 from [email protected] wrote:
> On Sun, 2013-09-22 at 17:34 +0800, Jia He wrote:
>> Thanks for the comments, but pls add my email as "from [email protected]"
>> if you have a better implementation.U know, it is my first kernel patch, maybe
>> will give me a brilliant memory in the future :)
> You can have the blame if you like :)
>
>> Anyway, your implementation looks not correct to me. Because from "man semop"
>> sem_otime will record the last sem operation time of semop. If you change the
>> otime in semget(), it changes the meanings in stardard, doesn't it?
> A Linux kernel doing a semop in 1970 would be a kinda neat trick :)
I will try to make it more clear
comes to my test case again:
process_a(server) process_b(client)
semget() <-seems you choose to set it here
--------------- <1> --------------------
semctl(SETVAL)
semop()
semget()
setctl(IP_STAT)
for(;;) {
check until sem_otime > 0
}
And assume that schedule() happenes at <1>, then sem_otime will >0
in process_b's for(;;), but at that time, the process_a's semctl()
hasn't been called yet.
>
> -Mike
>
> .
>
On Sun, 22 Sep 2013 12:42:05 +0200 from [email protected] wrote:
> Hi all,
>
> On 09/22/2013 10:26 AM, Mike Galbraith wrote:
>> On Sun, 2013-09-22 at 10:17 +0200, Mike Galbraith wrote:
>>> On Sun, 2013-09-22 at 10:11 +0800, Jia He wrote:
>>>> In commit 0a2b9d4c,the update of semaphore's sem_otime(last semop time)
>>>> was removed because he wanted to move setting sem->sem_otime to one
>>>> place. But after that, the initial semop() will not set the otime
>>>> because its sem_op value is 0(in semtimedop,will not change
>>>> otime if alter == 1).
>>>>
>>>> the error case:
>>>> process_a(server) process_b(client)
>>>> semget()
>>>> semctl(SETVAL)
>>>> semop()
>>>> semget()
>>>> setctl(IP_STAT)
>>>> for(;;) { <--not successful here
>>>> check until sem_otime > 0
>>>> }
> Good catch:
> Since commit 0a2b9d4c, wait-for-zero semops do not update sem_otime anymore.
>
> Let's reverse that part of my commit and move the update of sem_otime back
> into perform_atomic_semop().
>
> Jia: If perform_atomic_semop() updates sem_otime, then the update in
> do_smart_update() is not necessary anymore.
> Thus the whole logic with passing arround "semop_completed" can be removed, too.
> Are you interested in writing that patch?
>
With pleasure.
>
>>> Why not..
>> (pokes evolution's don't-munge-me button)
>>
>> ipc,sem: Create semaphores with plausible sem_otime.
> Mike: no, your patch makes it worse:
> - wait-for-zero semops still don't update sem_otime
> - sem_otime is initialized to sem_ctime. That's not mentioned in the sysv
> standard.
>
Agree.
> --
> Manfred
>
Hi Manfred
On Sun, 22 Sep 2013 12:42:05 +0200 from [email protected] wrote:
> Hi all,
>
> On 09/22/2013 10:26 AM, Mike Galbraith wrote:
>> On Sun, 2013-09-22 at 10:17 +0200, Mike Galbraith wrote:
>>> On Sun, 2013-09-22 at 10:11 +0800, Jia He wrote:
>>>> In commit 0a2b9d4c,the update of semaphore's sem_otime(last semop time)
>>>> was removed because he wanted to move setting sem->sem_otime to one
>>>> place. But after that, the initial semop() will not set the otime
>>>> because its sem_op value is 0(in semtimedop,will not change
>>>> otime if alter == 1).
>>>>
>>>> the error case:
>>>> process_a(server) process_b(client)
>>>> semget()
>>>> semctl(SETVAL)
>>>> semop()
>>>> semget()
>>>> setctl(IP_STAT)
>>>> for(;;) { <--not successful here
>>>> check until sem_otime > 0
>>>> }
> Good catch:
> Since commit 0a2b9d4c, wait-for-zero semops do not update sem_otime anymore.
>
> Let's reverse that part of my commit and move the update of sem_otime back
> into perform_atomic_semop().
>
> Jia: If perform_atomic_semop() updates sem_otime, then the update in
> do_smart_update() is not necessary anymore.
> Thus the whole logic with passing arround "semop_completed" can be removed, too.
> Are you interested in writing that patch?
>
Not all perform_atomic_semop() can cover the points of do_smart_update()
after I move the parts of updating otime.
Eg. in semctl_setval/exit_sem/etc. That is, it seems I need to write some
other codes to update sem_otime outside perform_atomic_semop(). That
still violate your original goal---let one function do all the update otime
things.
IMO, what if just remove the condition alter in sys_semtimedop:
- if (alter && error == 0)
+ if (error == 0)
do_smart_update(sma, sops, nsops, 1, &tasks);
In old codes, it would set the otime even when sem_op == 0
>
>>> Why not..
>> (pokes evolution's don't-munge-me button)
>>
>> ipc,sem: Create semaphores with plausible sem_otime.
> Mike: no, your patch makes it worse:
> - wait-for-zero semops still don't update sem_otime
> - sem_otime is initialized to sem_ctime. That's not mentioned in the sysv
> standard.
>
> --
> Manfred
>
On Sun, 2013-09-22 at 12:42 +0200, Manfred Spraul wrote:
> Mike: no, your patch makes it worse:
> - wait-for-zero semops still don't update sem_otime
> - sem_otime is initialized to sem_ctime. That's not mentioned in the
> sysv standard.
So sem_otime = 0 is a specified semaphore state? I thought the proggy
was busted for spinning on a (busted and) irrelevant stamp.
Man lernt nie aus.
-Mike
On Mon, 23 Sep 2013 03:08:36 +0200 from [email protected] wrote:
> On Sun, 2013-09-22 at 12:42 +0200, Manfred Spraul wrote:
>
>> Mike: no, your patch makes it worse:
>> - wait-for-zero semops still don't update sem_otime
>> - sem_otime is initialized to sem_ctime. That's not mentioned in the
>> sysv standard.
> So sem_otime = 0 is a specified semaphore state? I thought the proggy
> was busted for spinning on a (busted and) irrelevant stamp.
Please refer to the words from Unix Network Programming - Volume 2 2nd
Edition Chapter 11
"Fortunately, there is a way around this race condition. We are guaranteed
that thesem-otime member of the semid-ds structure is set to 0 when a
new semaphore set iscreated. (The System V manuals have stated this
fact for a long time, as do the XPG3and Unix 98 standards.) This member
is set to the current time only by a successful callto semop."
>
> Man lernt nie aus.
>
> -Mike
>
>
On 09/22/2013 05:14 PM, Jia He wrote:
> Hi Manfred
>
> On Sun, 22 Sep 2013 12:42:05 +0200 from [email protected] wrote:
>> Hi all,
>>
>> On 09/22/2013 10:26 AM, Mike Galbraith wrote:
>>> On Sun, 2013-09-22 at 10:17 +0200, Mike Galbraith wrote:
>>>> On Sun, 2013-09-22 at 10:11 +0800, Jia He wrote:
>>>>> In commit 0a2b9d4c,the update of semaphore's sem_otime(last semop time)
>>>>> was removed because he wanted to move setting sem->sem_otime to one
>>>>> place. But after that, the initial semop() will not set the otime
>>>>> because its sem_op value is 0(in semtimedop,will not change
>>>>> otime if alter == 1).
>>>>>
>>>>> the error case:
>>>>> process_a(server) process_b(client)
>>>>> semget()
>>>>> semctl(SETVAL)
>>>>> semop()
>>>>> semget()
>>>>> setctl(IP_STAT)
>>>>> for(;;) { <--not successful here
>>>>> check until sem_otime > 0
>>>>> }
>> Good catch:
>> Since commit 0a2b9d4c, wait-for-zero semops do not update sem_otime anymore.
>>
>> Let's reverse that part of my commit and move the update of sem_otime back
>> into perform_atomic_semop().
>>
>> Jia: If perform_atomic_semop() updates sem_otime, then the update in
>> do_smart_update() is not necessary anymore.
>> Thus the whole logic with passing arround "semop_completed" can be removed, too.
>> Are you interested in writing that patch?
>>
> Not all perform_atomic_semop() can cover the points of do_smart_update()
> after I move the parts of updating otime.
> Eg. in semctl_setval/exit_sem/etc. That is, it seems I need to write some
> other codes to update sem_otime outside perform_atomic_semop(). That
> still violate your original goal---let one function do all the update otime
> things.
No. The original goal was an optimization:
The common case is one semop that increases a semaphore value - and that
allows another semop that is sleeping to proceed.
Before, this caused two get_seconds() calls.
The current (buggy) code avoids the 2nd call.
> IMO, what if just remove the condition alter in sys_semtimedop:
> - if (alter && error == 0)
> + if (error == 0)
> do_smart_update(sma, sops, nsops, 1, &tasks);
> In old codes, it would set the otime even when sem_op == 0
do_smart_update() can be expensive - thus it shouldn't be called if we
didn't change anything.
Attached is a proposed patch - fully untested. It is intended to be
applied on top of Jia's patch.
_If_ the performance degradation is too large, then the alternative
would be to set sem_otime directly in semtimedop() for successfull
non-alter operations.
--
Manfred
Hi Manfred
IIUC after reivewing your patch and src code, does it seem
sem_otime lost the chance to be updated when calling
semctl_main/semctl_setval?
In old codes, even whendo_smart_update(sma, NULL, 0, 0, &tasks),
the otime can be updated after several conditions checking.
On Tue, 24 Sep 2013 23:09:33 +0200 from [email protected] wrote:
> On 09/22/2013 05:14 PM, Jia He wrote:
>> Hi Manfred
>>
>> On Sun, 22 Sep 2013 12:42:05 +0200 from [email protected] wrote:
>>> Hi all,
>>>
>>> On 09/22/2013 10:26 AM, Mike Galbraith wrote:
>>>> On Sun, 2013-09-22 at 10:17 +0200, Mike Galbraith wrote:
>>>>> On Sun, 2013-09-22 at 10:11 +0800, Jia He wrote:
>>>>>> In commit 0a2b9d4c,the update of semaphore's sem_otime(last semop time)
>>>>>> was removed because he wanted to move setting sem->sem_otime to one
>>>>>> place. But after that, the initial semop() will not set the otime
>>>>>> because its sem_op value is 0(in semtimedop,will not change
>>>>>> otime if alter == 1).
>>>>>>
>>>>>> the error case:
>>>>>> process_a(server) process_b(client)
>>>>>> semget()
>>>>>> semctl(SETVAL)
>>>>>> semop()
>>>>>> semget()
>>>>>> setctl(IP_STAT)
>>>>>> for(;;) { <--not successful here
>>>>>> check until sem_otime > 0
>>>>>> }
>>> Good catch:
>>> Since commit 0a2b9d4c, wait-for-zero semops do not update sem_otime anymore.
>>>
>>> Let's reverse that part of my commit and move the update of sem_otime back
>>> into perform_atomic_semop().
>>>
>>> Jia: If perform_atomic_semop() updates sem_otime, then the update in
>>> do_smart_update() is not necessary anymore.
>>> Thus the whole logic with passing arround "semop_completed" can be removed,
>>> too.
>>> Are you interested in writing that patch?
>>>
>> Not all perform_atomic_semop() can cover the points of do_smart_update()
>> after I move the parts of updating otime.
>> Eg. in semctl_setval/exit_sem/etc. That is, it seems I need to write some
>> other codes to update sem_otime outside perform_atomic_semop(). That
>> still violate your original goal---let one function do all the update otime
>> things.
> No. The original goal was an optimization:
> The common case is one semop that increases a semaphore value - and that
> allows another semop that is sleeping to proceed.
> Before, this caused two get_seconds() calls.
> The current (buggy) code avoids the 2nd call.
>
>> IMO, what if just remove the condition alter in sys_semtimedop:
>> - if (alter && error == 0)
>> + if (error == 0)
>> do_smart_update(sma, sops, nsops, 1, &tasks);
>> In old codes, it would set the otime even when sem_op == 0
> do_smart_update() can be expensive - thus it shouldn't be called if we didn't
> change anything.
>
> Attached is a proposed patch - fully untested. It is intended to be applied
> on top of Jia's patch.
>
> _If_ the performance degradation is too large, then the alternative would be
> to set sem_otime directly in semtimedop() for successfull non-alter operations.
>
> --
> Manfred
Hi Jia,
On 09/25/2013 05:05 AM, Jia He wrote:
> Hi Manfred
> IIUC after reivewing your patch and src code, does it seem
> sem_otime lost the chance to be updated when calling
> semctl_main/semctl_setval?
> In old codes, even whendo_smart_update(sma, NULL, 0, 0, &tasks),
> the otime can be updated after several conditions checking.
The update is performed now performed inside perform_atomic_semop():
Old code:
perform_atomic_semop() does not update sem_otime. It just returns 0 for
successfull operations.
This "0 returned" is passed upwards ("semop_completed") into
do_smart_update()
do_smart_update() updates sem_otime.
New code:
perform_atomic_semop() updates sem_otime immediately (your change).
No need to keep track if a waiting operation was completed (my change).
I don't see a problem - perhaps I overlook something.
Which problem do you see?
--
Manfred
Hi Manfred
got it :) I am so glad that my minor is on top of yours
Anyway,
Do you think it is more safe to update the otime like this:
- sma->sem_base[sops[0].sem_num].sem_otime =
- get_seconds();
+ if (sops == NULL) {
+ sma->sem_base[0].sem_otime = get_seconds();
+ } else {
+ sma->sem_base[sops[0].sem_num].sem_otime =
+ get_seconds();
+ }
If u think so, i will update my patch according to it
On Wed, 25 Sep 2013 08:55:16 +0200 from [email protected] wrote:
> Hi Jia,
>
> On 09/25/2013 05:05 AM, Jia He wrote:
>> Hi Manfred
>> IIUC after reivewing your patch and src code, does it seem
>> sem_otime lost the chance to be updated when calling
>> semctl_main/semctl_setval?
>> In old codes, even whendo_smart_update(sma, NULL, 0, 0, &tasks),
>> the otime can be updated after several conditions checking.
> The update is performed now performed inside perform_atomic_semop():
>
> Old code:
> perform_atomic_semop() does not update sem_otime. It just returns 0 for
> successfull operations.
> This "0 returned" is passed upwards ("semop_completed") into do_smart_update()
> do_smart_update() updates sem_otime.
>
> New code:
> perform_atomic_semop() updates sem_otime immediately (your change).
> No need to keep track if a waiting operation was completed (my change).
>
> I don't see a problem - perhaps I overlook something.
> Which problem do you see?
>
> --
> Manfred
>
>