On Thu, Apr 27, 2017 at 05:09:26PM +0200, Laurent Vivier wrote:
> On 27/04/2017 16:09, Eduardo Habkost wrote:
> > On Thu, Apr 27, 2017 at 12:12:58PM +0200, Laurent Vivier wrote:
> >> We need to change the way we distribute the memory across
> >> the nodes. To keep compatibility between machine type version
> >> introduce a machine type dependent function.
> >>
> >> Signed-off-by: Laurent Vivier <lviv...@redhat.com>
> > [...]
> >> +static void numa_auto_assign_ram(MachineClass *mc, NodeInfo *nodes,
> >> + int nb_nodes, ram_addr_t size)
> >> +{
> >> + int i;
> >> + uint64_t usedmem = 0;
> >> +
> >> + if (mc->numa_auto_assign_ram) {
> >> + uint64_t *mem = g_new(uint64_t, nb_nodes);
> >> +
> >> + mc->numa_auto_assign_ram(mem, nb_nodes, size);
> >> +
> >> + for (i = 0; i < nb_nodes; i++) {
> >> + nodes[i].node_mem = mem[i];
> >> + }
> >> +
> >> + g_free(mem);
> >> +
> >> + return;
> >> + }
> >> +
> >> + /* Align each node according to the alignment
> >> + * requirements of the machine class
> >> + */
> >> +
> >> + for (i = 0; i < nb_nodes - 1; i++) {
> >> + nodes[i].node_mem = (size / nb_nodes) &
> >> + ~((1 << mc->numa_mem_align_shift) - 1);
> >> + usedmem += nodes[i].node_mem;
> >> + }
> >> + nodes[i].node_mem = size - usedmem;
> >> +}
> >
> > I would prefer to make your new algorithm the default, and move
> > this code to a legacy_auto_assign_ram() function set by the 2.9
> > machine-types.
>
> I think it's easier to do as I've done because otherwise, we need:
>
> - to add the numa_mem_align_shift to the parameters list of the
> numa_auto_assign_ram() function.
You can take MachineClass or MachineState as paramter.
>
> - set the function by default to numa_auto_assign_ram in
> hw/core/machine.c:machine_class_init()
Yep.
>
> - set the pointer to NULL in 2.10 pseries machine type,
Won't pseries-2.10 have the same behavior as all other machines
except pc/pseries <= 2.9? pseries-2.10 and pc-2.10 would just
reuse the default value set by machine_class_init().
>
> - export the function to re-set the legacy function in the 2.9 pseries
> machine type definition.
Well, this is the part where I agree it's too much hassle. :)
>
> So, we need to add one argument to the function, export the function to
> use it from machine.c and at least spapr.c, to set the function in
> machine_class_init() and spapr_machine_2_9_class_options() (as we clear
> it in 2.10 function).
>
> I can do that, but is this what you want?
I don't have a strong opinion. If you believe your way is
simpler, we can keep it.
--
Eduardo
