Message ID | 20040927102552.GA19183@gondor.apana.org.au |
---|---|
State | New, archived |
Headers | show |
Series |
|
Related | show |
On Llu, 2004-09-27 at 11:25, Herbert Xu wrote: > The continue is just paranoia in case something relies on the sleep > to take 2 seconds or more. If the signal occurs then you'll spin for 2 seconds because the signal is still waiting to be serviced. This therefore looks broken A more interesting question is why this isn't being driven off a timer ? - 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/
On Mer, 2004-09-29 at 02:58, Herbert Xu wrote: > > A more interesting question is why this isn't being driven off a > > timer ? > > It probably could if the stuff afterwards doesn't sleep. schedule_work() ? - 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/
On Mon, Sep 27, 2004 at 01:51:42PM +0100, Alan Cox wrote: > On Llu, 2004-09-27 at 11:25, Herbert Xu wrote: > > The continue is just paranoia in case something relies on the sleep > > to take 2 seconds or more. > > If the signal occurs then you'll spin for 2 seconds because the signal > is still waiting to be serviced. This therefore looks broken Yes you're right. However I'd say that msleep_interruptible should mirror the behaviour of schedule_timeout and at least sleep once. BTW, msleep_interruptible() is white-space damaged. Can someone please fix it up? > A more interesting question is why this isn't being driven off a > timer ? It probably could if the stuff afterwards doesn't sleep. Cheers,
On Wed, 2004-09-29 at 11:24, Alan Cox wrote: > On Mer, 2004-09-29 at 02:58, Herbert Xu wrote: > > > A more interesting question is why this isn't being driven off a > > > timer ? > > > > It probably could if the stuff afterwards doesn't sleep. > > schedule_work() ? I don't like that. I wrote the g5 therm driver (from which this one is derivated) as a kernel thread because, at least on the g5, I do a lot of i2c accesses. If I were to do that in schedule_work, I would "hog" keventd a very long time each time, which is bad. schedule_work() is always way too much abused in this way, thus beeing a source of latencies. Creating my own work queue would have been silly since (at least back then), it would have meant creating one additional kernel thread on every CPU... so I decided just to create my own kernel thread and be done with it. Now, using a timer and waiting on it would eventually work too, but the way it is now just works so ... Ben. - 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/
On Wed, 2004-09-29 at 11:58, Herbert Xu wrote: > On Mon, Sep 27, 2004 at 01:51:42PM +0100, Alan Cox wrote: > > On Llu, 2004-09-27 at 11:25, Herbert Xu wrote: > > > The continue is just paranoia in case something relies on the sleep > > > to take 2 seconds or more. > > > > If the signal occurs then you'll spin for 2 seconds because the signal > > is still waiting to be serviced. This therefore looks broken > > Yes you're right. However I'd say that msleep_interruptible should > mirror the behaviour of schedule_timeout and at least sleep once. Mask all signals then, there is no need to get any signal in that kernel thread anyway > BTW, msleep_interruptible() is white-space damaged. Can someone please > fix it up? > > > A more interesting question is why this isn't being driven off a > > timer ? > > It probably could if the stuff afterwards doesn't sleep. > > Cheers,
Alan Cox <alan@lxorguk.ukuu.org.uk> wrote: > > On Llu, 2004-09-27 at 11:25, Herbert Xu wrote: > > The continue is just paranoia in case something relies on the sleep > > to take 2 seconds or more. > > If the signal occurs then you'll spin for 2 seconds because the signal > is still waiting to be serviced. This therefore looks broken It's a kernel thread, so unless it has taken explicit action to enable signals, it should be OK. - 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/
===== drivers/macintosh/therm_adt746x.c 1.5 vs edited ===== --- 1.5/drivers/macintosh/therm_adt746x.c 2004-09-23 06:31:14 +10:00 +++ edited/drivers/macintosh/therm_adt746x.c 2004-09-27 20:24:58 +10:00 @@ -246,7 +246,8 @@ while(monitor_running) { - msleep(2000); + if (msleep_interruptible(2000)) + continue; /* Check status */ /* local : chip */
Hi: Using msleep() in a kernel thread causes it to show up in the D state and contribute towards the system load average. The following patch converts it to msleep_interruptible(). The continue is just paranoia in case something relies on the sleep to take 2 seconds or more. This bug was reported at https://bugzilla.no-name-yet.com/show_bug.cgi?id=1804 Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> Cheers,