Re: [PATCH v2] asm-generic: remove a broken and needless ifdef conditional

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jul 22, 2022 at 01:07:11PM +0200, Lukas Bulwahn wrote:
> Commit 527701eda5f1 ("lib: Add a generic version of devmem_is_allowed()")
> introduces the config symbol GENERIC_LIB_DEVMEM_IS_ALLOWED, but then
> falsely refers to CONFIG_GENERIC_DEVMEM_IS_ALLOWED (note the missing LIB
> in the reference) in ./include/asm-generic/io.h.
> 
> Luckily, ./scripts/checkkconfigsymbols.py warns on non-existing configs:
> 
> GENERIC_DEVMEM_IS_ALLOWED
> Referencing files: include/asm-generic/io.h
> 
> The actual fix, though, is simply to not to make this function declaration
> dependent on any kernel config. For architectures that intend to use
> the generic version, the arch's 'select GENERIC_LIB_DEVMEM_IS_ALLOWED' will
> lead to picking the function definition, and for other architectures, this
> function is simply defined elsewhere.
> 
> The wrong '#ifndef' on a non-existing config symbol also always had the
> same effect (although more by mistake than by intent). So, there is no
> functional change.
> 
> Remove this broken and needless ifdef conditional.
> 
> Fixes: 527701eda5f1 ("lib: Add a generic version of devmem_is_allowed()")
> Signed-off-by: Lukas Bulwahn <lukas.bulwahn@xxxxxxxxx>
> ---

There is only one architecture which defines this as a static inline.
Should that architecture include asm-generic/io.h, or if it already
does, it may end up with a compilation error.

So if arch/s390/include/asm/page.h can end up including asm-generic/io.h
or if any file including for s390 can include its arch page.h and
asm-generic/io.h then we'll still need the guard.

This may compile today for s390 because s390 may not use asm-generic/io.h.

So why not just keep the guard and correct this as intended?

  Luis

> v1: https://lore.kernel.org/all/20211006145859.9564-1-lukas.bulwahn@xxxxxxxxx/
> 
> Arnd, please pick this v2 for your asm-generic branch. 
> 
> 
>  include/asm-generic/io.h | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> index ce4c90601300..a68f8fbf423b 100644
> --- a/include/asm-generic/io.h
> +++ b/include/asm-generic/io.h
> @@ -1221,9 +1221,7 @@ static inline void memcpy_toio(volatile void __iomem *addr, const void *buffer,
>  }
>  #endif
>  
> -#ifndef CONFIG_GENERIC_DEVMEM_IS_ALLOWED
>  extern int devmem_is_allowed(unsigned long pfn);
> -#endif
>  
>  #endif /* __KERNEL__ */
>  
> -- 
> 2.17.1
> 



[Index of Archives]     [Linux Kernel]     [Kernel Newbies]     [x86 Platform Driver]     [Netdev]     [Linux Wireless]     [Netfilter]     [Bugtraq]     [Linux Filesystems]     [Yosemite Discussion]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux