Re: [RFC] free_pages stuff

[Linus Torvalds <torvalds@linux-foundation.org>]

[ Grr. Resending because the stupid android gmail app still can't do
text emails ]

On Dec 21, 2015 17:04, "Al Viro" <viro@zeniv.linux.org.uk> wrote:
>
> > And quite frankly, even the "new name" is likely a bad idea. If you
> > want to allocate a page, and get a pointer, just use "kmalloc()".
> > Boom, done!
>
> Erm...  You've just described the absolute majority of callers.  Really.

That wasn't my point.

I totally believe that most of the legacy users actually wanted a pointer.

But that doesn't mean that we should just convert a legacy interface.
We should either just create a new interface and leave old users
alone, or if we care about that code and really want to remove the
cast, maybe it should just use kmalloc() instead.

Long ago, allocating a page using kmalloc() was a bad idea, because
there was overhead for it in the allocation and the code.

These days, kmalloc() not only doesn't have the allocation overhead,
but may actually scale better too, thanks to percpu caches etc.

So my point here is that not only is it wrong to change the calling
convention for a legacy function (and it really probably doesn't get
much more legacy than get_free_page - I think it's been around
forever), but even the "let's make up a new name" conversion may be
wrong, because it's entirely possible that the code in question should
just be using kmalloc().

So I don't think an automatic conversion is a good idea. I suspect
that old code that somebody isn't actively working on should just be
left alone, and code that *is* actively worked on should maybe
consider kmalloc().

And if the code really explicitly wants a page (or set of aligned
pages) for some vm reason, I suspect having the cast there isn't a bad
thing. It's clearly not just a random pointer allocation if the bit
pattern of the pointer matters.

And yes, most of the people who used to want "unsigned long" have long
since been converted to take "struct page *" instead, since things
like the VM wants highmem pages etc.  There's a reason why the
historical interface returns "unsigned long": it _used_ to be the
right thing for a lot of code. The fact that there now are more casts
than not are about changing use patterns, but I don't think that means
that we should change the calling convention that has a historical
reason for it.

     Linus