RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit

From	Felipe Balbi <balbi@kernel.org>
Newsgroups	linux.kernel
Subject	RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit
Date	2016-04-12 08:30 +0200
Message-ID	<rmZZL-2Fs-1@gated-at.bofh.it> (permalink)
References	(2 earlier) <rmHTc-4Qu-27@gated-at.bofh.it> <rmI2S-4Vk-19@gated-at.bofh.it> <rmImf-57L-33@gated-at.bofh.it> <rmIYW-5N5-15@gated-at.bofh.it> <rmW5P-7US-5@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

Hi Changbin,

"Du, Changbin" <changbin.du@intel.com> writes:
>> Hi,
>> 
>> "Du, Changbin" <changbin.du@intel.com> writes:
>> >>
>> >> >> > +	dwc->regset = NULL;
>> >> >>
>> >> >> setting regset to NULL is unnecessary. We only call
>> dwc3_debugfs_exit()
>> >> >> when removing the driver.
>> >> >>
>> >> >> --
>> >> >> Balbi
>> >> > I'd like keep this line even it is unnecessary, because It is a good habit to
>> >> > Avoid wild pointers. Just like the dwc->root = NULL.
>> >>
>> >> there won't be any wild pointers here, we'll free struct dwc3 *dwc itself.
>> >>
>> >> --
>> >> Balbi
>> > I agree the dwc will be freed in current code. But the 'free' logical is out
>> > of the debugfs code. They should be treat as some logical independent.
>> Per
>> > this point, I still think set pointer to null is not bad. For example, if dwc3
>> core
>> > code invoke dwc3_debugfs_exit twice by mistake(just an example case,
>> not
>> > really), then no crash/impact for the second call.
>> 
>> the second call should crash because it's clearly wrong ;-) If dwc3 ever
>> calls dwc3_debugfs_exit() twice, it really deserves to crash. It's
>> something so wrong that we want the verbosity and urgency of a kernel
>> oops to make sure we fix it ASAP.
>> 
>> If, however, we set it to null, it might be years before we notice
>> anything's wrong.
>> 
>> --
>> Balbi
>
> Hmm, I agree from this point. I will combine this patch with other two patches
> (due to their dependency). And I'd like remove the 'dwc->root=NULL' as well,

you are creating a dependency that doesn't exist. Please stop that. You
should have two separate branches based on v4.6-rc3 (or, if you prefer,
one based on my testing/fixes and another based on my testing/next). On
one branch you have *only* $subject and you fix *all* the memory
leaks. On the other branch you have the other two patches.

Ignore the fact that we might have a conflict, that's for git (and
maintainers) to handle when they happen.

Again, don't create dependencies between fixes for the -rc cycle and
changes for the next merge window.

> Is it ok for you?

yeah, please remove root = NULL as that's completely unnecessary, but
split these patches in separate branches and fix all memory leaks.

-- 
balbi

Thread

[PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit changbin.du@intel.com - 2016-04-08 12:00 +0200
  Re: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit Felipe Balbi <balbi@kernel.org> - 2016-04-11 10:10 +0200
    RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit "Du, Changbin" <changbin.du@intel.com> - 2016-04-11 13:10 +0200
      RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit Felipe Balbi <balbi@kernel.org> - 2016-04-11 13:20 +0200
        RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit "Du, Changbin" <changbin.du@intel.com> - 2016-04-11 13:40 +0200
          RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit Felipe Balbi <balbi@kernel.org> - 2016-04-11 14:20 +0200
            RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit "Du, Changbin" <changbin.du@intel.com> - 2016-04-12 04:20 +0200
              RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit Felipe Balbi <balbi@kernel.org> - 2016-04-12 08:30 +0200
                RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit "Du, Changbin" <changbin.du@intel.com> - 2016-04-12 09:00 +0200
                RE: [PATCH] usb: dwc3: free dwc->regset on dwc3_debugfs_exit Felipe Balbi <balbi@kernel.org> - 2016-04-12 10:10 +0200
                Re: [PATCH v2] usb: dwc3: fix memory leak of dwc->regset Felipe Balbi <balbi@kernel.org> - 2016-04-12 10:30 +0200
                [PATCH v3] usb: dwc3: fix memory leak of dwc->regset changbin.du@intel.com - 2016-04-12 10:40 +0200
                Re: [PATCH v3] usb: dwc3: fix memory leak of dwc->regset Felipe Balbi <balbi@kernel.org> - 2016-04-12 12:20 +0200
                [PATCH v2] usb: dwc3: fix memory leak of dwc->regset changbin.du@intel.com - 2016-04-12 10:30 +0200

csiph-web