Message-ID: <CAHk-=wh4bZdCkhng3EsJCDhHLxHT6x4S66v5JQvusihVfYrc5Q@mail.gmail.com>
Date:   Sun, 24 May 2020 10:05:28 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Stephen Rothwell <sfr@...b.auug.org.au>
Subject: Re: [GIT PULL] Driver core fixes for 5.7-rc7 - take 2

On Sun, May 24, 2020 at 8:00 AM Greg KH <gregkh@...uxfoundation.org> wrote:
>
> On Sat, May 23, 2020 at 11:14:28AM -0700, Linus Torvalds wrote:
> >
> > 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).

Note that the "obviously buggy or at least not documented" behavior of
that commit 4ef12f719802 ("kobject: Make sure the parent does not get
released before its children") that got reverted is true regardless.

Should the parent be released unconditionally (like that commit does),
or should it be released only when kobject_del() was called when it
had "state_in_sysfs" set?

Even if the problem Guenter reported was due to something else, that
other change is a rather fundamental change and should at least be
mentioned by the commit log.

It's entirely possible that the parent dropping should always be done,
but the way it was done in that reverted commit it looked kind of
accidental.

> What is really odd now, is that 'git log lib/kobject.c' does not show
> the change/revert at all.  Is that because there was a revert?  Or is it
> a git config option/default somewhere that prevents that from showing
> up?

No, it's fundamentally how git works.

Remember: git does _not_ track "changes".

Any SCM that tracks changes to a file is fundamentally broken, for
fundamental reasons. It mostly boils down to "what happens when the
source of the change the same file in two branches is different".
Think "rename to X" and "create X", and remember all the problems SVN
has when that happens.

So no, git never _ever_ tracks "what changed". Instead, git
fundamentally tracks "what is the state". The "change" is not
fundamental, it's something that gets computed afterwards when you
have a "before and after" state.

Why does that matter?

In the current git tree, when you start looking at the history of
lib/kobject.c, it looks at my merge of your tree, and goes "the
contents of that file were the same before and after the merge, so the
side history from you is clearly irrelevant".

And git is clearly right: your branch made changes to the file, but
then reverted them all, so clearly that branch doesn't matter. Git
will by default only show the simplified history - the part that
matters.

If you want it all, use "git log --full-history", but then you will
_really_ get the full history and a lot of pointless noise. And even
then, things like "blame" won't waste time on following merges that
made no difference in the end.

(This, btw, is also true if your branch _did_ make real changes, but
the merge itself ended up throwing them away - either because somebody
undid them in the merge, or because the main development line had
those same changes already, so that the branch that got merged didn't
actually matter. Again, this comes from the fact that git tracks the
history of the full _state_ of the tree, not "these are the changes
done here").

Sasha mentioned "--follow", which also happens to show that commit,
but that's more of an incidental happenstance than anything else. "git
log --follow" is kind of a special case, where git stops doing some of
the pathname-based simplifying, because if the file shows up from
nothing, git will try to then figure out where it came from. The fact
that "--follow" this ends up not pruning irrelevant history as
aggressively is more of an implementation artifact than anything else.

So generally, don't use "--follow". It's kind of a hack to emulate
"track changes to a file", but it is a hack, and it fundamentally is a
bogus operation (for all the same reasons that the CVS/SCCS/SVN/etc
notion of a "file identity" is complete garbage and leads to
fundamental problems).

So "--follow" also can't handle multiple paths (or directories), and
is generally just a "placate people who don't understand why SVN is
wrong" option. It can be very useful in practice for the simple cases,
but it can also end up missing real changes in other situations.

              Linus

Powered by blists - more mailing lists