[Kernel-janitors] Re: [PATCH 2.6.9-rc2 19/33] char/pcxx: replace

[Kernel-janitors] Re: [PATCH 2.6.9-rc2 19/33] char/pcxx: replace

[Christoph Lameter]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 793 bytes --]

Looks fine to me.

On Thu, 16 Sep 2004, Nishanth Aravamudan wrote:

> Any comments would be appreciated.
>
> Description: Use msleep_interruptible() instead of schedule_timeout() to
> guarantee the task delays as expected.
>
> Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
>
> --- 2.6.9-rc1-mm4-vanilla/drivers/char/pcxx.c	2004-09-09 23:05:41.000000000 -0700
> +++ 2.6.9-rc1-mm4/drivers/char/pcxx.c	2004-09-10 11:10:10.000000000 -0700
> @@ -561,8 +561,7 @@ static void pcxe_close(struct tty_struct
>  #endif
>  		if(info->blocked_open) {
>  			if(info->close_delay) {
> -				current->state = TASK_INTERRUPTIBLE;
> -				schedule_timeout(info->close_delay);
> +				msleep_interruptible(jiffies_to_msecs(info->close_delay));
>  			}
>  			wake_up_interruptible(&info->open_wait);
>  		}
>

[-- 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

[Kernel-janitors] Re: [PATCH 2.6.9-rc2 11/33] char/i830_irq:

[Jon Smirl]

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

schedule_timeout() is also used in DRM_WAIT_ON() in drm_os_linux.h 
Does it need to be adjusted too?

#define DRM_WAIT_ON( ret, queue, timeout, condition )		\
do {								\
	DECLARE_WAITQUEUE(entry, current);			\
	unsigned long end = jiffies + (timeout);		\
	add_wait_queue(&(queue), &entry);			\
								\
	for (;;) {						\
		__set_current_state(TASK_INTERRUPTIBLE);	\
		if (condition)					\
			break;					\
		if (time_after_eq(jiffies, end)) {		\
			ret = -EBUSY;				\
			break;					\
		}						\
		schedule_timeout((HZ/100 > 1) ? HZ/100 : 1);	\
		if (signal_pending(current)) {			\
			ret = -EINTR;				\
			break;					\
		}						\
	}							\
	__set_current_state(TASK_RUNNING);				\
	remove_wait_queue(&(queue), &entry);			\
} while (0)




On Thu, 16 Sep 2004 09:43:49 -0700, Nishanth Aravamudan <nacc@us.ibm.com> wrote:
> Any comments would be appreciated.
> 
> Description: Use msleep_interruptible() instead of schedule_timeout() to
> guarantee the task delays as expected.
> 
> Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> 
> --- 2.6.9-rc1-mm4-vanilla/drivers/char/drm/i830_irq.c   2004-09-09 23:05:37.000000000 -0700
> +++ 2.6.9-rc1-mm4/drivers/char/drm/i830_irq.c   2004-09-10 10:34:30.000000000 -0700
> @@ -105,7 +105,7 @@ int i830_wait_irq(drm_device_t *dev, int
>                         ret = -EBUSY;   /* Lockup?  Missed irq? */
>                         break;
>                 }
> -               schedule_timeout(HZ*3);
> +               msleep_interruptible(3000);
>                 if (signal_pending(current)) {
>                         ret = -EINTR;
>                         break;
> 
> -------------------------------------------------------
> This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
> Project Admins to receive an Apple iPod Mini FREE for your judgement on
> who ports your project to Linux PPC the best. Sponsored by IBM.
> Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
> --
> _______________________________________________
> Dri-devel mailing list
> Dri-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/dri-devel
> 



-- 
Jon Smirl
jonsmirl@gmail.com

[-- 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

[Kernel-janitors] Re: [PATCH 2.6.9-rc2 11/33] char/i830_irq:

[Nishanth Aravamudan]

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

On Fri, Sep 17, 2004 at 12:38:40AM -0400, Jon Smirl wrote:
> schedule_timeout() is also used in DRM_WAIT_ON() in drm_os_linux.h 
> Does it need to be adjusted too?
> 
> #define DRM_WAIT_ON( ret, queue, timeout, condition )		\
> do {								\
> 	DECLARE_WAITQUEUE(entry, current);			\
> 	unsigned long end = jiffies + (timeout);		\
> 	add_wait_queue(&(queue), &entry);			\
> 								\
> 	for (;;) {						\
> 		__set_current_state(TASK_INTERRUPTIBLE);	\
> 		if (condition)					\
> 			break;					\
> 		if (time_after_eq(jiffies, end)) {		\
> 			ret = -EBUSY;				\
> 			break;					\
> 		}						\
> 		schedule_timeout((HZ/100 > 1) ? HZ/100 : 1);	\
> 		if (signal_pending(current)) {			\
> 			ret = -EINTR;				\
> 			break;					\
> 		}						\
> 	}							\
> 	__set_current_state(TASK_RUNNING);				\
> 	remove_wait_queue(&(queue), &entry);			\
> } while (0)

I did notice this other occurrence, but since the timeout is not
necessarily a long one (measurable in msecs), i.e. in system's where
HZ < 100, I didn't want to make the code more complicated than it is.

But, if some of the DRM people who are more familiar with the hardware
are ok with there always being a 10 ms delay (which it would be for all
system's with HZ > 100 anyways), I could make that change.

Thanks,
Nish

[-- 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

[Kernel-janitors] Re: [PATCH 2.6.9-rc2 11/33] char/i830_irq:

[Jon Smirl]

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

Dave, what do you want to do about this one, inline function?
msleep_interuptible is only available on recent kernels.


On Thu, 16 Sep 2004 09:43:49 -0700, Nishanth Aravamudan <nacc@us.ibm.com> wrote:
> Any comments would be appreciated.
> 
> Description: Use msleep_interruptible() instead of schedule_timeout() to
> guarantee the task delays as expected.
> 
> Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> 
> --- 2.6.9-rc1-mm4-vanilla/drivers/char/drm/i830_irq.c   2004-09-09 23:05:37.000000000 -0700
> +++ 2.6.9-rc1-mm4/drivers/char/drm/i830_irq.c   2004-09-10 10:34:30.000000000 -0700
> @@ -105,7 +105,7 @@ int i830_wait_irq(drm_device_t *dev, int
>                         ret = -EBUSY;   /* Lockup?  Missed irq? */
>                         break;
>                 }
> -               schedule_timeout(HZ*3);
> +               msleep_interruptible(3000);
>                 if (signal_pending(current)) {
>                         ret = -EINTR;
>                         break;
> 
> -------------------------------------------------------
> This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
> Project Admins to receive an Apple iPod Mini FREE for your judgement on
> who ports your project to Linux PPC the best. Sponsored by IBM.
> Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
> --
> _______________________________________________
> Dri-devel mailing list
> Dri-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/dri-devel
> 



-- 
Jon Smirl
jonsmirl@gmail.com

[-- 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

[Kernel-janitors] Re: [PATCH 2.6.9-rc2 11/33] char/i830_irq:

[Dave Airlie]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2039 bytes --]


yeah we'll need to implement a msleep_interruptible in drm_compat.h if we
are running on a kernel which doesn't have it ...

drm_os_linux.h also has a schedule_timeout, that i830_wait_irq function
looks a bit bogus to me .. not sure it shouldnt look like some of the
others...

Dave.

On Sat, 18 Sep 2004, Jon Smirl wrote:

> Dave, what do you want to do about this one, inline function?
> msleep_interuptible is only available on recent kernels.
>
>
> On Thu, 16 Sep 2004 09:43:49 -0700, Nishanth Aravamudan <nacc@us.ibm.com> wrote:
> > Any comments would be appreciated.
> >
> > Description: Use msleep_interruptible() instead of schedule_timeout() to
> > guarantee the task delays as expected.
> >
> > Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> >
> > --- 2.6.9-rc1-mm4-vanilla/drivers/char/drm/i830_irq.c   2004-09-09 23:05:37.000000000 -0700
> > +++ 2.6.9-rc1-mm4/drivers/char/drm/i830_irq.c   2004-09-10 10:34:30.000000000 -0700
> > @@ -105,7 +105,7 @@ int i830_wait_irq(drm_device_t *dev, int
> >                         ret = -EBUSY;   /* Lockup?  Missed irq? */
> >                         break;
> >                 }
> > -               schedule_timeout(HZ*3);
> > +               msleep_interruptible(3000);
> >                 if (signal_pending(current)) {
> >                         ret = -EINTR;
> >                         break;
> >
> > -------------------------------------------------------
> > This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
> > Project Admins to receive an Apple iPod Mini FREE for your judgement on
> > who ports your project to Linux PPC the best. Sponsored by IBM.
> > Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
> > --
> > _______________________________________________
> > Dri-devel mailing list
> > Dri-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/dri-devel
> >
>
>
>
>

-- 
David Airlie, Software Engineer
http://www.skynet.ie/~airlied / airlied at skynet.ie
pam_smb / Linux DECstation / Linux VAX / ILUG person


[-- 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

[Kernel-janitors] Re: [PATCH 2.6.9-rc2 17/17] media/zr36120:

[Nishanth Aravamudan]

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

On Mon, Sep 20, 2004 at 03:07:07PM -0700, Nishanth Aravamudan wrote:
> Any comments would be appreciated.
> 
> Description: Use ssleep() instead of schedule_timeout()
> to guarantee the task delays as expected.

Sorry, clearly the patch uses msleep() not ssleep() [Subject line is
correct].

-Nish

[-- 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

[Kernel-janitors] Re: [PATCH 2.6.9-rc2 1/40] net/3c505: replace

[Phil Blundell]

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

Looks good to me.  

Thanks.

p.

On Tue, 2004-09-21 at 14:56 -0700, Nishanth Aravamudan wrote:
> Any comments would be appreciated.
> 
> Description: Use msleep() instead of schedule_timeout()
> to guarantee the task delays as expected.
> 
> Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> 
> --- 2.6.9-rc2-vanilla/drivers/net/3c505.c	2004-09-13 17:15:46.000000000 -0700
> +++ 2.6.9-rc2/drivers/net/3c505.c	2004-09-14 17:07:02.000000000 -0700
> @@ -1327,8 +1327,7 @@ static int __init elp_sense(struct net_d
>  	if (orig_HSR & DIR) {
>  		/* If HCR.DIR is up, we pull it down. HSR.DIR should follow. */
>  		outb(0, dev->base_addr + PORT_CONTROL);
> -		set_current_state(TASK_UNINTERRUPTIBLE);
> -		schedule_timeout(30*HZ/100);
> +		msleep(300);
>  		if (inb_status(addr) & DIR) {
>  			if (elp_debug > 0)
>  				printk(notfound_msg, 2);
> @@ -1337,8 +1336,7 @@ static int __init elp_sense(struct net_d
>  	} else {
>  		/* If HCR.DIR is down, we pull it up. HSR.DIR should follow. */
>  		outb(DIR, dev->base_addr + PORT_CONTROL);
> -		set_current_state(TASK_UNINTERRUPTIBLE);
> -		schedule_timeout(30*HZ/100);
> +		msleep(300);
>  		if (!(inb_status(addr) & DIR)) {
>  			if (elp_debug > 0)
>  				printk(notfound_msg, 3);
> 

[-- 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

[Kernel-janitors] Re: [PATCH 2.6.9-rc2 1/2] i2c/i2c-algo-ite:

[Greg KH]

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

On Thu, Sep 16, 2004 at 03:56:40PM -0700, Nishanth Aravamudan wrote:
> Any comments would be appreciated.
> 
> Description: Removes unused iic_sleep().
> 

This is already in my tree, but for some reason the latest -mm release
doesn't have the bk-i2c tree in it, so you wouldn't have realized that
:)

thanks anyway.

greg k-h

[-- 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

[Kernel-janitors] Re: [PATCH 2.6.9-rc2 1/16] scsi/53c700: replace

[James Bottomley]

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

On Mon, 2004-09-27 at 14:11, Nishanth Aravamudan wrote:
> --- 2.6.9-rc2-vanilla/drivers/scsi/53c700.c	2004-09-13 17:15:58.000000000 -0700
> +++ 2.6.9-rc2/drivers/scsi/53c700.c	2004-09-14 09:10:24.000000000 -0700
> @@ -1961,7 +1961,7 @@ NCR_700_bus_reset(struct scsi_cmnd * SCp
>  	 * reset via sg or something */
>  	while(hostdata->eh_complete != NULL) {
>  		spin_unlock_irq(SCp->device->host->host_lock);
> -		schedule_timeout(HZ/10);
> +		msleep_interruptible(100);
>  		spin_lock_irq(SCp->device->host->host_lock);
>  	}
>  	hostdata->eh_complete = &complete;

Firstly, could you copy linux-scsi@vger.kernel.org on these, and
secondly there isn't really much point in replacing this, is there?  The
only thing msleep_interruptible() would add is making sure that we slept
for the full time.  However, the code really doesn't care.  All it's
doing is relinquishing the processor for a short period while it waits
for the host eh_complete semaphore to become available.

James



[-- 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

[Kernel-janitors] Re: [PATCH 2.6.9-rc2 1/16] scsi/53c700: replace

[Nishanth Aravamudan]

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

On Mon, Sep 27, 2004 at 02:21:31PM -0400, James Bottomley wrote:
> On Mon, 2004-09-27 at 14:11, Nishanth Aravamudan wrote:
> > --- 2.6.9-rc2-vanilla/drivers/scsi/53c700.c	2004-09-13 17:15:58.000000000 -0700
> > +++ 2.6.9-rc2/drivers/scsi/53c700.c	2004-09-14 09:10:24.000000000 -0700
> > @@ -1961,7 +1961,7 @@ NCR_700_bus_reset(struct scsi_cmnd * SCp
> >  	 * reset via sg or something */
> >  	while(hostdata->eh_complete != NULL) {
> >  		spin_unlock_irq(SCp->device->host->host_lock);
> > -		schedule_timeout(HZ/10);
> > +		msleep_interruptible(100);
> >  		spin_lock_irq(SCp->device->host->host_lock);
> >  	}
> >  	hostdata->eh_complete = &complete;
> 
> Firstly, could you copy linux-scsi@vger.kernel.org on these, and

Sorry, I had already sent out all 16 patches when I saw your mail. I can
go back through and forward them on to linux-scsi, if you would like,
though.

> secondly there isn't really much point in replacing this, is there?  The
> only thing msleep_interruptible() would add is making sure that we slept
> for the full time.  However, the code really doesn't care.  All it's
> doing is relinquishing the processor for a short period while it waits
> for the host eh_complete semaphore to become available.

Well, in this case, msleep_interruptible() makes sure you sleep at all.
In the original code, schedule_timeout() will return immediately because
current->state is not being set beforehand. That fact, combined with the
clarity resulting from using msecs makes these changes useful, I think.

-Nish

[-- 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

[Kernel-janitors] Re: [PATCH 2.6.9-rc2 15/16] scsi/st: replace

[Kai Makisara]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 304 bytes --]

On Mon, 27 Sep 2004, Nishanth Aravamudan wrote:

> Any comments would be appreciated.
> 
> Description: Use msleep_interruptible() instead of
> schedule_timeout() to guarantee the task delays as expected.
> 
Looks fine to me (assuming msleep_interruptible() does what the name 
suggests :-)

Thanks,
Kai

[-- 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

[Kernel-janitors] Re: [PATCH 2.6.9-rc2 1/6] serial/68328serial:

[Nishanth Aravamudan]

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

On Mon, Sep 27, 2004 at 02:12:57PM -0700, Nishanth Aravamudan wrote:
> Any comments would be appreciated.
> 
> Description: Use msleep_interruptible() instead of
> schedule_timeout() to guarantee the task delays as expected.

Sorry, I was missing part of the description. The patch also uses
set_current_state() instead of direct assignment of current->state.

-Nish

[-- 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

[Kernel-janitors] Re: [PATCH 2.6.9-rc2 1/4] video/pxafb: replace

[Nishanth Aravamudan]

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

On Mon, Sep 27, 2004 at 04:06:20PM -0700, Nishanth Aravamudan wrote:
> Any comments would be appreciated.
> 
> Description: Use msleep() instead of schedule_timeout() to
> guarantee the task delays as expected.
> 
> --- 2.6.9-rc2-vanilla/drivers/video/pxafb.c	2004-09-13 17:15:38.000000000 -0700
> +++ 2.6.9-rc2/drivers/video/pxafb.c	2004-09-14 10:42:13.000000000 -0700
> @@ -747,7 +747,7 @@ static void pxafb_disable_controller(str
>  	LCCR0 &= ~LCCR0_LDM;	/* Enable LCD Disable Done Interrupt */
>  	LCCR0 |= LCCR0_DIS;	/* Disable LCD Controller */
>  
> -	schedule_timeout(20 * HZ / 1000);
> +	msleep(20);
>  	remove_wait_queue(&fbi->ctrlr_wait, &wait);
>  }

Sorry, this patch should *not* be applied - because the
schedule_timeout(), I believe, is there for the wait queue event, but
msleep() will ignore that event.

-Nish

[-- 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

[Kernel-janitors] Re: [PATCH 2.6.9-rc2 1/6] serial/68328serial:

[David McCullough]

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


Jivin Nishanth Aravamudan lays it down ...
> On Mon, Sep 27, 2004 at 02:12:57PM -0700, Nishanth Aravamudan wrote:
> > Any comments would be appreciated.
> > 
> > Description: Use msleep_interruptible() instead of
> > schedule_timeout() to guarantee the task delays as expected.
> 
> Sorry, I was missing part of the description. The patch also uses
> set_current_state() instead of direct assignment of current->state.

Looks ok to me :-)

Cheers,
Davidm

-- 
David McCullough, davidm@snapgear.com  Ph:+61 7 34352815 http://www.SnapGear.com
Custom Embedded Solutions + Security   Fx:+61 7 38913630 http://www.uCdot.org

[-- 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

[Kernel-janitors] Re: [PATCH 2.6.9-rc2 1/4] w1/dscore: replace

[Evgeniy Polyakov]

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

On Tue, 2004-09-28 at 03:17, Nishanth Aravamudan wrote:
> Any comments would be appreciated.

Following code is buggy, I know, the latest -mm tree already contains
codepath which properly sets process state and checks signals.
I've not used msleep*() due to future ability of special signal
handling.

> Description: Use msleep_interruptible() instead of schedule_timeout() to
> guarantee the task delays as expected.
> 
> --- 2.6.9-rc2-vanilla/drivers/w1/dscore.c	2004-09-13 17:16:07.000000000 -0700
> +++ 2.6.9-rc2/drivers/w1/dscore.c	2004-09-27 16:16:47.000000000 -0700
> @@ -23,6 +23,7 @@
>  #include <linux/kernel.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/usb.h>
> +#include <linux/delay.h>
>  
>  #include "dscore.h"
>  
> @@ -731,7 +732,7 @@ void ds_disconnect(struct usb_interface 
>  	usb_set_intfdata (intf, NULL);
>  
>  	while(atomic_read(&dev->refcnt))
> -		schedule_timeout(HZ);
> +		msleep_interruptible(1000);
>  
>  	usb_put_dev(dev->udev);
>  	kfree(dev);
-- 
	Evgeniy Polyakov

Crash is better than data corruption. -- Art Grabowski

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- 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

[Kernel-janitors] Re: [PATCH 2.6.9-rc2 1/4] w1/dscore: replace

[Evgeniy Polyakov]

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

On Tue, 2004-09-28 at 07:19, Evgeniy Polyakov wrote:
> On Tue, 2004-09-28 at 03:17, Nishanth Aravamudan wrote:
> > Any comments would be appreciated.
> 
> Following code is buggy, I know, the latest -mm tree already contains
> codepath which properly sets process state and checks signals.
> I've not used msleep*() due to future ability of special signal
> handling.

Hmm, I've rechecked -mm3, no it is not containing such fixes yet. I hope
next -mm release will have them.
Anyway if you set TASK_INTERRUPTIBLE you must be prepared to check
signal pending and either flush them or proper handle.

> > Description: Use msleep_interruptible() instead of schedule_timeout() to
> > guarantee the task delays as expected.
> > 
> > --- 2.6.9-rc2-vanilla/drivers/w1/dscore.c	2004-09-13 17:16:07.000000000 -0700
> > +++ 2.6.9-rc2/drivers/w1/dscore.c	2004-09-27 16:16:47.000000000 -0700
> > @@ -23,6 +23,7 @@
> >  #include <linux/kernel.h>
> >  #include <linux/mod_devicetable.h>
> >  #include <linux/usb.h>
> > +#include <linux/delay.h>
> >  
> >  #include "dscore.h"
> >  
> > @@ -731,7 +732,7 @@ void ds_disconnect(struct usb_interface 
> >  	usb_set_intfdata (intf, NULL);
> >  
> >  	while(atomic_read(&dev->refcnt))
> > -		schedule_timeout(HZ);
> > +		msleep_interruptible(1000);
> >  
> >  	usb_put_dev(dev->udev);
> >  	kfree(dev);
-- 
	Evgeniy Polyakov

Crash is better than data corruption. -- Art Grabowski

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- 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

[Kernel-janitors] Re: [PATCH 2.6.9-rc2 1/4] pci

[Greg KH]

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

On Fri, Sep 24, 2004 at 04:18:30PM -0700, Nishanth Aravamudan wrote:
> Any comments would be appreciated.
> 
> Description: Use msleep_interruptible() instead of
> schedule_timeout() to guarantee the task delays as expected.
> 
> Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>

Applied, thanks.

greg k-h


[-- 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