| Message ID | 20240708075632.95857-1-tursulin@igalia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v4] mm/numa_balancing: Teach mpol_to_str about the balancing mode | expand |
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. > > v4: > * Use is_power_of_2. > * Use ARRAY_SIZE and update recommended buffer size for two flags. > > 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+ LGTM, Thanks! Reviewed-by: "Huang, Ying" <ying.huang@intel.com> > --- > mm/mempolicy.c | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index aec756ae5637..a1bf9aa15c33 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 51 for the longest mode, "weighted > + * interleave", plus the longest flag flags, "relative|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[ARRAY_SIZE(preferred_node_policy) - 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 (!is_power_of_2(flags & MPOL_MODE_FLAGS)) > + p += snprintf(p, buffer + maxlen - p, "|"); > + p += snprintf(p, buffer + maxlen - p, "balancing"); > + } > } > > if (!nodes_empty(nodes))
On 09/07/2024 07:19, Huang, Ying wrote: > 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. >> >> v4: >> * Use is_power_of_2. >> * Use ARRAY_SIZE and update recommended buffer size for two flags. >> >> 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+ > > LGTM, Thanks! > > Reviewed-by: "Huang, Ying" <ying.huang@intel.com> Thank you! Andrew, this appears safe to pick up now as a replacement for the identically named patch you previously picked up but then had to back out from the queue. Thanks! Tvrtko >> --- >> mm/mempolicy.c | 18 ++++++++++++++---- >> 1 file changed, 14 insertions(+), 4 deletions(-) >> >> diff --git a/mm/mempolicy.c b/mm/mempolicy.c >> index aec756ae5637..a1bf9aa15c33 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 51 for the longest mode, "weighted >> + * interleave", plus the longest flag flags, "relative|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[ARRAY_SIZE(preferred_node_policy) - 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 (!is_power_of_2(flags & MPOL_MODE_FLAGS)) >> + p += snprintf(p, buffer + maxlen - p, "|"); >> + p += snprintf(p, buffer + maxlen - p, "balancing"); >> + } >> } >> >> if (!nodes_empty(nodes))
diff --git a/mm/mempolicy.c b/mm/mempolicy.c index aec756ae5637..a1bf9aa15c33 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 51 for the longest mode, "weighted + * interleave", plus the longest flag flags, "relative|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[ARRAY_SIZE(preferred_node_policy) - 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 (!is_power_of_2(flags & MPOL_MODE_FLAGS)) + p += snprintf(p, buffer + maxlen - p, "|"); + p += snprintf(p, buffer + maxlen - p, "balancing"); + } } if (!nodes_empty(nodes))