[Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Rafael J. Wysocki]

Hi Alan,

As stated at http://bugzilla.kernel.org/show_bug.cgi?id=14229, your commit
b50989dc444599c8b21edc23536fc305f4e9b7d5
(tty: make the kref destructor occur asynchronously) appears to have broken
resume from hibernation on MSI Wind U100.

I verified that hibernation works correctly with this commit reverted.

I'm still not sure what the root cause of the problem is, though.

Thanks,
Rafael

Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Alan Cox]

On Sat, 26 Sep 2009 00:38:21 +0200
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> Hi Alan,
> 
> As stated at http://bugzilla.kernel.org/show_bug.cgi?id=14229, your commit
> b50989dc444599c8b21edc23536fc305f4e9b7d5
> (tty: make the kref destructor occur asynchronously) appears to have broken
> resume from hibernation on MSI Wind U100.
> 
> I verified that hibernation works correctly with this commit reverted.
> 
> I'm still not sure what the root cause of the problem is, though.

The root cause is that the patch to fix the console async/sync behaviour
went into Andrews tree, back out of it and vanished.

It's a known bug, with a known confirmed fix. You may want to pick it up
and submit it as there is no tty maintainer currently.

Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Alan Cox]

On Sat, 26 Sep 2009 00:38:21 +0200
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> Hi Alan,
> 
> As stated at http://bugzilla.kernel.org/show_bug.cgi?id=14229, your commit
> b50989dc444599c8b21edc23536fc305f4e9b7d5
> (tty: make the kref destructor occur asynchronously) appears to have broken
> resume from hibernation on MSI Wind U100.
> 
> I verified that hibernation works correctly with this commit reverted.
> 
> I'm still not sure what the root cause of the problem is, though.

The root cause is that the patch to fix the console async/sync behaviour
went into Andrews tree, back out of it and vanished.

It's a known bug, with a known confirmed fix. You may want to pick it up
and submit it as there is no tty maintainer currently.

Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Rafael J. Wysocki]

On Sunday 27 September 2009, Alan Cox wrote:
> On Sat, 26 Sep 2009 00:38:21 +0200
> "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> 
> > Hi Alan,
> > 
> > As stated at http://bugzilla.kernel.org/show_bug.cgi?id=14229, your commit
> > b50989dc444599c8b21edc23536fc305f4e9b7d5
> > (tty: make the kref destructor occur asynchronously) appears to have broken
> > resume from hibernation on MSI Wind U100.
> > 
> > I verified that hibernation works correctly with this commit reverted.
> > 
> > I'm still not sure what the root cause of the problem is, though.
> 
> The root cause is that the patch to fix the console async/sync behaviour
> went into Andrews tree, back out of it and vanished.
> 
> It's a known bug, with a known confirmed fix. You may want to pick it up
> and submit it as there is no tty maintainer currently.

Thanks, I guess it's this one:
tty-fix-regression-caused-by-tty-make-the-kref-destructor-occur-asynchronously.patch ?

Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Rafael J. Wysocki]

On Sunday 27 September 2009, Alan Cox wrote:
> On Sat, 26 Sep 2009 00:38:21 +0200
> "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> 
> > Hi Alan,
> > 
> > As stated at http://bugzilla.kernel.org/show_bug.cgi?id=14229, your commit
> > b50989dc444599c8b21edc23536fc305f4e9b7d5
> > (tty: make the kref destructor occur asynchronously) appears to have broken
> > resume from hibernation on MSI Wind U100.
> > 
> > I verified that hibernation works correctly with this commit reverted.
> > 
> > I'm still not sure what the root cause of the problem is, though.
> 
> The root cause is that the patch to fix the console async/sync behaviour
> went into Andrews tree, back out of it and vanished.
> 
> It's a known bug, with a known confirmed fix. You may want to pick it up
> and submit it as there is no tty maintainer currently.

Thanks, I guess it's this one:
tty-fix-regression-caused-by-tty-make-the-kref-destructor-occur-asynchronously.patch ?

[PATCH, fix] Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Rafael J. Wysocki]

On Sunday 27 September 2009, Rafael J. Wysocki wrote:
> On Sunday 27 September 2009, Alan Cox wrote:
> > On Sat, 26 Sep 2009 00:38:21 +0200
> > "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > 
> > > Hi Alan,
> > > 
> > > As stated at http://bugzilla.kernel.org/show_bug.cgi?id=14229, your commit
> > > b50989dc444599c8b21edc23536fc305f4e9b7d5
> > > (tty: make the kref destructor occur asynchronously) appears to have broken
> > > resume from hibernation on MSI Wind U100.
> > > 
> > > I verified that hibernation works correctly with this commit reverted.
> > > 
> > > I'm still not sure what the root cause of the problem is, though.
> > 
> > The root cause is that the patch to fix the console async/sync behaviour
> > went into Andrews tree, back out of it and vanished.
> > 
> > It's a known bug, with a known confirmed fix. You may want to pick it up
> > and submit it as there is no tty maintainer currently.
> 
> Thanks, I guess it's this one:
> tty-fix-regression-caused-by-tty-make-the-kref-destructor-occur-asynchronously.patch ?

Tested, works.

Greg, could you please consider taking the patch below?  It fixes a recent
hibernation regression for me, so if not this one, another fix is necessary.

Best,
Rafael

---
From: Dave Young <hidave.darkstar@gmail.com>
Subject: tty: Fix regressions caused by commit b50989dc

The following commit made console open fails while booting:

	commit b50989dc444599c8b21edc23536fc305f4e9b7d5
	Author: Alan Cox <alan@linux.intel.com>
	Date:   Sat Sep 19 13:13:22 2009 -0700

	tty: make the kref destructor occur asynchronously

Due to tty release routines run in a workqueue now, error like the
following will be reported while booting:

INIT open /dev/console Input/output error

It also causes hibernation regression to appear as reported at
http://bugzilla.kernel.org/show_bug.cgi?id=14229

The reason is that now there's latency issue with closing, but when
we open a "closing not finished" tty, -EIO will be returned.

Fix it as per the following Alan's suggestion:

Fun but it's actually not a bug and the fix is wrong in itself as
the port may be closing but not yet being destructed, in which case
it seems to do the wrong thing.  Opening a tty that is closing (and
could be closing for long periods) is supposed to return -EIO.

I suspect a better way to deal with this and keep the old console
timing is to split tty->shutdown into two functions.

tty->shutdown() - called synchronously just before we dump the tty
onto the waitqueue for destruction

tty->cleanup() - called when the destructor runs.

We would then do the shutdown part which can occur in IRQ context
fine, before queueing the rest of the release (from tty->magic = 0
...  the end) to occur asynchronously

The USB update in -next would then need a call like

       if (tty->cleanup)
               tty->cleanup(tty);

at the top of the async function and the USB shutdown to be split
between shutdown and cleanup as the USB resource cleanup and final
tidy cannot occur synchronously as it needs to sleep.

In other words the logic becomes

       final kref put
               make object unfindable

       async
               clean it up

[rjw: Rebased on top of 2.6.31-git, reworked the changelog.]

Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
Signed-off-by: "Rafael J. Wysocki" <rjw@sisk.pl>
---

 drivers/char/tty_io.c           |   15 ++++++++++-----
 drivers/usb/serial/usb-serial.c |    2 +-
 include/linux/tty_driver.h      |   13 +++++++++++--
 3 files changed, 22 insertions(+), 8 deletions(-)

Index: linux-2.6/drivers/char/tty_io.c
===================================================================
--- linux-2.6.orig/drivers/char/tty_io.c
+++ linux-2.6/drivers/char/tty_io.c
@@ -1389,7 +1389,7 @@ EXPORT_SYMBOL(tty_shutdown);
  *	of ttys that the driver keeps.
  *
  *	This method gets called from a work queue so that the driver private
- *	shutdown ops can sleep (needed for USB at least)
+ *	cleanup ops can sleep (needed for USB at least)
  */
 static void release_one_tty(struct work_struct *work)
 {
@@ -1397,10 +1397,9 @@ static void release_one_tty(struct work_
 		container_of(work, struct tty_struct, hangup_work);
 	struct tty_driver *driver = tty->driver;
 
-	if (tty->ops->shutdown)
-		tty->ops->shutdown(tty);
-	else
-		tty_shutdown(tty);
+	if (tty->ops->cleanup)
+		tty->ops->cleanup(tty);
+
 	tty->magic = 0;
 	tty_driver_kref_put(driver);
 	module_put(driver->owner);
@@ -1415,6 +1414,12 @@ static void release_one_tty(struct work_
 static void queue_release_one_tty(struct kref *kref)
 {
 	struct tty_struct *tty = container_of(kref, struct tty_struct, kref);
+
+	if (tty->ops->shutdown)
+		tty->ops->shutdown(tty);
+	else
+		tty_shutdown(tty);
+
 	/* The hangup queue is now free so we can reuse it rather than
 	   waste a chunk of memory for each port */
 	INIT_WORK(&tty->hangup_work, release_one_tty);
Index: linux-2.6/drivers/usb/serial/usb-serial.c
===================================================================
--- linux-2.6.orig/drivers/usb/serial/usb-serial.c
+++ linux-2.6/drivers/usb/serial/usb-serial.c
@@ -1210,7 +1210,7 @@ static const struct tty_operations seria
 	.chars_in_buffer =	serial_chars_in_buffer,
 	.tiocmget =		serial_tiocmget,
 	.tiocmset =		serial_tiocmset,
-	.shutdown = 		serial_release,
+	.cleanup = 		serial_release,
 	.install = 		serial_install,
 	.proc_fops =		&serial_proc_fops,
 };
Index: linux-2.6/include/linux/tty_driver.h
===================================================================
--- linux-2.6.orig/include/linux/tty_driver.h
+++ linux-2.6/include/linux/tty_driver.h
@@ -45,8 +45,16 @@
  *
  * void (*shutdown)(struct tty_struct * tty);
  *
- * 	This routine is called when a particular tty device is closed for
- *	the last time freeing up the resources.
+ * 	This routine is called synchronously when a particular tty device
+ *	is closed for the last time freeing up the resources.
+ *
+ *
+ * void (*cleanup)(struct tty_struct * tty);
+ *
+ *	This routine is called asynchronously when a particular tty device
+ *	is closed for the last time freeing up the resources. This is
+ *	actually the second part of shutdown for routines that might sleep.
+ *
  *
  * int (*write)(struct tty_struct * tty,
  * 		 const unsigned char *buf, int count);
@@ -233,6 +241,7 @@ struct tty_operations {
 	int  (*open)(struct tty_struct * tty, struct file * filp);
 	void (*close)(struct tty_struct * tty, struct file * filp);
 	void (*shutdown)(struct tty_struct *tty);
+	void (*cleanup)(struct tty_struct *tty);
 	int  (*write)(struct tty_struct * tty,
 		      const unsigned char *buf, int count);
 	int  (*put_char)(struct tty_struct *tty, unsigned char ch);

[PATCH, fix] Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Rafael J. Wysocki]

On Sunday 27 September 2009, Rafael J. Wysocki wrote:
> On Sunday 27 September 2009, Alan Cox wrote:
> > On Sat, 26 Sep 2009 00:38:21 +0200
> > "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > 
> > > Hi Alan,
> > > 
> > > As stated at http://bugzilla.kernel.org/show_bug.cgi?id=14229, your commit
> > > b50989dc444599c8b21edc23536fc305f4e9b7d5
> > > (tty: make the kref destructor occur asynchronously) appears to have broken
> > > resume from hibernation on MSI Wind U100.
> > > 
> > > I verified that hibernation works correctly with this commit reverted.
> > > 
> > > I'm still not sure what the root cause of the problem is, though.
> > 
> > The root cause is that the patch to fix the console async/sync behaviour
> > went into Andrews tree, back out of it and vanished.
> > 
> > It's a known bug, with a known confirmed fix. You may want to pick it up
> > and submit it as there is no tty maintainer currently.
> 
> Thanks, I guess it's this one:
> tty-fix-regression-caused-by-tty-make-the-kref-destructor-occur-asynchronously.patch ?

Tested, works.

Greg, could you please consider taking the patch below?  It fixes a recent
hibernation regression for me, so if not this one, another fix is necessary.

Best,
Rafael

---
From: Dave Young <hidave.darkstar@gmail.com>
Subject: tty: Fix regressions caused by commit b50989dc

The following commit made console open fails while booting:

	commit b50989dc444599c8b21edc23536fc305f4e9b7d5
	Author: Alan Cox <alan@linux.intel.com>
	Date:   Sat Sep 19 13:13:22 2009 -0700

	tty: make the kref destructor occur asynchronously

Due to tty release routines run in a workqueue now, error like the
following will be reported while booting:

INIT open /dev/console Input/output error

It also causes hibernation regression to appear as reported at
http://bugzilla.kernel.org/show_bug.cgi?id=14229

The reason is that now there's latency issue with closing, but when
we open a "closing not finished" tty, -EIO will be returned.

Fix it as per the following Alan's suggestion:

Fun but it's actually not a bug and the fix is wrong in itself as
the port may be closing but not yet being destructed, in which case
it seems to do the wrong thing.  Opening a tty that is closing (and
could be closing for long periods) is supposed to return -EIO.

I suspect a better way to deal with this and keep the old console
timing is to split tty->shutdown into two functions.

tty->shutdown() - called synchronously just before we dump the tty
onto the waitqueue for destruction

tty->cleanup() - called when the destructor runs.

We would then do the shutdown part which can occur in IRQ context
fine, before queueing the rest of the release (from tty->magic = 0
...  the end) to occur asynchronously

The USB update in -next would then need a call like

       if (tty->cleanup)
               tty->cleanup(tty);

at the top of the async function and the USB shutdown to be split
between shutdown and cleanup as the USB resource cleanup and final
tidy cannot occur synchronously as it needs to sleep.

In other words the logic becomes

       final kref put
               make object unfindable

       async
               clean it up

[rjw: Rebased on top of 2.6.31-git, reworked the changelog.]

Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
Signed-off-by: "Rafael J. Wysocki" <rjw@sisk.pl>
---

 drivers/char/tty_io.c           |   15 ++++++++++-----
 drivers/usb/serial/usb-serial.c |    2 +-
 include/linux/tty_driver.h      |   13 +++++++++++--
 3 files changed, 22 insertions(+), 8 deletions(-)

Index: linux-2.6/drivers/char/tty_io.c
===================================================================
--- linux-2.6.orig/drivers/char/tty_io.c
+++ linux-2.6/drivers/char/tty_io.c
@@ -1389,7 +1389,7 @@ EXPORT_SYMBOL(tty_shutdown);
  *	of ttys that the driver keeps.
  *
  *	This method gets called from a work queue so that the driver private
- *	shutdown ops can sleep (needed for USB at least)
+ *	cleanup ops can sleep (needed for USB at least)
  */
 static void release_one_tty(struct work_struct *work)
 {
@@ -1397,10 +1397,9 @@ static void release_one_tty(struct work_
 		container_of(work, struct tty_struct, hangup_work);
 	struct tty_driver *driver = tty->driver;
 
-	if (tty->ops->shutdown)
-		tty->ops->shutdown(tty);
-	else
-		tty_shutdown(tty);
+	if (tty->ops->cleanup)
+		tty->ops->cleanup(tty);
+
 	tty->magic = 0;
 	tty_driver_kref_put(driver);
 	module_put(driver->owner);
@@ -1415,6 +1414,12 @@ static void release_one_tty(struct work_
 static void queue_release_one_tty(struct kref *kref)
 {
 	struct tty_struct *tty = container_of(kref, struct tty_struct, kref);
+
+	if (tty->ops->shutdown)
+		tty->ops->shutdown(tty);
+	else
+		tty_shutdown(tty);
+
 	/* The hangup queue is now free so we can reuse it rather than
 	   waste a chunk of memory for each port */
 	INIT_WORK(&tty->hangup_work, release_one_tty);
Index: linux-2.6/drivers/usb/serial/usb-serial.c
===================================================================
--- linux-2.6.orig/drivers/usb/serial/usb-serial.c
+++ linux-2.6/drivers/usb/serial/usb-serial.c
@@ -1210,7 +1210,7 @@ static const struct tty_operations seria
 	.chars_in_buffer =	serial_chars_in_buffer,
 	.tiocmget =		serial_tiocmget,
 	.tiocmset =		serial_tiocmset,
-	.shutdown = 		serial_release,
+	.cleanup = 		serial_release,
 	.install = 		serial_install,
 	.proc_fops =		&serial_proc_fops,
 };
Index: linux-2.6/include/linux/tty_driver.h
===================================================================
--- linux-2.6.orig/include/linux/tty_driver.h
+++ linux-2.6/include/linux/tty_driver.h
@@ -45,8 +45,16 @@
  *
  * void (*shutdown)(struct tty_struct * tty);
  *
- * 	This routine is called when a particular tty device is closed for
- *	the last time freeing up the resources.
+ * 	This routine is called synchronously when a particular tty device
+ *	is closed for the last time freeing up the resources.
+ *
+ *
+ * void (*cleanup)(struct tty_struct * tty);
+ *
+ *	This routine is called asynchronously when a particular tty device
+ *	is closed for the last time freeing up the resources. This is
+ *	actually the second part of shutdown for routines that might sleep.
+ *
  *
  * int (*write)(struct tty_struct * tty,
  * 		 const unsigned char *buf, int count);
@@ -233,6 +241,7 @@ struct tty_operations {
 	int  (*open)(struct tty_struct * tty, struct file * filp);
 	void (*close)(struct tty_struct * tty, struct file * filp);
 	void (*shutdown)(struct tty_struct *tty);
+	void (*cleanup)(struct tty_struct *tty);
 	int  (*write)(struct tty_struct * tty,
 		      const unsigned char *buf, int count);
 	int  (*put_char)(struct tty_struct *tty, unsigned char ch);

Re: [linux-pm] [PATCH, fix] Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Alan Stern]

On Sun, 27 Sep 2009, Rafael J. Wysocki wrote:

> Tested, works.
> 
> Greg, could you please consider taking the patch below?  It fixes a recent
> hibernation regression for me, so if not this one, another fix is necessary.

This patch has a mistake.

> Index: linux-2.6/drivers/usb/serial/usb-serial.c
> ===================================================================
> --- linux-2.6.orig/drivers/usb/serial/usb-serial.c
> +++ linux-2.6/drivers/usb/serial/usb-serial.c
> @@ -1210,7 +1210,7 @@ static const struct tty_operations seria
>  	.chars_in_buffer =	serial_chars_in_buffer,
>  	.tiocmget =		serial_tiocmget,
>  	.tiocmset =		serial_tiocmset,
> -	.shutdown = 		serial_release,
> +	.cleanup = 		serial_release,
>  	.install = 		serial_install,
>  	.proc_fops =		&serial_proc_fops,
>  };

It isn't enough to change the method pointer.  The code in 
serial_release() has to be changed too; it must not call tty_shutdown() 
any more.

Alan Stern

Re: [PATCH, fix] Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Alan Stern]

On Sun, 27 Sep 2009, Rafael J. Wysocki wrote:

> Tested, works.
> 
> Greg, could you please consider taking the patch below?  It fixes a recent
> hibernation regression for me, so if not this one, another fix is necessary.

This patch has a mistake.

> Index: linux-2.6/drivers/usb/serial/usb-serial.c
> ===================================================================
> --- linux-2.6.orig/drivers/usb/serial/usb-serial.c
> +++ linux-2.6/drivers/usb/serial/usb-serial.c
> @@ -1210,7 +1210,7 @@ static const struct tty_operations seria
>  	.chars_in_buffer =	serial_chars_in_buffer,
>  	.tiocmget =		serial_tiocmget,
>  	.tiocmset =		serial_tiocmset,
> -	.shutdown = 		serial_release,
> +	.cleanup = 		serial_release,
>  	.install = 		serial_install,
>  	.proc_fops =		&serial_proc_fops,
>  };

It isn't enough to change the method pointer.  The code in 
serial_release() has to be changed too; it must not call tty_shutdown() 
any more.

Alan Stern

Re: [linux-pm] [PATCH, fix] Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Rafael J. Wysocki]

On Sunday 27 September 2009, Alan Stern wrote:
> On Sun, 27 Sep 2009, Rafael J. Wysocki wrote:
> 
> > Tested, works.
> > 
> > Greg, could you please consider taking the patch below?  It fixes a recent
> > hibernation regression for me, so if not this one, another fix is necessary.
> 
> This patch has a mistake.
> 
> > Index: linux-2.6/drivers/usb/serial/usb-serial.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/usb/serial/usb-serial.c
> > +++ linux-2.6/drivers/usb/serial/usb-serial.c
> > @@ -1210,7 +1210,7 @@ static const struct tty_operations seria
> >  	.chars_in_buffer =	serial_chars_in_buffer,
> >  	.tiocmget =		serial_tiocmget,
> >  	.tiocmset =		serial_tiocmset,
> > -	.shutdown = 		serial_release,
> > +	.cleanup = 		serial_release,
> >  	.install = 		serial_install,
> >  	.proc_fops =		&serial_proc_fops,
> >  };
> 
> It isn't enough to change the method pointer.  The code in 
> serial_release() has to be changed too; it must not call tty_shutdown() 
> any more.

Would it be sufficient to remove the tty_shutdown() call from
serial_release()?

Rafael

Re: [PATCH, fix] Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Rafael J. Wysocki]

