[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC PATCH v3 05/24] x86: NUMA: Add accessors for nodes[] and node_memblk_range[] structs
On Wed, Jul 19, 2017 at 10:48 PM, Julien Grall <julien.grall@xxxxxxx> wrote: > > > On 19/07/17 07:40, Vijay Kilari wrote: >> >> On Tue, Jul 18, 2017 at 8:59 PM, Wei Liu <wei.liu2@xxxxxxxxxx> wrote: >>> >>> On Tue, Jul 18, 2017 at 05:11:27PM +0530, vijay.kilari@xxxxxxxxx wrote: >>>> >>>> From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx> >>>> >>>> Add accessors for nodes[] and other static variables and >>>> use those accessors. These variables are later accessed >>>> outside the file when the code made generic in later >>>> patches. However the coding style is not changed. >>>> >>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx> >>>> --- >>>> v3: - Changed accessors parameter from int to unsigned int >>>> - Updated commit message >>>> - Fixed wrong indentation >>>> --- >>>> xen/arch/x86/srat.c | 106 >>>> +++++++++++++++++++++++++++++++++++++++------------- >>>> 1 file changed, 81 insertions(+), 25 deletions(-) >>>> >>>> diff --git a/xen/arch/x86/srat.c b/xen/arch/x86/srat.c >>>> index 535c9d7..42cca5a 100644 >>>> --- a/xen/arch/x86/srat.c >>>> +++ b/xen/arch/x86/srat.c >>>> @@ -41,6 +41,44 @@ static struct node >>>> node_memblk_range[NR_NODE_MEMBLKS]; >>>> static nodeid_t memblk_nodeid[NR_NODE_MEMBLKS]; >>>> static __initdata DECLARE_BITMAP(memblk_hotplug, NR_NODE_MEMBLKS); >>>> >>>> +static struct node *get_numa_node(unsigned int id) >>>> +{ >>>> + return &nodes[id]; >>>> +} >>>> + >>>> +static nodeid_t get_memblk_nodeid(unsigned int id) >>>> +{ >>>> + return memblk_nodeid[id]; >>>> +} >>>> + >>>> +static nodeid_t *get_memblk_nodeid_map(void) >>>> +{ >>>> + return &memblk_nodeid[0]; >>>> +} >>>> + >>>> +static struct node *get_node_memblk_range(unsigned int memblk) >>>> +{ >>>> + return &node_memblk_range[memblk]; >>>> +} >>>> + >>>> +static int get_num_node_memblks(void) >>>> +{ >>>> + return num_node_memblks; >>>> +} >>> >>> >>> They should all be inline functions. And maybe at once lift to a header >>> and add proper prefix since you mention they are going to be used later. >> >> >> Currently these are static variables in x86/srat.c file. >> In patch #9 I move them to common/numa.c file and make these functions >> non-static. >> >> If I lift them to header file and make inline, then I have to make these >> as >> global variables. > > > As I said on v2, I am not sure to understand the usefulness of those > accessors over global variables... These are static variables. To access across other files (arch specific) these accessors are added. I have to make them global variables to use outside of this file. I am happy to make them global and make these accessors static inline as suggested by Wei. > > You don't have any kind of sanity check, so they would do exactly the same > job. The global variables would avoid so much churn. > > More that you tend to sometimes use global and other time static helpers... > > Cheers, > > -- > Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |