From: Nishanth Aravamudan <nacc@us.ibm.com>
To: Margit Schubert-While <margitsw@t-online.de>,
netdev@oss.sgi.com, kj <kernel-janitors@osdl.org>
Subject: Re: [Kernel-janitors] Re: [patch 8/8] prism54/islpci_dev: replace schedule_timeout() with msleep()
Date: Thu, 2 Sep 2004 16:23:01 +0000 [thread overview]
Message-ID: <20040902162301.GB1944@us.ibm.com> (raw)
In-Reply-To: <20040902100322.GD1876@stro.at>
On Thu, Sep 02, 2004 at 12:03:22PM +0200, maximilian attems wrote:
> On Thu, 02 Sep 2004, Margit Schubert-While wrote:
>
> > On Thu, 02 Sep 2004, Maximilian scribeth:
> > > it shouldn't hinder 2.6 in it's progression.
> > I consider this a regression.
> > As schedule_timeout is used elesewhere in the prism54 code,
> > we are using a consistent and documented method.
A grep of drivers/net/wireless/prism54 for schedule_timeout showed three
occurrences (in 2.6.9-rc1-bk7):
islpci_dev.c: schedule_timeout(50*HZ/1000);
islpci_dev.c: remaining = schedule_timeout(HZ);
islpci_mgt.c: timeleft = schedule_timeout(wait_cycle_jiffies);
The first is removed by my patch.
The second & third are potentially bugs as there is no
set_current_state() preceding the call to schedule_timeout(). As per the
source:
/**
* 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()).
Therefore, in the current code, the schedule_timeout() call does not
have the desired effect (the same information is available in
kernel-hacking.ps).
Both of these calls should probably be fixed, but I'm not sure if you
wish to sleep in TASK_INTERUPTIBLE or TASK_UNINTERRUPTIBLE. Keep in mind
that msleep_interruptible() is also (hopefully) being pushed to the
kernel soon.
As to consistency or documentation . . . I have no evidence to suggest
that msleep() is inconsistent. And I don't think there is any need for
more documentation than the source in this case:
/**
* msleep - sleep safely even with waitqueue interruptions
* @msecs: Time in milliseconds to sleep for
*/
Hope this helps clear things up.
-Nish
next prev parent reply other threads:[~2004-09-02 16:23 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2004-09-02 9:35 [patch 8/8] prism54/islpci_dev: replace schedule_timeout() with msleep() Margit Schubert-While
2004-09-02 10:03 ` maximilian attems
2004-09-02 16:23 ` Nishanth Aravamudan [this message]
-- strict thread matches above, loose matches on Subject: below --
2004-09-02 6:50 Margit Schubert-While
2004-09-02 8:24 ` [Kernel-janitors] " maximilian attems
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20040902162301.GB1944@us.ibm.com \
--to=nacc@us.ibm.com \
--cc=kernel-janitors@osdl.org \
--cc=margitsw@t-online.de \
--cc=netdev@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).