Messages in this thread | | | Date | Mon, 25 May 2020 09:33:10 +0200 | From | Greg KH <> | Subject | Re: [GIT PULL] Driver core fixes for 5.7-rc7 - take 2 |
| |
On Sun, May 24, 2020 at 11:42:19AM -0400, Sasha Levin wrote: > On Sun, May 24, 2020 at 05:00:18PM +0200, Greg KH wrote: > > On Sat, May 23, 2020 at 11:14:28AM -0700, Linus Torvalds wrote: > > > On Sat, May 23, 2020 at 8:29 AM Greg KH <gregkh@linuxfoundation.org> wrote: > > > > > > > > The kobject patch that was originally in here has now been reverted, as > > > > Guenter reported boot problems with it on some of his systems. > > > > > > Hmm. That original patch looks obviously buggy: in kobject_cleanup() > > > it would end up doing "kobject_put(parent)" regardless of whether it > > > had actually done __kobject_del() or not. > > > > > > That _could_ have been intentional, but considering the commit > > > message, it clearly wasn't in this case. It might be worth re-trying > > > to the commit, just with that fixed. > > > > Turns out that wasn't the real problem here, the culprit is the > > lib/test_printf.c code trying to tear down a kobject tree from the > > parent down to the children (i.e. in the backwards order). > > > > > Btw, when you end up reverting a patch that was already the top patch, > > > you might as well just remove it entirely from that tree instead (ie > > > "git reset --hard HEAD^" instead of "git revert HEAD"). > > > > > > Unless somebody else uses your branches and you are afraid that the > > > non-reverted commit escaped out in the wild that way? > > > > I don't like rebasing or changing the HEAD like that on a public branch. > > As proof, syzbot started sending me a bunch of "this is the failed > > commit" messages right after your email, based on it's testing of the > > tree in linux-next. > > OTOH, leaving commits like this may result in confusion later on because > of confusion around the "correct" patch. > > Consider this: > > 1. Someone writes a patch named "close memory leak when freeing XYZ" > 2. We revert it a day later with 'Revert "close memory leak when > freeing XYZ"'
And the sha1 is in the commit, showing which patch was reverted.
> 3. Now, what would the author of the original patch do? That's right - > re-submit a patch with an identical subject line and patch description, > but with a subtle change in the code to fix the bug the original patch > was reverted for.
Sometimes, yes, but sometimes, as in this case, a totally different patch will be submitted for the problem :)
But, even if it was there, the sha1 in the revert should allow us to track this properly. I can't remember a time where this has caused problems in the past, can you?
> So now we end up with two "close memory leak when freeing XYZ" commits > in our git history that are nearly identical. Recipe for a disaster :)
Time and sha1 should show them being different :)
thanks,
greg k-h
| |