* RE: [ACPI] down_timeout
@ 2003-10-03 20:29 Moore, Robert
2003-10-03 23:29 ` Alan Cox
0 siblings, 1 reply; 2+ messages in thread
From: Moore, Robert @ 2003-10-03 20:29 UTC (permalink / raw)
To: Matthew Wilcox, Yury Umanets; +Cc: acpi-devel, linux-kernel
I would say that the whole thing is wrong -- the kernel should provide a
semaphore wait function that includes a timeout parameter.
Bob
-----Original Message-----
From: acpi-devel-admin@lists.sourceforge.net
[mailto:acpi-devel-admin@lists.sourceforge.net] On Behalf Of Matthew
Wilcox
Sent: Friday, October 03, 2003 7:25 AM
To: Yury Umanets
Cc: acpi-devel@lists.sourceforge.net; linux-kernel@vger.kernel.org
Subject: [ACPI] down_timeout
[l-k people, skip to the bottom, that's where down_timeout is]
On Fri, Oct 03, 2003 at 04:37:53PM +0400, Yury Umanets wrote:
> Thus, @quantum_ms will be calculated longer for shorter HZ and this is
> definitelly not good in my opinion. Am I right?
You're right, but for the wrong reason. This code is pretty inaccurate
as it's relying on the result of integer divides. This code should
work better (disclaimer: compiled, not tested):
Index: drivers/acpi/osl.c
===================================================================
RCS file: /var/cvs/linux-2.6/drivers/acpi/osl.c,v
retrieving revision 1.3
diff -u -p -r1.3 osl.c
--- drivers/acpi/osl.c 23 Aug 2003 02:46:37 -0000 1.3
+++ drivers/acpi/osl.c 3 Oct 2003 14:02:44 -0000
@@ -827,7 +827,6 @@ acpi_os_wait_semaphore(
{
acpi_status status = AE_OK;
struct semaphore *sem = (struct semaphore*)handle;
- int ret = 0;
ACPI_FUNCTION_TRACE ("os_wait_semaphore");
@@ -842,56 +841,28 @@ acpi_os_wait_semaphore(
if (in_atomic())
timeout = 0;
- switch (timeout)
- {
- /*
- * No Wait:
- * --------
- * A zero timeout value indicates that we shouldn't wait
- just
- * acquire the semaphore if available otherwise return
AE_TIME
- * (a.k.a. 'would block').
- */
- case 0:
- if(down_trylock(sem))
- status = AE_TIME;
- break;
-
- /*
- * Wait Indefinitely:
- * ------------------
- */
- case ACPI_WAIT_FOREVER:
+ if (timeout == ACPI_WAIT_FOREVER) {
down(sem);
- break;
-
- /*
- * Wait w/ Timeout:
- * ----------------
- */
- default:
- // TODO: A better timeout algorithm?
- {
- int i = 0;
- static const int quantum_ms = 1000/HZ;
-
+ } else if (down_trylock(sem) == 0) {
+ /* Success, do nothing */
+ } else {
+ long now = jiffies;
+ int ret = 1;
+ while (jiffies < now + timeout * HZ) {
+ current->state = TASK_INTERRUPTIBLE;
+ schedule_timeout(1);
ret = down_trylock(sem);
- for (i = timeout; (i > 0 && ret < 0); i -=
quantum_ms) {
- current->state = TASK_INTERRUPTIBLE;
- schedule_timeout(1);
- ret = down_trylock(sem);
- }
-
- if (ret != 0)
- status = AE_TIME;
+ if (!ret)
+ break;
}
- break;
+ if (ret)
+ status = AE_TIME;
}
if (ACPI_FAILURE(status)) {
ACPI_DEBUG_PRINT ((ACPI_DB_ERROR, "Failed to acquire
semaphore[%p|%d|%d], %s\n",
handle, units, timeout,
acpi_format_exception(status)));
- }
- else {
+ } else {
ACPI_DEBUG_PRINT ((ACPI_DB_MUTEX, "Acquired
semaphore[%p|%d|%d]\n", handle, units, timeout));
}
[l-k people, this is the interesting bit]
It's still not great because it doesn't preserve ordering.
down_timeout()
would be a much better primitive. We have down_interruptible() which
could be used for this purpose. Something like (completely uncompiled):
/* Returns -EINTR if the timeout expires */
int down_timeout(struct semaphore *sem, long timeout)
{
struct timer_list timer;
int result;
init_timer(&timer);
timer.expires = timeout + jiffies;
timer.data = (unsigned long) current;
timer.function = process_timeout;
add_timer(&timer);
result = down_interruptible(sem);
del_timer_sync(&timer);
return result;
}
(This would have to go in kernel/timer.c as that's where process_timeout
lives).
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead
bodies.
Do you think I want to have an academic debate on this subject?" --
Robert Fisk
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
Acpi-devel mailing list
Acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/acpi-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
* RE: [ACPI] down_timeout
2003-10-03 20:29 [ACPI] down_timeout Moore, Robert
@ 2003-10-03 23:29 ` Alan Cox
0 siblings, 0 replies; 2+ messages in thread
From: Alan Cox @ 2003-10-03 23:29 UTC (permalink / raw)
To: Moore, Robert
Cc: Matthew Wilcox, Yury Umanets, acpi-devel, Linux Kernel Mailing List
On Gwe, 2003-10-03 at 21:29, Moore, Robert wrote:
> I would say that the whole thing is wrong -- the kernel should provide a
> semaphore wait function that includes a timeout parameter.
Thats probably the right thing to fix. A timeout aware version of down()
doesnt actually look too hard.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2003-10-03 23:32 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-10-03 20:29 [ACPI] down_timeout Moore, Robert
2003-10-03 23:29 ` Alan Cox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).