On Mon, 12 Mar 2012 07:28:02 -0300, Mauro Carvalho Chehab wrote: > Em 12-03-2012 07:04, Jean Delvare escreveu: > > "!d" can't actually happen, so it doesn't matter. d is passed by > > dib0700_rc_setup() when calling usb_fill_bulk_urb(), and > > dib0700_rc_setup() starts with dereferencing d, if it was NULL we'd > > crash right away. Hence d is never NULL in dib0700_rc_urb_completion(). > > > > So this "if (d == NULL)" is just paranoia and might as well be removed. > > Ok. Feel free to remove it on your patch. I'll send a patch later today, yes. > > (...) > > Indeed, as I read the code, unless disable_rc_polling=1 or a fatal > > error occurs, dib0700_rc_urb_completion will loop over and over > > endlessly. I guess it's what "RC polling" is all about. No surprise why > > my DVB-T card sucks so much power... > > This code should not poll anymore, at least with a v1.2 firmware. > > The URB code will run only when the URB arrives. The URB will only arrive > when some key is pressed. At key press, the URB handling code will re-trigger > the URB handler to be prepared for a next key. You're completely right. I enabled debugging, I have no remote control on my card and the callback function is simply never called during run time. It is called when I rmmod the driver though, with a negative status value, which causes the urb to be freed. So there is no leak in this case already, even without applying the fix that we discussed earlier. I have no idea who calls dib0700_rc_urb_completion, but the debug log clearly shows it is called. I re-enabled kmemleak to understand exactly what was happening and I get it now. My card behaves differently on cold boots and warm reboots. In case of warm reboots I see the following error message in the log: rc submit urb failed This is where the actual leak occurs, as the URB was never submitted, dib0700_rc_urb_completion is never called, not even on module removal, so the URB never has a chance to be freed. Furthermore, the transfer_buffer of the urb is also never freed, not even in the regular case. So a kfree must be added before any call to usb_free_urb(). Patch follows. (I guess this all means that the remote control wouldn't work on warm reboots, but as I have no remote control I never noticed.) > > (...) > > Sure, we can do that, but what surprises me is that I don't remember > > removing the module when kmemleak reported the leak to me. Oh well, > > kmemleak is pretty new, maybe that was a false positive after all. > > It seems weird that kmemleak would warn about a leak before the > memory removal. There are several places during driver init where data > is alloced, and will only be freed at driver removal. The same happens > with device probe/disconnect. If there are no references left to allocated memory, it makes sense to report immediately. -- Jean Delvare -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
