Re: [patch RFC 02/15] highmem: Provide generic variant of kmap_atomic*

Christoph Hellwig
2020-09-21T06:28:36Z

> +# ifndef ARCH_NEEDS_KMAP_HIGH_GET
> +static inline void *arch_kmap_temporary_high_get(struct page *page)
> +{
> + return NULL;
> +}
> +# endif

Turn this into a macro and use #ifndef on the symbol name?

> +static inline void __kunmap_atomic(void *addr)
> +{
> + kumap_atomic_indexed(addr);
> +}
> +
> +
> +#endif /* CONFIG_KMAP_ATOMIC_GENERIC */

Stange double empty line above the endif.

> -#define kunmap_atomic(addr) \
> -do { \
> - BUILD_BUG_ON(__same_type((addr), struct page *)); \
> - kunmap_atomic_high(addr); \
> - pagefault_enable(); \
> - preempt_enable(); \
> -} while (0)
> -
> +#define kunmap_atomic(addr) \
> + do { \
> + BUILD_BUG_ON(__same_type((addr), struct page *)); \
> + __kunmap_atomic(addr); \
> + preempt_enable(); \
> + } while (0)

Why the strange re-indent to a form that is much less common and less
readable?

> +void *kmap_atomic_pfn_prot(unsigned long pfn, pgprot_t prot)
> +{
> + pagefault_disable();
> + return __kmap_atomic_pfn_prot(pfn, prot);
> +}
> +EXPORT_SYMBOL(kmap_atomic_pfn_prot);

The existing kmap_atomic_pfn & co implementation is EXPORT_SYMBOL_GPL,
and this stuff should preferably stay that way.