Re: [PATCH 3/3] sparse provide pfn_to_nid

[Andrew Morton]

Andy Whitcroft <apw@xxxxxxxxxxxx> wrote:
>
> sparsemem: provide pfn_to_nid
> 
> Before SPARSEMEM is initialised we cannot provide an efficient
> pfn_to_nid() implmentation; before initialisation is complete we use
> early_pfn_to_nid() to provide location information.  Until recently
> there was no non-init user of this functionality.  Provide a post
> init pfn_to_nid() implementation.
> 
> Note that this implmentation assumes that the pfn passed has
> been validated with pfn_valid().  The current single user of this
> function already has this check.
> 
> Signed-off-by: Andy Whitcroft <apw@xxxxxxxxxxxx>
> ---
>  mmzone.h |   13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> diff -upN reference/include/linux/mmzone.h current/include/linux/mmzone.h
> --- reference/include/linux/mmzone.h
> +++ current/include/linux/mmzone.h
> @@ -598,14 +598,11 @@ static inline int pfn_valid(unsigned lon
>  	return valid_section(__nr_to_section(pfn_to_section_nr(pfn)));
>  }
>  
> -/*
> - * These are _only_ used during initialisation, therefore they
> - * can use __initdata ...  They could have names to indicate
> - * this restriction.
> - */
> -#ifdef CONFIG_NUMA
> -#define pfn_to_nid		early_pfn_to_nid
> -#endif
> +#define pfn_to_nid(pfn)							\
> +({									\
> + 	unsigned long __pfn = (pfn);                                    \
> +	page_to_nid(pfn_to_page(pfn));					\
> +})
>  
>  #define early_pfn_valid(pfn)	pfn_valid(pfn)
>  void sparse_init(void);

This causes a problem because we already have a definition of pfn_to_nid()
in include/linux/mmzone.h.  Effectively:

#ifndef CONFIG_NEED_MULTIPLE_NODES

#define pfn_to_nid(pfn)		(0)

#else /* CONFIG_NEED_MULTIPLE_NODES */

#include <asm/mmzone.h>

#endif /* !CONFIG_NEED_MULTIPLE_NODES */


If someone does !CONFIG_NEED_MULTIPLE_NODES, pfn_to_nid() gets a duplicate
definition (from inspection).

If someone does CONFIG_NEED_MULTIPLE_NODES && CONFIG_DISCONTIGMEM we get
duplicate definitions of pfn_to_nid(): one in include/linux/mmzone.h and
one in include/asm/mmzone.h.

It's a big mess - can someone please fix it up?  The maze of config options
is just over the top.

Meanwhile, I'll drop this patch.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/