* [PATCH v2 08/45] drivers: tty: serial: 8250_lpc18xx: use devm_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-14 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: gregkh, eric, stefan.wahren, f.fainelli, rjui, sbranden,
bcm-kernel-feedback-list, andriy.shevchenko, vz, matthias.bgg,
yamada.masahiro, tklauser, richard.genoud, macro, u.kleine-koenig,
kernel, slemieux.tyco, andy.gross, david.brown, shawnguo, s.hauer,
festevam, linux-imx, baohua, jacmet, linux-serial, linux-arm-msm,
linuxppc-dev
In-Reply-To: <1552602855-26086-1-git-send-email-info@metux.net>
Instead of fetching out data from a struct resource for passing
it to devm_ioremap(), directly use devm_ioremap_resource()
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
drivers/tty/serial/8250/8250_lpc18xx.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_lpc18xx.c b/drivers/tty/serial/8250/8250_lpc18xx.c
index eddf119..f36902b 100644
--- a/drivers/tty/serial/8250/8250_lpc18xx.c
+++ b/drivers/tty/serial/8250/8250_lpc18xx.c
@@ -119,8 +119,7 @@ static int lpc18xx_serial_probe(struct platform_device *pdev)
memset(&uart, 0, sizeof(uart));
- uart.port.membase = devm_ioremap(&pdev->dev, res->start,
- resource_size(res));
+ uart.port.membase = devm_ioremap_resource(&pdev->dev, res);
if (!uart.port.membase)
return -ENOMEM;
--
1.9.1
^ permalink raw reply related
* [PATCH v2 07/45] drivers: tty: serial: 8250_uniphier: use devm_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-14 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: gregkh, eric, stefan.wahren, f.fainelli, rjui, sbranden,
bcm-kernel-feedback-list, andriy.shevchenko, vz, matthias.bgg,
yamada.masahiro, tklauser, richard.genoud, macro, u.kleine-koenig,
kernel, slemieux.tyco, andy.gross, david.brown, shawnguo, s.hauer,
festevam, linux-imx, baohua, jacmet, linux-serial, linux-arm-msm,
linuxppc-dev
In-Reply-To: <1552602855-26086-1-git-send-email-info@metux.net>
Instead of fetching out data from a struct resource for passing
it to devm_ioremap(), directly use devm_ioremap_resource()
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
drivers/tty/serial/8250/8250_uniphier.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_uniphier.c b/drivers/tty/serial/8250/8250_uniphier.c
index 164ba13..9c1244e 100644
--- a/drivers/tty/serial/8250/8250_uniphier.c
+++ b/drivers/tty/serial/8250/8250_uniphier.c
@@ -171,7 +171,7 @@ static int uniphier_uart_probe(struct platform_device *pdev)
return -EINVAL;
}
- membase = devm_ioremap(dev, regs->start, resource_size(regs));
+ membase = devm_ioremap_resource(dev, regs);
if (!membase)
return -ENOMEM;
--
1.9.1
^ permalink raw reply related
* [PATCH v2 06/45] drivers: tty: serial: samsung: use devm_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-14 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: gregkh, eric, stefan.wahren, f.fainelli, rjui, sbranden,
bcm-kernel-feedback-list, andriy.shevchenko, vz, matthias.bgg,
yamada.masahiro, tklauser, richard.genoud, macro, u.kleine-koenig,
kernel, slemieux.tyco, andy.gross, david.brown, shawnguo, s.hauer,
festevam, linux-imx, baohua, jacmet, linux-serial, linux-arm-msm,
linuxppc-dev
In-Reply-To: <1552602855-26086-1-git-send-email-info@metux.net>
Instead of fetching out data from a struct resource for passing
it to devm_ioremap(), directly use devm_ioremap_resource()
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
drivers/tty/serial/samsung.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index 83fd516..a49cf15 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -1775,7 +1775,7 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport,
dbg("resource %pR)\n", res);
- port->membase = devm_ioremap(port->dev, res->start, resource_size(res));
+ port->membase = devm_ioremap_resource(port->dev, res);
if (!port->membase) {
dev_err(port->dev, "failed to remap controller address\n");
return -EBUSY;
--
1.9.1
^ permalink raw reply related
* [PATCH v2 05/45] drivers: tty: serial: sirfsoc_uart: use devm_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-14 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: gregkh, eric, stefan.wahren, f.fainelli, rjui, sbranden,
bcm-kernel-feedback-list, andriy.shevchenko, vz, matthias.bgg,
yamada.masahiro, tklauser, richard.genoud, macro, u.kleine-koenig,
kernel, slemieux.tyco, andy.gross, david.brown, shawnguo, s.hauer,
festevam, linux-imx, baohua, jacmet, linux-serial, linux-arm-msm,
linuxppc-dev
In-Reply-To: <1552602855-26086-1-git-send-email-info@metux.net>
Instead of fetching out data from a struct resource for passing
it to devm_ioremap(), directly use devm_ioremap_resource()
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
drivers/tty/serial/sirfsoc_uart.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/tty/serial/sirfsoc_uart.c b/drivers/tty/serial/sirfsoc_uart.c
index 38622f2..db2e08d 100644
--- a/drivers/tty/serial/sirfsoc_uart.c
+++ b/drivers/tty/serial/sirfsoc_uart.c
@@ -1359,8 +1359,7 @@ static int sirfsoc_uart_probe(struct platform_device *pdev)
goto err;
}
port->mapbase = res->start;
- port->membase = devm_ioremap(&pdev->dev,
- res->start, resource_size(res));
+ port->membase = devm_ioremap_resource(&pdev->dev, res);
if (!port->membase) {
dev_err(&pdev->dev, "Cannot remap resource.\n");
ret = -ENOMEM;
--
1.9.1
^ permalink raw reply related
* [PATCH v2 04/45] drivers: tty: serial: amba-pl010: use devm_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-14 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: gregkh, eric, stefan.wahren, f.fainelli, rjui, sbranden,
bcm-kernel-feedback-list, andriy.shevchenko, vz, matthias.bgg,
yamada.masahiro, tklauser, richard.genoud, macro, u.kleine-koenig,
kernel, slemieux.tyco, andy.gross, david.brown, shawnguo, s.hauer,
festevam, linux-imx, baohua, jacmet, linux-serial, linux-arm-msm,
linuxppc-dev
In-Reply-To: <1552602855-26086-1-git-send-email-info@metux.net>
Instead of fetching out data from a struct resource for passing
it to devm_ioremap(), directly use devm_ioremap_resource()
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
drivers/tty/serial/amba-pl010.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/tty/serial/amba-pl010.c b/drivers/tty/serial/amba-pl010.c
index 2c37d11..109b8df 100644
--- a/drivers/tty/serial/amba-pl010.c
+++ b/drivers/tty/serial/amba-pl010.c
@@ -713,8 +713,7 @@ static int pl010_probe(struct amba_device *dev, const struct amba_id *id)
if (!uap)
return -ENOMEM;
- base = devm_ioremap(&dev->dev, dev->res.start,
- resource_size(&dev->res));
+ base = devm_ioremap_resource(&dev->dev, &dev->res);
if (!base)
return -ENOMEM;
--
1.9.1
^ permalink raw reply related
* [PATCH v2 03/45] drivers: tty: serial: 8250_ingenic: use devm_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-14 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: gregkh, eric, stefan.wahren, f.fainelli, rjui, sbranden,
bcm-kernel-feedback-list, andriy.shevchenko, vz, matthias.bgg,
yamada.masahiro, tklauser, richard.genoud, macro, u.kleine-koenig,
kernel, slemieux.tyco, andy.gross, david.brown, shawnguo, s.hauer,
festevam, linux-imx, baohua, jacmet, linux-serial, linux-arm-msm,
linuxppc-dev
In-Reply-To: <1552602855-26086-1-git-send-email-info@metux.net>
Instead of fetching out data from a struct resource for passing
it to devm_ioremap(), directly use devm_ioremap_resource()
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
drivers/tty/serial/8250/8250_ingenic.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_ingenic.c b/drivers/tty/serial/8250/8250_ingenic.c
index 424c07c..90ae3e2 100644
--- a/drivers/tty/serial/8250/8250_ingenic.c
+++ b/drivers/tty/serial/8250/8250_ingenic.c
@@ -249,8 +249,7 @@ static int ingenic_uart_probe(struct platform_device *pdev)
if (line >= 0)
uart.port.line = line;
- uart.port.membase = devm_ioremap(&pdev->dev, regs->start,
- resource_size(regs));
+ uart.port.membase = devm_ioremap_resource(&pdev->dev, regs);
if (!uart.port.membase)
return -ENOMEM;
--
1.9.1
^ permalink raw reply related
* [PATCH v2 02/45] drivers: tty: serial: 8250_dw: use devm_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-14 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: gregkh, eric, stefan.wahren, f.fainelli, rjui, sbranden,
bcm-kernel-feedback-list, andriy.shevchenko, vz, matthias.bgg,
yamada.masahiro, tklauser, richard.genoud, macro, u.kleine-koenig,
kernel, slemieux.tyco, andy.gross, david.brown, shawnguo, s.hauer,
festevam, linux-imx, baohua, jacmet, linux-serial, linux-arm-msm,
linuxppc-dev
In-Reply-To: <1552602855-26086-1-git-send-email-info@metux.net>
Instead of fetching out data from a struct resource for passing
it to devm_ioremap(), directly use devm_ioremap_resource()
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
drivers/tty/serial/8250/8250_dw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index d31b975..f0a294d 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -526,7 +526,7 @@ static int dw8250_probe(struct platform_device *pdev)
p->set_ldisc = dw8250_set_ldisc;
p->set_termios = dw8250_set_termios;
- p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
+ p->membase = devm_ioremap_resource(dev, regs);
if (!p->membase)
return -ENOMEM;
--
1.9.1
^ permalink raw reply related
* [PATCH v2 01/45] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-14 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: gregkh, eric, stefan.wahren, f.fainelli, rjui, sbranden,
bcm-kernel-feedback-list, andriy.shevchenko, vz, matthias.bgg,
yamada.masahiro, tklauser, richard.genoud, macro, u.kleine-koenig,
kernel, slemieux.tyco, andy.gross, david.brown, shawnguo, s.hauer,
festevam, linux-imx, baohua, jacmet, linux-serial, linux-arm-msm,
linuxppc-dev
In-Reply-To: <1552602855-26086-1-git-send-email-info@metux.net>
For the common platform_get_resource()+devm_platform_ioremap() combination,
there is a helper, so use it and make the code a bit more compact.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
drivers/tty/serial/8250/8250_bcm2835aux.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c b/drivers/tty/serial/8250/8250_bcm2835aux.c
index bd53661..0738d14 100644
--- a/drivers/tty/serial/8250/8250_bcm2835aux.c
+++ b/drivers/tty/serial/8250/8250_bcm2835aux.c
@@ -25,7 +25,6 @@ struct bcm2835aux_data {
static int bcm2835aux_serial_probe(struct platform_device *pdev)
{
struct bcm2835aux_data *data;
- struct resource *res;
int ret;
/* allocate the custom structure */
@@ -63,15 +62,12 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev)
data->uart.port.irq = ret;
/* map the main registers */
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(&pdev->dev, "memory resource not found");
- return -EINVAL;
- }
- data->uart.port.membase = devm_ioremap_resource(&pdev->dev, res);
+ data->uart.port.membase = devm_platform_ioremap_resource(pdev, 0);
ret = PTR_ERR_OR_ZERO(data->uart.port.membase);
- if (ret)
+ if (ret) {
+ dev_err(&pdev->dev, "could not map memory resource");
return ret;
+ }
/* Check for a fixed line number */
ret = of_alias_get_id(pdev->dev.of_node, "serial");
--
1.9.1
^ permalink raw reply related
* serial driver cleanups v2
From: Enrico Weigelt, metux IT consult @ 2019-03-14 22:33 UTC (permalink / raw)
To: linux-kernel
Cc: gregkh, eric, stefan.wahren, f.fainelli, rjui, sbranden,
bcm-kernel-feedback-list, andriy.shevchenko, vz, matthias.bgg,
yamada.masahiro, tklauser, richard.genoud, macro, u.kleine-koenig,
kernel, slemieux.tyco, andy.gross, david.brown, shawnguo, s.hauer,
festevam, linux-imx, baohua, jacmet, linux-serial, linux-arm-msm,
linuxppc-dev
Hello folks,
here's v2 of my serial cleanups queue - part I:
essentially using helpers to code more compact and switching to
devm_*() functions for mmio management.
Part II will be about moving the mmio range from mapbase and
mapsize (which are used quite inconsistently) to a struct resource
and using helpers for that. But this one isn't finished yet.
(if somebody likes to have a look at it, I can send it, too)
happy hacking
--mtx
^ permalink raw reply
* Re: [PATCH 1/4] printk: Introduce per-console loglevel setting
From: Tetsuo Handa @ 2019-03-14 14:12 UTC (permalink / raw)
To: Petr Mladek
Cc: Calvin Owens, Sergey Senozhatsky, Steven Rostedt,
Greg Kroah-Hartman, Jonathan Corbet, linux-kernel, linux-serial
In-Reply-To: <20190308150914.ifqp36en3pyd4ljy@pathway.suse.cz>
Petr Mladek wrote:
> It might be even more straightforward when the per-console value
> defines the effective console level. I mean the following semantic:
>
> + "console_loglevel" would define the default loglevel used
> by consoles at runtime.
>
> + the per-console loglevel could override the default
> console_loglevel.
>
> + We would need a custom handler for the sysctl "console_loglevel".
> It would write the given value to the global console_loglevel
> variable and for all already registered consoles (con->loglevel).
But some functions change console_loglevel without sysctl (e.g.
console_verbose() when reporting hung tasks and panic()). Should
con->loglevel be changed (which might result in too much messages to
slow consoles) when console_loglevel changes?
>
> The value will be used also for all newly registered consoles
> when they do not have any custom one.
>
>
> + The handler for "loglevel" early param should behave the same
> as the sysctl handler.
>
>
> IMHO, there is no perfect solution. The advantage of the above
> proposal is that you "see" and "use" exactly what you "set".
^ permalink raw reply
* Re: Serial console is causing system lock-up
From: Sergey Senozhatsky @ 2019-03-14 10:30 UTC (permalink / raw)
To: John Ogness
Cc: Petr Mladek, Nigel Croxon, Theodore Y. Ts'o,
Sergey Senozhatsky, Greg Kroah-Hartman, Steven Rostedt,
Sergey Senozhatsky, dm-devel, Mikulas Patocka, linux-serial
In-Reply-To: <878sxj9nbb.fsf@linutronix.de>
On (03/13/19 09:43), John Ogness wrote:
> I don't understand how you can think "print or die trying" is replaced
> with another "print or die trying".
Sorry, let me explain. In some contexts CPUs which are spinning on
prb_lock don't do anything else. A careful placement of
touch_softlockup_watchdog_sync();
clocksource_touch_watchdog();
rcu_cpu_stall_reset();
touch_nmi_watchdog();
keeps the watchdogs away, yes, but that doesn't mean that we are not
sitting on a time bomb. Think of RCU, for instance. We keep rcu_cpu_stall
silent and things can look OK, but that doesn't mean that RCU is OK in
reality; spinning CPUs may hold off grace periods. So now a relatively
simple issue - raid checksum mismatch in this particular case - has
potential to become OOM. Quadratic CPU serialisation doesn't scale.
Throw enough reporting CPUs on it and we may get very close to some
big problems. Does this make sense?
This bug report demonstrates that we can have N CPUs reporting warns
simultaneously. And I think that people would want to have pr_warns
and WARN_ONs to be printed as emergency level messages (it sort of
sounds reasonable. I understand that you have different opinion on this).
And what I'm thinking is that *probably* we can have a bit less radical
approach - the system is not always doomed when it WARNs us - and a bit
more "best effort" one. *May be* we don't need to apply full serialisation
all the time. *May be* full serialisation can be applied only when we see
that we are about to run out of free space in logbuf. Or may be can
start dynamically resize the logbuf. And so on.
> By the way, Sergey, I appreciate your skepticism.
Sorry, John. I know I'm a PITA.
-ss
^ permalink raw reply
* Re: [RFC PATCH v1 00/25] printk: new implementation
From: John Ogness @ 2019-03-14 9:35 UTC (permalink / raw)
To: Petr Mladek
Cc: Sergey Senozhatsky, linux-kernel, Peter Zijlstra, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky, Sebastian Siewior
In-Reply-To: <20190314091434.ci26htgagjx6mk4k@pathway.suse.cz>
On 2019-03-14, Petr Mladek <pmladek@suse.com> wrote:
>>> I suggest the following way forward (separate patchsets):
>>>
>>> 1. Replace log buffer (least controversial thing)
>>
>> Yes. I will post a series that only implements the ringbuffer using
>> your simplified API. That will be enough to remove printk_safe and
>> actually does most of the work of updating devkmsg, kmsg_dump, and
>> syslog.
>
> Great. I just wonder if it is going to be fully lockless or
> still using the prb_lock. I could understand the a fully lockless
> solution will be much more complicated. But I think that it would
> make a lot of things easier in the long run. Especially it might
> help to avoid the big-kernel-lock-like approach.
I will re-visit my lockless implementation and see if I can minimize
complexity. We have since identified a couple other negative affects of
the prb_lock for PREEMPT_RT relating to CPU isolated setups. So the
lockless version is starting to look more attractive from a PREEMPT_RT
perspective as well.
>>> 2. Reliable offload to kthread (would be useful anyway)
>>
>> Yes. I would like to implement per-console kthreads for this
>> series. I think the advantages are obvious. For PREEMPT_RT the
>> offloading will need to be always active. (PREEMPT_RT cannot call the
>> console->write() from atomic contexts.) But I think this would be
>> acceptable at first. It would certainly be better than what
>> PREEMPT_RT is doing now.
>
> I would personally start with one kthread. I am afraid that
> the discussion about it will be complicated enough. We could
> always make it more complicated later.
>
> I understand the immediate offloading might be necessary for
> PREEMPT_RT. But a more conservative approach is needed for
> non-rt kernels.
>
> Well, if there won't be a big difference in the complexity
> between one and more threads then we could mix it. But
> I personally see this a two steps that are better be done
> separately.
I will create the series in a way that a complete solution with 1
kthread exists and all following patches in the series add per-console
kthreads. Then we have a clean cut if we decide we only want the first
part of the series.
>>> 3. Atomic consoles (a lot of tricky code, might not be
>>> worth the effort)
>>
>> I think this will be necessary. PREEMPT_RT cannot support reliable
>> emergency console messages without it. And for kernel developers this
>> is also very helpful. People like PeterZ are using their own patches
>> because the mainline kernel is not providing this functionality.
>>
>> The decision about _when_ to use it is still in the air. But I guess
>> we'll worry about that when we get that far. There's enough to do
>> until then.
>
> I am sure that there are situations where the direct output
> to atomic consoles would help with debugging. PeteZ and Steven
> are using their own patches for a reason.
>
> Let's see how the code is complicated and how many consoles
> might get supported a reasonable way.
Agreed.
I will post each of the above series as RFCv2 because I expect we still
need some discussion. Especially if I post the fully lockless version of
the ringbuffer.
I have taken a *lot* of notes about things that need changing during
this thread. I think a lot of great feedback has come out of this
RFC. Thank you to everyone that responded (publicly and privately)! I'll
need several weeks before the RFCv2 for the ringbuffer is ready.
John Ogness
^ permalink raw reply
* Re: [RFC PATCH v1 00/25] printk: new implementation
From: Petr Mladek @ 2019-03-14 9:27 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Sebastian Siewior, John Ogness, linux-kernel, Peter Zijlstra,
Steven Rostedt, Daniel Wang, Andrew Morton, Linus Torvalds,
Greg Kroah-Hartman, Alan Cox, Jiri Slaby, Peter Feiner,
linux-serial, Sergey Senozhatsky
In-Reply-To: <20190313092759.GA10060@jagdpanzerIV>
On Wed 2019-03-13 18:27:59, Sergey Senozhatsky wrote:
> On (03/13/19 09:40), Sebastian Siewior wrote:
> > On 2019-03-13 09:19:32 [+0100], John Ogness wrote:
> > > recursive situation. As you are pointing out, the notification/wake
> > > component of printk_safe will still be needed. I will leave that (small)
> > > part in printk_safe.c.
> >
> > Does this mean we keep irq_work part or we bury it and solve it by other
> > means?
>
> *May be* we can take a closer look and find cases when ->atomic
> consoles don't really depend on console_sem. And *may be* we can
> split the console drivers list and somehow forbid removal and
> modification (ioctl) of ->atomic consoles under us. Assuming that
> this is doable we then can start iterating ->atomic consoles list
> with unlocked console_sem.
I believe that this is actually the plan. Atomic consoles depending
on console_sem would not be a real step forward.
> Non->atomic consoles or consoles which depend on console_sem (VT,
> fbcon and so on) will stay in another list, and we will take
> console_sem before we iterate that list and invoke those drivers.
This might be also needed for "less" important messages
that people would not want to get to the console atomically
because it would serialize CPUs and slow down the entire
system.
I think that we would still need irq_work for this mode.
But it should be necessary only for messages from NMI context
and printk() recursion. It means that it should be a rare
situation and the amount of messages should get limited.
It should not be much worse than handling few printk()
calls from any irq handler.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH v1 00/25] printk: new implementation
From: Petr Mladek @ 2019-03-14 9:14 UTC (permalink / raw)
To: John Ogness
Cc: Sergey Senozhatsky, linux-kernel, Peter Zijlstra, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky, Sebastian Siewior
In-Reply-To: <874l8815uc.fsf@linutronix.de>
On Tue 2019-03-12 16:15:55, John Ogness wrote:
> On 2019-03-12, Petr Mladek <pmladek@suse.com> wrote:
> > On Tue 2019-03-12 09:17:49, John Ogness wrote:
> >> The current printk implementation is handling all console printing as
> >> best effort. Trying hard enough to dramatically affect the system, but
> >> not trying hard enough to guarantee success.
> >
> > I agree that direct output is more reliable. It might be very useful
> > for debugging some types of problems. The question is if it is worth
> > the cost (code complexity, serializing CPUs == slowing down the
> > entire system).
> >
> > But it is is possible that a reasonable offloading (in the direction
> > of last Sergey's approach) might be a better deal.
> >
> >
> > I suggest the following way forward (separate patchsets):
> >
> > 1. Replace log buffer (least controversial thing)
>
> Yes. I will post a series that only implements the ringbuffer using your
> simplified API. That will be enough to remove printk_safe and actually
> does most of the work of updating devkmsg, kmsg_dump, and syslog.
Great. I just wonder if it is going to be fully lockless or
still using the prb_lock. I could understand the a fully lockless
solution will be much more complicated. But I think that it would
make a lot of things easier in the long run. Especially it might
help to avoid the big-kernel-lock-like approach.
> > 2. Reliable offload to kthread (would be useful anyway)
>
> Yes. I would like to implement per-console kthreads for this series. I
> think the advantages are obvious. For PREEMPT_RT the offloading will
> need to be always active. (PREEMPT_RT cannot call the console->write()
> from atomic contexts.) But I think this would be acceptable at first. It
> would certainly be better than what PREEMPT_RT is doing now.
I would personally start with one kthread. I am afraid that
the discussion about it will be complicated enough. We could
always make it more complicated later.
I understand the immediate offloading might be necessary for
PREEMPT_RT. But a more conservative approach is needed for
non-rt kernels.
Well, if there won't be a big difference in the complexity
between one and more threads then we could mix it. But
I personally see this a two steps that are better be done
separately.
> > 3. Atomic consoles (a lot of tricky code, might not be
> > worth the effort)
>
> I think this will be necessary. PREEMPT_RT cannot support reliable
> emergency console messages without it. And for kernel developers this is
> also very helpful. People like PeterZ are using their own patches
> because the mainline kernel is not providing this functionality.
>
> The decision about _when_ to use it is still in the air. But I guess
> we'll worry about that when we get that far. There's enough to do until
> then.
I am sure that there are situations where the direct output
to atomic consoles would help with debugging. PeteZ and Steven
are using their own patches for a reason.
Let's see how the code is complicated and how many consoles
might get supported a reasonable way.
Anyway, it will be a long run. I am personally curious where
this will end :-)
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH] tty: atmel_serial: fix a NULL pointer dereference
From: Richard Genoud @ 2019-03-14 8:25 UTC (permalink / raw)
To: Kangjie Lu
Cc: Alexandre Belloni, Greg Kroah-Hartman, pakki001, linux-kernel,
Ludovic Desroches, linux-serial, Jiri Slaby, linux-arm-kernel
In-Reply-To: <20190314071759.18845-1-kjlu@umn.edu>
Hi,
Good catch !
Le 14/03/2019 à 08:17, Kangjie Lu a écrit :
> In case dmaengine_prep_dma_cyclic fails, the fix return a proper
> error code to avoid NULL pointer dereference.
>
you could add:
Fixes: 34df42f59a60 ("serial: at91: add rx dma support")
So that -stable branches get this.
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>
> ---
> drivers/tty/serial/atmel_serial.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index 05147fe24343..cf560d05008c 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -1237,8 +1237,10 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
> dma_cap_set(DMA_CYCLIC, mask);
>
> atmel_port->chan_rx = dma_request_slave_channel(mfd_dev, "rx");
> - if (atmel_port->chan_rx == NULL)
> + if (atmel_port->chan_rx == NULL) {
> + ret = -EINVAL;
> goto chan_err;
> + }
> dev_info(port->dev, "using %s for rx DMA transfers\n",
> dma_chan_name(atmel_port->chan_rx));
>
> @@ -1257,6 +1259,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
>
> if (!nent) {
> dev_dbg(port->dev, "need to release resource of dma\n");
> + ret = -EINVAL;
> goto chan_err;
> } else {
> dev_dbg(port->dev, "%s: mapped %d@%p to %pad\n", __func__,
> @@ -1288,6 +1291,11 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
> sg_dma_len(&atmel_port->sg_rx)/2,
> DMA_DEV_TO_MEM,
> DMA_PREP_INTERRUPT);
> + if (!desc) {
> + dev_err(port->dev, "Preparing DMA cyclic failed\n");
> + ret = -ENOMEM;
IMHO, we don't really know why dmaengine_prep_dma_cyclic() failed, it
could be because it's already in use, or bad value, or...
(and anyway, we just check if the return value is < 0 in atmel _startup.)
Is there a specific reason you choose -ENOMEM ?
If not, maybe keeping this patch smaller with a simple dev_err()+goto
here would be a better choice.
> + goto chan_err;
> + }
> desc->callback = atmel_complete_rx_dma;
> desc->callback_param = port;
> atmel_port->desc_rx = desc;
> @@ -1300,7 +1308,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
> atmel_port->use_dma_rx = 0;
> if (atmel_port->chan_rx)
> atmel_release_rx_dma(port);
> - return -EINVAL;
> + return ret;
> }
>
> static void atmel_uart_timer_callback(struct timer_list *t)
>
Thanks !
Richard.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] tty: mxs-auart: fix a NULL pointer dereference
From: Kangjie Lu @ 2019-03-14 7:21 UTC (permalink / raw)
To: kjlu
Cc: pakki001, Greg Kroah-Hartman, Jiri Slaby, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
linux-serial, linux-arm-kernel, linux-kernel
In case ioremap fails, the fix returns -ENOMEM to avoid NULL
pointer dereferences.
Multiple places use port.membase.
Signed-off-by: Kangjie Lu <kjlu@umn.edu>
---
drivers/tty/serial/mxs-auart.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 27235a526cce..4c188f4079b3 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -1686,6 +1686,10 @@ static int mxs_auart_probe(struct platform_device *pdev)
s->port.mapbase = r->start;
s->port.membase = ioremap(r->start, resource_size(r));
+ if (!s->port.membase) {
+ ret = -ENOMEM;
+ goto out_disable_clks;
+ }
s->port.ops = &mxs_auart_ops;
s->port.iotype = UPIO_MEM;
s->port.fifosize = MXS_AUART_FIFO_SIZE;
--
2.17.1
^ permalink raw reply related
* [PATCH] tty: atmel_serial: fix a NULL pointer dereference
From: Kangjie Lu @ 2019-03-14 7:17 UTC (permalink / raw)
To: kjlu
Cc: pakki001, Richard Genoud, Greg Kroah-Hartman, Jiri Slaby,
Nicolas Ferre, Alexandre Belloni, Ludovic Desroches, linux-serial,
linux-arm-kernel, linux-kernel
In case dmaengine_prep_dma_cyclic fails, the fix return a proper
error code to avoid NULL pointer dereference.
Signed-off-by: Kangjie Lu <kjlu@umn.edu>
---
drivers/tty/serial/atmel_serial.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 05147fe24343..cf560d05008c 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -1237,8 +1237,10 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
dma_cap_set(DMA_CYCLIC, mask);
atmel_port->chan_rx = dma_request_slave_channel(mfd_dev, "rx");
- if (atmel_port->chan_rx == NULL)
+ if (atmel_port->chan_rx == NULL) {
+ ret = -EINVAL;
goto chan_err;
+ }
dev_info(port->dev, "using %s for rx DMA transfers\n",
dma_chan_name(atmel_port->chan_rx));
@@ -1257,6 +1259,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
if (!nent) {
dev_dbg(port->dev, "need to release resource of dma\n");
+ ret = -EINVAL;
goto chan_err;
} else {
dev_dbg(port->dev, "%s: mapped %d@%p to %pad\n", __func__,
@@ -1288,6 +1291,11 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
sg_dma_len(&atmel_port->sg_rx)/2,
DMA_DEV_TO_MEM,
DMA_PREP_INTERRUPT);
+ if (!desc) {
+ dev_err(port->dev, "Preparing DMA cyclic failed\n");
+ ret = -ENOMEM;
+ goto chan_err;
+ }
desc->callback = atmel_complete_rx_dma;
desc->callback_param = port;
atmel_port->desc_rx = desc;
@@ -1300,7 +1308,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
atmel_port->use_dma_rx = 0;
if (atmel_port->chan_rx)
atmel_release_rx_dma(port);
- return -EINVAL;
+ return ret;
}
static void atmel_uart_timer_callback(struct timer_list *t)
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 1/8] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-13 17:13 UTC (permalink / raw)
To: Greg KH
Cc: Enrico Weigelt, metux IT consult, jslaby, linux-serial,
linux-kernel
In-Reply-To: <20190313143654.GA7489@kroah.com>
On 13.03.19 15:36, Greg KH wrote:
> Also, usually RFC like patches and series are ignored, I know I almost
> always ignore them because if the author doesn't think they are ready to
> be reviewed, why would I spend time on them compared to other patches
> that their authors think are ready for review.
Okay, but then, how is the best way to ask for help ?
In my case here, there're several things still unclear to me, eg:
* struct uart_port -> mapbase:
=> seems to be used by most drivers, but some also clear it later, eg.
uartlite does so when uart_add_one_port() call failed. is that
really necessary ?
* struct uart_port -> mapsize:
=> seems to be used only rarely. most drivers use either fixed params
on the memory mapping functions, or take it from some function.
wouldn't it be better if all would fill out that field and later
use it consistently ?
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
^ permalink raw reply
* Re: [PATCH 1/8] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource()
From: Greg KH @ 2019-03-13 14:36 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult
Cc: Enrico Weigelt, metux IT consult, jslaby, linux-serial,
linux-kernel
In-Reply-To: <947da751-bbce-4364-a1bf-5487908e59a5@metux.net>
On Wed, Mar 13, 2019 at 07:59:53AM +0100, Enrico Weigelt, metux IT consult wrote:
> On 12.03.19 17:33, Greg KH wrote:
> > On Tue, Mar 12, 2019 at 03:57:33PM +0100, Enrico Weigelt, metux IT consult wrote:
> >> ---
> >> drivers/tty/serial/8250/8250_bcm2835aux.c | 12 ++++--------
> >> 1 file changed, 4 insertions(+), 8 deletions(-)
> >
> > No changelog or signed-off-by, sorry, please fix up the series and
> > resend.
>
> Maybe the threading got messed up somehow (at least my tbird doesn't
> show it correctly), so you've probably didn't see my intro letter: there
> I wrote that this is yet WIP, not meant for actual submission, instead
> just to ask you folks whether my approach in general would be work.
>
Threading worked just fine, but you still shouldn't send out patches
like this without any independant information in the patch itself saying
what it does and what it is for. That's just bad engineering :)
Also, usually RFC like patches and series are ignored, I know I almost
always ignore them because if the author doesn't think they are ready to
be reviewed, why would I spend time on them compared to other patches
that their authors think are ready for review.
Remember, patch review is the most limited resource we have.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 1/8] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-13 14:09 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Greg KH, Enrico Weigelt, metux IT consult, Jiri Slaby,
linux-serial, linux-kernel
In-Reply-To: <CANiq72=SBFzRVhpkH8V+=5E-adzJ4v6UGAPKfr18wnCB0rcoQA@mail.gmail.com>
On 13.03.19 12:28, Miguel Ojeda wrote:
Hi,
> The intro letter seems to be an independent message with a different> subject line -- not sure what threading you expected (?).
hmm, I've sent it via git-send-email --compose. IMHO, this should
send the intro as the first message and the actual patches as reply.
But it seems that didn't work as I hoped.
>> I wrote that this is yet WIP, not meant for actual submission, instead>> just to ask you folks whether my approach in general would be work.>
> That will indeed cause confusion. If you are requesting comments
only,> please do so placing [RFC] or similar in the subject of each patch
Ah, good idea. So I also have this tag in the git repo.
BTW: are there more such conventions I should be aware of ?
> Another option is sending a message with the repo URL where> development is happening, so interested parties can take a look.
Ok. I had the impression that people here don't really like to look
at some git repos for reviews.
> Even if you are sending some draft patches, it is still a bad idea to> send them incomplete. Basically you are making it harder for early>
reviewers simply by not having written an explanation or at least a>
reference to the explanation.
Understood.
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
^ permalink raw reply
* Re: [PATCH 1/8] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource()
From: Miguel Ojeda @ 2019-03-13 11:28 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult
Cc: Greg KH, Enrico Weigelt, metux IT consult, Jiri Slaby,
linux-serial, linux-kernel
In-Reply-To: <947da751-bbce-4364-a1bf-5487908e59a5@metux.net>
On Wed, Mar 13, 2019 at 8:03 AM Enrico Weigelt, metux IT consult
<lkml@metux.net> wrote:
>
> On 12.03.19 17:33, Greg KH wrote:
> > On Tue, Mar 12, 2019 at 03:57:33PM +0100, Enrico Weigelt, metux IT consult wrote:
> >> ---
> >> drivers/tty/serial/8250/8250_bcm2835aux.c | 12 ++++--------
> >> 1 file changed, 4 insertions(+), 8 deletions(-)
> >
> > No changelog or signed-off-by, sorry, please fix up the series and
> > resend.
>
> Maybe the threading got messed up somehow (at least my tbird doesn't
> show it correctly), so you've probably didn't see my intro letter: there
The intro letter seems to be an independent message with a different
subject line -- not sure what threading you expected (?).
> I wrote that this is yet WIP, not meant for actual submission, instead
> just to ask you folks whether my approach in general would be work.
That will indeed cause confusion. If you are requesting comments only,
please do so placing [RFC] or similar in the subject of each patch
(you did in the letter, but that had a different title, and it is
not how cover letters are usually sent, they are normally patch 0).
Another option is sending a message with the repo URL where
development is happening, so interested parties can take a look.
However:
> Of course, when the stuff becomes ready for submission, all these
> things will be cleaned up.
Even if you are sending some draft patches, it is still a bad idea to
send them incomplete. Basically you are making it harder for early
reviewers simply by not having written an explanation or at least a
reference to the explanation.
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH 3/4] printk: Add consoles to a virtual "console" bus
From: Petr Mladek @ 2019-03-13 10:08 UTC (permalink / raw)
To: Calvin Owens
Cc: Sergey Senozhatsky, Steven Rostedt, Greg Kroah-Hartman,
Jonathan Corbet, linux-kernel@vger.kernel.org,
linux-serial@vger.kernel.org
In-Reply-To: <20190312215209.GD5982@Haydn>
On Tue 2019-03-12 21:52:13, Calvin Owens wrote:
> On Monday 03/11 at 14:33 +0100, Petr Mladek wrote:
> > On Fri 2019-03-01 16:48:19, Calvin Owens wrote:
> > > This patch embeds a device struct in the console struct, and registers
> > > them on a "console" bus so we can expose attributes in sysfs.
> > >
> > > Currently, most drivers declare static console structs, and that is
> > > incompatible with the dev refcount model. So we end up needing to patch
> > > all of the console drivers to:
> > >
> > > 1. Dynamically allocate the console struct using a new helper
> > > 2. Handle the allocation in (1) possibly failing
> > > 3. Dispose of (1) with put_device()
> > >
> > > Early console structures must still be static, since they're required
> > > before we're able to allocate memory. The least ugly way I can come up
> > > with to handle this is an "is_static" flag in the structure which makes
> > > the gets and puts NOPs, and is checked in ->release() to catch mistakes.
> > >
> > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > > index 2e0eb89f046c..67e1e993ab80 100644
> > > --- a/kernel/printk/printk.c
> > > +++ b/kernel/printk/printk.c
> > > @@ -2706,6 +2770,8 @@ void register_console(struct console *newcon)
> > > console_drivers->next = newcon;
> > > }
> > >
> > > + get_console(newcon);
> > > +
> > > if (newcon->flags & CON_EXTENDED)
> > > nr_ext_console_drivers++;
> > >
> > > @@ -2730,6 +2796,7 @@ void register_console(struct console *newcon)
> > > exclusive_console_stop_seq = console_seq;
> > > logbuf_unlock_irqrestore(flags);
> > > }
> > > + console_register_device(newcon);
> >
> > This calls console_init_device() for the statically defined
> > early consoles. I guess that it would invalidate the above
> > get_console().
>
> The get_console() macro checks for ->is_static and is a NOP if it's
> set, which is definitely confusing.
>
> > Also it is not symmetric with unregister_console(). We add it
> > to the console_subsys here and did not remove it when
> > the console is unregistered.
>
> We do a put() in the unregister path, somebody could hold a reference
> through sysfs so we can't just remove it. Or am I misunderstanding?
To be honest, I am not sure what the effect of get_device() and
put_device() is. But they are called also in device_add().
This suggests that the sysfs interface and struct device
stay even when we call device_put() in unregister_console().
This is an asymmetry. The sysfs interface is created only
for successfully registered console but it stays even
after unregistration (if it works as I expect).
Another problem is that register_console()/unregister_console()
might get called repeatedly for the same console. But device_add()
should get called only once.
I think that we could do better, see below.
> > IMHO, this might be done already in allocate_console().
> > And eventually in printk_late_init() for consoles that
> > are allocated earlier.
> >
> > I am not familiar with the device-related sysfs API.
> > But I see that device_initialize() and device_add()
> > can be done by device_register(). The separate
> > calls are needed only when a special reference
> > handling is needed. Are we special?
>
> We're special: the console initcalls run before the device core is
> initialized, initialize() lets us use the refcount.
But console_init_device() is called later from printk_late_init()
for the statically defined structures (early consoles). This
would reset the refcount for these consoles.
I think that we should delay calling console_init_device() for
all consoles until printk_late_done is set. Then it should
be safe to call device_register() there.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH v1 00/25] printk: new implementation
From: Sergey Senozhatsky @ 2019-03-13 10:06 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Sebastian Siewior, John Ogness, Petr Mladek, linux-kernel,
Peter Zijlstra, Steven Rostedt, Daniel Wang, Andrew Morton,
Linus Torvalds, Greg Kroah-Hartman, Alan Cox, Jiri Slaby,
Peter Feiner, linux-serial, Sergey Senozhatsky
In-Reply-To: <20190313092759.GA10060@jagdpanzerIV>
On (03/13/19 18:27), Sergey Senozhatsky wrote:
> > Does this mean we keep irq_work part or we bury it and solve it by other
> > means?
>
> That's a very good question. Because if we add console_trylock()
> to printk(), then we can't invoke ->atomic() consoles when console_sem
> is already locked, because one of the registered drivers is currently
> being modified by a 3rd party and printk(), thus, must stay away.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
one of the drivers or the list itself.
-ss
^ permalink raw reply
* Re: [RFC PATCH v1 00/25] printk: new implementation
From: Sergey Senozhatsky @ 2019-03-13 9:27 UTC (permalink / raw)
To: Sebastian Siewior
Cc: John Ogness, Sergey Senozhatsky, Petr Mladek, linux-kernel,
Peter Zijlstra, Steven Rostedt, Daniel Wang, Andrew Morton,
Linus Torvalds, Greg Kroah-Hartman, Alan Cox, Jiri Slaby,
Peter Feiner, linux-serial, Sergey Senozhatsky
In-Reply-To: <20190313084028.f2m4qhxd5sjzzawr@linutronix.de>
On (03/13/19 09:40), Sebastian Siewior wrote:
> On 2019-03-13 09:19:32 [+0100], John Ogness wrote:
> > recursive situation. As you are pointing out, the notification/wake
> > component of printk_safe will still be needed. I will leave that (small)
> > part in printk_safe.c.
>
> Does this mean we keep irq_work part or we bury it and solve it by other
> means?
That's a very good question. Because if we add console_trylock()
to printk(), then we can't invoke ->atomic() consoles when console_sem
is already locked, because one of the registered drivers is currently
being modified by a 3rd party and printk(), thus, must stay away.
Once that modification will be done console_unlock() will print all
pending messages. This is current design. And this conflicts with the
whole idea of ->atomic() consoles.
So may be we need a whole new scheme in this department as well.
For instance [*and this is completely untested idea* !!!]
*May be* we can take a closer look and find cases when ->atomic
consoles don't really depend on console_sem. And *may be* we can
split the console drivers list and somehow forbid removal and
modification (ioctl) of ->atomic consoles under us. Assuming that
this is doable we then can start iterating ->atomic consoles list
with unlocked console_sem.
Non->atomic consoles or consoles which depend on console_sem (VT,
fbcon and so on) will stay in another list, and we will take
console_sem before we iterate that list and invoke those drivers.
One more time - a completely random thought.
-ss
^ permalink raw reply
* Re: [RFC PATCH v1 00/25] printk: new implementation
From: Sergey Senozhatsky @ 2019-03-13 8:46 UTC (permalink / raw)
To: John Ogness
Cc: Sergey Senozhatsky, Petr Mladek, linux-kernel, Peter Zijlstra,
Steven Rostedt, Daniel Wang, Andrew Morton, Linus Torvalds,
Greg Kroah-Hartman, Alan Cox, Jiri Slaby, Peter Feiner,
linux-serial, Sergey Senozhatsky, Sebastian Siewior
In-Reply-To: <87d0mv9off.fsf@linutronix.de>
On (03/13/19 09:19), John Ogness wrote:
> >> Yes. I will post a series that only implements the ringbuffer using
> >> your simplified API. That will be enough to remove printk_safe and
> >> actually does most of the work of updating devkmsg, kmsg_dump, and
> >> syslog.
> >
> > This may _not_ be enough to remove printk_safe. One of the reasons
> > printk_safe "condom" came into existence was console_sem (which
> > is a bit too important to ignore it):
> >
> > printk()
> > console_trylock()
> > console_unlock()
> > up()
> > raw_spin_lock_irqsave(&sem->lock, flags)
> > __up()
> > wake_up_process()
> > WARN/etc
> > printk()
> > console_trylock()
> > down_trylock()
> > raw_spin_lock_irqsave(&sem->lock, flags) << deadlock
> >
> > Back then we were looking at
> >
> > printk->console_sem->lock->printk->console_sem->lock
> >
> > deadlock report from LG, if I'm not mistaken.
>
> The main drawback of printk_safe is the safe buffers, which, aside from
> bogus timestamping, may never make it back to the printk log buffer.
>
> With the new ring buffer the safe buffers are not needed, even in the
> recursive situation. As you are pointing out, the notification/wake
> component of printk_safe will still be needed. I will leave that (small)
> part in printk_safe.c.
Yeah, all I'm saying is that as it stands new printk() is missing a huge
and necessary part - console semaphore. And things can get very different
once you add that missing part. It brings a lot of stuff back to printk.
logbuf and logbuf_lock are not really huge printk problems. scheduler,
timekeeping locks, etc. are much bigger ones. Those dependencies don't
come from logbuf/logbuf_lock.
-ss
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox