lkml.org 
[lkml]   [2014]   [Nov]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] sparc64: Add preprocessor symbols for PAGE_* pgprot_t values.
David Miller wrote:
>> Kernel code assumes that the PAGE_* values are preprocessor symbols, and
>> that therefore arch support can be checked for with #ifdef.
>>
>> At the moment, sparc64 does not implement any of the symbols checked
>> for, so these checks happen to work.
>>
>> To prevent potential breakage when another #ifdef check is added or when
>> sparc64 implements another PAGE_ value, make such #ifdef checks work by
>> adding preprocessor symbols on top of the PAGE_* variables.
>>
>> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
>
> "This change actually doesn't have any effect, and doesn't matter,
> so let's make this change."
>
> I really don't buy this.

At the moment, sparc64 does not support symbols such as PAGE_KERNEL_RO,
and has no mechanism for arch-independent code to detect this.
Some code tries anyway:

$ git grep '#ifn\?def PAGE_KERNEL_'
drivers/base/firmware_class.c:#ifndef PAGE_KERNEL_RO
drivers/staging/comedi/comedi_buf.c:#ifdef PAGE_KERNEL_NOCACHE
mm/nommu.c:#ifndef PAGE_KERNEL_EXEC
mm/vmalloc.c:#ifndef PAGE_KERNEL_EXEC

I don't want to add more such code to the kernel without a guarantee
that it actually works, or without replacing it with some other kind
of check that does work.

> I'd also rather the kernel use Kconfig based symbols to detect for
> arch availability of feature X or Y, assuming things are CPP symbols
> is very fragile at best.

It is certainly possible to use Kconfig-based symbols. However, this
would have its own fragility: adding, e.g., PAGE_KERNEL_SO to an arch
requires that one remembers to update Kconfig, and if one forgets,
a check like this:

#ifndef CONFIG_ARCH_HAS_PAGE_KERNEL_RO
#define PAGE_KERNEL_RO PAGE_KERNEL /* fallback for this code */
#endif

will not detect the error on sparc64 (if PAGE_KERNEL_RO were a CPP
symbol, the compiler would complain about the redefinition).

So even if PAGE_KERNEL_RO is a variable, making it into a CPP symbol
is beneficial. And once it is a CPP symbol, introducing a separate
Kconfig-based symbol is not necessary and just increases the chances
of an error.


Regards,
Clemens


\
 
 \ /
  Last update: 2014-11-03 11:01    [W:0.038 / U:0.524 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site