On Sunday 27 September 2009, Alan Stern wrote:
> On Sun, 27 Sep 2009, Rafael J. Wysocki wrote:
> 
> > Tested, works.
> > 
> > Greg, could you please consider taking the patch below?  It fixes a recent
> > hibernation regression for me, so if not this one, another fix is necessary.
> 
> This patch has a mistake.
> 
> > Index: linux-2.6/drivers/usb/serial/usb-serial.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/usb/serial/usb-serial.c
> > +++ linux-2.6/drivers/usb/serial/usb-serial.c
> > @@ -1210,7 +1210,7 @@ static const struct tty_operations seria
> >  	.chars_in_buffer =	serial_chars_in_buffer,
> >  	.tiocmget =		serial_tiocmget,
> >  	.tiocmset =		serial_tiocmset,
> > -	.shutdown = 		serial_release,
> > +	.cleanup = 		serial_release,
> >  	.install = 		serial_install,
> >  	.proc_fops =		&serial_proc_fops,
> >  };
> 
> It isn't enough to change the method pointer.  The code in 
> serial_release() has to be changed too; it must not call tty_shutdown() 
> any more.

Would it be sufficient to remove the tty_shutdown() call from
serial_release()?

Rafael

Re: [linux-pm] [PATCH, fix] Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Dave Young]

On Mon, Sep 28, 2009 at 2:16 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Sunday 27 September 2009, Alan Stern wrote:
>> On Sun, 27 Sep 2009, Rafael J. Wysocki wrote:
>>
>> > Tested, works.
>> >
>> > Greg, could you please consider taking the patch below?  It fixes a recent
>> > hibernation regression for me, so if not this one, another fix is necessary.
>>
>> This patch has a mistake.
>>
>> > Index: linux-2.6/drivers/usb/serial/usb-serial.c
>> > ===================================================================
>> > --- linux-2.6.orig/drivers/usb/serial/usb-serial.c
>> > +++ linux-2.6/drivers/usb/serial/usb-serial.c
>> > @@ -1210,7 +1210,7 @@ static const struct tty_operations seria
>> >     .chars_in_buffer =      serial_chars_in_buffer,
>> >     .tiocmget =             serial_tiocmget,
>> >     .tiocmset =             serial_tiocmset,
>> > -   .shutdown =             serial_release,
>> > +   .cleanup =              serial_release,
>> >     .install =              serial_install,
>> >     .proc_fops =            &serial_proc_fops,
>> >  };
>>
>> It isn't enough to change the method pointer.  The code in
>> serial_release() has to be changed too; it must not call tty_shutdown()
>> any more.
>
> Would it be sufficient to remove the tty_shutdown() call from
> serial_release()?

I think so, because standard shutdown will be called in queue_release_one_tty.

Alan, could you confirm about this?  Thus I'd like to update the patch.



>
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>



-- 
Regards
dave

Re: [PATCH, fix] Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Dave Young]

On Mon, Sep 28, 2009 at 2:16 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Sunday 27 September 2009, Alan Stern wrote:
>> On Sun, 27 Sep 2009, Rafael J. Wysocki wrote:
>>
>> > Tested, works.
>> >
>> > Greg, could you please consider taking the patch below?  It fixes a recent
>> > hibernation regression for me, so if not this one, another fix is necessary.
>>
>> This patch has a mistake.
>>
>> > Index: linux-2.6/drivers/usb/serial/usb-serial.c
>> > ===================================================================
>> > --- linux-2.6.orig/drivers/usb/serial/usb-serial.c
>> > +++ linux-2.6/drivers/usb/serial/usb-serial.c
>> > @@ -1210,7 +1210,7 @@ static const struct tty_operations seria
>> >     .chars_in_buffer =      serial_chars_in_buffer,
>> >     .tiocmget =             serial_tiocmget,
>> >     .tiocmset =             serial_tiocmset,
>> > -   .shutdown =             serial_release,
>> > +   .cleanup =              serial_release,
>> >     .install =              serial_install,
>> >     .proc_fops =            &serial_proc_fops,
>> >  };
>>
>> It isn't enough to change the method pointer.  The code in
>> serial_release() has to be changed too; it must not call tty_shutdown()
>> any more.
>
> Would it be sufficient to remove the tty_shutdown() call from
> serial_release()?

I think so, because standard shutdown will be called in queue_release_one_tty.

Alan, could you confirm about this?  Thus I'd like to update the patch.



>
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>



-- 
Regards
dave
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

Re: [linux-pm] [PATCH, fix] Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Alan Stern]

On Mon, 28 Sep 2009, Dave Young wrote:

