linux-kernel - Re: [GIT PULL] MM updates for 6.10-rc1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=whEAquncw0wb59-XVAHmVh4CWSfwuWh4bTJjJzvx0=PzQ@mail.gmail.com>
Date: Sun, 19 May 2024 08:32:44 -0700
From: Linus Torvalds <torvalds@...uxfoundation.org>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-mm@...ck.org, mm-commits@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] MM updates for 6.10-rc1

On Fri, 17 May 2024 at 19:22, Andrew Morton <akpm@...ux-foundation.org> wrote:
>
> include/linux/slab.h
>         https://lkml.kernel.org/r/20240429114302.7af809e8@canb.auug.org.au

This is not only a merge conflict, your tree is actively buggy.

You have introduced changes like this:

  -static inline __alloc_size(1, 2) void *kvcalloc(size_t n, size_t
size, gfp_t flags)
  -{
  - return kvmalloc_array(n, size, flags | __GFP_ZERO);
  -}
  +#define kvcalloc(_n, _size, _flags) kvmalloc_array(_n, _size,
_flags|__GFP_ZERO)

and that's just completely wrong. Note the "_flags|__GFP_ZERO": yes,
the bitwise or is fairly low down in the operator precedence rules,
and it probably work sin practice because most cases will just pass in
a simple expression for the flags, but it's still *horribly* wrong.

I'm going to take this pull and fix up the cases I find, but I'm not
happy with this kind of trivial C preprocessor misuse.

I also note that you have *SEVEN* pointless merges that have no
explanation for them. I'm happy that you use git, but that means that
you also need to either

 (a) not do merges at all and treat it as a patch queue

 (b) do merges _properly_ and not throw them around like some madman

And doing them properly means not only writing good commit messages,
but actually having good reasons for them. As it is, we have

  5d1bc760583f ("merge mm-hotfixes-stable into mm-nonmm-stable to pick
up needed changes")
  640958fde130 ("Merge branch 'master' into mm-stable")
  4e2e36129225 ("Merge branch 'master' into mm-stable")
  1dd4505cf4c8 ("Merge branch 'master' into mm-stable")
  71919308943d ("Merge branch 'master' into mm-stable")
  b228ab57e51b ("Merge branch 'master' into mm-stable")
  5e2806112864 ("Merge branch 'master' into mm-stable")

and those one-liners are all the explanation there are for the merges,
and NONE OF THOSE MERGES ARE VALID IN THE FIRST PLACE!

They shouldn't have been merges! You literally have no new code on the
other side. You should have just fast-forwarded to the new state,
since your side didn't contain anything relevant any more.

So the merges are doubly wrong. They have no explanation, but part of
the reason that they have no explanation is probably exactly the fact
that there *IS* no explanation for them.

There is one fundamental rule of merges: if you can't explain why
you'd need to merge, DON'T DO IT.

There are lots of smaller rules too. See

    Documentation/maintainer/rebasing-and-merging.rst

for at least some of them.

                 Linus

          Linus