Skip to contentSkip to conversation

12/34: cdrom/cdu31a: replace interruptible_sleep_on() with wait_event_interruptible()

PATCH In review [email protected] 2 participants 4 messages Mar 6, 2005

lore
Share & tools
lore
#0 lore raw

PATCH 2/6 12/34: cdrom/cdu31a: replace interruptible_sleep_on() with wait_event_interruptible()

Use wait_event_interruptible() instead of the deprecated interruptible_sleep_on(). The patch is straight-forward as the macros should result in the same execution. Patch is compile-tested (still throws out warnings regarding {save,restore}_flags()).

---

 kj-domen/drivers/cdrom/cdu31a.c |   40 +++++++++++++++++-----------------------
 1 files changed, 17 insertions(+), 23 deletions(-)
drivers/cdrom/cdu31a.c renamed from drivers/cdrom/cdu31a.c~int_sleep_on-drivers_cdrom_cdu31a +17 -23
diff -puN drivers/cdrom/cdu31a.c~int_sleep_on-drivers_cdrom_cdu31a drivers/cdrom/cdu31a.c
@@ -166,6 +166,7 @@
 #include <linux/slab.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/wait.h>
 
 #include <asm/system.h>
 #include <asm/io.h>
@@ -865,15 +866,13 @@ do_sony_cd_cmd(unsigned char cmd,
 	save_flags(flags);
 	cli();
 	if (current != has_cd_task) {	/* Allow recursive calls to this routine */
-		while (sony_inuse) {
-			interruptible_sleep_on(&sony_wait);
-			if (signal_pending(current)) {
-				result_buffer[0] = 0x20;
-				result_buffer[1] = SONY_SIGNAL_OP_ERR;
-				*result_size = 2;
-				restore_flags(flags);
-				return;
-			}
+		wait_event_interruptible(sony_wait, !sony_inuse);
+		if (signal_pending(current)) {
+			result_buffer[0] = 0x20;
+			result_buffer[1] = SONY_SIGNAL_OP_ERR;
+			*result_size = 2;
+			restore_flags(flags);
+			return;
 		}
 		sony_inuse = 1;
 		has_cd_task = current;
@@ -1372,16 +1371,13 @@ static void do_cdu31a_request(request_qu
 	 */
 	save_flags(flags);
 	cli();
-	while (sony_inuse) {
-		interruptible_sleep_on(&sony_wait);
-		if (signal_pending(current)) {
-			restore_flags(flags);
+	wait_event_interruptible(sony_wait, !sony_inuse);
+	if (signal_pending(current)) {
+		restore_flags(flags);
 #if DEBUG
-			printk("Leaving do_cdu31a_request at %d\n",
-			       __LINE__);
+		printk("Leaving do_cdu31a_request at %d\n", __LINE__);
 #endif
-			return;
-		}
+		return;
 	}
 	sony_inuse = 1;
 	has_cd_task = current;
@@ -2326,12 +2322,10 @@ static int read_audio(struct cdrom_read_
 	 */
 	save_flags(flags);
 	cli();
-	while (sony_inuse) {
-		interruptible_sleep_on(&sony_wait);
-		if (signal_pending(current)) {
-			restore_flags(flags);
-			return -EAGAIN;
-		}
+	wait_event_interruptible(sony_wait, sony_inuse);
+	if (signal_pending(current)) {
+		restore_flags(flags);
+		return -EAGAIN;
 	}
 	sony_inuse = 1;
 	has_cd_task = current;

_

#1 lore raw
Quoted 6 lines · [email protected] wrote:

Use wait_event_interruptible() instead of the deprecated
interruptible_sleep_on(). The patch is straight-forward as the macros should result in the same execution. Patch is compile-tested (still throws out warnings regarding {save,restore}_flags()).

I've posted a patch for the cdu31a driver some time ago that removes almost all usage of interruptible_sleep_on() and also {save,restore}_flags() - it uses semaphore instead. The only remaining code is in sony_sleep() function when using IRQ-driven operation.

See http://lkml.org/lkml/2004/12/18/107
The patch is big because I've messed with the formatting...

Signature · 1 line
Ondrej Zary
#2 lore raw
Quoted 10 lines · Ondrej Zary
Quoted 8 lines · [email protected] wrote:

Use wait_event_interruptible() instead of the deprecated interruptible_sleep_on(). The patch is straight-forward as the macros should result in the same execution. Patch is compile-tested (still throws out warnings
regarding {save,restore}_flags()).

Uh, this one escaped me, as schedule() (sleep_on) after cli() is clearly wrong. Btw. what was the reason for this?

Quoted 8 lines

I've posted a patch for the cdu31a driver some time ago that removes almost all usage of interruptible_sleep_on() and also {save,restore}_flags() - it uses semaphore instead.
The only remaining code is in sony_sleep() function when using IRQ-driven operation.

See http://lkml.org/lkml/2004/12/18/107
The patch is big because I've messed with the formatting...

I looked at it, and rewrote some of it into smaller patches. If you don't mind, can i send them to you for review and testing?

Domen

#3 lore raw
Quoted 8 lines · Domen Puncer
Quoted 6 lines · Ondrej Zary
Quoted 4 lines · [email protected] wrote:

Use wait_event_interruptible() instead of the deprecated
interruptible_sleep_on(). The patch is straight-forward as the macros should result in the same execution. Patch is compile-tested (still throws out warnings regarding {save,restore}_flags()).

Quoted 6 lines
Quoted 3 lines
Quoted 2 lines

Uh, this one escaped me, as schedule() (sleep_on) after cli() is clearly wrong. Btw. what was the reason for this?

I've never tried the IRQ-driven operation so I don't know if that code still works (it certainly used to for the original author some years ago). I think that it should wait until the IRQ from the drive arrives.

Quoted 9 lines
Quoted 5 lines

I've posted a patch for the cdu31a driver some time ago that removes almost all usage of interruptible_sleep_on() and also {save,restore}_flags() - it uses semaphore instead. The only remaining code is in sony_sleep() function when using IRQ-driven operation.

See http://lkml.org/lkml/2004/12/18/107
The patch is big because I've messed with the formatting...

I looked at it, and rewrote some of it into smaller patches. If you don't mind, can i send them to you for review and testing?

Thanks. I'll test them. I guess that these smaller patches are more likely to be accepted into mainline.

Signature · 1 line
Ondrej Zary