Messages in this thread Patch in this message | | | Date | Wed, 17 Aug 2005 15:24:15 -0700 | From | Nishanth Aravamudan <> | Subject | Re: [UPDATE PATCH] push rounding up of relative request to schedule_timeout() |
| |
On 17.08.2005 [12:51:17 -0700], George Anzinger wrote: > Nishanth Aravamudan wrote: > ~ > >>IMNSHO we should not get too parental with kernel only interfaces. > >>Adding 1 is easy enough for the caller and even easier to explain in the > >>instructions (i.e. this call sleeps for X jiffies edges). This allows > >>the caller to do more if needed and, should he ever just want to sync to > >>the next jiffie he does not have to deal with backing out that +1. > > > > > >I don't want to be too parental either, but I also am trying to avoid > >code duplication. Lots of drivers basically do something like > >poll_event() does (or could do with some changes), i.e. looping a > >constant amount multiple times, checking something every so often. The > >patch was just a thought, though. I will keep evaluating drivers and see > >if it's a useful interface to have eventually. > > > >I guess I'm just concerned with making an unintuitive interface. As was > >brought up at OLS, drivers are a major source of bugs/buggy code. The > >simpler, more useful we can make interfaces, the better, I think. I'm > >not claiming you disagree, I just want to make my own motives clear. > >While fixing up the schedule_timeout() comment would make it clear what > >schedule_timeout() achieves, I'm not sure how useful such an interface > >is, if every caller adds 1 :) I need to mull it over, though... Lots to > >consider. I also, of course, want to stay flexible for the reasons you > >mention (letting the driver adjust the timeout as they expect to). > > I would leave the +1 alone and put in the correct documentation. This > way _more_ folks will be made aware of the mid jiffie issue. Far to > often we see (and let get in) patches that mess up user interfaces > around this issue. The recent changes to itimer come to mind...
Ok, makes sense to me; does the following patch work for everybody? The wording is a bit awkward, but so is the issue :)
Description: Fix schedule_timeout()'s comment, indicated the inter-tick rounding issue. Since the kernel does not keep track of an inter-tick position in jiffies, a caller which wishes to sleep for at least a certain number of jiffies should add its request to the *next* jiffies value (meaning add 1 to its relative request). Make that clear in the comment.
Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
---
diff -urpN 2.6.13-rc6/kernel/timer.c 2.6.13-rc6-dev/kernel/timer.c --- 2.6.13-rc6/kernel/timer.c 2005-08-09 15:22:57.000000000 -0700 +++ 2.6.13-rc6-dev/kernel/timer.c 2005-08-17 15:21:35.000000000 -0700 @@ -1077,9 +1077,15 @@ static void process_timeout(unsigned lon * schedule_timeout - sleep until timeout * @timeout: timeout value in jiffies * - * Make the current task sleep until @timeout jiffies have - * elapsed. The routine will return immediately unless - * the current task state has been set (see set_current_state()). + * Make the current task sleep until @timeout timer interrupts have + * occurred, meaning jiffies has incremented @timeout times and not + * necessarily that @timeout jiffies have elapsed. If the task wishes to + * sleep until (at least) @timeout jiffies have elapsed, then it should + * add 1 to its request. This is necessary, as the kernel does not keep + * track of an inter-jiffy position, so the caller must "round up" its + * request so that it begins at the next jiffy. The routine will return + * immediately unless the current task state has been set (see + * set_current_state()). * * You can set the task state as follows - * - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |