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@xxxxxxxxxx> > > "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 -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
