Re: [PATCHv3 2/9] ir-rx51: Handle signals properly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Heippa,

Timo Kokkonen wrote:
Terve,

On 09/01/12 20:14, Sakari Ailus wrote:
Moi,

On Thu, Aug 30, 2012 at 08:54:24PM +0300, Timo Kokkonen wrote:
@@ -273,9 +281,18 @@ static ssize_t lirc_rx51_write(struct file *file, const char *buf,

  	/*
  	 * Don't return back to the userspace until the transfer has
-	 * finished
+	 * finished. However, we wish to not spend any more than 500ms
+	 * in kernel. No IR code TX should ever take that long.
+	 */
+	i = wait_event_timeout(lirc_rx51->wqueue, lirc_rx51->wbuf_index < 0,
+			HZ / 2);

Why such an arbitrary timeout? In reality it might not bite the user space
in practice ever, but is it (and if so, why) really required in the first
place?

Well, I can think of two cases:

1) Something goes wrong. Such before I converted the patch to use the up
to date PM QoS implementation, the transmitting could take very long
time because the interrupts were not waking up the MPU. Now that this is
sorted out only unknown bugs can cause transmitting to hang indefinitely.

2) User is (intentionally?) doing something wrong. For example by
feeding in an IR code that has got very long pulses, he could end up
having the lircd process hung in kernel unkillable for long time. That
could be avoided quite easily by counting the pulse lengths and
rejecting any IR codes that are obviously too long. But since I'd like
to also protect against 1) case, I think this solution works just fine.

In the end, this is just safety measure that this driver behaves well.

In that case I think you should use wait_event_interruptible() instead. It's not the driver's job to decide what the user can do with the hardware and what not, is it?

Terveisin,

--
Sakari Ailus
sakari.ailus@xxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux