Re: [PATCH 2.6.9-rc2 17/38] net/islpci_dev: replace schedule_timeout() with msleep()

Re: [PATCH 2.6.9-rc2 17/38] net/islpci_dev: replace schedule_timeout() with msleep()

[Luis R. Rodriguez]

[-- Attachment #1.1: Type: text/plain, Size: 1619 bytes --]

On Thu, Sep 23, 2004 at 03:13:03PM -0700, Nishanth Aravamudan wrote:
> Any comments would be appreciated.
> 
> Description: Use msleep() instead of schedule_timeout()
> to guarantee the task delays as expected. Also set_current_state() is
> inserted before schedule_timeout(). If the for-loop were to execute
> twice, the second time would not set the state before sleeping in the
> current code; this causes schedule_timeout() to return immediately.
> 
> Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> 
> --- 2.6.9-rc2-vanilla/drivers/net/wireless/prism54/islpci_dev.c	2004-09-13 17:15:41.000000000 -0700
> +++ 2.6.9-rc2/drivers/net/wireless/prism54/islpci_dev.c	2004-09-23 13:58:42.000000000 -0700
> @@ -436,8 +436,7 @@ prism54_bring_down(islpci_private *priv)
>  	wmb();
>  
>  	/* wait a while for the device to reset */
> -	set_current_state(TASK_UNINTERRUPTIBLE);
> -	schedule_timeout(50*HZ/1000);
> +	msleep(50);
>  
>  	return 0;
>  }
> @@ -489,6 +488,7 @@ islpci_reset_if(islpci_private *priv)
>  		/* The software reset acknowledge needs about 220 msec here.
>  		 * Be conservative and wait for up to one second. */
>  	
> +		set_current_state(TASK_UNINTERRUPTIBLE);
>  		remaining = schedule_timeout(HZ);
>  
>  		if(remaining > 0) {


Looks good to me. IIRC Margit had something to say about this last time
this popped around -- CC'ing her to see if there are any outstanding
comments.

PS. For future prism54 patches please feel free to CC prism54-devel and
netdev.

	Luis

-- 
GnuPG Key fingerprint = 113F B290 C6D2 0251 4D84  A34A 6ADD 4937 E20A 525E

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 151 bytes --]

_______________________________________________
Prism54-devel mailing list
Prism54-devel@prism54.org
http://prism54.org/mailman/listinfo/prism54-devel

Re: [PATCH 2.6.9-rc2 17/38] net/islpci_dev: replace schedule_timeout() with msleep()

[Margit Schubert-While]

Hi Nish,
At 18:55 23.09.2004 -0400, Luis scribeth:
>On Thu, Sep 23, 2004 at 03:13:03PM -0700, Nishanth Aravamudan wrote:
> > Any comments would be appreciated.
> >
> > Description: Use msleep() instead of schedule_timeout()
> > to guarantee the task delays as expected. Also set_current_state() is
> > inserted before schedule_timeout(). If the for-loop were to execute
> > twice, the second time would not set the state before sleeping in the
> > current code; this causes schedule_timeout() to return immediately.
> >
> > Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> >
> > --- 
> 2.6.9-rc2-vanilla/drivers/net/wireless/prism54/islpci_dev.c 
> 2004-09-13 17:15:41.000000000 -0700
> > +++ 
> 2.6.9-rc2/drivers/net/wireless/prism54/islpci_dev.c       2004-09-23 
> 13:58:42.000000000 -0700
> > @@ -436,8 +436,7 @@ prism54_bring_down(islpci_private *priv)
> >       wmb();
> >
> >       /* wait a while for the device to reset */
> > -     set_current_state(TASK_UNINTERRUPTIBLE);
> > -     schedule_timeout(50*HZ/1000);
> > +     msleep(50);
> >
> >       return 0;
> >  }
> > @@ -489,6 +488,7 @@ islpci_reset_if(islpci_private *priv)
> >               /* The software reset acknowledge needs about 220 msec here.
> >               * Be conservative and wait for up to one second. */
> >
> > +             set_current_state(TASK_UNINTERRUPTIBLE);
> >               remaining = schedule_timeout(HZ);
> >
> >               if(remaining > 0) {
>
>
>Looks good to me. IIRC Margit had something to say about this last time
>this popped around -- CC'ing her to see if there are any outstanding
>comments.

You bet she has.

The patch has wrong line numbers. Doesn't take into account the stacked up
netdev changes. (Therefore CC'ing Jeff Garzik)

This breaks 2.4 compatibility.
So either backport to 2.4 or Nish can take over prism54 2.4 maintenance ;-)

Don't say a backport is not possible/reasonable, it happened with 
netdev_priv().
(In 2.4.27; At least there, we have HAVE_NETDEV_PRIV).

If this is going to be forced, can we at least have a
define HAVE_MSLEEP in delay.h ?

I am somewhat confused by the second part of the patch.
What has that got to do with msleep ?
Actually, the fix would appear to be correct, but that is a seperate issue
and nothing to do with msleep.
(Prims54 developers -> I'll take a look over the weekend)

I am sceptical about the whole msleep patchset as, by their own admission,
the janitors have/can not (no hardware) test the majority of the changes.
Even more worrying is that incorrect code has directly appeared in
mainline kernel BK.

Margit

Re: [Kernel-janitors] [PATCH 2.6.9-rc2 17/38] net/islpci_dev: replace schedule_timeout() with msleep()

[maximilian attems]

On Fri, 24 Sep 2004, Margit Schubert-While wrote:
..
> The patch has wrong line numbers. Doesn't take into account the stacked up
> netdev changes. (Therefore CC'ing Jeff Garzik)

sure the kj patches are against mainline.
 
> This breaks 2.4 compatibility.
> So either backport to 2.4 or Nish can take over prism54 2.4 maintenance ;-)

can't remember the last time when Randy submitted janitorial patches to
2.4.x, but it's long ago.  2.4 is in maintenance mode.
 
> Don't say a backport is not possible/reasonable, it happened with 
> netdev_priv().
> (In 2.4.27; At least there, we have HAVE_NETDEV_PRIV).

there must have been serious reasons for that.
 
> If this is going to be forced, can we at least have a
> define HAVE_MSLEEP in delay.h ?
> 
> I am somewhat confused by the second part of the patch.
> What has that got to do with msleep ?

basically a lot, because as prism54 lots of drivers forgo/et to set
there state when calling schedule_timeout().

> Actually, the fix would appear to be correct, but that is a seperate issue
> and nothing to do with msleep.
> (Prims54 developers -> I'll take a look over the weekend)

great, please also remove the unused TRACE macro.
(patch was sent to netdev on 3. Sept).
the kj mailing list got another submission to correct 
the __FUNCTION__ use their. :)
 
> I am sceptical about the whole msleep patchset as, by their own admission,
> the janitors have/can not (no hardware) test the majority of the changes.
> Even more worrying is that incorrect code has directly appeared in
> mainline kernel BK.

the named small errors were quickly corrected.
it's up to the driver MAINTAINER to prove us wrong.
we got lots of ack in between.

--
maks
kernel janitor  	http://janitor.kernelnewbies.org/

Re: [PATCH 2.6.9-rc2 17/38] net/islpci_dev: replace schedule_timeout() with msleep()

[Jeff Garzik]

I would rather see an msleep implementation in 2.4.x...

Re: [PATCH 2.6.9-rc2 17/38] net/islpci_dev: replace schedule_timeout() with msleep()

[Margit Schubert-While]

At 00:15 01.10.2004 -0400, Jeff scribeth:
>I would rather see an msleep implementation in 2.4.x...

Hear, hear.
And can we PLEASE have a
"#define HAVE_MSLEEP"  in delay.h ?!!!
(2.4. AND 2.6)

Margit

Re: [Kernel-janitors] Re: [PATCH 2.6.9-rc2 17/38] net/islpci_dev: replace schedule_timeout() with msleep()

[Greg KH]

On Fri, Oct 01, 2004 at 09:04:27AM +0200, Margit Schubert-While wrote:
> At 00:15 01.10.2004 -0400, Jeff scribeth:
> >I would rather see an msleep implementation in 2.4.x...
> 
> Hear, hear.
> And can we PLEASE have a
> "#define HAVE_MSLEEP"  in delay.h ?!!!
> (2.4. AND 2.6)

No, this is not how the kernel source tree works, sorry.

greg k-h

Re: [Kernel-janitors] Re: [PATCH 2.6.9-rc2 17/38] net/islpci_dev: replace schedule_timeout() with msleep()

[Margit Schubert-While]

At 09:55 01.10.2004 -0700, Greg scribeth:
>On Fri, Oct 01, 2004 at 09:04:27AM +0200, Margit Schubert-While wrote:
> > At 00:15 01.10.2004 -0400, Jeff scribeth:
> > >I would rather see an msleep implementation in 2.4.x...
> >
> > Hear, hear.
> > And can we PLEASE have a
> > "#define HAVE_MSLEEP"  in delay.h ?!!!
> > (2.4. AND 2.6)
>
>No, this is not how the kernel source tree works, sorry.

Well it happened with netdev_priv  ;-)
(HAVE_NETDEV_PRIV)

Margit

Re: [Kernel-janitors] Re: [PATCH 2.6.9-rc2 17/38] net/islpci_dev: replace schedule_timeout() with msleep()

[Jeff Garzik]

Margit Schubert-While wrote:
> At 09:55 01.10.2004 -0700, Greg scribeth:
> 
>> On Fri, Oct 01, 2004 at 09:04:27AM +0200, Margit Schubert-While wrote:
>> > At 00:15 01.10.2004 -0400, Jeff scribeth:
>> > >I would rather see an msleep implementation in 2.4.x...
>> >
>> > Hear, hear.
>> > And can we PLEASE have a
>> > "#define HAVE_MSLEEP"  in delay.h ?!!!
>> > (2.4. AND 2.6)
>>
>> No, this is not how the kernel source tree works, sorry.
> 
> 
> Well it happened with netdev_priv  ;-)
> (HAVE_NETDEV_PRIV)


Net driver API is intentionally more friendly to cross-version kernel 
compat than the kernel as a whole ;-)

	Jeff

[KJ] [patch 2.4] back port msleep(), msleep_interruptible()

[maximilian attems]

[-- Attachment #1: Type: text/plain, Size: 4286 bytes --]

On Fri, 01 Oct 2004, Jeff Garzik wrote:

> I would rather see an msleep implementation in 2.4.x...

thanks for Domen Puncer at helping out.

msleep() and msleep_interruptible() as found in current 2.6 to 2.4.
therefor also adds the helper functions ssleep(), jiffies_to_msecs(),
jiffies_to_usecs(), msecs_to_jiffies().

compile and boot tested.


Signed-off-by: Maximilian Attems <janitor@sternwelten.at>


---

 include/linux/delay.h |    8 ++++++
 include/linux/time.h  |   41 +++++++++++++++++++++++++++++++++
 kernel/Makefile       |    3 +-
 kernel/timer.c        |   33 ++++++++++++++++++++++++++
 4 files changed, 84 insertions(+), 1 deletion(-)

diff -puN kernel/Makefile~add-msleep-2.4 kernel/Makefile
--- a/kernel/Makefile~add-msleep-2.4	2004-10-30 22:48:46.000000000 +0200
+++ b/kernel/Makefile	2004-10-30 22:50:45.000000000 +0200
@@ -9,7 +9,8 @@
 
 O_TARGET := kernel.o
 
-export-objs = signal.o sys.o kmod.o context.o ksyms.o pm.o exec_domain.o printk.o
+export-objs   = signal.o sys.o kmod.o context.o ksyms.o pm.o exec_domain.o \
+		printk.o timer.o
 
 obj-y     = sched.o dma.o fork.o exec_domain.o panic.o printk.o \
 	    module.o exit.o itimer.o info.o time.o softirq.o resource.o \
diff -puN kernel/timer.c~add-msleep-2.4 kernel/timer.c
--- a/kernel/timer.c~add-msleep-2.4	2004-10-30 22:48:46.000000000 +0200
+++ b/kernel/timer.c	2004-10-30 22:50:09.000000000 +0200
@@ -22,6 +22,7 @@
 #include <linux/smp_lock.h>
 #include <linux/interrupt.h>
 #include <linux/kernel_stat.h>
+#include <linux/module.h>
 
 #include <asm/uaccess.h>
 
@@ -874,3 +875,35 @@ asmlinkage long sys_nanosleep(struct tim
 	return 0;
 }
 
+/**
+ * msleep - sleep safely even with waitqueue interruptions
+ * @msecs: Time in milliseconds to sleep for
+ */
+void msleep(unsigned int msecs)
+{
+	unsigned long timeout = msecs_to_jiffies(msecs) + 1;
+
+	while (timeout) {
+		set_current_state(TASK_UNINTERRUPTIBLE);
+		timeout = schedule_timeout(timeout);
+	}
+}
+
+EXPORT_SYMBOL(msleep);
+
+/**
+ * msleep_interruptible - sleep waiting for waitqueue interruptions
+ * @msecs: Time in milliseconds to sleep for
+ */
+unsigned long msleep_interruptible(unsigned int msecs)
+{
+	unsigned long timeout = msecs_to_jiffies(msecs) + 1;
+
+	while (timeout && !signal_pending(current)) {
+		set_current_state(TASK_INTERRUPTIBLE);
+		timeout = schedule_timeout(timeout);
+	}
+	return jiffies_to_msecs(timeout);
+}
+
+EXPORT_SYMBOL(msleep_interruptible);
diff -puN include/linux/delay.h~add-msleep-2.4 include/linux/delay.h
--- a/include/linux/delay.h~add-msleep-2.4	2004-10-30 22:48:46.000000000 +0200
+++ b/include/linux/delay.h	2004-10-30 22:48:46.000000000 +0200
@@ -34,4 +34,12 @@ extern unsigned long loops_per_jiffy;
 	({unsigned long msec=(n); while (msec--) udelay(1000);}))
 #endif
 
+void msleep(unsigned int msecs);
+unsigned long msleep_interruptible(unsigned int msecs);
+
+static inline void ssleep(unsigned int seconds)
+{
+	msleep(seconds * 1000);
+}
+
 #endif /* defined(_LINUX_DELAY_H) */
diff -puN include/linux/time.h~add-msleep-2.4 include/linux/time.h
--- a/include/linux/time.h~add-msleep-2.4	2004-10-30 22:48:46.000000000 +0200
+++ b/include/linux/time.h	2004-10-30 22:48:46.000000000 +0200
@@ -126,4 +126,45 @@ struct	itimerval {
 	struct	timeval it_value;	/* current value */
 };
 