> On Mon, Sep 28, 2009 at 2:16 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Sunday 27 September 2009, Alan Stern wrote:
> >> On Sun, 27 Sep 2009, Rafael J. Wysocki wrote:
> >>
> >> > Tested, works.
> >> >
> >> > Greg, could you please consider taking the patch below?  It fixes a recent
> >> > hibernation regression for me, so if not this one, another fix is necessary.
> >>
> >> This patch has a mistake.
> >>
> >> > Index: linux-2.6/drivers/usb/serial/usb-serial.c
> >> > ===================================================================
> >> > --- linux-2.6.orig/drivers/usb/serial/usb-serial.c
> >> > +++ linux-2.6/drivers/usb/serial/usb-serial.c
> >> > @@ -1210,7 +1210,7 @@ static const struct tty_operations seria
> >> >     .chars_in_buffer =      serial_chars_in_buffer,
> >> >     .tiocmget =             serial_tiocmget,
> >> >     .tiocmset =             serial_tiocmset,
> >> > -   .shutdown =             serial_release,
> >> > +   .cleanup =              serial_release,
> >> >     .install =              serial_install,
> >> >     .proc_fops =            &serial_proc_fops,
> >> >  };
> >>
> >> It isn't enough to change the method pointer.  The code in
> >> serial_release() has to be changed too; it must not call tty_shutdown()
> >> any more.
> >
> > Would it be sufficient to remove the tty_shutdown() call from
> > serial_release()?
> 
> I think so, because standard shutdown will be called in queue_release_one_tty.
> 
> Alan, could you confirm about this?  Thus I'd like to update the patch.

Yes, that's right.  Just remove the function call.  But what happens if
the device is a serial console?  Shouldn't the call to tty_shutdown()  
be skipped in that case?  (Eventually this won't matter; the console
code will be straightened out so that we never release a serial console
device.  But for now it's important.)

If you think it would be appropriate, you could also rename
serial_release() to serial_cleanup() -- it's up to you.

Alan Stern

Re: [PATCH, fix] Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Alan Stern]

On Mon, 28 Sep 2009, Dave Young wrote:

> On Mon, Sep 28, 2009 at 2:16 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Sunday 27 September 2009, Alan Stern wrote:
> >> On Sun, 27 Sep 2009, Rafael J. Wysocki wrote:
> >>
> >> > Tested, works.
> >> >
> >> > Greg, could you please consider taking the patch below?  It fixes a recent
> >> > hibernation regression for me, so if not this one, another fix is necessary.
> >>
> >> This patch has a mistake.
> >>
> >> > Index: linux-2.6/drivers/usb/serial/usb-serial.c
> >> > ===================================================================
> >> > --- linux-2.6.orig/drivers/usb/serial/usb-serial.c
> >> > +++ linux-2.6/drivers/usb/serial/usb-serial.c
> >> > @@ -1210,7 +1210,7 @@ static const struct tty_operations seria
> >> >     .chars_in_buffer =      serial_chars_in_buffer,
> >> >     .tiocmget =             serial_tiocmget,
> >> >     .tiocmset =             serial_tiocmset,
> >> > -   .shutdown =             serial_release,
> >> > +   .cleanup =              serial_release,
> >> >     .install =              serial_install,
> >> >     .proc_fops =            &serial_proc_fops,
> >> >  };
> >>
> >> It isn't enough to change the method pointer.  The code in
> >> serial_release() has to be changed too; it must not call tty_shutdown()
> >> any more.
> >
> > Would it be sufficient to remove the tty_shutdown() call from
> > serial_release()?
> 
> I think so, because standard shutdown will be called in queue_release_one_tty.
> 
> Alan, could you confirm about this?  Thus I'd like to update the patch.

Yes, that's right.  Just remove the function call.  But what happens if
the device is a serial console?  Shouldn't the call to tty_shutdown()  
be skipped in that case?  (Eventually this won't matter; the console
code will be straightened out so that we never release a serial console
device.  But for now it's important.)

If you think it would be appropriate, you could also rename
serial_release() to serial_cleanup() -- it's up to you.

Alan Stern

Re: [linux-pm] [PATCH, fix] Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Rafael J. Wysocki]

