On 15.11.2023 15:11, Oleksii wrote: > On Wed, 2023-11-15 at 11:07 +0100, Jan Beulich wrote: >> On 10.11.2023 17:30, Oleksii Kurochko wrote: >>> --- /dev/null >>> +++ b/xen/include/asm-generic/numa.h >>> @@ -0,0 +1,40 @@ >>> +/* SPDX-License-Identifier: GPL-2.0-only */ >>> +#ifndef __ARCH_GENERIC_NUMA_H >>> +#define __ARCH_GENERIC_NUMA_H >>> + >>> +#include <xen/types.h> >>> +#include <xen/mm.h> >> >> I'm afraid I can't spot what you need these for here. You clearly >> need >> xen/stdint.h, and you need xen/mm-frame.h for mfn_t. Yes, max_page is >> declared in xen/mm.h, but its questionable whether the header needs >> including here for that reason, as all uses are in macros. (We aren't >> anywhere near consistent in this regard). Plus you don't also include >> xen/cpumask.h to pull in cpu_online_map (which another macro >> references). > I agree with almost you wrote but should <xen/cpumas.h> be included > then? It looks like it is the same situation as with max_page and > <xen/mm.h>.
Well, yes and no: Indeed the minimal requirement is to be consistent (either include both or include neither). Personally I'd prefer if headers would be included only if they are needed to successfully compiler the header on its own. That limits needless dependencies on the (otherwise) included headers. But I can easily see that others might take the other sensible perspective. >>> +typedef uint8_t nodeid_t; >>> + >>> +#ifndef CONFIG_NUMA >> >> Isn't it an error to include this header when NUMA=y? > It's still need to define arch_want_default_dmazone() macros which is > used by common code. Oh, yes. I somehow managed to overlook that. Some of the #include-s then want to move inside the #ifndef, though. (Ultimately I question the placement of this #define in this header, but we can deal with that separately.) Jan
