Re: [PATCH v3 4/4] mm/mempolicy: change cur_il_weight to atomic and carry the node with it

[Gregory Price]

On Mon, Jan 29, 2024 at 10:48:47AM -0500, Gregory Price wrote:
> On Mon, Jan 29, 2024 at 04:17:46PM +0800, Huang, Ying wrote:
> > Gregory Price <gregory.price@xxxxxxxxxxxx> writes:
> > 
> > But, in contrast, it's bad to put task-local "current weight" in
> > mempolicy.  So, I think that it's better to move cur_il_weight to
> > task_struct.  And maybe combine it with current->il_prev.
> > 
> Style question: is it preferable add an anonymous union into task_struct:
> 
> union {
>     short il_prev;
>     atomic_t wil_node_weight;
> };
> 
> Or should I break out that union explicitly in mempolicy.h?
> 

Having attempted this, it looks like including mempolicy.h into sched.h
is a non-starter.  There are build issues likely associated from the
nested include of uapi/linux/mempolicy.h

So I went ahead and did the following.  Style-wise If it's better to just
integrate this as an anonymous union in task_struct, let me know, but it
seemed better to add some documentation here.

I also added static get/set functions to mempolicy.c to touch these
values accordingly.

As suggested, I changed things to allow 0-weight in il_prev.node_weight
adjusted the logic accordingly. Will be testing this for a day or so
before sending out new patches.

~Gregory



diff --git a/include/linux/sched.h b/include/linux/sched.h
index ffe8f618ab86..f0d2af3bbc69 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -745,6 +745,29 @@ struct kmap_ctrl {
 #endif
 };

+
+/*
+ * Describes task_struct interleave settings
+ *
+ * Interleave uses mpol_interleave.node
+ *   last node allocated from
+ *   intended for use in next_node_in() on the next allocation
+ *
+ * Weighted interleave uses mpol_interleave.node_weight
+ *   node is the value of the current node to allocate from
+ *   weight is the number of allocations left on that node
+ *   when weight is 0, next_node_in(node) will be invoked
+ */
+union mpol_interleave {
+       struct {
+               short node;
+               short resv;
+       };
+       /* structure: short node; u8 resv; u8 weight; */
+       atomic_t node_weight;
+};
+
+
 struct task_struct {
 #ifdef CONFIG_THREAD_INFO_IN_TASK
        /*
@@ -1258,7 +1281,7 @@ struct task_struct {
 #ifdef CONFIG_NUMA
        /* Protected by alloc_lock: */
        struct mempolicy                *mempolicy;
-       short                           il_prev;
+       union mpol_interleave           il_prev;
        short                           pref_node_fork;
 #endif
 #ifdef CONFIG_NUMA_BALANCING



diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 92740b8f0eb5..48e365b507b2 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -149,6 +149,66 @@ static struct mempolicy preferred_node_policy[MAX_NUMNODES];
 static u8 __rcu *iw_table;
 static DEFINE_MUTEX(iw_table_lock);

+static u8 get_il_weight(int node)
+{
+       u8 __rcu *table;
+       u8 weight;
+
+       rcu_read_lock();
+       table = rcu_dereference(iw_table);
+       /* if no iw_table, use system default */
+       weight = table ? table[node] : 1;
+       /* if value in iw_table is 0, use system default */
+       weight = weight ? weight : 1;
+       rcu_read_unlock();
+       return weight;
+}
+
+/* Clear any interleave values from task->il_prev */
+static void clear_il_prev(void)
+{
+       int node_weight;
+
+       node_weight = MAKE_WIL_PREV(MAX_NUMNODES - 1, 0);
+       atomic_set(&current->il_prev.node_weight, node_weight);
+}
+
+/* get the next value for weighted interleave */
+static void get_wil_prev(int *node, u8 *weight)
+{
+       int node_weight;
+
+       node_weight = atomic_read(&current->il_prev.node_weight);
+       *node = WIL_NODE(node_weight);
+       *weight = WIL_WEIGHT(node_weight);
+}
+
+/* set the next value for weighted interleave */
+static void set_wil_prev(int node, u8 weight)
+{
+       int node_weight;
+
+       if (node == MAX_NUMNODES)
+               node -= 1;
+       node_weight = MAKE_WIL_PREV(node, weight);
+       atomic_set(&current->il_prev.node_weight, node_weight);
+}
+
+/* get the previous interleave node */
+static short get_il_prev(void)
+{
+       return current->il_prev.node;
+}
+
+/* set the previous interleave node */
+static void set_il_prev(int node)
+{
+       if (unlikely(node >= MAX_NUMNODES))
+               node = MAX_NUMNODES - 1;
+
+       current->il_prev.node = node;
+}
+