On Monday 28 September 2009, Alan Stern wrote:
> On Mon, 28 Sep 2009, Dave Young wrote:
> 
> > On Mon, Sep 28, 2009 at 2:16 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > > On Sunday 27 September 2009, Alan Stern wrote:
> > >> On Sun, 27 Sep 2009, Rafael J. Wysocki wrote:
> > >>
> > >> > Tested, works.
> > >> >
> > >> > Greg, could you please consider taking the patch below? Â It fixes a recent
> > >> > hibernation regression for me, so if not this one, another fix is necessary.
> > >>
> > >> This patch has a mistake.
> > >>
> > >> > Index: linux-2.6/drivers/usb/serial/usb-serial.c
> > >> > ===================================================================
> > >> > --- linux-2.6.orig/drivers/usb/serial/usb-serial.c
> > >> > +++ linux-2.6/drivers/usb/serial/usb-serial.c
> > >> > @@ -1210,7 +1210,7 @@ static const struct tty_operations seria
> > >> > Â  Â  .chars_in_buffer = Â  Â  Â serial_chars_in_buffer,
> > >> > Â  Â  .tiocmget = Â  Â  Â  Â  Â  Â  serial_tiocmget,
> > >> > Â  Â  .tiocmset = Â  Â  Â  Â  Â  Â  serial_tiocmset,
> > >> > - Â  .shutdown = Â  Â  Â  Â  Â  Â  serial_release,
> > >> > + Â  .cleanup = Â  Â  Â  Â  Â  Â  Â serial_release,
> > >> > Â  Â  .install = Â  Â  Â  Â  Â  Â  Â serial_install,
> > >> > Â  Â  .proc_fops = Â  Â  Â  Â  Â  Â &serial_proc_fops,
> > >> > Â };
> > >>
> > >> It isn't enough to change the method pointer. Â The code in
> > >> serial_release() has to be changed too; it must not call tty_shutdown()
> > >> any more.
> > >
> > > Would it be sufficient to remove the tty_shutdown() call from
> > > serial_release()?
> > 
> > I think so, because standard shutdown will be called in queue_release_one_tty.
> > 
> > Alan, could you confirm about this?  Thus I'd like to update the patch.
> 
> Yes, that's right.  Just remove the function call.  But what happens if
> the device is a serial console?  Shouldn't the call to tty_shutdown()  
> be skipped in that case?  (Eventually this won't matter; the console
> code will be straightened out so that we never release a serial console
> device.  But for now it's important.)
> 
> If you think it would be appropriate, you could also rename
> serial_release() to serial_cleanup() -- it's up to you.

The patch has been fixed up by Linus and merged as
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=f278a2f7bbc2239f479eaf63d0b3ae573b1d746c

If any more fixes are necessary, they'll have to go on top of it.

Best,
Rafael

Re: [PATCH, fix] Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Rafael J. Wysocki]

On Monday 28 September 2009, Alan Stern wrote:
> On Mon, 28 Sep 2009, Dave Young wrote:
> 
> > On Mon, Sep 28, 2009 at 2:16 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > > On Sunday 27 September 2009, Alan Stern wrote:
> > >> On Sun, 27 Sep 2009, Rafael J. Wysocki wrote:
> > >>
> > >> > Tested, works.
> > >> >
> > >> > Greg, could you please consider taking the patch below? Â It fixes a recent
> > >> > hibernation regression for me, so if not this one, another fix is necessary.
> > >>
> > >> This patch has a mistake.
> > >>
> > >> > Index: linux-2.6/drivers/usb/serial/usb-serial.c
> > >> > ===================================================================
> > >> > --- linux-2.6.orig/drivers/usb/serial/usb-serial.c
> > >> > +++ linux-2.6/drivers/usb/serial/usb-serial.c
> > >> > @@ -1210,7 +1210,7 @@ static const struct tty_operations seria
> > >> > Â  Â  .chars_in_buffer = Â  Â  Â serial_chars_in_buffer,
> > >> > Â  Â  .tiocmget = Â  Â  Â  Â  Â  Â  serial_tiocmget,
> > >> > Â  Â  .tiocmset = Â  Â  Â  Â  Â  Â  serial_tiocmset,
> > >> > - Â  .shutdown = Â  Â  Â  Â  Â  Â  serial_release,
> > >> > + Â  .cleanup = Â  Â  Â  Â  Â  Â  Â serial_release,
> > >> > Â  Â  .install = Â  Â  Â  Â  Â  Â  Â serial_install,
> > >> > Â  Â  .proc_fops = Â  Â  Â  Â  Â  Â &serial_proc_fops,
> > >> > Â };
> > >>
> > >> It isn't enough to change the method pointer. Â The code in
> > >> serial_release() has to be changed too; it must not call tty_shutdown()
> > >> any more.
> > >
> > > Would it be sufficient to remove the tty_shutdown() call from
> > > serial_release()?
> > 
> > I think so, because standard shutdown will be called in queue_release_one_tty.
> > 
> > Alan, could you confirm about this?  Thus I'd like to update the patch.
> 
> Yes, that's right.  Just remove the function call.  But what happens if
> the device is a serial console?  Shouldn't the call to tty_shutdown()  
> be skipped in that case?  (Eventually this won't matter; the console
> code will be straightened out so that we never release a serial console
> device.  But for now it's important.)
> 
> If you think it would be appropriate, you could also rename
> serial_release() to serial_cleanup() -- it's up to you.

The patch has been fixed up by Linus and merged as
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=f278a2f7bbc2239f479eaf63d0b3ae573b1d746c

If any more fixes are necessary, they'll have to go on top of it.

Best,
Rafael

Re: [linux-pm] [PATCH, fix] Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Dave Young]

On Tue, Sep 29, 2009 at 4:20 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Monday 28 September 2009, Alan Stern wrote:
>> On Mon, 28 Sep 2009, Dave Young wrote:
>>
>> > On Mon, Sep 28, 2009 at 2:16 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > > On Sunday 27 September 2009, Alan Stern wrote:
>> > >> On Sun, 27 Sep 2009, Rafael J. Wysocki wrote:
>> > >>
>> > >> > Tested, works.
>> > >> >
>> > >> > Greg, could you please consider taking the patch below? Â It fixes a recent
>> > >> > hibernation regression for me, so if not this one, another fix is necessary.
>> > >>
>> > >> This patch has a mistake.
>> > >>
>> > >> > Index: linux-2.6/drivers/usb/serial/usb-serial.c
>> > >> > ===================================================================
>> > >> > --- linux-2.6.orig/drivers/usb/serial/usb-serial.c
>> > >> > +++ linux-2.6/drivers/usb/serial/usb-serial.c
>> > >> > @@ -1210,7 +1210,7 @@ static const struct tty_operations seria
>> > >> > Â  Â  .chars_in_buffer = Â  Â  Â serial_chars_in_buffer,
>> > >> > Â  Â  .tiocmget = Â  Â  Â  Â  Â  Â  serial_tiocmget,
>> > >> > Â  Â  .tiocmset = Â  Â  Â  Â  Â  Â  serial_tiocmset,
>> > >> > - Â  .shutdown = Â  Â  Â  Â  Â  Â  serial_release,
>> > >> > + Â  .cleanup = Â  Â  Â  Â  Â  Â  Â serial_release,
>> > >> > Â  Â  .install = Â  Â  Â  Â  Â  Â  Â serial_install,
>> > >> > Â  Â  .proc_fops = Â  Â  Â  Â  Â  Â &serial_proc_fops,
>> > >> > Â };
>> > >>
>> > >> It isn't enough to change the method pointer. Â The code in
>> > >> serial_release() has to be changed too; it must not call tty_shutdown()
>> > >> any more.
>> > >
>> > > Would it be sufficient to remove the tty_shutdown() call from
>> > > serial_release()?
>> >
>> > I think so, because standard shutdown will be called in queue_release_one_tty.
>> >
>> > Alan, could you confirm about this?  Thus I'd like to update the patch.
>>
>> Yes, that's right.  Just remove the function call.  But what happens if
>> the device is a serial console?  Shouldn't the call to tty_shutdown()
>> be skipped in that case?  (Eventually this won't matter; the console
>> code will be straightened out so that we never release a serial console
>> device.  But for now it's important.)
>>
>> If you think it would be appropriate, you could also rename
>> serial_release() to serial_cleanup() -- it's up to you.
>
> The patch has been fixed up by Linus and merged as
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=f278a2f7bbc2239f479eaf63d0b3ae573b1d746c
>
> If any more fixes are necessary, they'll have to go on top of it.

Oh, I'm late. Thanks for tell

>
> Best,
> Rafael
>



-- 
Regards
dave

Re: [PATCH, fix] Re: [Regression] 2.6.31-git: tty change broke resume from hibernation on MSI Wind U100

[Dave Young]

On Tue, Sep 29, 2009 at 4:20 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Monday 28 September 2009, Alan Stern wrote:
>> On Mon, 28 Sep 2009, Dave Young wrote:
>>
>> > On Mon, Sep 28, 2009 at 2:16 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > > On Sunday 27 September 2009, Alan Stern wrote:
>> > >> On Sun, 27 Sep 2009, Rafael J. Wysocki wrote:
>> > >>
>> > >> > Tested, works.
>> > >> >
>> > >> > Greg, could you please consider taking the patch below? Â It fixes a recent
>> > >> > hibernation regression for me, so if not this one, another fix is necessary.
>> > >>
>> > >> This patch has a mistake.
>> > >>
>> > >> > Index: linux-2.6/drivers/usb/serial/usb-serial.c
>> > >> > ===================================================================
>> > >> > --- linux-2.6.orig/drivers/usb/serial/usb-serial.c
>> > >> > +++ linux-2.6/drivers/usb/serial/usb-serial.c
>> > >> > @@ -1210,7 +1210,7 @@ static const struct tty_operations seria
>> > >> > Â  Â  .chars_in_buffer = Â  Â  Â serial_chars_in_buffer,
>> > >> > Â  Â  .tiocmget = Â  Â  Â  Â  Â  Â  serial_tiocmget,
>> > >> > Â  Â  .tiocmset = Â  Â  Â  Â  Â  Â  serial_tiocmset,
>> > >> > - Â  .shutdown = Â  Â  Â  Â  Â  Â  serial_release,
>> > >> > + Â  .cleanup = Â  Â  Â  Â  Â  Â  Â serial_release,
>> > >> > Â  Â  .install = Â  Â  Â  Â  Â  Â  Â serial_install,
>> > >> > Â  Â  .proc_fops = Â  Â  Â  Â  Â  Â &serial_proc_fops,
>> > >> > Â };
>> > >>
>> > >> It isn't enough to change the method pointer. Â The code in
>> > >> serial_release() has to be changed too; it must not call tty_shutdown()
>> > >> any more.
>> > >
>> > > Would it be sufficient to remove the tty_shutdown() call from
>> > > serial_release()?
>> >
>> > I think so, because standard shutdown will be called in queue_release_one_tty.
>> >
>> > Alan, could you confirm about this?  Thus I'd like to update the patch.
>>
>> Yes, that's right.  Just remove the function call.  But what happens if
>> the device is a serial console?  Shouldn't the call to tty_shutdown()
>> be skipped in that case?  (Eventually this won't matter; the console
>> code will be straightened out so that we never release a serial console
>> device.  But for now it's important.)
>>
>> If you think it would be appropriate, you could also rename
>> serial_release() to serial_cleanup() -- it's up to you.
>
> The patch has been fixed up by Linus and merged as
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=f278a2f7bbc2239f479eaf63d0b3ae573b1d746c
>
> If any more fixes are necessary, they'll have to go on top of it.

Oh, I'm late. Thanks for tell

>
> Best,
> Rafael
>



-- 
Regards
dave
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm