On 11/1/21 07:22, Pasha Tatashin wrote:
Yes, you are just repeating what the diffs say.
But it's still not good to have this function name doing something completely
different than its name indicates.
I see, I can rename it to: 'set_page_recounted/get_page_recounted' ?
What? No, that's not where I was going at all. The function is already
named set_page_refcounted(), and one of the problems I see is that your
changes turn it into something that most certainly does not
set_page_refounted(). Instead, this patch *increments* the refcount.
That is not the same thing.
And then it uses a .config-sensitive assertion to "prevent" problems.
And by that I mean, the wording throughout this series seems to equate
VM_BUG_ON_PAGE() assertions with real assertions. They are only active,
however, in CONFIG_DEBUG_VM configurations, and provide no protection at
all for normal (most distros) users. That's something that the wording,
comments, and even design should be tweaked to account for.
VM_BUG_ON and BUG_ON should be treated the same. Yes, they are config
sensitive, but in both cases *BUG_ON() means that there is an
unrecoverable problem that occured. The only difference between the
two is that VM_BUG_ON() is not enabled when distros decide to reduce
the size of their kernel and improve runtime performance by skipping
some extra checking.
There is no logical separation between VM_BUG_ON and BUG_ON, there is
been a lengthy discussion about this:
Actually I do want to mention one more thing about this, before we move
on to the next version of the patchset. The above is inaccurate. The
intent of VM_BUG_ON() and BUG_ON() is similar, but there is *definitely*
a logical separation between them: they do not behave the same at runtime.
Just because some distros enable VM_BUG_ON(), does not mean that we can
treat it the same as BUG_ON() in "both directions". From a "don't BUG()
crash the kernel unless really warranted", they are about the same, as
Linus keeps repeating. From the other direction, though ("I need to BUG()-
crash the kernel"), they are NOT the same. BUG_ON() is more reliably
available.
And that's the essence of my object to treating this as if you have
reliably stopped the kernel with a VM_BUG_ON(). It's not really the same!
thanks,
--
John Hubbard
NVIDIA
