lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87ttgzaxzi.fsf@yhuang6-desk2.ccr.corp.intel.com>
Date: Tue, 09 Jul 2024 14:19:29 +0800
From: "Huang, Ying" <ying.huang@...el.com>
To: Tvrtko Ursulin <tursulin@...lia.com>
Cc: linux-mm@...ck.org,  linux-kernel@...r.kernel.org,
  kernel-dev@...lia.com,  Tvrtko Ursulin <tvrtko.ursulin@...lia.com>,  Mel
 Gorman <mgorman@...e.de>,  Peter Zijlstra <peterz@...radead.org>,  Ingo
 Molnar <mingo@...hat.com>,  Rik van Riel <riel@...riel.com>,  Johannes
 Weiner <hannes@...xchg.org>,  "Matthew Wilcox (Oracle)"
 <willy@...radead.org>,  Dave Hansen <dave.hansen@...el.com>,  Andi Kleen
 <ak@...ux.intel.com>,  Michal Hocko <mhocko@...e.com>,  David Rientjes
 <rientjes@...gle.com>,  stable@...r.kernel.org
Subject: Re: [PATCH v4] mm/numa_balancing: Teach mpol_to_str about the
 balancing mode

Tvrtko Ursulin <tursulin@...lia.com> writes:

> From: Tvrtko Ursulin <tvrtko.ursulin@...lia.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@...lia.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@...el.com>
> Cc: Mel Gorman <mgorman@...e.de>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Rik van Riel <riel@...riel.com>
> Cc: Johannes Weiner <hannes@...xchg.org>
> Cc: "Matthew Wilcox (Oracle)" <willy@...radead.org>
> Cc: Dave Hansen <dave.hansen@...el.com>
> Cc: Andi Kleen <ak@...ux.intel.com>
> Cc: Michal Hocko <mhocko@...e.com>
> Cc: David Rientjes <rientjes@...gle.com>
> Cc: <stable@...r.kernel.org> # v5.12+

LGTM, Thanks!

Reviewed-by: "Huang, Ying" <ying.huang@...el.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))

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