Re: [GIT PULL] Please pull powerpc/linux.git powerpc-5.7-1 tag

[Linus Torvalds]

On Sun, Apr 5, 2020 at 5:53 AM Michael Ellerman <mpe@xxxxxxxxxxxxxx> wrote:
>
> There is one conflict in fs/sysfs/group.c, between our:
>
>   9255782f7061 ("sysfs: Wrap __compat_only_sysfs_link_entry_to_kobj function to change the symlink name")
[...]

The conflict was trivial.

But I want to kvetch a bit about that commit. It's doing some odd stuff.

In particular, it's wrapping things "the wrong way". Our naming rules
are that the double underscore versions are the internal helper
functions that you generally shouldn't use unless you have some extra
reason for it, and then the non-underscore versions are the preferred
and simpler user interface to those internal implementations.

IOW, the _wrapper_ doesn't have double underscores, it's the _wrappee_
that has the underscores.

That commit does the exact reverse of that usual pattern, which is
very confusing.

Now, I see _why_ you do that - normally the non-underscore version is
the "real" interface and the one we've always exported, and then the
double underscore is the special internal thing that maybe exposes
some internal detail (or maybe only does one special case of it and
leaves out locking or whatever).

In this case, for hysterical raisins, we only _had_ that
double-underscore version, and you basically added the new case and
did it without the underscores.

So I see why it happened the way it did, but I do think the end result
makes no sense and is odd and surprising.

The thing is, we have exactly *one* user of that double-underscore
version: tpm-chip.c (ok, there are two calls in that file, but it's a
single user).

So I think it should just have removed the __ version entirely. Make
tpm-chip just use the new semantics, and pass in the extra NULL
argument.

I guess I'll just do that as a cleanup patch on top, but it feels a
bit odd to have to do that cleanup when the original patch could have
just done the obvious thing.

                    Linus