Re: [RFC] free_pages stuff

From: Al Viro
Date: Mon Dec 21 2015 - 20:04:13 EST


On Mon, Dec 21, 2015 at 04:03:11PM -0800, Linus Torvalds wrote:

> If you want to have versions of the function that return pointers, you
> had damn well better give them new names. Not use the same name for a
> different function, causing confusion and forcing this kind of crazy
> "change everything at once" flag-day patches, and pain for
> backporting.

*shrug*

Up to you. I'll cherry-pick the fixes for bugs found in process and leave
the rest alone.

> 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.
Counting typecasts demonstrates _that_ very clearly. Any place that
wanted to allocate a page and get a pointer needs a typecast in mainline;
any place that wanted to allocate a page and get a number would need
one after this series. Similar for freeing something that is a pointer
and is a number respectively.

The total after that series was 70 typecasts added and 1408 removed. In other
words, "want to allocate a page and get a pointer" outnumbers the "want to
allocate a page and get a number" a _lot_.

Do you really mean that we are overusing __get_free_page() and friends that
much and that we should simply use kmalloc() instead? I'm not saying that
it's wrong - a lot of places clearly would be fine with kmalloc/kfree.
For something like page table allocations kmalloc() is obviously wrong (and
they also are of "get a pointer" sort), but that's a very small fraction.

> So I don't know how many ways I can say "NO", but I'll not take
> anythign like this. It's *completely* wrong.

OK. Don't get me wrong - I'm not fond of all-over-the-tree changes either;
I wanted to figure out how the damn thing is actually used and I have found
that. What (if anything) to do with that is a separate question.

For me the bottom line so far is that we have a lot of places where page
allocator is used and the majority of those uses the result as a pointer.
That, with the calling conventions we have (and had all along), means tons
of boilerplate. It also means a lot of opportunities to mix physical,
virtual and DMA addresses, since typechecking is completely bypassed by
those typecasts.

If I understood you correctly, in a lot of those cases the answer should've
been "just use kmalloc() and be done with that". And something like
e.g. debugfs read and write methods of some wireless NIC driver
certainly could use kmalloc(); any concerns about extra overhead are
ridiculous there. The same goes for e.g. sysfs symlink body generated
when we run into one, etc. I'm not suggesting to start converting existing
code to kmalloc; that only goes for new code being written, obviously.

PS: in case you've said that "NO" earlier and I'd missed your replies, my
apologies for keeping that up; this is really the first response from you I've
seen on this topic.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/