* [PATCH] fix oops in usbserial_cleanup function;
@ 2010-10-15 4:14 m00150988
2010-10-15 4:25 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: m00150988 @ 2010-10-15 4:14 UTC (permalink / raw)
To: linux-usb, linux-usb; +Cc: linux-kernel, linux-kernel, Franko Fang, greg, greg
From: marui <m00150988@huawei.com>
1. I find this bug on OpenSUSE 11.3 which kernel vesion is 2.6.34, but the latest kernel vesion 2.6.36-rc7 aslo have this bug. This patch is based on the kernel of 2.6.36-rc7.
2. Bug report:
a. Install huawei datacard dashboard on OpenSUSE 11.3
b. Plug in huawei datacard into OpenSUSE 11.3 which kernel verison is 2.6.36-rc7
c. After the dashboard has detected the device, I pull out the usb datacard
d. close datashboard,then kernel panic will happen in usbserial_cleanup function.
3. fix the bug:
I find usbserial_cleanup should judge the usb device wheher has been disconnected firtly.
Signed-off-by: marui<m00150988@huawei.com>
--- linux-2.6.36-rc7_orig/drivers/usb/serial/usb-serial.c 2010-10-06 16:39:52.000000000 -0400
+++ linux-2.6.36-rc7/drivers/usb/serial/usb-serial.c 2010-10-14 20:59:47.000000000 -0400
@@ -328,6 +328,20 @@ static void serial_cleanup(struct tty_st
/* The console is magical. Do not hang up the console hardware
* or there will be tears.
*/
+ dbg("%s start\n",__func__);
+ if(NULL == port)
+ {
+ dbg("%s NULL == port\n",__func__);
+ return;
+ }
+ mutex_lock(&port->serial->disc_mutex);
+ if (port->serial->disconnected)
+ {
+ dbg("%s port->serial->disconnected\n",__func__);
+ return_serial(port->serial);
+ return;
+ }
+ mutex_unlock(&port->serial->disc_mutex);
if (port->port.console)
return;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] fix oops in usbserial_cleanup function;
2010-10-15 4:14 [PATCH] fix oops in usbserial_cleanup function; m00150988
@ 2010-10-15 4:25 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2010-10-15 4:25 UTC (permalink / raw)
To: m00150988; +Cc: linux-usb, linux-kernel, Franko Fang
On Fri, Oct 15, 2010 at 12:14:37PM +0800, m00150988@huawei.com wrote:
> From: marui <m00150988@huawei.com>
>
> 1. I find this bug on OpenSUSE 11.3 which kernel vesion is 2.6.34, but the latest kernel vesion 2.6.36-rc7 aslo have this bug. This patch is based on the kernel of 2.6.36-rc7.
> 2. Bug report:
> a. Install huawei datacard dashboard on OpenSUSE 11.3
> b. Plug in huawei datacard into OpenSUSE 11.3 which kernel verison is 2.6.36-rc7
> c. After the dashboard has detected the device, I pull out the usb datacard
> d. close datashboard,then kernel panic will happen in usbserial_cleanup function.
>
>
> 3. fix the bug:
> I find usbserial_cleanup should judge the usb device wheher has been disconnected firtly.
>
> Signed-off-by: marui<m00150988@huawei.com>
I need a full name here please.
> --- linux-2.6.36-rc7_orig/drivers/usb/serial/usb-serial.c 2010-10-06 16:39:52.000000000 -0400
> +++ linux-2.6.36-rc7/drivers/usb/serial/usb-serial.c 2010-10-14 20:59:47.000000000 -0400
> @@ -328,6 +328,20 @@ static void serial_cleanup(struct tty_st
> /* The console is magical. Do not hang up the console hardware
> * or there will be tears.
> */
> + dbg("%s start\n",__func__);
> + if(NULL == port)
> + {
> + dbg("%s NULL == port\n",__func__);
> + return;
> + }
> + mutex_lock(&port->serial->disc_mutex);
Please read Documentation/CodingStyle for the tabs and brace placements
you need to resolve.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <00c601cb6c1c$2c768500$ba260b0a@china.huawei.com>]
* Re: [PATCH] fix oops in usbserial_cleanup function;
[not found] <00c601cb6c1c$2c768500$ba260b0a@china.huawei.com>
@ 2010-10-15 4:20 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2010-10-15 4:20 UTC (permalink / raw)
To: marui; +Cc: linux-usb, linux-kernel, zihan, Lin Lei, Franko Fang, wangyeqi
On Fri, Oct 15, 2010 at 11:50:56AM +0800, marui wrote:
> 1. I find this bug on OpenSUSE 11.3 which kernel vesion is 2.6.34, but the latest kernel vesion 2.6.36-rc7 aslo have this bug. This patch is based on the kernel of 2.6.36-rc7.
> 2. Bug report:
> a. Install huawei datacard dashboard on OpenSUSE 11.3
> b. Plug in huawei datacard into OpenSUSE 11.3 which kernel verison is 2.6.36-rc7
> c. After the dashboard has detected the device, I pull out the usb datacard
> d. close datashboard,then kernel panic will happen in usbserial_cleanup function.
What does the "dashboard" program do? Hold the port open?
> 3. fix the bug:
> I find usbserial_cleanup should judge the usb device wheher has been disconnected firtly.
>
>
> diff -uprN -X linux-2.6.36-rc7-orig/Documentation/dontdiff linux-2.6.36-rc7-orig/drivers/usb/serial/usb-serial.c linux-2.6.36-rc7/drivers/usb/serial/usb-serial.c
>
> --- linux-2.6.36-rc7_orig/drivers/usb/serial/usb-serial.c 2010-10-06 16:39:52.000000000 -0400
> +++ linux-2.6.36-rc7/drivers/usb/serial/usb-serial.c 2010-10-14 20:59:47.000000000 -0400
> @@ -328,6 +328,20 @@ static void serial_cleanup(struct tty_st
> /* The console is magical. Do not hang up the console hardware
> * or there will be tears.
> */
> + dbg("%s start\n",__func__);
> + if(NULL == port)
> + {
> + dbg("%s NULL == port\n",__func__);
> + return;
We don't need to keep the dbg statements here, do we?
> + }
> + mutex_lock(&port->serial->disc_mutex);
> + if (port->serial->disconnected)
> + {
> + dbg("%s port->serial->disconnected\n",__func__);
> + return_serial(port->serial);
> + return;
You can't return with a lock held.
> + }
> + mutex_unlock(&port->serial->disc_mutex);
> if (port->port.console)
> return;
Your tabs and spaces got all mixed up, and it can't be applied. Also, I
need a "Signed-off-by:" line to be able to accept it. Care to read the
file Documentation/SubmittingPatches for how to format a patch so we can
take it?
Also, run the patch through scripts/checkpatch.pl and resolve the coding
style issues before resending please.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fix oops in usbserial_cleanup function;
@ 2010-10-15 8:30 m00150988
2010-10-15 14:15 ` Alan Stern
0 siblings, 1 reply; 6+ messages in thread
From: m00150988 @ 2010-10-15 8:30 UTC (permalink / raw)
To: linux-usb, linux-usb
Cc: linux-kernel, linux-kernel, zihan, zihan, Lin Lei, Franko Fang,
wangyeqi, wangyeqi
From:ma rui <m00150988@huawei.com>
1. I find this bug on OpenSUSE 11.3 which kernel version is 2.6.34, but the latest kernel version 2.6.36-rc7 aslo have this bug. This patch is based on
the kernel of 2.6.36-rc7
2. bug report:
a. Install huawei datacard dashboard on OpenSUSE 11.3
b. Plug in huawei datacard into OpenSUSE 11.3 which kernel version is 2.6.36-rc7
c. After the dashboard has detected the device, I pull out the usb datacard
d. Close dashboard,then kernel panic will happen in usbserial_clean function
Yes, the datacard exit without close the port.
But after the dashboard connect internet with hauwei datacard, then Hibernate/resume, the bug will happen too.
Do you have any other good idea to resolve this bug,or please apply my patch,thanks. :)
Signed-off-by: ma rui <m00150988@huawei.com>
diff -uprN -X linux-2.6.36-rc7_orig/Documentation/dontdiff linux-2.6.36-rc7_orig/drivers/usb/serial/usb-serial.c linux-2.6.36-rc7/drivers/usb/serial/usb-serial.c
--- linux-2.6.36-rc7_orig/drivers/usb/serial/usb-serial.c 2010-10-06 16:39:52.000000000 -0400
+++ linux-2.6.36-rc7/drivers/usb/serial/usb-serial.c 2010-10-15 01:57:36.000000000 -0400
@@ -328,6 +328,16 @@ static void serial_cleanup(struct tty_st
/* The console is magical. Do not hang up the console hardware
* or there will be tears.
*/
+ if (NULL == port)
+ return;
+ mutex_lock(&port->serial->disc_mutex);
+ if (port->serial->disconnected) {
+ return_serial(port->serial);
+ mutex_unlock(&port->serial->disc_mutex);
+ return;
+ }
+ mutex_unlock(&port->serial->disc_mutex);
+
if (port->port.console)
return;
----- Original Message -----
From: "Greg KH" <greg@kroah.com>
To: "marui" <m00150988@huawei.com>
Cc: <linux-usb@vger.kernel.org>; <linux-kernel@vger.kernel.org>; <zihan@huawei.com>; "Lin Lei" <Lin.Lei@huawei.com>; "Franko Fang" <huananhu@huawei.com>; <wangyeqi@huawei.com>
Sent: Friday, October 15, 2010 12:20 PM
Subject: Re: [PATCH] fix oops in usbserial_cleanup function;
> On Fri, Oct 15, 2010 at 11:50:56AM +0800, marui wrote:
>> 1. I find this bug on OpenSUSE 11.3 which kernel vesion is 2.6.34, but the latest kernel vesion 2.6.36-rc7 aslo have this bug. This patch is based on the kernel of 2.6.36-rc7.
>> 2. Bug report:
>> a. Install huawei datacard dashboard on OpenSUSE 11.3
>> b. Plug in huawei datacard into OpenSUSE 11.3 which kernel verison is 2.6.36-rc7
>> c. After the dashboard has detected the device, I pull out the usb datacard
>> d. close datashboard,then kernel panic will happen in usbserial_cleanup function.
>
> What does the "dashboard" program do? Hold the port open?
>
>> 3. fix the bug:
>> I find usbserial_cleanup should judge the usb device wheher has been disconnected firtly.
>>
>>
>> diff -uprN -X linux-2.6.36-rc7-orig/Documentation/dontdiff linux-2.6.36-rc7-orig/drivers/usb/serial/usb-serial.c linux-2.6.36-rc7/drivers/usb/serial/usb-serial.c
>>
>> --- linux-2.6.36-rc7_orig/drivers/usb/serial/usb-serial.c 2010-10-06 16:39:52.000000000 -0400
>> +++ linux-2.6.36-rc7/drivers/usb/serial/usb-serial.c 2010-10-14 20:59:47.000000000 -0400
>> @@ -328,6 +328,20 @@ static void serial_cleanup(struct tty_st
>> /* The console is magical. Do not hang up the console hardware
>> * or there will be tears.
>> */
>> + dbg("%s start\n",__func__);
>> + if(NULL == port)
>> + {
>> + dbg("%s NULL == port\n",__func__);
>> + return;
>
> We don't need to keep the dbg statements here, do we?
>
>> + }
>> + mutex_lock(&port->serial->disc_mutex);
>> + if (port->serial->disconnected)
>> + {
>> + dbg("%s port->serial->disconnected\n",__func__);
>> + return_serial(port->serial);
>> + return;
>
> You can't return with a lock held.
>
>> + }
>> + mutex_unlock(&port->serial->disc_mutex);
>> if (port->port.console)
>> return;
>
> Your tabs and spaces got all mixed up, and it can't be applied. Also, I
> need a "Signed-off-by:" line to be able to accept it. Care to read the
> file Documentation/SubmittingPatches for how to format a patch so we can
> take it?
>
> Also, run the patch through scripts/checkpatch.pl and resolve the coding
> style issues before resending please.
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] fix oops in usbserial_cleanup function;
2010-10-15 8:30 m00150988
@ 2010-10-15 14:15 ` Alan Stern
2010-10-15 19:09 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Alan Stern @ 2010-10-15 14:15 UTC (permalink / raw)
To: m00150988
Cc: USB list, Kernel development list, zihan, Lin Lei, Franko Fang,
wangyeqi
On Fri, 15 Oct 2010 m00150988@huawei.com wrote:
> From:ma rui <m00150988@huawei.com>
> 1. I find this bug on OpenSUSE 11.3 which kernel version is 2.6.34, but the latest kernel version 2.6.36-rc7 aslo have this bug. This patch is based on
> the kernel of 2.6.36-rc7
> 2. bug report:
> a. Install huawei datacard dashboard on OpenSUSE 11.3
> b. Plug in huawei datacard into OpenSUSE 11.3 which kernel version is 2.6.36-rc7
> c. After the dashboard has detected the device, I pull out the usb datacard
> d. Close dashboard,then kernel panic will happen in usbserial_clean function
>
> Yes, the datacard exit without close the port.
>
> But after the dashboard connect internet with hauwei datacard, then Hibernate/resume, the bug will happen too.
> Do you have any other good idea to resolve this bug,or please apply my patch,thanks. :)
>
>
> Signed-off-by: ma rui <m00150988@huawei.com>
>
>
> diff -uprN -X linux-2.6.36-rc7_orig/Documentation/dontdiff linux-2.6.36-rc7_orig/drivers/usb/serial/usb-serial.c linux-2.6.36-rc7/drivers/usb/serial/usb-serial.c
> --- linux-2.6.36-rc7_orig/drivers/usb/serial/usb-serial.c 2010-10-06 16:39:52.000000000 -0400
> +++ linux-2.6.36-rc7/drivers/usb/serial/usb-serial.c 2010-10-15 01:57:36.000000000 -0400
> @@ -328,6 +328,16 @@ static void serial_cleanup(struct tty_st
> /* The console is magical. Do not hang up the console hardware
> * or there will be tears.
> */
> + if (NULL == port)
> + return;
> + mutex_lock(&port->serial->disc_mutex);
> + if (port->serial->disconnected) {
> + return_serial(port->serial);
> + mutex_unlock(&port->serial->disc_mutex);
> + return;
> + }
> + mutex_unlock(&port->serial->disc_mutex);
> +
> if (port->port.console)
> return;
This patch is clearly wrong, since it skips some of the actions that
should be taken by serial_cleanup even if the port is already
disconnected.
Besides, the main point of the patch is to avoid problems when
port = tty->driver_data turns out to be NULL. But the only place where
tty->driver_data is set to NULL is further below in this same function!
So the problems should never arise.
If they do arise, it indicates there's a bug somewhere else. That
other bug can't be fixed by changing this function.
Alan Stern
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] fix oops in usbserial_cleanup function;
2010-10-15 14:15 ` Alan Stern
@ 2010-10-15 19:09 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2010-10-15 19:09 UTC (permalink / raw)
To: Alan Stern
Cc: m00150988, USB list, Kernel development list, zihan, Lin Lei,
Franko Fang, wangyeqi
On Fri, Oct 15, 2010 at 10:15:34AM -0400, Alan Stern wrote:
> On Fri, 15 Oct 2010 m00150988@huawei.com wrote:
>
> > From:ma rui <m00150988@huawei.com>
> > 1. I find this bug on OpenSUSE 11.3 which kernel version is 2.6.34, but the latest kernel version 2.6.36-rc7 aslo have this bug. This patch is based on
> > the kernel of 2.6.36-rc7
> > 2. bug report:
> > a. Install huawei datacard dashboard on OpenSUSE 11.3
> > b. Plug in huawei datacard into OpenSUSE 11.3 which kernel version is 2.6.36-rc7
> > c. After the dashboard has detected the device, I pull out the usb datacard
> > d. Close dashboard,then kernel panic will happen in usbserial_clean function
> >
> > Yes, the datacard exit without close the port.
> >
> > But after the dashboard connect internet with hauwei datacard, then Hibernate/resume, the bug will happen too.
> > Do you have any other good idea to resolve this bug,or please apply my patch,thanks. :)
> >
> >
> > Signed-off-by: ma rui <m00150988@huawei.com>
> >
> >
> > diff -uprN -X linux-2.6.36-rc7_orig/Documentation/dontdiff linux-2.6.36-rc7_orig/drivers/usb/serial/usb-serial.c linux-2.6.36-rc7/drivers/usb/serial/usb-serial.c
> > --- linux-2.6.36-rc7_orig/drivers/usb/serial/usb-serial.c 2010-10-06 16:39:52.000000000 -0400
> > +++ linux-2.6.36-rc7/drivers/usb/serial/usb-serial.c 2010-10-15 01:57:36.000000000 -0400
> > @@ -328,6 +328,16 @@ static void serial_cleanup(struct tty_st
> > /* The console is magical. Do not hang up the console hardware
> > * or there will be tears.
> > */
> > + if (NULL == port)
> > + return;
> > + mutex_lock(&port->serial->disc_mutex);
> > + if (port->serial->disconnected) {
> > + return_serial(port->serial);
> > + mutex_unlock(&port->serial->disc_mutex);
> > + return;
> > + }
> > + mutex_unlock(&port->serial->disc_mutex);
> > +
> > if (port->port.console)
> > return;
>
> This patch is clearly wrong, since it skips some of the actions that
> should be taken by serial_cleanup even if the port is already
> disconnected.
>
> Besides, the main point of the patch is to avoid problems when
> port = tty->driver_data turns out to be NULL. But the only place where
> tty->driver_data is set to NULL is further below in this same function!
> So the problems should never arise.
>
> If they do arise, it indicates there's a bug somewhere else. That
> other bug can't be fixed by changing this function.
Yeah, I agree.
Ma, what is the full oops message that you are seeing here when you
remove the device? And does userspace still have the device open at
that time? I'm guessing so as it sounds like the oops happens when the
port is then closed. I can't duplicate that problem here.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-15 19:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-15 4:14 [PATCH] fix oops in usbserial_cleanup function; m00150988
2010-10-15 4:25 ` Greg KH
[not found] <00c601cb6c1c$2c768500$ba260b0a@china.huawei.com>
2010-10-15 4:20 ` Greg KH
-- strict thread matches above, loose matches on Subject: below --
2010-10-15 8:30 m00150988
2010-10-15 14:15 ` Alan Stern
2010-10-15 19:09 ` Greg KH
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.