Re: [git pull] device mapper fixes for 6.7-rc2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 18 Nov 2023 at 08:04, Mike Snitzer <snitzer@xxxxxxxxxx> wrote:
>
> - Update DM crypt target in response to the treewide change that made
>   MAX_ORDER inclusive.

Your fix is obviously correct, and was an unfortunate semantic
conflict that I (and in my defense, apparently linux-next) missed this
merge window.

But now that I notice the mis-merge I also think the original change
may just have been wrong.

Not only did it change MAX_ORDER semantics from their historical
definition, the *argument* for changing it is bogus.

That commit claims that the traditional MAX_ORDER definition was
counter-intuitive, and that was really the *only* argument for the
change.

But the thing is, the 0..MAX-1 is often the *norm* in the kernel
because we count from 0 and often have max values determined by
powers-of-two etc

Just in the mm, we have MPOL_MAX, MAX_NUMNODES, KM_MAX_IDX,
MAX_SWAPFILES, MAX_NR_GENS, COMPACT_CLUSTER_MAX, MAX_LRU_BATCH, and
probably others.  And as far as I can tell, *none* of them are any
kind of "inclusive" max (that's from a very quick grep for 'MAX', I'm
sure I missed some, and maybe I missed some case where it was actually
inclusive).

So the dm fix in commit 13648e04a9b8 ("dm-crypt: start allocating with
MAX_ORDER") is clearly and obviously a fix, and I have pulled this,
but the more I look at it, the more I think that commit 23baf831a32c
("mm, treewide: redefine MAX_ORDER sanely") was just *complete*
garbage.

Calling the old MAXORDER insane (by implication: "redefine sanely")
and counter-intuitive is clearly bogus. It's neither unusual, insane
_or_ counter-intuitive, just by all the other similar cases we have.

Andrew, Kirill - I'm inclined to just revert that commit (and the new
dm fix it resulted in), unless you can back it up with more than a
completely bogus commit message.

What are the alleged "number of bugs all over the kernel" that the old
MAX_ORDER definition had? In light of the other "MAX" definitions I
found from a second of grepping, I really think the argument for that
was just wrong.

And old MAX_ORDER it is. I went back in history, and the MAX_ORDER
define comes from 2.3.27pre6, where we renamed NR_MEM_LISTS to
MAX_ORDER.

And that was back in 1999.

So we have literally 24 years of MAX_ORDER that was upended for some
very questionable reasons. And changing the meaning of it
*immediately* caused a bug.

                   Linus




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux