Skip to contentSkip to conversation

no set_current_state() before schedule_timeout()

Kernel-janitors Nishanth Aravamudan 3 participants 19 messages Jul 13, 2004

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Hi,

In continuing to replace, where appropriate, code with msleep() calls, I ran across the following files / functions, all of which do not invoke set_current_state() before schedule_timeout(), which causes the latter to return immediately:

drivers/mtd/chips/cfi_cmdset_0001.c::do_xxlock_one_block() drivers/mtd/chips/cfi_cmdset_0001.c::do_erase_chip() drivers/mtd/chips/amd_flash.c::erase_one_block() drivers/mtd/chips/cfi_cmdset_0002.c::do_erase_chip() drivers/mtd/chips/cfi_cmdset_0002.c::do_erase_oneblock() drivers/mtd/chips/cfi_cmdset_0020.c::do_erase_oneblock() drivers/mtd/chips/cfi_cmdset_0020.c::do_lock_oneblock() drivers/mtd/chips/cfi_cmdset_0020.c::do_unlock_oneblock()

If someone could tell me which state (TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE) is desired, I can fix this and perhaps replace the calls with msleep().

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#1 lore raw

Hi,

In continuing to replace, where appropriate, code with msleep() calls, I ran across the following files / functions, which do not invoke set_current_state() before schedule_timeout(), which causes the latter to return immediately:

drivers/isdn/icn/icn.c::icn_command() drivers/isdn/isdnloop/isdnloop.c::isdnloop_command()

Also, in

drivers/isdn/icn/icn.c::SLEEP()

I am wondering if the macro must necessarily run in TASK_INTERRUPTIBLE? If not, then I can replace it directly with a call to msleep().

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#2 lore raw
In reply to Nishanth Aravamudan

Hi,

In continuing to replace, where appropriate, code with msleep() calls, I ran across the following file(s) / function(s), which do not invoke set_current_state() before schedule_timeout(), which causes the latter to return immediately:

drivers/net/tokenring/ibmtr.c::ibmtr_cleanup_card()

If someone could tell me which state (TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE) is desired, I can fix this and perhaps replace the calls with msleep().

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#3 lore raw
In reply to Nishanth Aravamudan

Hi,

In continuing to replace, where appropriate, code with msleep() calls, I ran across the following file(s) / function(s), which do not invoke set_current_state() before schedule_timeout(), which causes the latter to return immediately:

drivers/net/wan/farsync.c::fst_issue_cmd()

If someone could tell me which state (TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE) is desired, I can fix this and perhaps replace the calls with msleep().

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#4 lore raw
In reply to Nishanth Aravamudan

Hi,

In continuing to replace, where appropriate, code with msleep() calls, I ran across the following file(s) / function(s), which do not invoke set_current_state() before schedule_timeout(), which causes the latter to return immediately:

drivers/net/wireless/airo.c::airo_pci_resume() drivers/net/wireless/airport.c::airport_hard_reset() drivers/net/wireless/prism54/islpci_dev.c::islpci_reset_if() drivers/net/wireless/prsim54/islpci_mgt.c::islpci_mgt_transaction()

If someone could tell me which state (TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE) is desired, I can fix this and perhaps replace the calls with msleep().

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#5 lore raw
In reply to Nishanth Aravamudan

Hi,

In continuing to replace, where appropriate, code with msleep() calls, I ran across the following file(s) / function(s), which do not invoke set_current_state() before schedule_timeout(), which causes the latter to return immediately:

drivers/s390/cio/device_ops.c::__ccw_device_retry_loop() drivers/s390/net/lcs.c::lcs_resetcard()

If someone could tell me which state (TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE) is desired, I can fix this and perhaps replace the calls with msleep().

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#6 lore raw
In reply to Nishanth Aravamudan

Hi,

In continuing to replace, where appropriate, code with msleep() calls, I ran across the following file(s) / function(s), which do not invoke set_current_state() before schedule_timeout(), which causes the latter to return immediately:

drivers/scsi/53c700.c::NCR_700_bus_reset()

If someone could tell me which state (TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE) is desired, I can fix this and perhaps replace the calls with msleep().

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#7 lore raw
In reply to Nishanth Aravamudan

Hi,

In continuing to replace, where appropriate, code with msleep() calls, I ran across the following file(s) / function(s), which do not invoke set_current_state() before schedule_timeout(), which causes the latter to return immediately:

drivers/scsi/ipr.c::ipr_store_diagnostics()

If someone could tell me which state (TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE) is desired, I can fix this and perhaps replace the calls with msleep().

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#8 lore raw
In reply to Nishanth Aravamudan

Hi,

In continuing to replace, where appropriate, code with msleep() calls, I ran across the following file(s) / function(s), which do not invoke set_current_state() before schedule_timeout(), which causes the latter to return immediately:

drivers/scsi/osst.c::osst_reposition_and_retry()

If someone could tell me which state (TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE) is desired, I can fix this and perhaps replace the calls with msleep().

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#9 lore raw

[Kernel-janitors] [PATCH] Re: no set_current_state() before

Quoted 7 lines · Nishanth Aravamudan

Hi,

In continuing to replace, where appropriate, code with msleep() calls, I ran across the following file(s) / function(s), which do not invoke set_current_state() before schedule_timeout(), which causes the latter to return immediately:

Quoted 5 lines

drivers/scsi/osst.c::osst_reposition_and_retry()

If someone could tell me which state (TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE) is desired, I can fix this and perhaps replace the calls with msleep().

You're right, there is a set_current_state(TASK_INTERRUPTIBLE) missing. I don't know why we would want to change to use msleep() though. Below is the patch to add the missing statement.