+/*
+ * Convert jiffies to milliseconds and back.
+ *
+ * Avoid unnecessary multiplications/divisions in the
+ * two most common HZ cases:
+ */
+static inline unsigned int jiffies_to_msecs(const unsigned long j)
+{
+#if HZ <= 1000 && !(1000 % HZ)
+	return (1000 / HZ) * j;
+#elif HZ > 1000 && !(HZ % 1000)
+	return (j + (HZ / 1000) - 1)/(HZ / 1000);
+#else
+	return (j * 1000) / HZ;
+#endif
+}
+
+static inline unsigned int jiffies_to_usecs(const unsigned long j)
+{
+#if HZ <= 1000 && !(1000 % HZ)
+	return (1000000 / HZ) * j;
+#elif HZ > 1000 && !(HZ % 1000)
+	return (j*1000 + (HZ - 1000))/(HZ / 1000);
+#else
+	return (j * 1000000) / HZ;
+#endif
+}
+
+static inline unsigned long msecs_to_jiffies(const unsigned int m)
+{
+	if (m > jiffies_to_msecs(MAX_JIFFY_OFFSET))
+		return MAX_JIFFY_OFFSET;
+#if HZ <= 1000 && !(1000 % HZ)
+	return (m + (1000 / HZ) - 1) / (1000 / HZ);
+#elif HZ > 1000 && !(HZ % 1000)
+	return m * (HZ / 1000);
+#else
+	return (m * HZ + 999) / 1000;
+#endif
+}
+
 #endif



[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [patch 2.4] back port msleep(), msleep_interruptible()

[Jeff Garzik]

maximilian attems wrote:
> diff -puN include/linux/delay.h~add-msleep-2.4 include/linux/delay.h
> --- a/include/linux/delay.h~add-msleep-2.4	2004-10-30 22:48:46.000000000 +0200
> +++ b/include/linux/delay.h	2004-10-30 22:48:46.000000000 +0200
> @@ -34,4 +34,12 @@ extern unsigned long loops_per_jiffy;
>  	({unsigned long msec=(n); while (msec--) udelay(1000);}))
>  #endif
>  
> +void msleep(unsigned int msecs);
> +unsigned long msleep_interruptible(unsigned int msecs);
> +
> +static inline void ssleep(unsigned int seconds)
[...]
> +static inline unsigned int jiffies_to_msecs(const unsigned long j)

> +static inline unsigned int jiffies_to_usecs(const unsigned long j)

> +static inline unsigned long msecs_to_jiffies(const unsigned int m)


I'm pretty sure more than one of these symbols clashes with a symbol 
defined locally in a driver.  I like the patch but we can't apply it 
until the impact on existing code is evaluated.

	Jeff

Re: [KJ] Re: [patch 2.4] back port msleep(), msleep_interruptible()

[Nish Aravamudan]

On Sat, 30 Oct 2004 18:41:10 -0400, Jeff Garzik <jgarzik@pobox.com> wrote:
> maximilian attems wrote:
> > diff -puN include/linux/delay.h~add-msleep-2.4 include/linux/delay.h
> > --- a/include/linux/delay.h~add-msleep-2.4    2004-10-30 22:48:46.000000000 +0200
> > +++ b/include/linux/delay.h   2004-10-30 22:48:46.000000000 +0200
> > @@ -34,4 +34,12 @@ extern unsigned long loops_per_jiffy;
> >       ({unsigned long msec=(n); while (msec--) udelay(1000);}))
> >  #endif
> >
> > +void msleep(unsigned int msecs);
> > +unsigned long msleep_interruptible(unsigned int msecs);
> > +
> > +static inline void ssleep(unsigned int seconds)
> [...]
> > +static inline unsigned int jiffies_to_msecs(const unsigned long j)
> 
> > +static inline unsigned int jiffies_to_usecs(const unsigned long j)
> 
> > +static inline unsigned long msecs_to_jiffies(const unsigned int m)
> 
> 
> I'm pretty sure more than one of these symbols clashes with a symbol
> defined locally in a driver.  I like the patch but we can't apply it
> until the impact on existing code is evaluated.

More than likely much of the code cleanup that was done before I began
my patches, like removing custom msleep()s from drivers will need to
be done again, as Jeff points out.

-Nish

Re: [KJ] Re: [patch 2.4] back port msleep(), msleep_interruptible()

[Nish Aravamudan]

On Sat, 30 Oct 2004 18:41:10 -0400, Jeff Garzik <jgarzik@pobox.com> wrote:
> maximilian attems wrote:
> > diff -puN include/linux/delay.h~add-msleep-2.4 include/linux/delay.h
> > --- a/include/linux/delay.h~add-msleep-2.4    2004-10-30 22:48:46.000000000 +0200
> > +++ b/include/linux/delay.h   2004-10-30 22:48:46.000000000 +0200
> > @@ -34,4 +34,12 @@ extern unsigned long loops_per_jiffy;
> >       ({unsigned long msec=(n); while (msec--) udelay(1000);}))
> >  #endif
> >
> > +void msleep(unsigned int msecs);
> > +unsigned long msleep_interruptible(unsigned int msecs);
> > +
> > +static inline void ssleep(unsigned int seconds)
> [...]
> > +static inline unsigned int jiffies_to_msecs(const unsigned long j)
> 
> > +static inline unsigned int jiffies_to_usecs(const unsigned long j)
> 
> > +static inline unsigned long msecs_to_jiffies(const unsigned int m)
> 
> 
> I'm pretty sure more than one of these symbols clashes with a symbol
> defined locally in a driver.  I like the patch but we can't apply it
> until the impact on existing code is evaluated.

More than likely much of the code cleanup that was done before I began
my patches, like removing custom msleep()s from drivers will need to
be done again, as Jeff points out.

-Nish

Re: [KJ] Re: [patch 2.4] back port msleep(), msleep_interruptible()

[maximilian attems]

On Sat, 30 Oct 2004, Nish Aravamudan wrote:

> On Sat, 30 Oct 2004 18:41:10 -0400, Jeff Garzik <jgarzik@pobox.com> wrote:
> > maximilian attems wrote:
> > > diff -puN include/linux/delay.h~add-msleep-2.4 include/linux/delay.h
> > > --- a/include/linux/delay.h~add-msleep-2.4    2004-10-30 22:48:46.000000000 +0200
> > > +++ b/include/linux/delay.h   2004-10-30 22:48:46.000000000 +0200
> > > @@ -34,4 +34,12 @@ extern unsigned long loops_per_jiffy;
> > >       ({unsigned long msec=(n); while (msec--) udelay(1000);}))
> > >  #endif
> > >
> > > +void msleep(unsigned int msecs);
> > > +unsigned long msleep_interruptible(unsigned int msecs);
> > > +
> > > +static inline void ssleep(unsigned int seconds)
> > [...]
> > > +static inline unsigned int jiffies_to_msecs(const unsigned long j)
> > 
> > > +static inline unsigned int jiffies_to_usecs(const unsigned long j)
> > 
> > > +static inline unsigned long msecs_to_jiffies(const unsigned int m)
> > 
> > 
> > I'm pretty sure more than one of these symbols clashes with a symbol
> > defined locally in a driver.  I like the patch but we can't apply it
> > until the impact on existing code is evaluated.
> 
> More than likely much of the code cleanup that was done before I began
> my patches, like removing custom msleep()s from drivers will need to
> be done again, as Jeff points out.
> 
> -Nish

thanks Jeff for your quick response,
ooh i seee libata is defining an msleep().
so there seems to be need for it.

ok we'll come up with tougher patchset.

--
maks
kernel janitor  	http://janitor.kernelnewbies.org/

Re: [KJ] Re: [patch 2.4] back port msleep(), msleep_interruptible()

[maximilian attems]

On Sat, 30 Oct 2004, Nish Aravamudan wrote:

> On Sat, 30 Oct 2004 18:41:10 -0400, Jeff Garzik <jgarzik@pobox.com> wrote:
> > maximilian attems wrote:
> > > diff -puN include/linux/delay.h~add-msleep-2.4 include/linux/delay.h
> > > --- a/include/linux/delay.h~add-msleep-2.4    2004-10-30 22:48:46.000000000 +0200
> > > +++ b/include/linux/delay.h   2004-10-30 22:48:46.000000000 +0200
> > > @@ -34,4 +34,12 @@ extern unsigned long loops_per_jiffy;
> > >       ({unsigned long msec=(n); while (msec--) udelay(1000);}))
> > >  #endif
> > >
> > > +void msleep(unsigned int msecs);
> > > +unsigned long msleep_interruptible(unsigned int msecs);
> > > +
> > > +static inline void ssleep(unsigned int seconds)
> > [...]
> > > +static inline unsigned int jiffies_to_msecs(const unsigned long j)
> > 
> > > +static inline unsigned int jiffies_to_usecs(const unsigned long j)
> > 
> > > +static inline unsigned long msecs_to_jiffies(const unsigned int m)
> > 
> > 
> > I'm pretty sure more than one of these symbols clashes with a symbol
> > defined locally in a driver.  I like the patch but we can't apply it
> > until the impact on existing code is evaluated.
> 
> More than likely much of the code cleanup that was done before I began
> my patches, like removing custom msleep()s from drivers will need to
> be done again, as Jeff points out.
> 
> -Nish

thanks Jeff for your quick response,
ooh i seee libata is defining an msleep().
so there seems to be need for it.

ok we'll come up with tougher patchset.

--
maks
kernel janitor  	http://janitor.kernelnewbies.org/

Re: [patch 2.4] back port msleep(), msleep_interruptible()

[maximilian attems]

On Sat, 30 Oct 2004, Jeff Garzik wrote:

> I'm pretty sure more than one of these symbols clashes with a symbol 
> defined locally in a driver.  I like the patch but we can't apply it 
> until the impact on existing code is evaluated.
> 
> 	Jeff

current 2.4 has no ssleep() nor jiffies_to_usecs() nor jiffies_to_msecs()
users. so no namespace conflicts on them.

i found a strange unsupported "msleep" syscall in
./arch/parisc/hpux/sys_hpux.c

left this one appart, i resend the msleep patch + 5 cleanup patches.
they remove duplicate msleep() or msecs_to_jiffies() definitions.
they are all compile tested, but the one touching drivers/char/shwdt.c
please show me if i forgot something.

--
maks
kernel janitor  	http://janitor.kernelnewbies.org/


ps dropped  prism54-devel@prism54.org from cc as this ml rejects my mails.

Re: [patch 2.4] back port msleep(), msleep_interruptible()

[maximilian attems]

On Sat, 30 Oct 2004, Jeff Garzik wrote:

> I'm pretty sure more than one of these symbols clashes with a symbol 
> defined locally in a driver.  I like the patch but we can't apply it 
> until the impact on existing code is evaluated.
> 
> 	Jeff

current 2.4 has no ssleep() nor jiffies_to_usecs() nor jiffies_to_msecs()
users. so no namespace conflicts on them.

i found a strange unsupported "msleep" syscall in
./arch/parisc/hpux/sys_hpux.c

left this one appart, i resend the msleep patch + 5 cleanup patches.
they remove duplicate msleep() or msecs_to_jiffies() definitions.
they are all compile tested, but the one touching drivers/char/shwdt.c
please show me if i forgot something.

--
maks
kernel janitor  	http://janitor.kernelnewbies.org/


ps dropped  prism54-devel@prism54.org from cc as this ml rejects my mails.

[KJ] [patch 1/6] back port msleep(), msleep_interruptible()

[maximilian attems]

[-- Attachment #1: Type: text/plain, Size: 4536 bytes --]


Backport suggested by prism54 folks. idea acked by Jeff.
thanks for Domen Puncer at helping out.

Belows patch adds msleep() and msleep_interruptible() as found
in current 2.6 to 2.4.
therefor adds the helper functions ssleep(), jiffies_to_msecs(),
jiffies_to_usecs(), msecs_to_jiffies().

The namespace clashes for msleep() and msecs_to_jiffies()
are cleanup by the next 5 patches.

Signed-off-by: Maximilian Attems <janitor@sternwelten.at>


---

 linux-2.4.28-rc1-max/include/linux/delay.h |    8 +++++
 linux-2.4.28-rc1-max/include/linux/time.h  |   41 +++++++++++++++++++++++++++++
 linux-2.4.28-rc1-max/kernel/Makefile       |    3 +-
 linux-2.4.28-rc1-max/kernel/timer.c        |   33 +++++++++++++++++++++++
 4 files changed, 84 insertions(+), 1 deletion(-)

diff -puN kernel/Makefile~add-msleep-2.4 kernel/Makefile
--- linux-2.4.28-rc1/kernel/Makefile~add-msleep-2.4	2004-10-30 22:48:46.000000000 +0200
+++ linux-2.4.28-rc1-max/kernel/Makefile	2004-10-30 22:50:45.000000000 +0200
@@ -9,7 +9,8 @@
 
 O_TARGET := kernel.o
 
-export-objs = signal.o sys.o kmod.o context.o ksyms.o pm.o exec_domain.o printk.o
+export-objs   = signal.o sys.o kmod.o context.o ksyms.o pm.o exec_domain.o \
+		printk.o timer.o
 
 obj-y     = sched.o dma.o fork.o exec_domain.o panic.o printk.o \
 	    module.o exit.o itimer.o info.o time.o softirq.o resource.o \
diff -puN kernel/timer.c~add-msleep-2.4 kernel/timer.c
--- linux-2.4.28-rc1/kernel/timer.c~add-msleep-2.4	2004-10-30 22:48:46.000000000 +0200
+++ linux-2.4.28-rc1-max/kernel/timer.c	2004-10-30 22:50:09.000000000 +0200
@@ -22,6 +22,7 @@
 #include <linux/smp_lock.h>
 #include <linux/interrupt.h>
 #include <linux/kernel_stat.h>
+#include <linux/module.h>
 
 #include <asm/uaccess.h>
 
@@ -874,3 +875,35 @@ asmlinkage long sys_nanosleep(struct tim
 	return 0;
 }
 
+/**
+ * msleep - sleep safely even with waitqueue interruptions
+ * @msecs: Time in milliseconds to sleep for
+ */
+void msleep(unsigned int msecs)
+{
+	unsigned long timeout = msecs_to_jiffies(msecs) + 1;
+
+	while (timeout) {
+		set_current_state(TASK_UNINTERRUPTIBLE);
+		timeout = schedule_timeout(timeout);
+	}
+}
+
+EXPORT_SYMBOL(msleep);
+
+/**
+ * msleep_interruptible - sleep waiting for waitqueue interruptions
+ * @msecs: Time in milliseconds to sleep for
+ */
+unsigned long msleep_interruptible(unsigned int msecs)
+{
+	unsigned long timeout = msecs_to_jiffies(msecs) + 1;
+
+	while (timeout && !signal_pending(current)) {
+		set_current_state(TASK_INTERRUPTIBLE);
+		timeout = schedule_timeout(timeout);
+	}
+	return jiffies_to_msecs(timeout);
+}
+
+EXPORT_SYMBOL(msleep_interruptible);
diff -puN include/linux/delay.h~add-msleep-2.4 include/linux/delay.h
--- linux-2.4.28-rc1/include/linux/delay.h~add-msleep-2.4	2004-10-30 22:48:46.000000000 +0200
+++ linux-2.4.28-rc1-max/include/linux/delay.h	2004-10-30 22:48:46.000000000 +0200
@@ -34,4 +34,12 @@ extern unsigned long loops_per_jiffy;
 	({unsigned long msec=(n); while (msec--) udelay(1000);}))
 #endif
 
+void msleep(unsigned int msecs);
+unsigned long msleep_interruptible(unsigned int msecs);
+
+static inline void ssleep(unsigned int seconds)
+{
+	msleep(seconds * 1000);
+}
+
 #endif /* defined(_LINUX_DELAY_H) */
diff -puN include/linux/time.h~add-msleep-2.4 include/linux/time.h
--- linux-2.4.28-rc1/include/linux/time.h~add-msleep-2.4	2004-10-30 22:48:46.000000000 +0200
+++ linux-2.4.28-rc1-max/include/linux/time.h	2004-10-30 22:57:44.000000000 +0200
@@ -126,4 +126,45 @@ struct	itimerval {
 	struct	timeval it_value;	/* current value */
 };
 
+/*
+ * Convert jiffies to milliseconds and back.
+ *
+ * Avoid unnecessary multiplications/divisions in the
+ * two most common HZ cases:
+ */
+static inline unsigned int jiffies_to_msecs(const unsigned long j)
+{
+#if HZ <= 1000 && !(1000 % HZ)
+	return (1000 / HZ) * j;
+#elif HZ > 1000 && !(HZ % 1000)
+	return (j + (HZ / 1000) - 1)/(HZ / 1000);
+#else
+	return (j * 1000) / HZ;
+#endif
+}
+
+static inline unsigned int jiffies_to_usecs(const unsigned long j)
+{
+#if HZ <= 1000 && !(1000 % HZ)
+	return (1000000 / HZ) * j;
+#elif HZ > 1000 && !(HZ % 1000)
+	return (j*1000 + (HZ - 1000))/(HZ / 1000);
+#else
+	return (j * 1000000) / HZ;
+#endif
+}
+
+static inline unsigned long msecs_to_jiffies(const unsigned int m)
+{
+	if (m > jiffies_to_msecs(MAX_JIFFY_OFFSET))
+		return MAX_JIFFY_OFFSET;
+#if HZ <= 1000 && !(1000 % HZ)
+	return (m + (1000 / HZ) - 1) / (1000 / HZ);
+#elif HZ > 1000 && !(HZ % 1000)
+	return m * (HZ / 1000);
+#else
+	return (m * HZ + 999) / 1000;
+#endif
+}
+
 #endif
_

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

[patch 2/6] libata remove duplicate definition msecs_to_jiffies()

[maximilian attems]

remove duplicate definition of msecs_to_jiffies().
already includes delay.h

Signed-off-by: Maximilian Attems <janitor@sternwelten.at>


---

 linux-2.4.28-rc1-max/include/linux/libata.h |    5 -----
 1 files changed, 5 deletions(-)

diff -puN include/linux/libata.h~remove-msecs_to_jiffies-libata.h include/linux/libata.h
--- linux-2.4.28-rc1/include/linux/libata.h~remove-msecs_to_jiffies-libata.h	2004-10-31 13:33:52.000000000 +0100
+++ linux-2.4.28-rc1-max/include/linux/libata.h	2004-10-31 13:36:16.000000000 +0100
@@ -419,11 +419,6 @@ extern int ata_std_bios_param(Disk * dis
 extern void libata_msleep(unsigned long msecs);
 
 
-static inline unsigned long msecs_to_jiffies(unsigned long msecs)
-{
-	return ((HZ * msecs + 999) / 1000);
-}
-
 static inline unsigned int ata_tag_valid(unsigned int tag)
 {
 	return (tag < ATA_MAX_QUEUE) ? 1 : 0;
_

[patch 3/6] sx8 remove duplicate definition msleep(), msecs_to_jiffies()

[maximilian attems]

remove duplicate definition msleep(), msecs_to_jiffies().
delay.h already included.

Signed-off-by: Maximilian Attems <janitor@sternwelten.at>


---

 linux-2.4.28-rc1-max/drivers/block/sx8.c |   11 -----------
 1 files changed, 11 deletions(-)

diff -puN drivers/block/sx8.c~remove-msleep+msecs_to_jiffies-drivers_block_sx8 drivers/block/sx8.c
--- linux-2.4.28-rc1/drivers/block/sx8.c~remove-msleep+msecs_to_jiffies-drivers_block_sx8	2004-10-31 13:37:56.000000000 +0100
+++ linux-2.4.28-rc1-max/drivers/block/sx8.c	2004-10-31 13:38:56.000000000 +0100
@@ -548,17 +548,6 @@ static int carm_bdev_ioctl(struct inode 
 	return -EOPNOTSUPP;
 }
 
-static inline unsigned long msecs_to_jiffies(unsigned long msecs)
-{
-	return ((HZ * msecs + 999) / 1000);
-}
-
-static void msleep(unsigned long msecs)
-{
-	set_current_state(TASK_UNINTERRUPTIBLE);
-	schedule_timeout(msecs_to_jiffies(msecs) + 1);
-}
-
 static const u32 msg_sizes[] = { 32, 64, 128, CARM_MSG_SIZE };
 
 static inline int carm_lookup_bucket(u32 msg_size)
_

[patch 4/6] char/shwdt remove duplicate msecs_to_jiffies()

[maximilian attems]

remove duplicate msecs_to_jiffies() definition.
add include <delay.h>.

Signed-off-by: Maximilian Attems <janitor@sternwelten.at>


---

 linux-2.4.28-rc1-max/drivers/char/shwdt.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

diff -puN drivers/char/shwdt.c~remove-msecs_to_jiffies-drivers_char_shwdt drivers/char/shwdt.c
--- linux-2.4.28-rc1/drivers/char/shwdt.c~remove-msecs_to_jiffies-drivers_char_shwdt	2004-10-31 13:40:49.000000000 +0100
+++ linux-2.4.28-rc1-max/drivers/char/shwdt.c	2004-10-31 13:42:05.000000000 +0100
@@ -27,6 +27,7 @@
 #include <linux/reboot.h>
 #include <linux/notifier.h>
 #include <linux/ioport.h>
+#include <linux/delay.h>
 
 #include <asm/io.h>
 #include <asm/uaccess.h>
@@ -113,7 +114,6 @@
  */
 static int clock_division_ratio = WTCSR_CKS_4096;
 
-#define msecs_to_jiffies(msecs)	(jiffies + (HZ * msecs + 9999) / 10000)
 #define next_ping_period(cks)	msecs_to_jiffies(cks - 4)
 
 static unsigned long shwdt_is_open;
_

[patch 5/6] sata_promise remove duplicate msleep() definition

[maximilian attems]

remove now duplicate define.
driver already includes delay.

Signed-off-by: Maximilian Attems <janitor@sternwelten.at>


---

 linux-2.4.28-rc1-max/drivers/scsi/sata_promise.c |    2 --
 1 files changed, 2 deletions(-)

diff -puN drivers/scsi/sata_promise.c~remove-msleep-drivers_scsi_sata_promise drivers/scsi/sata_promise.c
--- linux-2.4.28-rc1/drivers/scsi/sata_promise.c~remove-msleep-drivers_scsi_sata_promise	2004-10-31 13:59:39.000000000 +0100
+++ linux-2.4.28-rc1-max/drivers/scsi/sata_promise.c	2004-10-31 14:00:12.000000000 +0100
@@ -42,8 +42,6 @@
 #define DRV_NAME	"sata_promise"
 #define DRV_VERSION	"1.00"
 
-#define msleep libata_msleep	/* 2.4-specific */
-
 enum {
 	PDC_PKT_SUBMIT		= 0x40, /* Command packet pointer addr */
 	PDC_INT_SEQMASK		= 0x40,	/* Mask of asserted SEQ INTs */
_

[patch 6/6] libata remove msleep_libata()

[maximilian attems]

remove msleep_libata(), now that msleep() is backported.
also remove duplicate msleep() definition.
delay.h is already included.

Signed-off-by: Maximilian Attems <janitor@sternwelten.at>



---

 linux-2.4.28-rc1-max/drivers/scsi/libata-core.c |   23 -----------------------
 linux-2.4.28-rc1-max/include/linux/libata.h     |    1 -
 2 files changed, 24 deletions(-)

diff -puN drivers/scsi/libata-core.c~remove-libata_msleep drivers/scsi/libata-core.c
--- linux-2.4.28-rc1/drivers/scsi/libata-core.c~remove-libata_msleep	2004-10-31 14:06:49.000000000 +0100
+++ linux-2.4.28-rc1-max/drivers/scsi/libata-core.c	2004-10-31 14:08:41.000000000 +0100
@@ -67,28 +67,6 @@ MODULE_DESCRIPTION("Library module for A
 MODULE_LICENSE("GPL");
 
 /**
- *	msleep - sleep for a number of milliseconds
- *	@msecs: number of milliseconds to sleep
- *
- *	Issues schedule_timeout call for the specified number
- *	of milliseconds.
- *
- *	LOCKING:
- *	None.
- */
-
-static void msleep(unsigned long msecs)
-{
-	set_current_state(TASK_UNINTERRUPTIBLE);
-	schedule_timeout(msecs_to_jiffies(msecs) + 1);
-}
-
-void libata_msleep(unsigned long msecs)
-{
-	msleep(msecs);
-}
-
-/**
  *	ata_tf_load - send taskfile registers to host controller
  *	@ap: Port to which output is sent
  *	@tf: ATA taskfile register set
@@ -3697,7 +3675,6 @@ EXPORT_SYMBOL_GPL(ata_scsi_queuecmd);
 EXPORT_SYMBOL_GPL(ata_scsi_error);
 EXPORT_SYMBOL_GPL(ata_scsi_detect);
 EXPORT_SYMBOL_GPL(ata_add_to_probe_list);
-EXPORT_SYMBOL_GPL(libata_msleep);
 EXPORT_SYMBOL_GPL(ata_scsi_release);
 EXPORT_SYMBOL_GPL(ata_host_intr);
 EXPORT_SYMBOL_GPL(ata_dev_classify);
diff -puN include/linux/libata.h~remove-libata_msleep include/linux/libata.h
--- linux-2.4.28-rc1/include/linux/libata.h~remove-libata_msleep	2004-10-31 14:06:49.000000000 +0100
+++ linux-2.4.28-rc1-max/include/linux/libata.h	2004-10-31 14:09:08.000000000 +0100
@@ -416,7 +416,6 @@ extern void ata_qc_complete(struct ata_q
 extern void ata_eng_timeout(struct ata_port *ap);
 extern void ata_add_to_probe_list (struct ata_probe_ent *probe_ent);
 extern int ata_std_bios_param(Disk * disk, kdev_t dev, int *ip);
-extern void libata_msleep(unsigned long msecs);
 
 
 static inline unsigned int ata_tag_valid(unsigned int tag)
_