Gregory Price <gregory.price@xxxxxxxxxxxx> writes:
> On Tue, Jan 02, 2024 at 03:41:08PM +0800, Huang, Ying wrote:
>> Gregory Price <gregory.price@xxxxxxxxxxxx> writes:
>>
>> That is, if we use "1" as default weight, we need to change weights of
>> nodes frequently because we haven't a "base" weight. The best candidate
>> base weight is the weight of DRAM node. For example, if we set the
>> default weight of DRAM node to be "16" and use that as the base weight,
>> we don't need to change it in most cases. The weight of other nodes can
>> be set according to the ratio of its memory bandwidth to that of DRAM.
>>
>> This makes it easy to set the default weight via HMAT/CDAT too.
>>
>> What do you think about that?
>>
>
> You're getting a bit ahead of the patch set. There is "what is a
> reasonable default weight" and "what is the minumum functionality".
I totally agree that we need the minimal functionality firstly.
> The minimum functionality is everything receiving a default weight of 1,
> such that weighted interleave's behavior defaults to round-robin
> interleave. This gets the system off the ground.
I don't think that we need to implement all functionalities now. But,
we may need to consider more especially if it may impact the user space
interface. The default base weight is something like that. If we
change the default base weight from "1" to "16" later, users may be
surprised. So, I think it's better to discuss it now.
> We can then expose an internal interface to drivers for them to set the
> default weight to some reasonable number during system and device
> initialization. The question at that point is what system is responsible
> for setting the default weights... node? cxl? anything? What happens on
> hotplug? etc. That seems outside the scope of this patch set.
>
>
> If you want me to add the default_iw_table with special value 0 denoting
> "use default" at each layer, I can do that.
>
> The basic change is this snippet:
> ```
> if (pol->flags & MPOL_F_GWEIGHT)
> pol_weights = iw_table;
> else
> pol_weights = pol->wil.weights;
>
> for_each_node_mask(nid, nodemask) {
> weight = pol_weights[nid];
> weight_total += weight;
> weights[nid] = weight;
> }
> ```
>
> changes to:
> ```
> for_each_node_mask(nid, nodemask) {
> weight = pol->wil.weights[node]
> if (!weight)
> weight = iw_table[node]
> if (!weight)
> weight = default_iw_table[node]
> weight_total += weight;
> weights[nid] = weight
> }
> ```
>
> It's a bit ugly,
We can use a wrapper function to hide the logic.
> but it allows a 0 value to represent "use default",
> and default_iw_table just ends up being initialized to `1` for now.
Because the contents of default_iw_table are just "default weight" for
now. We don't need it for now. We can add it later.
> I think it also allows MPOL_F_GWEIGHT to be eliminated.
Do we need a way to distinguish whether to copy the global weights to
local weights when the memory policy is created? That is, when the
global weights are changed later, will the changes be used? One
possible solution is
- If no weights are specified in set_mempolicy2(), the global weights
will be used always.
- If at least one weight is specified in set_mempolicy2(), it will be
used, and the other weights in global weights will be copied to the
local weights. That is, changes to the global weights will not be
used.
--
Best Regards,
Huang, Ying
