Thread (12 messages) 12 messages, 3 authors, 2021-03-24

Re: [PATCH] serial: imx: drop workaround for forced irq threading

From: Uwe Kleine-König <hidden>
Date: 2021-03-24 11:28:47
Also in: lkml

Hello Johan,

On Tue, Mar 23, 2021 at 03:37:29PM +0100, Johan Hovold wrote:
On Mon, Mar 22, 2021 at 02:40:32PM +0100, Uwe Kleine-König wrote:
quoted
On Mon, Mar 22, 2021 at 02:20:57PM +0100, Johan Hovold wrote:
quoted
On Mon, Mar 22, 2021 at 12:55:36PM +0100, Uwe Kleine-König wrote:
quoted
On Mon, Mar 22, 2021 at 12:39:18PM +0100, Sebastian Andrzej Siewior wrote:
quoted
On 2021-03-22 12:34:02 [+0100], Uwe Kleine-König wrote:
quoted
On Mon, Mar 22, 2021 at 12:10:36PM +0100, Johan Hovold wrote:
quoted
Force-threaded interrupt handlers used to run with interrupts enabled,
something which could lead to deadlocks in case a threaded handler
shared a lock with code running in hard interrupt context (e.g. timer
callbacks) and did not explicitly disable interrupts.

This was specifically the case for serial drivers that take the port
lock in their console write path as printk can be called from hard
interrupt context also with forced threading ("threadirqs").

Since commit 81e2073c175b ("genirq: Disable interrupts for force
threaded handlers") interrupt handlers always run with interrupts
disabled on non-RT so that drivers no longer need to do handle this.
So we're breaking RT knowingly here? If this is the case I'm not happy
with your change. (And if RT is not affected a different wording would
be good.)
Which wording, could you be more specific? It looks good from here and
no, RT is not affected.
The commit log says essentially: "The change is fine on non-RT" which
suggests there is a problem on RT.
I don't think you can read that into the commit message.
From a strictly logically point of view you indeed cannot. But if you go
to the street and say to people there that they can park their car in
this street free of charge between Monday and Friday, I expect that most
of them will assume that they have to pay for parking on weekends.
That analogy would almost seem to suggest bad intent on my side.
That analogy's purpose was to put over my point that writing
(paraphrased) "Since non-RT changed, this workaround isn't necessary any
more" suggests to me that the change might be bad for RT. So again,
there was no harm intended, this is just a call for clearing up either
the commit log to make it obvious the change is right or to fix the
problem on RT if there is any.
To say that this workaround is no longer needed on !RT does not imply
that it is needed on RT. If anything it suggests I have considered RT,
I'd say.
The code in question was used for both RT and non-RT. You drop it for
both cases and only justify one of them. OK, fine, you considered both
cases. Just from reading the commit log I considered you didn't. It's
completely ok for me to be wrong here, but I still think the chosen
words are not optimal and stumbling as I did is easy. So I still see a
potential to improve the wording.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help