Messages in this thread Patch in this message |  | | Date | Tue, 13 Mar 2007 08:30:50 -0700 | From | Jim Radford <> | Subject | Re: [PATCH] usb-serial regression fix |
| |
On Tue, Mar 13, 2007 at 09:55:41AM -0400, Mark Lord wrote: > Jim Radford wrote: > >On Mon, Mar 12, 2007 at 09:55:14PM -0400, Mark Lord wrote: > > > >>So where does the memory get freed -- the structure pointed at > >>by the serial->port[i] thingie ? It's not a leak, is it? > > > >It gets free'd through device_unregister > > > > for (i = 0; i < num_ports; ++i) { > > ... > > port->dev.release = &port_release; > > ... > > retval = device_register(&port->dev); > > > >which means that until all the drivers get converted to use > >->port_probe() and ->port_remove() (which gets called by > >device_unregister) and stop using the ->port[] array in ->shutdown() > >we need to have ->shutdown() called before device_unregister. > > > >>>>>Look at changeset d9a7ecacac5f8274d2afce09aadcf37bdb42b93a in Linus's > >>>>>tree from Jim Radford: > > > >>>>>http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=d9a7ecacac5f8274d2afce09aadcf37bdb42b93a
> >So, this patch should be reverted for now.
> Okay, so.. Jim, could you spell it out for us now?
> I'm confused.
> Based on the current 2.6.21-rc3-git*, tell us *exactly* what (if > any) needs to be reverted, and *exactly* which (if any) of my > suggested patches to apply ?
The simple change to not NULL the ->port[] array after unregister was not quite enough, as ->shutdown() would, as hinted at by Mark, then access kfree'd memory.
The best thing to do is to just revert d9a7ecacac5f8274d2afce09aadcf37bdb42b93a and work out a better fix for the next release. My patches to fix ftdi_sio no longer require this patch, so it is pointless at this late state if it breaks *anything*, and it does.
This patch reverts d9a7ecacac5f8274d2afce09aadcf37bdb42b93a since it breaks drivers that need to access the ->port[] array in shutdown (most of them).
Signed-Off: Jim Radford <radford@blackbean.org>
--- b/drivers/usb/serial/usb-serial.c +++ a/drivers/usb/serial/usb-serial.c @@ -137,6 +135,11 @@ dbg("%s - %s", __FUNCTION__, serial->type->description); + serial->type->shutdown(serial); + + /* return the minor range that this device had */ + return_serial(serial); + for (i = 0; i < serial->num_ports; ++i) serial->port[i]->open_count = 0; @@ -147,12 +150,6 @@ serial->port[i] = NULL; } - if (serial->type->shutdown) - serial->type->shutdown(serial); - - /* return the minor range that this device had */ - return_serial(serial); - /* If this is a "fake" port, we have to clean it up here, as it will * not get cleaned up in port_release() as it was never registered with * the driver core */
- 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/
|  |