[Kernel-janitors] Re: [PATCH] pci/cpci_hotplug_core: replace

[Kernel-janitors] Re: [PATCH] pci/cpci_hotplug_core: replace

[Scott Murray]

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

On Tue, 27 Jul 2004, Nishanth Aravamudan wrote:

> I would appreciate any comments from the janitors list. This is one (of
> many) cases where I made a decision about replacing
> 
> set_current_state(TASK_INTERRUPTIBLE);
> schedule_timeout(some_time);
> 
> with
> 
> msleep(jiffies_to_msecs(some_time));
> 
> msleep() is not exactly the same as the previous code, but I only did
> this replacement where I thought long delays were *desired*. If this is
> not the case here, then just disregard this patch. 

msleep sets TASK_UNINTERRUPTIBLE and not TASK_INTERRUPTIBLE, which I think 
will work okay in this instance, but am not actually sure.  I'm leaving 
for two weeks vacation tomorrow, so I'm not in a position to test this out 
at the moment.  Greg, can you comment on the correctness of this?

My only other comment is that I would prefer you keep the braces in the 
if/else statements.  There may only be one actual line of code in the 
bodies, but with a comment above that one line I find it looks odd.  

Scott


-- 
Scott Murray
SOMA Networks, Inc.
Toronto, Ontario
e-mail: scottm@somanetworks.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] pci/cpci_hotplug_core: replace

[Greg KH]

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

On Tue, Jul 27, 2004 at 06:04:19PM -0400, Scott Murray wrote:
> On Tue, 27 Jul 2004, Nishanth Aravamudan wrote:
> 
> > I would appreciate any comments from the janitors list. This is one (of
> > many) cases where I made a decision about replacing
> > 
> > set_current_state(TASK_INTERRUPTIBLE);
> > schedule_timeout(some_time);
> > 
> > with
> > 
> > msleep(jiffies_to_msecs(some_time));
> > 
> > msleep() is not exactly the same as the previous code, but I only did
> > this replacement where I thought long delays were *desired*. If this is
> > not the case here, then just disregard this patch. 
> 
> msleep sets TASK_UNINTERRUPTIBLE and not TASK_INTERRUPTIBLE, which I think 
> will work okay in this instance, but am not actually sure.  I'm leaving 
> for two weeks vacation tomorrow, so I'm not in a position to test this out 
> at the moment.  Greg, can you comment on the correctness of this?

Yes, this should be safe at this point in time.  I'll apply the patch in
my next round of PCI patches.

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

[Kernel-janitors] Re: [PATCH] pci/cpci_hotplug_core: replace

[Greg KH]

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

On Tue, Jul 27, 2004 at 02:14:12PM -0700, Nishanth Aravamudan wrote:
> I would appreciate any comments from the janitors list. This is one (of
> many) cases where I made a decision about replacing
> 
> set_current_state(TASK_INTERRUPTIBLE);
> schedule_timeout(some_time);
> 
> with
> 
> msleep(jiffies_to_msecs(some_time));
> 
> msleep() is not exactly the same as the previous code, but I only did
> this replacement where I thought long delays were *desired*. If this is
> not the case here, then just disregard this patch. 

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