Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
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 manf...@colorfullife.com 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, ),
>> 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
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
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, ),
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
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 manf...@colorfullife.com 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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
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, ),
the otime can be updated after several conditions checking.
On Tue, 24 Sep 2013 23:09:33 +0200 from manf...@colorfullife.com wrote:
> On 09/22/2013 05:14 PM, Jia He wrote:
>> Hi Manfred
>>
>> On Sun, 22 Sep 2013 12:42:05 +0200 from manf...@colorfullife.com 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, );
>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
On 09/22/2013 05:14 PM, Jia He wrote:
Hi Manfred
On Sun, 22 Sep 2013 12:42:05 +0200 from manf...@colorfullife.com 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, );
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
>From 07da483abc88748a666f655f3e1d81a94df88e38 Mon Sep 17 00:00:00 2001
From: Manfred Spraul
Date: Tue, 24 Sep 2013 22:53:27 +0200
Subject: [PATCH] ipc/sem.c: Simplify sem_otime handling.
The sem_otime handling tries to minimize the number of calls to
get_seconds().
This generates some complexity (i.e.: pass around if any operation
completed) and introduced one bug (See previous commit to ipc/sem.c).
Therefore: Remove the whole logic - this removes 25 lines.
Disadvantages:
- One line of code duplication in exit_sem():
The function must now update sem_otime, it can't rely on
do_smart_update_queue() to perform that task.
- Two get_seconds() calls instead of one call for the common
case of increasing a semaphore and waking up a sleeping task.
TODO:
1) How fast is get_seconds()?
Is the optimization worth the effort?
2) Test it.
---
ipc/sem.c | 61 ++---
1 file changed, 18 insertions(+), 43 deletions(-)
diff --git a/ipc/sem.c b/ipc/sem.c
index 8e01e76..5e5d7b1 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -713,15 +713,13 @@ static int check_restart(struct sem_array *sma, struct sem_queue *q)
* semaphore.
* The tasks that must be woken up are added to @pt. The return code
* is stored in q->pid.
- * The function returns 1 if at least one operation was completed successfully.
*/
-static int wake_const_ops(struct sem_array *sma, int semnum,
+static void wake_const_ops(struct sem_array *sma, int semnum,
struct list_head *pt)
{
struct sem_queue *q;
struct list_head *walk;
struct list_head *pending_list;
- int semop_completed = 0;
if (semnum == -1)
pending_list = >pending_const;
@@ -742,13 +740,9 @@ static int wake_const_ops(struct sem_array *sma, int semnum,
/* operation completed, remove from queue & wakeup */
unlink_queue(sma, q);
-
wake_up_sem_queue_prepare(pt, q, error);
- if (error == 0)
-semop_completed = 1;
}
}
- return semop_completed;
}
/**
@@ -761,13 +755,11 @@ static int wake_const_ops(struct sem_array *sma, int semnum,
* do_smart_wakeup_zero() checks all required queue for wait-for-zero
* operations, based on the actual changes that were performed on the
* semaphore array.
- * The function returns 1 if at least one operation was completed successfully.
*/
-static int do_smart_wakeup_zero(struct sem_array *sma, struct sembuf *sops,
+static void
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
On 09/22/2013 05:14 PM, Jia He wrote:
Hi Manfred
On Sun, 22 Sep 2013 12:42:05 +0200 from manf...@colorfullife.com 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
From 07da483abc88748a666f655f3e1d81a94df88e38 Mon Sep 17 00:00:00 2001
From: Manfred Spraul manf...@colorfullife.com
Date: Tue, 24 Sep 2013 22:53:27 +0200
Subject: [PATCH] ipc/sem.c: Simplify sem_otime handling.
The sem_otime handling tries to minimize the number of calls to
get_seconds().
This generates some complexity (i.e.: pass around if any operation
completed) and introduced one bug (See previous commit to ipc/sem.c).
Therefore: Remove the whole logic - this removes 25 lines.
Disadvantages:
- One line of code duplication in exit_sem():
The function must now update sem_otime, it can't rely on
do_smart_update_queue() to perform that task.
- Two get_seconds() calls instead of one call for the common
case of increasing a semaphore and waking up a sleeping task.
TODO:
1) How fast is get_seconds()?
Is the optimization worth the effort?
2) Test it.
---
ipc/sem.c | 61 ++---
1 file changed, 18 insertions(+), 43 deletions(-)
diff --git a/ipc/sem.c b/ipc/sem.c
index 8e01e76..5e5d7b1 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -713,15 +713,13 @@ static int check_restart(struct sem_array *sma, struct sem_queue *q)
* semaphore.
* The tasks that must be woken up are added to @pt. The return code
* is stored in q-pid.
- * The function returns 1 if at least one operation was completed successfully.
*/
-static int wake_const_ops(struct sem_array *sma, int semnum,
+static void wake_const_ops(struct sem_array *sma, int semnum,
struct list_head *pt)
{
struct sem_queue *q;
struct list_head *walk;
struct list_head *pending_list;
- int semop_completed = 0;
if (semnum == -1)
pending_list = sma-pending_const;
@@ -742,13 +740,9 @@ static int wake_const_ops(struct sem_array *sma, int semnum,
/* operation completed, remove from queue wakeup */
unlink_queue(sma, q);
-
wake_up_sem_queue_prepare(pt, q, error);
- if (error == 0)
-semop_completed = 1;
}
}
- return semop_completed;
}
/**
@@ -761,13 +755,11 @@ static int wake_const_ops(struct sem_array *sma, int semnum,
* do_smart_wakeup_zero() checks all required queue for wait-for-zero
* operations, based on the actual changes that were performed on the
* semaphore array.
- * The function returns 1 if at least one operation was completed successfully.
*/
-static int do_smart_wakeup_zero(struct sem_array *sma, struct sembuf *sops,
+static void
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
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 manf...@colorfullife.com wrote:
On 09/22/2013 05:14 PM, Jia He wrote:
Hi Manfred
On Sun, 22 Sep 2013 12:42:05 +0200 from manf...@colorfullife.com 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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
On Mon, 23 Sep 2013 03:08:36 +0200 from bitbuc...@online.de 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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
Hi Manfred
On Sun, 22 Sep 2013 12:42:05 +0200 from manf...@colorfullife.com 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, );
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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
On Sun, 22 Sep 2013 12:42:05 +0200 from manf...@colorfullife.com 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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
On Sun, 22 Sep 2013 12:00:21 +0200 from bitbuc...@online.de wrote:
> On Sun, 2013-09-22 at 17:34 +0800, Jia He wrote:
>> Thanks for the comments, but pls add my email as "from jiaker...@gmail.com"
>> 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
>
> .
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
On Sun, 2013-09-22 at 17:34 +0800, Jia He wrote: > Thanks for the comments, but pls add my email as "from jiaker...@gmail.com" > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
Thanks for the comments, but pls add my email as "from jiaker...@gmail.com"
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 bitbuc...@online.de 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
>
> 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 *) [1];
> + sma->complex_count = 0;
> + INIT_LIST_HEAD(>pending_alter);
> + INIT_LIST_HEAD(>pending_const);
> + INIT_LIST_HEAD(>list_id);
> + sma->sem_nsems = nsems;
> + sma->sem_ctime = get_seconds();
>
> for (i = 0; i < nsems; i++) {
> INIT_LIST_HEAD(>sem_base[i].pending_alter);
> INIT_LIST_HEAD(>sem_base[i].pending_const);
> spin_lock_init(>sem_base[i].lock);
> + sma->sem_base[i].sem_otime = sma->sem_ctime;
> }
>
> - sma->complex_count = 0;
> - INIT_LIST_HEAD(>pending_alter);
> - INIT_LIST_HEAD(>pending_const);
> - INIT_LIST_HEAD(>list_id);
> - sma->sem_nsems = nsems;
> - sma->sem_ctime = get_seconds();
> sem_unlock(sma, -1);
> rcu_read_unlock();
>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
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
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 *) [1];
+ sma->complex_count = 0;
+ INIT_LIST_HEAD(>pending_alter);
+ INIT_LIST_HEAD(>pending_const);
+ INIT_LIST_HEAD(>list_id);
+ sma->sem_nsems = nsems;
+ sma->sem_ctime = get_seconds();
for (i = 0; i < nsems; i++) {
INIT_LIST_HEAD(>sem_base[i].pending_alter);
INIT_LIST_HEAD(>sem_base[i].pending_const);
spin_lock_init(>sem_base[i].lock);
+ sma->sem_base[i].sem_otime = sma->sem_ctime;
}
- sma->complex_count = 0;
- INIT_LIST_HEAD(>pending_alter);
- INIT_LIST_HEAD(>pending_const);
- INIT_LIST_HEAD(>list_id);
- sma->sem_nsems = nsems;
- sma->sem_ctime = get_seconds();
sem_unlock(sma, -1);
rcu_read_unlock();
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
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
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 *) [1];
+ sma->complex_count = 0;
+ INIT_LIST_HEAD(>pending_alter);
+ INIT_LIST_HEAD(>pending_const);
+ INIT_LIST_HEAD(>list_id);
+ sma->sem_nsems = nsems;
+ sma->sem_ctime = get_seconds();
for (i = 0; i < nsems; i++) {
INIT_LIST_HEAD(>sem_base[i].pending_alter);
INIT_LIST_HEAD(>sem_base[i].pending_const);
spin_lock_init(>sem_base[i].lock);
+ sma->sem_base[i].sem_otime = sma->sem_ctime;
}
- sma->complex_count = 0;
- INIT_LIST_HEAD(>pending_alter);
- INIT_LIST_HEAD(>pending_const);
- INIT_LIST_HEAD(>list_id);
- sma->sem_nsems = nsems;
- sma->sem_ctime = get_seconds();
sem_unlock(sma, -1);
rcu_read_unlock();
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
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 bitbuc...@online.de
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();
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
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 bitbuc...@online.de
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();
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
Thanks for the comments, but pls add my email as from jiaker...@gmail.com
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 bitbuc...@online.de 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 bitbuc...@online.de
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();
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
On Sun, 2013-09-22 at 17:34 +0800, Jia He wrote: Thanks for the comments, but pls add my email as from jiaker...@gmail.com 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
On Sun, 22 Sep 2013 12:00:21 +0200 from bitbuc...@online.de wrote:
On Sun, 2013-09-22 at 17:34 +0800, Jia He wrote:
Thanks for the comments, but pls add my email as from jiaker...@gmail.com
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
.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
On Sun, 22 Sep 2013 12:42:05 +0200 from manf...@colorfullife.com 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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
Hi Manfred
On Sun, 22 Sep 2013 12:42:05 +0200 from manf...@colorfullife.com 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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
On Mon, 23 Sep 2013 03:08:36 +0200 from bitbuc...@online.de 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
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, , 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 = _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
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ipc/sem.c: fix update sem_otime when calling sem_op in semaphore initialization
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 jiaker...@gmail.com
---
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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
