On Sat, Mar 12, 2022 at 05:39:05PM +0800, Dongliang Mu wrote: > On Fri, Feb 25, 2022 at 5:15 PM Greg KH <greg@xxxxxxxxx> wrote: > > > > On Thu, Feb 17, 2022 at 10:25:02AM -0500, Alan Stern wrote: > > > On Thu, Feb 17, 2022 at 11:04:59AM +0300, Dan Carpenter wrote: > > > > Salah sent a bunch of these. The reasoning was explained in this email. > > > > > > > > https://www.spinics.net/lists/kernel/msg4026672.html > > > > > > > > When he resent the patch, Greg said that taking the reference wasn't > > > > needed so the patch wasn't applied. (Also it had the same reference > > > > leak so that's a second reason it wasn't applied). > > > > > > Indeed, the kerneldoc for usb_get_intf() does say that each reference > > > held by a driver must be refcounted. And there's nothing wrong with > > > doing that, _provided_ you do it correctly. > > > > > > But if you know the extra refcount will never be needed (because the > > > reference will be dropped before the usb_interface in question is > > > removed), fiddling with the reference count is unnecessary. I guess > > > whether or not to do it could be considered a matter of taste. > > > > > > On the other hand, it wouldn't hurt to update the kerneldoc for > > > usb_get_intf() (and usb_get_dev() also). We could point out that if a > > > driver does not access the usb_interface structure after its disconnect > > > routine returns, incrementing the refcount isn't mandatory. > > > > That would be good to add to prevent this type of confusion in the > > future. > > Hi Jiri Kosina, > > from my understanding, my previous patch and patch from Alan Stern can > all fix the underlying issue. But as Dan said, the patch from Alan is > more elegant. > > However, the revert commit from you said, my commit introduces memory > leak, which confuses me. > > HID: elo: Revert USB reference counting > > Commit 817b8b9 ("HID: elo: fix memory leak in elo_probe") introduced > memory leak on error path, but more importantly the whole USB reference > counting is not needed at all in the first place ...... > > If it is really my patch that introduces the memory leak, please let me know. Jiri named the wrong commit in his Changelog. The memory leak was introduced by commit fbf42729d0e9 ("HID: elo: update the reference count of the usb device structure"). not by your commit 817b8b9c5396 ("HID: elo: fix memory leak in elo_probe"). Alan Stern
