[dm-devel] slab-nomerge (was Re: [git pull] device mapper changes for 4.3)

[Linus Torvalds]

On Wed, Sep 2, 2015 at 10:39 AM, Mike Snitzer <snitzer at redhat.com> wrote:
>
> - last but not least: add SLAB_NO_MERGE flag to mm/slab_common and
>   disable slab merging for all of DM's slabs (XFS will also use
>   SLAB_NO_MERGE once merged).

So I'm not at all convinced this is the right thing to do. In fact,
I'm pretty convinced it shouldn't be done this way. Since those
commits were at the top of your tree, I just didn't pull them, but
took the rest..

You are basically making this one-sided decision based on your notion
of convenience, and just forcing that thing unconditionally on people.

Your rationale seems _totally_ bogus: you say that it's to be able to
observe the sizes of the dm slabs without using slab debugging.

First off, you don't have to enable slab debugging. You can just
disable slab merging.  It's called "slab_nomerge". It does exactly
what you would think it does.

And what is it that makes dm slabs such a special little princess?
What makes you think that the fact that _you_ want to look at slab
statistics means that everybody else suddenly must have separate slabs
for dm, and dm only? Or xfs?

The other "rationale" was that not merging slabs limits
cross-subsystem memory corruption. Again, what the _hell_ is special
about device mapper that dm - and only dm - would make this a special
thing? That is just pure and utter garbage. Again, we already have
that "slab_nomerge" option, exactly so that when odd slab corruption
issues happen (they are rare, but they do occasionally happen), you
can try that to see if that pinpoints the problem more. And it is
*not* limited to some random set of subsystems. Which makes it clearly
superior to your broken approach, wouldn't you agree?

The only possible true rationale for why dm is special is "because dm
is such a buggy piece of sh*t that it's much more likely to have these
slab corruption bugs than anything else, so I'm just protecting the
rest of the system".

Is that really your rationale? Somehow I doubt it. But if it is, you
really should have said so. At least then it would make sense why this
thing came in through the dm tree, and why dm is so special than it -
and only it - would disable slab merging.

So I'm not pulling things like this from the device mapper tree. There
is just no excuse that I can see for something like SLAB_NO_MERGE to
go through the dm tree in the first place, but that's doubly true when
the rationale for these things were bogus and had nothing what-so-ever
to do with dm.

Things like this aren't supposed to come in through random irrelevant
trees like this, and with no discussion (at least judging by the
commits) with the maintainers of the other pieces of code.

If you have issues with slab merging, then those should be discussed
as such, not as some magical and bogus dm or xfs special case when
they damn well aren't, and damn well will never be.

Yes, I'm annoyed. This was not done well. I realize that everybody
thinks that _their_ code is so special and exceptional that
"obviously" they should be treated specially, but I don't see that
that is the case at all in this case.

If you want to argue that slab merging should be disabled by default,
then that is an argument that I'm willing to believe might be valid
("the downsides are bigger than the upsides").  Or if you are able to
explain why dm really _is_ special, that's an option too. But this
kind of "random subsystems decide unilaterally to not follow the
normal rules" is not acceptable. Not when the "arguments" for it have
absolutely nothing in particular to do with that subsystem.

                    Linus