* [PATCH] serial: Fix wakeup init logic to speed up startup
@ 2012-01-19 19:28 Simon Glass
2012-01-19 19:42 ` Paul E. McKenney
2012-01-19 23:46 ` Rafael J. Wysocki
0 siblings, 2 replies; 6+ messages in thread
From: Simon Glass @ 2012-01-19 19:28 UTC (permalink / raw)
To: LKML
Cc: Greg Kroah-Hartman, linux-serial, Rafael J. Wysocki, Alan Cox,
Paul E. McKenney, Simon Glass
The synchronize_rcu() call resulting from making every serial driver
wake-up capable (commit b3b708fa) slows boot down on my Tegra2x system
(with CONFIG_PREEMPT disabled).
But this is avoidable since it is the device_set_wakeup_enable() and then
subsequence disable which causes the delay. We might as well just make
the device wakeup capable but not actually enable it for wakeup until
needed.
Effectively the current code does this:
device_set_wakeup_capable(dev, 1);
device_set_wakeup_enable(dev, 1);
device_set_wakeup_enable(dev, 0);
We can just drop the last two lines.
Before this change my boot log says:
[ 0.227062] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
[ 0.702928] serial8250.0: ttyS0 at MMIO 0x70006040 (irq = 69) is a Tegra
after:
[ 0.227264] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
[ 0.227983] serial8250.0: ttyS0 at MMIO 0x70006040 (irq = 69) is a Tegra
for saving of 450ms.
Suggested-by: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Simon Glass <sjg@chromium.org>
---
drivers/tty/serial/serial_core.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index c7bf31a..1305618 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2348,11 +2348,11 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
*/
tty_dev = tty_register_device(drv->tty_driver, uport->line, uport->dev);
if (likely(!IS_ERR(tty_dev))) {
- device_init_wakeup(tty_dev, 1);
- device_set_wakeup_enable(tty_dev, 0);
- } else
+ device_set_wakeup_capable(tty_dev, 1);
+ } else {
printk(KERN_ERR "Cannot register tty device on line %d\n",
uport->line);
+ }
/*
* Ensure UPF_DEAD is not set.
--
1.7.7.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] serial: Fix wakeup init logic to speed up startup
2012-01-19 19:28 [PATCH] serial: Fix wakeup init logic to speed up startup Simon Glass
@ 2012-01-19 19:42 ` Paul E. McKenney
2012-01-19 19:51 ` Simon Glass
2012-01-19 23:46 ` Rafael J. Wysocki
1 sibling, 1 reply; 6+ messages in thread
From: Paul E. McKenney @ 2012-01-19 19:42 UTC (permalink / raw)
To: Simon Glass
Cc: LKML, Greg Kroah-Hartman, linux-serial, Rafael J. Wysocki, Alan Cox
On Thu, Jan 19, 2012 at 11:28:56AM -0800, Simon Glass wrote:
> The synchronize_rcu() call resulting from making every serial driver
> wake-up capable (commit b3b708fa) slows boot down on my Tegra2x system
> (with CONFIG_PREEMPT disabled).
>
> But this is avoidable since it is the device_set_wakeup_enable() and then
> subsequence disable which causes the delay. We might as well just make
> the device wakeup capable but not actually enable it for wakeup until
> needed.
>
> Effectively the current code does this:
>
> device_set_wakeup_capable(dev, 1);
> device_set_wakeup_enable(dev, 1);
> device_set_wakeup_enable(dev, 0);
>
> We can just drop the last two lines.
>
> Before this change my boot log says:
> [ 0.227062] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
> [ 0.702928] serial8250.0: ttyS0 at MMIO 0x70006040 (irq = 69) is a Tegra
>
> after:
> [ 0.227264] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
> [ 0.227983] serial8250.0: ttyS0 at MMIO 0x70006040 (irq = 69) is a Tegra
>
> for saving of 450ms.
You have multiple CPUs running at this point, correct? Before that
second CPU starts up, synchronize_rcu() is a no-op.
The patch looks good to me, but then again, I do not consider myself
qualified to have an opinion on the TTY layer. ;-)
Thanx, Paul
> Suggested-by: Rafael J. Wysocki <rjw@sisk.pl>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> drivers/tty/serial/serial_core.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index c7bf31a..1305618 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -2348,11 +2348,11 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
> */
> tty_dev = tty_register_device(drv->tty_driver, uport->line, uport->dev);
> if (likely(!IS_ERR(tty_dev))) {
> - device_init_wakeup(tty_dev, 1);
> - device_set_wakeup_enable(tty_dev, 0);
> - } else
> + device_set_wakeup_capable(tty_dev, 1);
> + } else {
> printk(KERN_ERR "Cannot register tty device on line %d\n",
> uport->line);
> + }
>
> /*
> * Ensure UPF_DEAD is not set.
> --
> 1.7.7.3
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] serial: Fix wakeup init logic to speed up startup
2012-01-19 19:42 ` Paul E. McKenney
@ 2012-01-19 19:51 ` Simon Glass
2012-01-19 20:12 ` Paul E. McKenney
0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2012-01-19 19:51 UTC (permalink / raw)
To: paulmck
Cc: LKML, Greg Kroah-Hartman, linux-serial, Rafael J. Wysocki, Alan Cox
Hi Paul,
On Thu, Jan 19, 2012 at 11:42 AM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Thu, Jan 19, 2012 at 11:28:56AM -0800, Simon Glass wrote:
>> The synchronize_rcu() call resulting from making every serial driver
>> wake-up capable (commit b3b708fa) slows boot down on my Tegra2x system
>> (with CONFIG_PREEMPT disabled).
>>
>> But this is avoidable since it is the device_set_wakeup_enable() and then
>> subsequence disable which causes the delay. We might as well just make
>> the device wakeup capable but not actually enable it for wakeup until
>> needed.
>>
>> Effectively the current code does this:
>>
>> device_set_wakeup_capable(dev, 1);
>> device_set_wakeup_enable(dev, 1);
>> device_set_wakeup_enable(dev, 0);
>>
>> We can just drop the last two lines.
>>
>> Before this change my boot log says:
>> [ 0.227062] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
>> [ 0.702928] serial8250.0: ttyS0 at MMIO 0x70006040 (irq = 69) is a Tegra
>>
>> after:
>> [ 0.227264] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
>> [ 0.227983] serial8250.0: ttyS0 at MMIO 0x70006040 (irq = 69) is a Tegra
>>
>> for saving of 450ms.
>
> You have multiple CPUs running at this point, correct? Before that
> second CPU starts up, synchronize_rcu() is a no-op.
Yes that's right, although I didn't get different behavior with 'nosmp'.
>
> The patch looks good to me, but then again, I do not consider myself
> qualified to have an opinion on the TTY layer. ;-)
Thanks, me neither :-)
Regards,
Simon
>
> Thanx, Paul
>
>> Suggested-by: Rafael J. Wysocki <rjw@sisk.pl>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>> drivers/tty/serial/serial_core.c | 6 +++---
>> 1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
>> index c7bf31a..1305618 100644
>> --- a/drivers/tty/serial/serial_core.c
>> +++ b/drivers/tty/serial/serial_core.c
>> @@ -2348,11 +2348,11 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
>> */
>> tty_dev = tty_register_device(drv->tty_driver, uport->line, uport->dev);
>> if (likely(!IS_ERR(tty_dev))) {
>> - device_init_wakeup(tty_dev, 1);
>> - device_set_wakeup_enable(tty_dev, 0);
>> - } else
>> + device_set_wakeup_capable(tty_dev, 1);
>> + } else {
>> printk(KERN_ERR "Cannot register tty device on line %d\n",
>> uport->line);
>> + }
>>
>> /*
>> * Ensure UPF_DEAD is not set.
>> --
>> 1.7.7.3
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] serial: Fix wakeup init logic to speed up startup
2012-01-19 19:51 ` Simon Glass
@ 2012-01-19 20:12 ` Paul E. McKenney
0 siblings, 0 replies; 6+ messages in thread
From: Paul E. McKenney @ 2012-01-19 20:12 UTC (permalink / raw)
To: Simon Glass
Cc: LKML, Greg Kroah-Hartman, linux-serial, Rafael J. Wysocki, Alan Cox
On Thu, Jan 19, 2012 at 11:51:47AM -0800, Simon Glass wrote:
> Hi Paul,
>
> On Thu, Jan 19, 2012 at 11:42 AM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > On Thu, Jan 19, 2012 at 11:28:56AM -0800, Simon Glass wrote:
> >> The synchronize_rcu() call resulting from making every serial driver
> >> wake-up capable (commit b3b708fa) slows boot down on my Tegra2x system
> >> (with CONFIG_PREEMPT disabled).
> >>
> >> But this is avoidable since it is the device_set_wakeup_enable() and then
> >> subsequence disable which causes the delay. We might as well just make
> >> the device wakeup capable but not actually enable it for wakeup until
> >> needed.
> >>
> >> Effectively the current code does this:
> >>
> >> device_set_wakeup_capable(dev, 1);
> >> device_set_wakeup_enable(dev, 1);
> >> device_set_wakeup_enable(dev, 0);
> >>
> >> We can just drop the last two lines.
> >>
> >> Before this change my boot log says:
> >> [ 0.227062] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
> >> [ 0.702928] serial8250.0: ttyS0 at MMIO 0x70006040 (irq = 69) is a Tegra
> >>
> >> after:
> >> [ 0.227264] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
> >> [ 0.227983] serial8250.0: ttyS0 at MMIO 0x70006040 (irq = 69) is a Tegra
> >>
> >> for saving of 450ms.
> >
> > You have multiple CPUs running at this point, correct? Before that
> > second CPU starts up, synchronize_rcu() is a no-op.
>
> Yes that's right, although I didn't get different behavior with 'nosmp'.
If you are running CONFIG_PREEMPT=y, that is expected behavior, though
that grace period is a bit on the long side. What is the value of HZ?
On the other hand, if you are running CONFIG_PREEMPT=n on recent kernels,
synchronize_rcu() will be a no-op any time that you are running with
only a single CPU.
The reason for this difference is that with CONFIG_PREEMPT=y, there might
be a preempted RCU reader, and RCU must check for this condition, waiting
for any such reader to complete. In contrast, when CONFIG_PREEMPT=n on a
single-CPU system, the fact that you are executing in synchronize_rcu()
guarantees that there are no running RCU readers, so synchronize_rcu()
need do nothing in that case.
Thanx, Paul
> > The patch looks good to me, but then again, I do not consider myself
> > qualified to have an opinion on the TTY layer. ;-)
>
> Thanks, me neither :-)
>
> Regards,
> Simon
>
> >
> > Thanx, Paul
> >
> >> Suggested-by: Rafael J. Wysocki <rjw@sisk.pl>
> >> Signed-off-by: Simon Glass <sjg@chromium.org>
> >> ---
> >> drivers/tty/serial/serial_core.c | 6 +++---
> >> 1 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> >> index c7bf31a..1305618 100644
> >> --- a/drivers/tty/serial/serial_core.c
> >> +++ b/drivers/tty/serial/serial_core.c
> >> @@ -2348,11 +2348,11 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
> >> */
> >> tty_dev = tty_register_device(drv->tty_driver, uport->line, uport->dev);
> >> if (likely(!IS_ERR(tty_dev))) {
> >> - device_init_wakeup(tty_dev, 1);
> >> - device_set_wakeup_enable(tty_dev, 0);
> >> - } else
> >> + device_set_wakeup_capable(tty_dev, 1);
> >> + } else {
> >> printk(KERN_ERR "Cannot register tty device on line %d\n",
> >> uport->line);
> >> + }
> >>
> >> /*
> >> * Ensure UPF_DEAD is not set.
> >> --
> >> 1.7.7.3
> >>
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] serial: Fix wakeup init logic to speed up startup
2012-01-19 19:28 [PATCH] serial: Fix wakeup init logic to speed up startup Simon Glass
2012-01-19 19:42 ` Paul E. McKenney
@ 2012-01-19 23:46 ` Rafael J. Wysocki
2012-01-19 23:57 ` Simon Glass
1 sibling, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2012-01-19 23:46 UTC (permalink / raw)
To: Simon Glass
Cc: LKML, Greg Kroah-Hartman, linux-serial, Alan Cox, Paul E. McKenney
On Thursday, January 19, 2012, Simon Glass wrote:
> The synchronize_rcu() call resulting from making every serial driver
> wake-up capable (commit b3b708fa) slows boot down on my Tegra2x system
> (with CONFIG_PREEMPT disabled).
>
> But this is avoidable since it is the device_set_wakeup_enable() and then
> subsequence disable which causes the delay. We might as well just make
> the device wakeup capable but not actually enable it for wakeup until
> needed.
>
> Effectively the current code does this:
>
> device_set_wakeup_capable(dev, 1);
> device_set_wakeup_enable(dev, 1);
> device_set_wakeup_enable(dev, 0);
>
> We can just drop the last two lines.
>
> Before this change my boot log says:
> [ 0.227062] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
> [ 0.702928] serial8250.0: ttyS0 at MMIO 0x70006040 (irq = 69) is a Tegra
>
> after:
> [ 0.227264] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
> [ 0.227983] serial8250.0: ttyS0 at MMIO 0x70006040 (irq = 69) is a Tegra
>
> for saving of 450ms.
>
> Suggested-by: Rafael J. Wysocki <rjw@sisk.pl>
> Signed-off-by: Simon Glass <sjg@chromium.org>
Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
Thanks!
> ---
> drivers/tty/serial/serial_core.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index c7bf31a..1305618 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -2348,11 +2348,11 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
> */
> tty_dev = tty_register_device(drv->tty_driver, uport->line, uport->dev);
> if (likely(!IS_ERR(tty_dev))) {
> - device_init_wakeup(tty_dev, 1);
> - device_set_wakeup_enable(tty_dev, 0);
> - } else
> + device_set_wakeup_capable(tty_dev, 1);
> + } else {
> printk(KERN_ERR "Cannot register tty device on line %d\n",
> uport->line);
> + }
>
> /*
> * Ensure UPF_DEAD is not set.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] serial: Fix wakeup init logic to speed up startup
2012-01-19 23:46 ` Rafael J. Wysocki
@ 2012-01-19 23:57 ` Simon Glass
0 siblings, 0 replies; 6+ messages in thread
From: Simon Glass @ 2012-01-19 23:57 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: LKML, Greg Kroah-Hartman, linux-serial, Alan Cox, Paul E. McKenney
Hi Rafael,
On Thu, Jan 19, 2012 at 3:46 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Thursday, January 19, 2012, Simon Glass wrote:
>> The synchronize_rcu() call resulting from making every serial driver
>> wake-up capable (commit b3b708fa) slows boot down on my Tegra2x system
>> (with CONFIG_PREEMPT disabled).
>>
>> But this is avoidable since it is the device_set_wakeup_enable() and then
>> subsequence disable which causes the delay. We might as well just make
>> the device wakeup capable but not actually enable it for wakeup until
>> needed.
>>
>> Effectively the current code does this:
>>
>> device_set_wakeup_capable(dev, 1);
>> device_set_wakeup_enable(dev, 1);
>> device_set_wakeup_enable(dev, 0);
>>
>> We can just drop the last two lines.
>>
>> Before this change my boot log says:
>> [ 0.227062] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
>> [ 0.702928] serial8250.0: ttyS0 at MMIO 0x70006040 (irq = 69) is a Tegra
>>
>> after:
>> [ 0.227264] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
>> [ 0.227983] serial8250.0: ttyS0 at MMIO 0x70006040 (irq = 69) is a Tegra
>>
>> for saving of 450ms.
>>
>> Suggested-by: Rafael J. Wysocki <rjw@sisk.pl>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
>
> Thanks!
Thanks for your help with this.
Regards,
Simon
>
>> ---
>> drivers/tty/serial/serial_core.c | 6 +++---
>> 1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
>> index c7bf31a..1305618 100644
>> --- a/drivers/tty/serial/serial_core.c
>> +++ b/drivers/tty/serial/serial_core.c
>> @@ -2348,11 +2348,11 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
>> */
>> tty_dev = tty_register_device(drv->tty_driver, uport->line, uport->dev);
>> if (likely(!IS_ERR(tty_dev))) {
>> - device_init_wakeup(tty_dev, 1);
>> - device_set_wakeup_enable(tty_dev, 0);
>> - } else
>> + device_set_wakeup_capable(tty_dev, 1);
>> + } else {
>> printk(KERN_ERR "Cannot register tty device on line %d\n",
>> uport->line);
>> + }
>>
>> /*
>> * Ensure UPF_DEAD is not set.
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-01-20 0:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-19 19:28 [PATCH] serial: Fix wakeup init logic to speed up startup Simon Glass
2012-01-19 19:42 ` Paul E. McKenney
2012-01-19 19:51 ` Simon Glass
2012-01-19 20:12 ` Paul E. McKenney
2012-01-19 23:46 ` Rafael J. Wysocki
2012-01-19 23:57 ` Simon Glass
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).