On 4/9/20 10:12 AM, Jason Gunthorpe wrote:
On Thu, Apr 09, 2020 at 05:47:44PM +0300, Kirill A. Shutemov wrote:
On Thu, Apr 09, 2020 at 07:32:38AM -0700, Matthew Wilcox wrote:
On Thu, Apr 09, 2020 at 05:28:51PM +0300, Kirill A. Shutemov wrote:
We have a few places that do things like
mm/filemap.c: if (unlikely(compound_head(page)->mapping != mapping)) {
Good point.
Acked-by: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
Darn, I hoped you'd have a better idea. I feel quite ashamed of this patch.
I had two ideas. Both awful.
- Rename compound_head() to __compound_head() or something and make it
return void *. Then wrap it into a macro that would cast return type to
type of the argument. It would allow to regain *some* type safety.
- Provide two implementations and use C11 _Generic() :P
(bump GCC version requirements first)
I saw people using static_assert stuff in the kernel, maybe C11 is
feasible now? It is 9 years old..
ohhh, if this patch is a reliable indicator of what const correctness looks
like in the kernel, then we're going to end up with a lot movement in that
direction, too. With that in mind, let's provide some counter-force, by:
1) Poking around at the C11 idea, just in case It Is Time. Because clearly
the C language dialect that we're using now is not *quite* up to doing const
correctness, so moving the language ahead is likely to give us the cleanest
solution. Better than writing our own language in macros, which yes, I
realize is a way to compensate. But, uggghhh. :)
2) Perhaps providing two routines with different names might be the answer
in this case? This avoids macros, still provides a const cast (which is
breaks the chain of const correctness, but on the other hand, we still get
most of the const goodness--the rest comes when the language supports it):
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 222f6f7b2bb3..71e2d07ddcf4 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -186,6 +186,11 @@ static inline struct page *compound_head(struct page *page)
return page;
}
+static inline const struct page *compound_head_const(struct page *page)
+{
+ return (const struct page*)compound_head(page);
+}
+
static __always_inline int PageTail(struct page *page)
{
return READ_ONCE(page->compound_head) & 1;
diff --git a/mm/debug.c b/mm/debug.c
index 2189357f0987..9a1f37a80ed7 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -44,7 +44,7 @@ const struct trace_print_flags vmaflag_names[] = {
void __dump_page(struct page *page, const char *reason)
{
- struct page *head = compound_head(page);
+ const struct page *head = compound_head_const(page);
struct address_space *mapping;
bool page_poisoned = PagePoisoned(page);
bool compound = PageCompound(page);
...etc
thanks,
--
John Hubbard
NVIDIA
