Hi Viktor,

On Fri, Jul 16, 2021 at 01:36:52AM +0300, Viktor Prutyanov wrote:

Hi Sean,

On Thu, 15 Jul 2021 22:40:01 +0100
Sean Young [off-list ref] wrote:

quoted

On Thu, Jul 15, 2021 at 12:27:06AM +0300, Viktor Prutyanov wrote:

 > > +	meson_irtx_fill_buf(ir, tx_buf, buf, len);

quoted

quoted

+	dev_dbg(ir->dev, "TX buffer filled, length = %u\n", len);
+
+	spin_lock_irqsave(&ir->lock, flags);
+	meson_irtx_update_buf(ir, tx_buf, len, 0);
+	reinit_completion(&ir->completion);
+	meson_irtx_send_buffer(ir);
+	spin_unlock_irqrestore(&ir->lock, flags);
+
+	ret = wait_for_completion_interruptible(&ir->completion);
+	dev_dbg(ir->dev, "TX %s\n", ret ? "interrupted" :
"completed");  

Here two things can happen. One is, the process received a signal
(e.g. ^C). The other is that the hardware didn't issue any interrupts
due some problem somewhere. In the latter case, we only escape this
wait_for_completion_interruptable() when the user gets fed up and
presses ^C or something like that.

quoted

+
+	spin_lock_irqsave(&ir->lock, flags);
+	kfree(ir->buf);
+	meson_irtx_update_buf(ir, NULL, 0, 0);
+	spin_unlock_irqrestore(&ir->lock, flags);  

Now it is possible that the buffer gets cleared before that IR was
sent, if the signal was received early enough. This means not all the
Tx was completed.

quoted

+
+	return len;  

Yet, we always return success.

In case no interrupts were generated we should return an error in a
timely manner, so the wait_for_completion() needs the timeout. You
can use the fact that the IR is never longer IR_MAX_DURATION (half a
second currently). Not sure what the returned error should be, maybe
-ETIMEDOUT?

As for me, ETIMEDOUT is OK.

quoted

The problem with the interruptable wait is that a process can receive
a signal at any time, and now when this happens your IR gets
truncated. I don't think this is what you want.

Should I replace wait_for_completion_interruptible by
wait_for_completion_timeout in order to wait in uninterruptible way?

Yes, the process can receive a signal if the terminal is resized
(SIGWINCH) or if the process is backgrounded and then foregrounded with
^Z and fg (SIGCONT). If this happens during tx then the tx might be
incomplete.

Thanks,

Sean

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel