* [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