From: Tvrtko Ursulin tvrtko.ursulin@igalia.com
Since balancing mode was added in bda420b98505 ("numa balancing: migrate on fault among multiple bound nodes"), it was possible to set this mode but it wouldn't be shown in /proc/<pid>/numa_maps since there was no support for it in the mpol_to_str() helper.
Furthermore, because the balancing mode sets the MPOL_F_MORON flag, it would be displayed as 'default' due a workaround introduced a few years earlier in 8790c71a18e5 ("mm/mempolicy.c: fix mempolicy printing in numa_maps").
To tidy this up we implement two changes:
Replace the MPOL_F_MORON check by pointer comparison against the preferred_node_policy array. By doing this we generalise the current special casing and replace the incorrect 'default' with the correct 'bind' for the mode.
Secondly, we add a string representation and corresponding handling for the MPOL_F_NUMA_BALANCING flag.
With the two changes together we start showing the balancing flag when it is set and therefore complete the fix.
Representation format chosen is to separate multiple flags with vertical bars, following what existed long time ago in kernel 2.6.25. But as between then and now there wasn't a way to display multiple flags, this patch does not change the format in practice.
Some /proc/<pid>/numa_maps output examples:
555559580000 bind=balancing:0-1,3 file=... 555585800000 bind=balancing|static:0,2 file=... 555635240000 prefer=relative:0 file=
v2: * Fully fix by introducing MPOL_F_KERNEL.
v3: * Abandoned the MPOL_F_KERNEL approach in favour of pointer comparisons. * Removed lookup generalisation for easier backporting. * Replaced commas as separator with vertical bars. * Added a few more words about the string format in the commit message.
Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@igalia.com Fixes: bda420b98505 ("numa balancing: migrate on fault among multiple bound nodes") References: 8790c71a18e5 ("mm/mempolicy.c: fix mempolicy printing in numa_maps") Cc: Huang Ying ying.huang@intel.com Cc: Mel Gorman mgorman@suse.de Cc: Peter Zijlstra peterz@infradead.org Cc: Ingo Molnar mingo@redhat.com Cc: Rik van Riel riel@surriel.com Cc: Johannes Weiner hannes@cmpxchg.org Cc: "Matthew Wilcox (Oracle)" willy@infradead.org Cc: Dave Hansen dave.hansen@intel.com Cc: Andi Kleen ak@linux.intel.com Cc: Michal Hocko mhocko@suse.com Cc: David Rientjes rientjes@google.com Cc: stable@vger.kernel.org # v5.12+ --- mm/mempolicy.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/mm/mempolicy.c b/mm/mempolicy.c index aec756ae5637..1bfb6c73a39c 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -3293,8 +3293,9 @@ int mpol_parse_str(char *str, struct mempolicy **mpol) * @pol: pointer to mempolicy to be formatted * * Convert @pol into a string. If @buffer is too short, truncate the string. - * Recommend a @maxlen of at least 32 for the longest mode, "interleave", the - * longest flag, "relative", and to display at least a few node ids. + * Recommend a @maxlen of at least 42 for the longest mode, "weighted + * interleave", the longest flag, "balancing", and to display at least a few + * node ids. */ void mpol_to_str(char *buffer, int maxlen, struct mempolicy *pol) { @@ -3303,7 +3304,10 @@ void mpol_to_str(char *buffer, int maxlen, struct mempolicy *pol) unsigned short mode = MPOL_DEFAULT; unsigned short flags = 0;
- if (pol && pol != &default_policy && !(pol->flags & MPOL_F_MORON)) { + if (pol && + pol != &default_policy && + !(pol >= &preferred_node_policy[0] && + pol <= &preferred_node_policy[MAX_NUMNODES - 1])) { mode = pol->mode; flags = pol->flags; } @@ -3331,12 +3335,18 @@ void mpol_to_str(char *buffer, int maxlen, struct mempolicy *pol) p += snprintf(p, buffer + maxlen - p, "=");
/* - * Currently, the only defined flags are mutually exclusive + * Static and relative are mutually exclusive. */ if (flags & MPOL_F_STATIC_NODES) p += snprintf(p, buffer + maxlen - p, "static"); else if (flags & MPOL_F_RELATIVE_NODES) p += snprintf(p, buffer + maxlen - p, "relative"); + + if (flags & MPOL_F_NUMA_BALANCING) { + if (hweight16(flags & MPOL_MODE_FLAGS) > 1) + p += snprintf(p, buffer + maxlen - p, "|"); + p += snprintf(p, buffer + maxlen - p, "balancing"); + } }
if (!nodes_empty(nodes))
On Fri, Jul 05, 2024 at 03:32:16PM +0100, Tvrtko Ursulin wrote:
if (flags & MPOL_F_NUMA_BALANCING) {
if (hweight16(flags & MPOL_MODE_FLAGS) > 1)
hweight() > 1 seems somewhat inefficient. !is_power_of_2() would be better. Or clear off the bits as they're printed and print the bar if the remaining flags are not 0.
p += snprintf(p, buffer + maxlen - p, "|");
p += snprintf(p, buffer + maxlen - p, "balancing");
}}
if (!nodes_empty(nodes)) -- 2.44.0
Tvrtko Ursulin tursulin@igalia.com writes:
From: Tvrtko Ursulin tvrtko.ursulin@igalia.com
Since balancing mode was added in bda420b98505 ("numa balancing: migrate on fault among multiple bound nodes"), it was possible to set this mode but it wouldn't be shown in /proc/<pid>/numa_maps since there was no support for it in the mpol_to_str() helper.
Furthermore, because the balancing mode sets the MPOL_F_MORON flag, it would be displayed as 'default' due a workaround introduced a few years earlier in 8790c71a18e5 ("mm/mempolicy.c: fix mempolicy printing in numa_maps").
To tidy this up we implement two changes:
Replace the MPOL_F_MORON check by pointer comparison against the preferred_node_policy array. By doing this we generalise the current special casing and replace the incorrect 'default' with the correct 'bind' for the mode.
Secondly, we add a string representation and corresponding handling for the MPOL_F_NUMA_BALANCING flag.
With the two changes together we start showing the balancing flag when it is set and therefore complete the fix.
Representation format chosen is to separate multiple flags with vertical bars, following what existed long time ago in kernel 2.6.25. But as between then and now there wasn't a way to display multiple flags, this patch does not change the format in practice.
Some /proc/<pid>/numa_maps output examples:
555559580000 bind=balancing:0-1,3 file=... 555585800000 bind=balancing|static:0,2 file=... 555635240000 prefer=relative:0 file=
v2:
- Fully fix by introducing MPOL_F_KERNEL.
v3:
- Abandoned the MPOL_F_KERNEL approach in favour of pointer comparisons.
- Removed lookup generalisation for easier backporting.
- Replaced commas as separator with vertical bars.
- Added a few more words about the string format in the commit message.
Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@igalia.com Fixes: bda420b98505 ("numa balancing: migrate on fault among multiple bound nodes") References: 8790c71a18e5 ("mm/mempolicy.c: fix mempolicy printing in numa_maps") Cc: Huang Ying ying.huang@intel.com Cc: Mel Gorman mgorman@suse.de Cc: Peter Zijlstra peterz@infradead.org Cc: Ingo Molnar mingo@redhat.com Cc: Rik van Riel riel@surriel.com Cc: Johannes Weiner hannes@cmpxchg.org Cc: "Matthew Wilcox (Oracle)" willy@infradead.org Cc: Dave Hansen dave.hansen@intel.com Cc: Andi Kleen ak@linux.intel.com Cc: Michal Hocko mhocko@suse.com Cc: David Rientjes rientjes@google.com Cc: stable@vger.kernel.org # v5.12+
mm/mempolicy.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/mm/mempolicy.c b/mm/mempolicy.c index aec756ae5637..1bfb6c73a39c 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -3293,8 +3293,9 @@ int mpol_parse_str(char *str, struct mempolicy **mpol)
- @pol: pointer to mempolicy to be formatted
- Convert @pol into a string. If @buffer is too short, truncate the string.
- Recommend a @maxlen of at least 32 for the longest mode, "interleave", the
- longest flag, "relative", and to display at least a few node ids.
- Recommend a @maxlen of at least 42 for the longest mode, "weighted
- interleave", the longest flag, "balancing", and to display at least a few
And we may display 2 flags now, +9 further?
*/
- node ids.
void mpol_to_str(char *buffer, int maxlen, struct mempolicy *pol) { @@ -3303,7 +3304,10 @@ void mpol_to_str(char *buffer, int maxlen, struct mempolicy *pol) unsigned short mode = MPOL_DEFAULT; unsigned short flags = 0;
- if (pol && pol != &default_policy && !(pol->flags & MPOL_F_MORON)) {
- if (pol &&
pol != &default_policy &&
!(pol >= &preferred_node_policy[0] &&
pol <= &preferred_node_policy[MAX_NUMNODES - 1])) {
Better to replace MAX_NUMNODES with ARRAY_SIZE() here.
mode = pol->mode; flags = pol->flags;
} @@ -3331,12 +3335,18 @@ void mpol_to_str(char *buffer, int maxlen, struct mempolicy *pol) p += snprintf(p, buffer + maxlen - p, "="); /*
* Currently, the only defined flags are mutually exclusive
*/ if (flags & MPOL_F_STATIC_NODES) p += snprintf(p, buffer + maxlen - p, "static"); else if (flags & MPOL_F_RELATIVE_NODES) p += snprintf(p, buffer + maxlen - p, "relative");* Static and relative are mutually exclusive.
if (flags & MPOL_F_NUMA_BALANCING) {
if (hweight16(flags & MPOL_MODE_FLAGS) > 1)
p += snprintf(p, buffer + maxlen - p, "|");
p += snprintf(p, buffer + maxlen - p, "balancing");
}
Still think that it's better to move this part to [2/3]. Unless you can make the change small and the resulting code looks good.
} if (!nodes_empty(nodes))
-- Best Regards, Huang, Ying
linux-stable-mirror@lists.linaro.org