James Bottomley, can you apply this patch?

Regards, Willem Riede.

--- linux-2.6.8-rc1/drivers/scsi/osst.c.orig 2004-07-13
18:59:16.026349000 -0400
+++ linux-2.6.8-rc1/drivers/scsi/osst.c 2004-07-13 19:01:58.040719208
-0400
@@ -1555,6 +1555,7 @@

			debugging = 0;
		}
#endif
+		set_current_state(TASK_INTERRUPTIBLE);
		schedule_timeout(HZ / 10);
	}
	printk(KERN_ERR "%s:E: Failed to find valid tape media\n", name);
#10 lore raw

Re: [Kernel-janitors] [PATCH] Re: no set_current_state() before

Quoted 3 lines · Willem Riede

On 07/13/2004 01:40:54 PM, Nishanth Aravamudan wrote:

<snip>

Quoted 6 lines
Quoted 3 lines

If someone could tell me which state (TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE) is desired, I can fix this and perhaps replace the calls with msleep().

You're right, there is a set_current_state(TASK_INTERRUPTIBLE) missing. I don't know why we would want to change to use msleep() though.

<snip>

The main reason I see for using msleep() instead is if the task should sleep for at least 100 ms. Using TASK_INTERRUPTIBLE (or really anything other than msleep()) is not guaranteed to sleep as long as requested. If that's ok / desired, then I won't convert it, of course.

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#11 lore raw

Re: [Kernel-janitors] [PATCH] Re: no set_current_state() before

Quoted 5 lines · Nish Aravamudan

The main reason I see for using msleep() instead is if the task should sleep for at least 100 ms. Using TASK_INTERRUPTIBLE (or really anything other than msleep()) is not guaranteed to sleep as long as requested. If that's ok / desired, then I won't convert it, of course.

There is no need for such guarantee. The current behavior is OK, by design.

Regards, Willem Riede.

#12 lore raw

Re: [Kernel-janitors] [PATCH] Re: no set_current_state() before

In reply to Nish Aravamudan
Quoted 16 lines · Nish Aravamudan
Quoted 2 lines · Willem Riede

On 07/13/2004 01:40:54 PM, Nishanth Aravamudan wrote:

<snip>

Quoted 6 lines
Quoted 3 lines

If someone could tell me which state (TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE) is desired, I can fix this and perhaps replace the calls with msleep().

You're right, there is a set_current_state(TASK_INTERRUPTIBLE) missing. I don't know why we would want to change to use msleep() though.

Quoted 6 lines

<snip>

The main reason I see for using msleep() instead is if the task should sleep for at least 100 ms. Using TASK_INTERRUPTIBLE (or really anything other than msleep()) is not guaranteed to sleep as long as requested. If that's ok / desired, then I won't convert it, of course.

To be clear, the 100 ms I mention above is specific to this example. In general, if the time you want to sleep (and you really want to *sleep* for that time) is measureable in msecs, then msleep() is the way to go.

-Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#13 lore raw
In reply to Nishanth Aravamudan

Hi,

In continuing to replace, where appropriate, code with msleep() calls, I ran across the following file(s) / function(s), which do not invoke set_current_state() before schedule_timeout(), which causes the latter to return immediately:

drivers/scsi/qla1280.c::qla1280_bus_reset

If someone could tell me which state (TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE) is desired, I can fix this and perhaps replace the calls with msleep().

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#14 lore raw
In reply to Nishanth Aravamudan

Hi,

In continuing to replace, where appropriate, code with msleep() calls, I ran across the following file(s) / function(s), which do not invoke set_current_state() before schedule_timeout(), which causes the latter to return immediately:

drivers/scsi/scsi_lib.c::scsi_device_quiesce()

If someone could tell me which state (TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE) is desired, I can fix this and perhaps replace the calls with msleep().

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#15 lore raw
In reply to Nishanth Aravamudan

Hi,

In continuing to replace, where appropriate, code with msleep() calls, I ran across the following file(s) / function(s), which do not invoke set_current_state() before schedule_timeout(), which causes the latter to return immediately:

drivers/usb/gadget/pxa2xx_udc.c::udc_enable()

If someone could tell me which state (TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE) is desired, I can fix this and perhaps replace the calls with msleep().

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#16 lore raw
In reply to Nishanth Aravamudan

Hi,

In continuing to replace, where appropriate, code with msleep() calls, I ran across the following file(s) / function(s), which do not invoke set_current_state() before schedule_timeout(), which causes the latter to return immediately:

drivers/usb/media/dabusb.c::dabusb_open()

If someone could tell me which state (TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE) is desired, I can fix this and perhaps replace the calls with msleep().

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#17 lore raw
In reply to Nishanth Aravamudan

Hi,

In continuing to replace, where appropriate, code with msleep() calls, I ran across the following file(s) / function(s), which do not invoke set_current_state() before schedule_timeout(), which causes the latter to return immediately:

drivers/usb/media/ov511.c::init_ov_sensor()
drivers/usb/media/ov511.c::ov7xx0_configure()

If someone could tell me which state (TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE) is desired, I can fix this and perhaps replace the calls with msleep().

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#18 lore raw
In reply to Nishanth Aravamudan

Hi,

In continuing to replace, where appropriate, code with msleep() calls, I ran across the following file(s) / function(s), which do not invoke set_current_state() before schedule_timeout(), which causes the latter to return immediately:

drivers/usb/misc/tiglusb.c::tiglusb_open()

If someone could tell me which state (TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE) is desired, I can fix this and perhaps replace the calls with msleep().

Thanks,
Nish

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors